The road to SSSD 1.0.3

So, today we released SSSD 1.0.3. We fixed a few bugs, but more importantly we did it in an open way.

I generally assume that anyone reading this blog has heard of open-source before. If not, I suggest you log on to the Internet more. More eloquent bloggers than I have done a better job explaining the open-source concepts. The SSSD is more of a case-study in why open-source works. In this last week, we’ve done a bit of work on our own inside the core SSSD team headed in the direction of our next feature release, but at the same time we were working with outside developers.

One of these developers took a look at the SSSD and decided that it might come in handy for use on embedded devices like smart routers. So he tried to build the SSSD on the ARM processor. This failed, because most of our development on the SSSD takes place on i686 or x86_64 systems (with some limited testing done on PowerPC). Here we have the first example of why open-source works: someone came in from Outside with an idea: that the SSSD might have a place beyond personal computers. Beyond that, with the source code available to them, they began the effort of implementing this idea. Unfortunately, in its current state of development, the SSSD was unable to run on the ARM platform. This developer then took it upon himself to dive into the code and identify the source of the problem (if you care about the details regarding alignment of memory, feel free to look at our git history. I’m not going to bore you any more than I have to). Once he identified the issues, he created a patch and submitted it to our mailing list. We reviewed it and included it in the 1.0.3 release today. Open-source works because people like this  not only identify new ideas, but they have the ability to run with them as well. In a traditional, closed-source development team, the process would have looked more like this: 1) Open a bug. 2) Wait to see if the company developing the product agrees with this idea. 3) Wait to see if the company has resources to expend on implementing the idea. 4) Wait for a release including this fix.  With open-source, even had the SSSD team decided not to release 1.0.3 for weeks or months, this gentleman’s patch was available for anyone to apply atop our public source and get to work on.

The second story I want to tell is about the benefits of cooperation between distributions. Another prominent GNU/Linux distribution recently began work on updating their SSSD package to 1.0.2 (previously they had been running a very old and buggy 0.5.0). They ran into a bit of trouble and contacted us through the usual channels to help them track down what was going wrong. It turned out that one of the differences between these two distributions was this (and I apologize, but there’s no real way to avoid going into a little bit of detail on this): on Fedora, when you link against a shared library, you automatically inheret any link that this library has. In our case, we linked against libldb, which in turn linked against libkrb5. On the other distribution, however, there is no implicit linking. All links need to be made explicitly. So we tracked down where we needed to link properly and added that. “But Steve,” you ask. “How is this an example of distro cooperation improving the product for both distros?”.  The answer is this: working with this other distribution helped us to avoid a painfully embarrassing situation in the coming months. The reason for this is because one of the features coming in Fedora 13 is the very same requirement for explicit linking. So thanks to the developers from the other distribution, we were able to fix this ahead of time and save ourselves a lot of trouble (and support calls!) when Fedora 13 arrives.

Code review methodologies

One of the most important parts of any development process is probably its least-beloved: the code-review. Any project that entertains a notion of one day being stable and production-ready knows that it needs to do at least periodic reviews of the source.

There are many different approaches to the code review process. Some projects allow open commits during the first phase of development, then lock the tree and do in-depth code reviews, followed by a “hard freeze” wherein only reviewed and approved changes are further permitted. On the other end of the spectrum, many open-source projects follow the “hard freeze” party for their entire development cycle. For these projects (among which the SSSD is one), every patch is reviewed by at least one gatekeeper.

The former approach can serve a young project quite well. At the start of development, there are usually many small commits, few of which are functional or testable. By the time the code hits its first stable (or at least public) release, I suggest that it’s time to switch to the gatekeeper approach. The team should examine its membership for a very short list of gatekeepers. These will be the only members from there on that will have privilege to commit to the master repository. These individuals are the final say on what patches are accepted into a particular development branch.

This may sound a bit authoritarian. Perhaps it even sounds contrary to an open development process. I contend that stability should be the ultimate goal of any project, above and beyond “shiny features”. To ensure that this remains an open process, it becomes necessary for the code review process to be designed to be completely transparent. For example, it is usually not necessary for the gatekeepers to be the only code-reviewers on a project. In most cases, the reviews can and should be performed by any member of the team. The gatekeepers need be only a sanity check to ensure that the code-review was performed by a qualified individual.

Ok, so that’s all well and good, but how does one design a transparent and effective code-review system? There are two common approaches, which I will discuss briefly, and then I’d like to discuss a less-common approach that is gaining traction.

All of the common code-review processes involve somehow getting a patch file in the hands of a reviewer.  One process favored by very well-established projects is the ticket-driven review process. For old and stable projects, it is common for the process to require that all changes to the source correspond to an issue-tracker ticket (such as a Bugzilla or Trac ticket). The code-review process will often be designed around this ticketing system. When a developer has a patch ready for review, it will be attached to the ticket and the appropriate flags/status will be set to indicate that it needs a reviewer. Potential reviewers will then be informed that the patch is ready (perhaps because a saved search will now reveal that the ticket is awaiting review). Code review comments and resubmissions will then be handled within ticket comments. Once the review is passed, a gatekeeper will take the patch and apply it to the master repository.

The second (and probably most popular) code-review process for open-source development is by email. When a patch is ready, a developer will email the patch as an attachment to an email sent to the developer mailing list for the project. Another developer will then take up the review, responding with an ack or nack (indicating corrections that need to be made). The gatekeeper will then take the patch and apply it to the master repository.

Both of these approaches have their merits and disadvantages. The ticket approach works well for well-established projects, providing easy history and a link to every issue or feature that the patches are meant to address. On the other hand, for young projects, the additional overhead of filing a ticket for every change can be unnecessarily heavy. Young projects tend to have a much more rapid development pace and filing a ticket for every change will usually serve only to train engineers to write short, worthless ticket descriptions.

The email approach is much more agile, but it is rather less transparent, as it requires all participants to be subscribed to the relevant list. It adds a barrier to entry for new contributors that may have only a single small patch to submit (and may not want to deal with a heavy-traffic development list). Further, the email approach adds additional effort on the part of the gatekeepers. It is necessary for gatekeepers to develop for themselves a system to ensure that patches are reviewed in a timely manner, as well as not forgotten.

To address these issues, some projects have begun looking to new code-review tools. As an example, I recently contributed to the packaging effort for the ReviewBoard tool. Tools such as this provide many benefits over the above approaches. It has built-in functionality to keep track of pending reviews and their status (approved, awaiting review, needs update etc.) Furthermore, they provide additional user interface enhancements, such as the ability to make review comments attached to the specific code that is being discussed. Such tools provide much greater visibility than the email approach, while retaining the same agility (especially if the tool is configured to announce updates to reviews on the developer mailing list).

As mentioned above, I’ve been working on packaging ReviewBoard for Fedora and EPEL, with the goal of making ReviewBoard available for projects hosted by the Fedora Hosted project. I think that the advantages provided by such a code-review system would contribute greatly to the security and stability of open-source software.