Bow-shaped branches: using vendor branches in Git

LATER EDIT: the method described here turned out not to scale very well for large vendored libraries with lots of changes. If your Broadcom (or similar) merges are killing you, you should probably be using a rebasing vendor flow instead.

Following on from the one about bow-shaped branches, one of the remaining ways that Git history can get untidy, despite following those precepts, is when you’ve got a long-running “vendor branch” for some third-party code. The usual pattern is, to have a branch containing successive deliveries of the third-party code; when a new delivery needs to be integrated, it’s checked-in unaltered on that branch, and then the branch is merged back to master. This means that the branch effectively has a diff that corresponds only to the vendor’s changes in the new release (relative to the previous release), so that Git’s merge infrastructure helps you integrate those changes with any changes you’ve made yourself to the third-party code.

Which is all to the good, but presents a problem for those striving for tidy Git history. Especially in the case of widespread changes or ugly merges, integrating the new delivery could end up being as much work as a new feature in its own right – which means it really ought to take place on a feature branch. But if you try and do that feature branch as a bow-shaped branch, starting off with a commit that’s the merge from the vendor branch, then the bow-shaped workflow (and indeed the simple rebase workflow) won’t work out of the box: every time you attempt to rebase the feature branch, Git tries to rebase the entire vendor branch, which is hopelessly not what you want.

The obscure Git feature which in this case turns out to be exactly what’s wanted, as if designed for it, is git rerere. Git can be instructed to remember merge-conflict resolutions, and replay them automatically if the precise same conflict is encountered again. This feature isn’t enabled by default, no doubt because it’d confuse people about how their conflicts somehow disappeared, but it’s just what’s needed for rebasing a feature branch that includes a merge from a vendor branch.

Let’s start by having a look at the desired result. On the right is a screenshot from gitk, which shows the newest commits at the top. It shows that the long-lived vendor branch “broadcom” has been merged following a new delivery of the third-party code, and that the work of integrating the new release itself happened on a bow-shaped branch. No superfluous merge commits are visible.

Here’s how to make history that looks like that. First of all I need to turn on the rerere feature:

git config --global rerere.enabled true

Then import the new delivery. This part is much like the process of using vendor branches in any other source-control system:

1. checkout the vendor branch
2. remove the old code entirely
3. untar/unzip the new delivery
4. rename if necessary (usually, at least, removing version numbers from directory names – the delivery depicted here untarred into a directory called WICED-SDK-2.3.1, but we wanted it called thirdparty/broadcom)
5. but don’t touch the source in any other way
6. add the changes, additions, and removals to Git (git add -A)
7. commit everything to the vendor branch with a commit message along the lines of “import Broadcom Wiced 2.3.1”

But then it’s time to merge those changes back to master. The steps are as follows:

1. create a merge branch, here called “pdh-broadcom” (this is the branch that will eventually look bow-shaped)

   git checkout master
   git checkout -b pdh-broadcom

2. merge the vendor branch

   git merge broadcom

   1. if there were merge conflicts, fix them up; don’t do any other edits at this stage
   2. once the merge conflicts are fixed, commit (to the merge branch)

      git commit

3. compile and test the result
4. commit any compile or test fixes to the merge branch
5. test some more, hold code reviews, commit more fixes, etc., until you’re satisfied that it’s ready to merge to master

At this stage, history looks like the picture on the right: a bit like the “ready to merge” stage of a normal bow-shaped branch, except of course with the long tail of the vendor branch heading off into the distant past.

So let’s get hold of the current state of master:

git checkout master
git pull --rebase

Aha, see on the right, someone’s beaten us to the punch again. We need to rebase our work on top of theirs. But here’s the point at which a bit of caution needs to be applied. Blithely using git rebase as if the vendor branch weren’t there, will instruct git to rebase the entire vendor branch on top of master, which will typically be somewhere between undesirable and complete carnage.

There’s a -p (“preserve branching”) option to git rebase option which will stop it doing that, but there is still the issue that re-doing our merge branch, will involve re-doing the merge itself – which will involve re-resolving all the conflicts that we got in stage 2 above.

And that is where git rerere comes in. Because we enabled rerere above, each time we committed the fix to a merge conflict, git remembered the conflict and our resolution of the conflict. And when we re-encounter the same conflict again, as a result of rebasing, git re-resolves it for us:

