Slashdot Mirror
Stop Breaking the Build
Cap'n Grumpy writes "You know the score - you've just finished some coding, do a final cvs update before commiting, and all of a sudden all hell breaks loose. Your code now refuses to compile, or xunit starts flashing up red - test failures! One of the other members of your team has checked in something which breaks the build, and they just went out for lunch ... Argh! Did you know there is a solution to this problem? It is a system which makes it impossible for people to check in code which does not compile or test successfully. It allows coders to review others coding efforts code before it goes into the baseline, rather than after. It organises your checkins into logical change sets. It enforces continuous integration. It is linux based, and GPL'd. It's called Aegis."
Is it just me or does this sound like an advertisement?
At work, we use CVS, and our build tools guru has it set up so the checkins fail unless we've build without errors first. The testing isn't integrated (it can't be, as we're cross-compiling for an embedded system), and you can break the build by doing a partial checking or with bad interaction between different checkins, but for the most part it works really well.
It seems like common sense for most projects to refuse to allow checkins without building first, and that's the sort of policy that can have a fairly effective mechanism for enforcement.
(And for that matter, if you need to track software bugs & other issues, RT rocks. Don't bother with Bugzilla, it's not half as good as RT is for most of the same tasks. And no, no one is paying me to endorse RT or anything, it's just great software and, in reference to Aegis, I respect the judgement of the guy developing it...)
DO NOT LEAVE IT IS NOT REAL
There is a rule for check-ins when my team is trying to stablize code; that is have another developer go through your changes in case you did something silly. It also serves as a mini code review. This includes making sure the code builds. :)
Editing the same source files is common when you have a shared resource DLL for multiple programs. Think about it...everyone has to add strings, icons, dialogs, etc to the same .rc file. When we did it at my company, we just used resource IDs in different ranges, so merging was never a problem. Multiple people working on the same file is common!
2-3 times a month we get Krispy Kremes. That is the penance for breaking a nightly build. Engineer or build meister. Screw it up and bring in the donuts.
I have used, in my time, Clearcase (which I rather liked despite the high price tag and apparent inefficiency repository-side), CVS, and most recently Perforce. For all the complaining about CVS lacking sophistication, it does get 95% of the job of source code control done. But, none of these address the root problem, and it's unfair to pick on one for not addressing it, implying that the others do a better job.
Inconsistent checkins of code, even code in different files, can break builds, unit, and integration tests. Consider the development process:
You check out a read-only version of the top of the development tree, that builds clean, and passes all the tests. Great. You get a write checkout on the stuff you want to change. Even if others don't touch those files (which can be difficult to enforce for some kinds of files, like headers of unique enumerations that everyone updates), you can still break things.
When you test build, you test build with your stuff changed, and everything else frozen. There is no guarantee that when you check in your changes, the break will build because something else on which your code depends got changed. Like a header file. I suppose you could get a write lock on all the files on which your code depends, but that still isn't good enough.
Consider a processing sequence where two function in two code files get called. One of them is supposed to increment some global "the sequence was run" counter. Both do. Oops. Builds fine, fails regression testing.
Short of locking the entire source tree when one developer changes something, you can't avoid this in general. Oh sure, you can resync all the files you didn't change, and run a build and regression test before checking your change in, but, lo, you'd have to lock the entire source tree during that time (or at least see if it changed again after your build and test, and repeat the process if it did, possibly indefinately). Serializing an otherwise parallel development process that way is murder on productivity: even if you run all the sanity builds at night, you now have a one day turnaround just to test build all changes, and hope they make it into the source repository. Kinda sucks, when you just changed a few lines.
Any automated solution will have to rely on serialization of source tree access, at some level. If the project can be broken down into independent components, serialization within a component can be relatively painless, with somewhat less frequent serialization across components (so called "integration" which the nightly build strives to avoid because it happens all the time). Experience shows that, unless you want to plan "integration" phases, this defeats a large benefit of the nightly build process, though, it is not unreasonable for very large projects, with clearly independent parts.
So, what's the solution?
The same, it's always been: divide and conquer.
While "integration" phases are to be shunned, and "repository serialization" kills productivity, one can take a statistical approach: instead of verifying everything before a checkin to a frozen repository, you design your project to try to isolate the effects of implementation changes from one another. Early on, this may be difficult as interfaces are still being tweaked, but you have less code then, and builds happen quickly. This is a large part of what a project architect is supposed to do.
If done correctly, a local delta built with a recent snapshot of the source repository is not likely to break the build and unit test of a more recent snapshot if it builds and passes regression tests against the somewhat older snapshot. This isn't foolproof, of course, but a proper design will have the necessary isolation: an implementation change in one area, or the addition of a new interface should be oblivious to your code.
Now, if interfaces need to change, or new features are to be added that have to be coordinated among developers (i.e. someone adds a feature and someone else writes code to use it), then you need greater coordination among all those that might be affected. Such work can take place on a side branch, and merged to the main development tree only when it is internally consistent. Again, this is generally the responsibility of the common lead programmer of those implementing the changes that need to be coordinated (perhaps the project architect, but in large projects, that kind of detail can be delegated).
Where trouble brews is when such a synchronized change has to take place across development teams: it usually is more effective if one person handles the integration of the new feature and code that uses it. Because the feature user has the need, it may be him or her. However, review of code to be added to a base for which another team is responsible should, of course, rest with that team: "You code didn't have this which I needed, so here's the patch, wanna check it out?" Yes, this can cause political friction, but in a mature development team, that won't happen. When communication between teams is minimal, hostile, or non-existent, and such functionality is to be added, is when builds break, and regression tests fail -- and the finger pointing begins.
You could've hired me.
Well, obviously no source code control software is gonna compensate for developers who can't write good code, use common sense, and follow *simple* process. Granted, requiring complex process of your developers is asking for trouble, but you can't live without simple rules. Some simple rules really require being backed up by the capabilities of the software, though. An example that comes to mind: consistent atomic checkins.
It goes something like this: say you change a function signature. It is your responsibility to grep for all the uses of that function and change them. It is also your responsibility to check in all of those changes atomically. That is: an all-or-none checkin of a group of files all at once. That group is also bound together into the future (the relationship is not severed after checkin).
Another simple point of process that saves your ass is JOB TRACKING. If your source control repository doesn't link into a job tracking system, then I pitty you. I've been there, and it sucks. It took a while for us to work out exactly what was missing and how to get it... but now that we have implemented it, it makes life livable again. The idea with a job tracking system is: assosciate all of your changes with a job. If you want a bleeding edge revision, then sync to the head revision and don't be surprised when stuff breaks. If you want a rough-around-the-edges version to test against, then sync to the highest revision that is entered in the job tracker. Use the various life-cycle statuses in the job tracker to sync to various points. On the whole: Get all files in QA status, or all files in QATESTED status, or CODECHECK status (or however you choose to name these things in your job tracker), or whatever status you want.
In that way, you don't easily break the build, because before you even try to build off of something, you've tracked its code review and its unit testing. Of course, there are always the possibilities for unit testing problemss, but they are usually going to end up being the fault of a developer not following simple processes. In the example I used above where we changed the signature of a function, and updated all of the calls to that function in the same atomic change... you could have had another developer creating a *new* call to that function in their own working copy of the code. You would, of course never catch that, and they might not either... but hopefully whoever is doing code reviews has an eye open to things like function signature changes, and can catch it at that point.
Make it clear to your developers that changes not assosciated with a job will never see the light of day. Every night, review any untracked changes and email the developer, asking them what the hell.
It's true, there is no way to make everything 100% bullet-proof against checking in bad code. Of course, that's why we do things like freeze integration and test before a release.
:Wq
Not an editor command: Wq