git checkout pdh-broadcom
git rebase -p master
Resolved 'thirdparty/broadcom/Wiced/WWD/internal/wwd_wifi.c' using previous resolution.
Automatic merge failed; fix conflicts and then commit the result.
Error redoing merge 7a485d1d

In fact, Git is being unduly alarmist here. The merge did technically fail, but rerere has in fact already solved all the issues automatically. So we just need to convince Git of that, noting which files or directories it thought were a problem:

git add thirdparty/broadcom/Wiced/WWD/internal
git commit

So we’ve now got a fully rebased merge branch, ready to merge back to master – as at right. The merge itself is now just like a normal bow-shaped branch:

git checkout master
git merge --no-ff pdh-broadcom

So at last (as at left) we’ve got where we need to be: it’s a merge from a vendor branch, but it looks like a feature branch.

The more I use Git, the more I feel the truth of these two rules of thumb about it: there is always a way to do X, and there is always a use for command Y. Blogging about Git is really an exercise in providing the Y corresponding to a certain X, which is not always obvious.

Bow-shaped branches: a Git workflow

Distributed version control systems, and Git in particular, have for all practical purposes wiped out CVS, Subversion and similar systems. But Git is new enough that there’s still discussion to be had on the best ways of using it: the best Git “workflows”. One goal of a good workflow is that Git history – the graph of all previous revisions – remains readable and comprehensible. Or, put conversely: one symptom of a bad workflow is a convoluted history that looks like a Tube map, or worse.

One way in which git history can end up looking a tangled mess, is if the history graph includes lots of merges caused by doing git pull when upstream has also been updated. Those merges don’t really add that much information – only that two things happened simultaneously, which is a perfectly normal occurrence in teams of more than one person. So most development teams, at least once they’ve been using Git for a while, decide that merges caused by git pull cause unnecessary tangles, and git pull --rebase gets mandated instead.

But what if development occurs on “feature branches” which are always getting merged? This case, too, can be dealt with using rebasing: in this case, rebasing the branch before merging it. But a pure rebase workflow serialises everything into a straight line, losing the information that these commits, in some sense, belong together.

So what’s really needed is a hybrid of the rebasing and merging styles: something that keeps the feature commits together, but also keeps history looking neat (and, in case of disaster, allows easy reverting of the whole feature). What’s needed is bow-shaped feature branches.

Let’s look at the desired end-result first. On the right is a snapshot from gitk --all, showing a feature branch that I’ve just merged, ready to push. As always with gitk, the newest commits are shown at the top. Two different routes join the same two endpoints, spanning the addition of my new feature: one direct, and another taking many small steps, so that the overall effect is a bow shape. This means that, though each individual commit is readily accessible, so too (by comparing the top and bottom of the bow) is the overall effect of the whole feature. Notice how the commits are all bunched together, but with a limited, local amount of branching: a run of N such branches consecutively would still, in some sense, have O(1) complexity, not O(N). (At least, it would have O(1) concurrently-active development branches, which is believably a useful measure of the complexity, and the comprehensibility burden, of history.)

Someone looking back through history who’s only interested in the large-scale changes, can thus move back via the “express train” lines and not the “stopping train” (or “local train” in US parlance) that goes via every commit. But they still wouldn’t have to follow many different branch lines, as they would in a pure merge workflow, because each bow-shape happens on top of the last, not in parallel with it.

So how to build history graphs that look like that? On the left is gitk’s view of my local repository after doing some work on a feature: in this case a feature made up of seven commits. (Commit early, commit often; but stay bisectable.) If I just did a git pull followed by git push, I’d get a merge with commits on both sides; if I did the same but pulling with --rebase, I’d get a straight-line history with my commits at the top.

In order to get the bow shape, though, I’m going to need to retrospectively turn my commits into a branch. Fortunately, that’s straightforward in Git. First, I create a new branch, named after my initials and my feature (the branch itself won’t be pushed upstream, but its name appears in the merge message, so it ought to be fairly descriptive):

git checkout -b pdh-regexp

At this point my repository looks like the picture on the right.

Then I need to wind back my local master branch to match origin/master:

git checkout master
git reset --hard origin/master

Now my repository looks like the picture on the left: a feature-branch ready to merge. (If at this point you can't see your feature branch, make sure you used the --all option to gitk.) This is the point at which to do any smartening-up of the branch, using interactive rebase or similar, that is needed. It’s also the point at which to do a code review with one of your colleagues – a practice that’s strongly recommended, for various reasons.

[Edited 2014-Apr-05→] Now if by this stage master has moved on – in other words, if it’s not still pointing to the base of my feature-branch as shown in the pictures – then I’ll first need to rebase my branch on top of master, so that it looks like the gitk pictures:

git checkout master
git pull --rebase
git checkout pdh-regexp
git rebase master
git checkout master

And now I’m once more at the ready to merge stage, as seen in gitk.

At this point a normal git merge would do a fast-forward merge, returning me to the picture above on the right – so what’s needed instead, is an explicitly no-fast-forwards merge (which can never produce conflicts, because nothing else has happened to the top of master). First I’d better just check that I’m ready to merge:

git checkout master
git branch --contains HEAD

which will print a list of branches, including at least master itself, that contain the commit at the top of master. If my branch isn’t in the list, then I wasn’t, in fact, ready to merge – which means that I need to rebase on top of master again. If, though, my branch is in the list, then it’s time to do the bow-shaped merge itself:

git merge --no-ff pdh-regexp

And now I’m where I need to be, like the picture above right. [←end edits]

So it’s time to push:

git push
To peter@git.electricimp.com
! [rejected] master -> master (non-fast-forward)
error: failed to push some refs to 'peter@git.electricimp.com/ei.git'
To prevent you from losing history, non-fast-forward updates were rejected
Merge the remote changes (e.g. 'git pull') before pushing again. See the
'Note about fast-forwards' section of 'git push --help' for details.

Ahh, someone else has pushed a different feature while I was doing all that. So I need to go round the loop again.

git reset --hard origin/master

This has the effect of undoing the (trivial) merge commit, and the repository ends up like the picture on the left.

Now I can fetch the new commits to the top of master:

git pull --rebase
git checkout pdh-regexp
git rebase master
git checkout master

Of course, git rebase master might produce conflicts, which should be cleared up in the usual way. (And if the conflicts are too hard to clear up, maybe a real merge is called-for, so that history records that the project had a hard time here.) But if all goes smoothly, I end up with the picture on the right: a feature branch ready to merge, as above – the only difference being that the branch is now on top of a newer commit as the head of origin/master. So I can carry on from that point.

Of course, sometimes a feature is too small to merit all of this ... mechanism. If something goes in in just one or two commits, then it’s probably not complex enough for its presence in history to be controversial: for those, just use the pure rebase workflow. But once a feature has grown to three or four commits, it’s worthwhile making it as clear as possible, to those who follow us, what’s gone on. As for an upper limit: I’ve seen bow shapes with 20–30 commits which still looked fairly sane. Any feature which takes more commits than that, could probably do with being subdivided anyway: not least because the rebasing is probably becoming a major pain at that scale. Again, perhaps a real merge commit is called-for, and again this helps signal to future historians that here is a point at which perhaps the project was having a bit of a hard time.

Giving An FTDI Serial Port A Persistent Device Node

There are plenty of perfectly good reasons why one might have several FTDI USB-to-serial adaptors attached to the same PC at the same time. Anyone who does, though, will notice that they’re a royal pain, because every time you unplug and replug one, it gets a different /dev/ttyUSBn device node.

Fortunately, there’s a way to use udev to set up unchanging aliases for those ever-changing device nodes. This is done by setting up a udev rule for each FTDI adaptor that you care about, that picks it out by serial number and gives it a custom symlink.

Some of the details are here, but just to sew it all together, here’s what you need to do.

• First attach your FTDI adaptor, making sure all others are unplugged.

• ls /dev/ttyUSB*
  — only one should be listed.
• udevadm info --name=/dev/ttyUSB0 --attribute-walk
  — (using the ttyUSB number listed in the previous step) should dump a huge list of attributes
• udevadm info --name=/dev/ttyUSB0 --attribute-walk | grep serial
  — will list only a few lines, such as the following —

  SUBSYSTEMS=="usb-serial"
  ATTRS{serial}=="XR00U1BU"
  ATTRS{serial}=="000000000000"
  ATTRS{serial}=="0000:00:1d.7"

• The first ATTRS line is the one we’re interested in. That’s the USB serial number of the FTDI adaptor. (The subsequent lines are the USB serial number of an intermediate hub, and the PCI address of the USB controller.)
• Now you need to go and find the udev rules directory: in a standard udev install, that’s /etc/udev/rules.d. In that directory, make a file called 99-local.rules (you’ll need to be root), and add a line like the following (and it must be all on one line; this blog might show it split) —
  

  KERNEL=="ttyUSB?", ATTRS{serial}=="XR00U1BU", SYMLINK+="ftdiDUINO", MODE="0666"

• (An earlier version of this page suggested the name 10-local.rules, which with modern versions of udev is checked too early in the process; using 99-local.rules makes it work with any version of udev.)
• Obviously the ATTRS clause must match the one your adaptor listed above. The SYMLINK clause can be anything (that doesn’t clash with any other device), so name it after the thing it’s plugged into at the other end.
• Unplug and replug your FTDI adaptor. Whatever ttyUSB number it gets, the symlink /dev/ftdiDUINO will be pointing to it.
• Oh yes, that udev rule also makes the device node world-writable. So you won’t need to be root to issue

  miniterm.py /dev/ftdiDUINO 115200

The Failure Mode Of Agile Development

Here’s the “traditional” waterfall model of software development:

+------------+------------+------------+
| Design     | Implement  | Test       |
+------------+------------+------------+
^ Start                                ^ Deadline

And here’s its almost inevitable failure mode: design takes too long, or implementation takes too long, but the deadline doesn’t move, so test is cut short and the product ships full of bugs:

+------------>>>+------------>>>+------+.....+
| Design        | Implement     | Test |
+------------>>>+------------>>>+------+.....+
^ Start                                ^ Deadline

Software development organisations have been wearily familiar with this outcome, whether in their own code or other people’s, since at least Brooks (1975). Both the cause and the effect of schedule crunch are widely-known and well-understood: often the effect is so well-known that it can be specifically detected by reviewers and end-users.

Here, by contrast, is the fashionable new agile model:

+------------+------------+------------+
| Red        | Green      | Refactor   |
+------------+------------+------------+
^ Start                                ^ Deadline

In this case, “red” means you write the unit-tests first, but of course they fail (red light), because you haven’t written the code yet. Then you write the code, so they pass (“green” light). But so far what you’ve done is “the simplest thing that could possibly work” – in other words, you’ve been deliberately closely-focussed, overtly short-termist, to get your tests to pass, and a refactoring stage is needed to reintegrate your new functionality with the bigger picture.

Agile, of course, involves many of these cycles between project start and project deadline, not just one. (Indeed, some say that each cycle should be as small as a small tomato. I find that the going rate is two small tomatoes to one cup of tea.) So the agile diagram isn’t drawn to quite the same scale as the waterfall one: still, though, developers acquire the same sense of schedule crunch, they skip on to the next task too soon, and the corresponding failure mode occurs:

+------------>>>+------------>>>+------+.....+
| Red           | Green         | Refac|
+------------>>>+------------>>>+------+.....+
^ Start                                ^ Deadline

The cause is the same, but what’s the effect? The code was complete, and – if not perfect – then at least unit-tested, at the end of the green phase. So the product as actually shipped works, which is more than can be said for its waterfallen equivalent.

All that’s missing, in fact, is some of the refactoring effort. Unfortunately, that’s the only place in agile development where any large-scale design work gets done: the design debt that an agile shop takes out by not doing Big Design Up-Front, is paid off only in these refactoring installments. This means that in fact the effect of schedule crunch on agile projects, is that the system ends up under-designed and directionless at any other than the lowest level.

And unlike the paper-bag obviousness of the waterfall model’s failure mode, the agile model’s failure mode is subtle and pernicious. Product 1.0 ships and works – because agile development “releases” every sprint, and thus is perfectly fitted for triaging features against time. But the system is a ball of mud. Feature development on Product 1.5 and Product 2.0 takes longer than expected – which agile development also helps to hide, given its stubborn reluctance to countenance long-term planning – because developers eventually spend all their time battling, not intrinsic problems of the target domain, but incidental problems caused by previous instances of themselves.

Only the most obsessed agiliste would claim that agile development doesn’t have a failure mode. But because agile development is new, its failure mode is unfamiliar to us; and because that failure mode is less visibly catastrophic than Brooks’s, it’s easier to overlook. It is, however, real; its very subtlety requires us to pay particular care to look out for it, and to get right on top of fixing it once we see it start to happen.

Conversely, the fact that there are problems that agile development can’t solve, isn’t a fatal blow. Inevitably such problems are the most visible ones – because all the problems which agile development does solve easily, come and go without anyone really noticing. And the failure mode of agile development – the system’s complexity spiralling out of control – can be fixed without doing too much damage to the theory.

And how to fix it? Schedule in some serious refactoring, one subsystem at a time. In his paper Checklist for Planning Software System Production, RW Bemer, writing in August 1966 (August 1966!) says:

Is periodic recoding recommended when a routine has lost a clean structural form?

Nearly 45 years later, that’s still effectively the best available advice. Whether you call it refactoring or periodic recoding, it of course takes advantage of all the unit tests that the ball of mud already contains. This time round, it also takes advantage of knowledge about how all the parts of the subsystem operate. That knowledge is unlikely to be acquired in a single two-week sprint, so unless you can put someone on the task who already knows the subsystem inside-out (and most of the time you’re in this state, there won’t even be such a person), you’ll find yourselves breaking some of the rules of agile development by formally or otherwise block-booking someone’s time for a larger period. (Agile development aims at having any team member able to take on any task in any sprint – but for that to be okay, there mustn’t be any software problems complex enough to require more than two weeks’ thought. Some software problems just are that big, and context-switching can break the developers’ train of thought.)

This is the answer to the sometimes-asked question, “If, in agile development, everyone does design [as often as once per small tomato], what’s the rôle of the architect?”. Agile development is, in a way, Christopher Alexander’s observation that most things can be made piecemeal. But simplicity cannot be made piecemeal. The contribution of the software architect is simplicity.

Is “Factory Method” An Anti-Pattern?

Let’s take another look at this version of the “two implementations, one interface” code from that one about portability:

// Event.h v8
class Event {
public:
  virtual ~Event() {}
  virtual void Wake() = 0;
  ...
};

std::auto_ptr<Event> CreateEvent();

// Event.cpp
std::auto_ptr<Event> CreateEvent()
{
  ... return whichever derived class of Event is appropriate ...
}

What I didn’t say at the time is that this is sort-of the Factory Method pattern, though a strict following of that pattern would instead have us put the CreateEvent function inside the class as a static member, Event::Create(). And the pattern also includes designs where CreateEvent is a factory method on a different class from Event, but it‘s specifically “self-factory methods” such as Event::Create that I’m concerned with here.

(As an aside: the patterns literature comes in for a lot of criticism for being simplistic. Which it is: the GoF book could be a quarter the size if it engaged in less spoon-feeding and pedagoguery. (And by pedagoguery I mean pedagoguery.) But in a sense the simplicity and obviousness of the patterns they’re describing is the whole point: thanks to the patternistas (patternauts?), a lot of simple and obvious things that often come up in programmers’ natural discourse about code, and that didn’t previously have names, now do. Reading a patterns book might not make your code much better unless you’re a n00b, but that’s not what it’s for. It’s for making your discourse about code better. In any n>1 team, being able to discourse well about code is a vital skill.)

But what I also didn’t say at the time, is that whether CreateEvent is a member or not, so long as it’s in event.h, this code appears to have cyclic dependencies — to be a violation of the principle of “levelisability” set out in the Lakos book.

What’s going on, as you can see on the left, is that, although the source file dependencies themselves don’t exhibit any cycles, viewing, as Lakos does, each header and its corresponding .cpp file as forming a component — the three grey rectangles — produces a component dependency graph with cycles: win32/event ↔ event ↔ posix/event.

One way around that would be to move CreateEvent out into its own component — a freestanding event factory — as seen on the right. With this change, the design is fairly clearly levelisable at both the file level and the component level. This refactoring is an example of what Lakos (5.2) calles escalation: knowledge of the multifarious nature of events has been kicked upstairs to a higher-level class that, being higher-level, is allowed to know everything. (The file event.cpp now gets a question-mark because, as the implementation file for what may now be a completely abstract class, it might not exist at all — or it might exist and contain the unit tests.)

But is it worth it? We’ve arguably complicated the code — requiring users of events to know also about event factories — for what’s essentially the synthetic goal of levelisability: synthetic in the sense that it won’t be appearing in any user-story. Any subsequent programmer working on the code would be very tempted to fold the factory back into event.h under the banner of the Factory Method pattern.

Moreover, in this case the warning is basically a false positive: if Event is truly an abstract class (give or take its factory method), then the apparent coupling between, say, posix/event and event is not in fact present: posix/event can be unit-tested without linking against event.o nor win32/event.o. (Not that, in this particular example, posix/event and win32/event would both exist for the same target — but factory methods obviously also get used for cases where both potential concrete products exist in the same build.) Though conversely, if Event had any non-abstract methods — any with implementations in event.cpp — then it’d be a true positive not a false positive, as all the different event types would be undesirably link-time coupled together.

One reason that the refactoring is worth it, is the same sort of reason that fixing compiler warnings, i.e. altering the code so they don’t trigger, is worth it, even in instances when the warning doesn’t point out a bug: because if you let warnings proliferate, real ones will get lost in the noise, and ideally you aim for the zero-warnings state in order that the introduction of any new warning is an easily-visible alert telling you that there’s a new potential bug to check for. Steve Maguire is talking here about unit tests, but the same applies to compiler warnings: “[W]hen they corner a bug, they grab it by the antennae, drag it to the broadcast studio, and interrupt your regularly-scheduled program”.

Exactly like compiler warnings, cyclic-dependency warnings — which are really design warnings — are sometimes false positives, but likewise it’s worth aiming for the “zero design warnings” state, because it makes new design warnings stand out so. I ran a cycle-checker script (in the spirit of the ones in Lakos) over my own Chorale project, and the result was that it effectively shone a spotlight on all the parts of the code where the design was a bit icky. Every cycle it produced — one was dvb::Service ↔ dvb::Recording, another was all the parts of db::steam depending on each other in a big loop — was a place where I’d at some time thought, “Hmm, this isn’t quite right, but it works for now and I’ll come back and do it properly”. And of course without anything to remind me, I never had gone back and done it properly.

So it turns out that you can’t have both factory methods and levelisability. You have to pick one or the other. And levelisability is better.

Quirking a Mac Pro into AHCI mode

The Apple Mac Pro (2006 model) has an Intel ESB2 south-bridge, which includes the SATA controller. Under MacOS, this SATA controller is driven in AHCI mode, which is a great improvement on the “traditional IDE” mode — for one thing, it enables “NCQ” SCSI-like command queueing. But the pseudo-BIOS that boots non-MacOS operating systems on Intel Macs, forces the chip into traditional-IDE mode. This is certainly a good idea for wide compatibility, but for an OS which has AHCI drivers — such as Linux — it’d be good to force it back again.

Instructions do exist for forcing it back again using a hacked Grub bootloader, but for one thing that’s a hack, and for another I don’t use Grub, I use Lilo. The “right” way (well, other than convincing Apple to put an AHCI mode into their pseudo-BIOS) is to use Linux’s “PCI quirk” infrastructure. This patch (against 2.6.35.6, but similar should apply almost anywhere) gives me toasty AHCI goodness under Linux:

--- drivers/pci/quirks.c~ 2010-09-27 01:19:16.000000000 +0100
+++ drivers/pci/quirks.c 2010-10-06 20:29:04.000000000 +0100
@@ -1044,6 +1044,15 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDO
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_HUDSON2_SATA_IDE, quirk_amd_ide_mode);
 DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_HUDSON2_SATA_IDE, quirk_amd_ide_mode);
 
+static void __devinit quirk_intel_esb2_ahci(struct pci_dev *pdev)
+{
+    pci_write_config_byte(pdev, 0x90, 0x40);
+    pdev->class = PCI_CLASS_STORAGE_SATA_AHCI;
+    dev_info(&pdev->dev, "Intel ESB2 AHCI enabled\n");
+}
+
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2680, quirk_intel_esb2_ahci);
+
 /*
  * Serverworks CSB5 IDE does not fully support native mode
  */

And here it is in my dmesg:

pci 0000:00:1f.2: Intel ESB2 AHCI enabled
ahci 0000:00:1f.2: flags: 64bit ncq pm led slum part 
ata1.00: 488397168 sectors, multi 16: LBA48 NCQ (depth 31/32)

All six SATA ports (four for the drive carriers, two by the optical drives) are seen by the ahci driver (which I have built-in to the kernel); loading the ata_piix driver as a module also enables the PATA port by the optical drives. (It's probably not a good idea to compile both ahci and ata_piix into the kernel, in case they fight over the ESB2 controller.)

Job’s a good ’un.

Portability, Or Cut-And-Paste?

Suppose you’ve got some functionality with two completely different implementations on different platforms:

// Event.h v1
class Event // Win32 waitable event
{

  void *m_hEvent;
public:
  Event() : m_hEvent(::CreateEvent()) {}

  void Wake();
  ...
};

versus

// Event.h v2
class Event // Posix waitable event
{
  int m_fd[2];
public:
  Event() { ::pipe(m_fd); } // or maybe eventfd on Linux
  void Wake();
  ...
};

And, by the way, if you’ve got a class where only some of the implementation is different on different platforms, you’ve first got an opportunity for refactoring to extract that part of the implementation as a self-contained class. Portability and maintainability are aided if each of your classes is either wholly portable, or wholly platform-specific. But as it is, you’ve got a more immediate problem, which is that there’s two classes both called Event, and any translation unit that includes both headers isn’t going to compile. Here’s one solution:

// Event.h v3
#ifdef WIN32
class Event
{
  ...
};
#else
class Event
{
  ...
};
#endif

This, plus its close relative

// Event.h v4
#ifdef WIN32
#include "win32/Event.h"
#else
#include "posix/Event.h"
#endif

could well be the most commonly-encountered solutions to this problem in real-world code. But they’re not without problems. One problem is with code analysis tools, including such things as production of automated documentation with Doxygen. Really you want such tools to analyse your whole codebase, and if they do they’ll get hopelessly confused with the two classes both called Event. (You can set up preprocessor defines in Doxygen so that it only sees one Event class -- but that only means the other doesn’t get documented, or that you have two entire sets of Doxygen output for the two platforms, neither of which sounds desirable.)

To solve this problem, you’ve going to have to give the classes different names:

// Event.h v5
class Win32Event
{
  ...
};

class PosixEvent
{
  ...
};

No, no, no, this is C++, there’s a built-in language facility especially for namespaces, you don’t have to reinvent it in the class names themselves:

// Event.h v6
namespace win32 {
class Event
{
  ...
};
} // namespace win32

namespace posix {
class Event
{
  ...
};
} // namespace posix

(Why the comments on the “}” end-of-namespace lines? Because the error messages you get from C++ when you accidentally miss out an end-of-namespace brace are often insane and impenetrable, especially if you do it in a header file, and it’s convenient to be able to grep namespace *.h *.cpp and check whether braces appear in matching pairs in the grep output.)

Notice that both declarations are parsed on every compilation on either platform. This might require you to move some method definitions (those calling such unportable APIs as ::CreateEvent and ::pipe) into a cpp file. But even compiling the other platform’s declarations helps ensure that changes on one platform won’t break the other platform without somebody noticing. And a Doxygen run, or other automated code analysis, will at least get to see both sets of declarations even if not both sets of definitions.

While going down the route of helping platform developers avoid breaking other platforms, it’s been conspicuous so far that there’s nothing in Event.h making sure that the Win32 and Posix implementations keep the same API. Without such enforced consistency, there’s a risk of client code unwittingly using non-portable APIs. The way to enforce the consistency, of course, is to derive from an abstract base class containing the API:

// Event.h v7
class EventAPI {
public:
  virtual ~EventAPI() {}
  virtual void Wake() = 0;
  ...
};

namespace win32 {
class Event: public EventAPI
{
  ...
};
} // namespace win32

namespace posix {
class Event: public EventAPI
{
  ...
};
} // namespace posix

This is, after all, exactly the sort of thing you’d do if you had two different sorts of Event that were both used in the same build of the program. Really the fact that no one individual binary will contain instances of both classes, doesn’t mean that the quest for well-factored design should be thrown out of the window and replaced with cut-and-paste.

Now, though, all the method calls have become virtual function calls. For most of any given codebase, that won’t matter, but there’ll always be hot paths and/or embedded systems where it does, and indeed an event class might quite plausibly be on such a critical path. And after all, compiling the client code itself provides a, potentially large, body of checks that the API has remained consistent over time. It’s reasonable to adopt the position that using design techniques to discourage unthinkingly changing core APIs in a non-portable way isn’t actually necessary, so long as the compilation-failure results of such a change are always speedily available from all target platforms, such as from an automated-build or continuous-integration system.

If, conversely, your class is only used in situations where nobody’s really counting individual machine cycles, you could hide the implementations altogether, at the cost of ruling out stack objects and member variables of type Event and requiring a heap (new) allocation every time one is created:

// Event.h v8
class Event {
public:
  virtual ~Event() {}
  virtual void Wake() = 0;
  ...
};

std::auto_ptr<Event> CreateEvent();

// Event.cpp
std::auto_ptr<Event> CreateEvent()
{
  ... return whichever derived class of Event is appropriate ...
}

But for now let’s assume you can’t really conceal the two platform-specific declarations, and go back to Event.h version 6 or version 7. Let’s look at how client code gets to pick which of the two implementations it uses. For a start, not like this:

class NominallyPortableDomainAbstraction
{
#ifdef WIN32
  win32::Event m_event;
#else
  posix::Event m_event;
#endif
  ...
  m_event.Wake();
  ...
};

Flouting our design rule that each class is either wholly portable or wholly platform-specific, this sort of thing couples portability decisions into all client code, which is very undesirable. This is much better:

// Event.h v9
... as before in v6 or v7

#ifdef WIN32
typedef win32::Event Event;
#else
typedef posix::Event Event;
#endif

// Client code
class NominallyPortableDomainAbstraction
{
  Event m_event;
  ...
  m_event.Wake();
  ...
};

It’s still not great, though: if there are lots of classes involved, lots of code ends up inside the #ifdefs. How can we minimise the amount of #ifdef’d code? Well, here’s one way:

// Event.h v10
... as before in v6 or v7

#ifdef WIN32
using namespace win32:
#else
using namespace posix;
#endif

But this, of course, introduces everything from those namespaces into the surrounding namespace. Not only is that namespace pollution, it’s potentially misleading for client code, as there may be Win32-specific classes or functions -- ones with no Posix equivalent, for use in Win32 situations only -- which are now accessible in portable code without the telltale win32:: prefix. (And, of course, vice versa for Posix-specific ones.) Really we want to explicitly enumerate which classes and APIs are intended to be portable. So we can do this:

// Event.h v11
... as before in v6 or v7

#ifdef WIN32
namespace platform = win32;
#else
namespace platform = posix;
#endif

using platform::Event;
using platform::PollThread;
...

The intention is that the using declarations list precisely those facilities which are available on all platforms, but with differing implementations on each. Non-portable classes or functions can stay in namespace win32 (or namespace posix) so that any use of them in client code immediately flags that code up as itself non-portable.

There is one problem with this neat solution, though: it doesn’t actually compile. Or rather, once you do the same thing in several different header files, you’ll find that multiple namespace X = Y; statements for the same namespace X aren’t allowed, even if Y is the same each time. So, sadly, you have to come up with a different name for “platform” each time (or centralise the using in one header file, causing potentially-undesirable widespread dependency on that file):

// Event.h v12
... as before in v6 or v7

#ifdef WIN32
namespace eventimpl = win32;
#else
namespace eventimpl = posix;
#endif

using eventimpl::Event;
using eventimpl::PollThread;
...

There’s only one remaining wrinkle, which is that you can’t forward-declare a class-name if that name only exists due to using. So if there are header files that traffic in Event* or Event& and could otherwise be satisfied with a forward declaration class Event; and avoid including Event.h, then you can’t use Event.h version 12. (Version 9 is also out, as you can’t forward-declare typedefs either.) The best you can do is probably this:

// Event.h v13
... as before in v6 or v7

#ifdef WIN32
namespace eventimpl = win32;
#else
namespace eventimpl = posix;
#endif

class Event: public eventimpl::Event {};
class PollThread: public eventimpl::PollThread {};
...

although naturally that only works as-written if the eventimpl base classes don’t have constructors other than the default constructor; if they did, you’d have to write forwarding constructors in each derived class, making the code a lot less neat.