Bazaar bundles as part of a review process

In my previous article, I outlined Bazaar‘s bundle feature. This article describes how the Bazaar developers use bundles as part of their development and code review process.

Proposed changes to Bazaar are generally posted as patches or bundles to the development mailing list. Each change is discussed on the mailing list (often going through a number of iterations), and ultimately approved or rejected by the core developers. To aide in managing these patches Aaron Bentley (one of the developers wrote a tool called Bundle Buggy.

Bundle Buggy watches messages sent to the mailing list, checking for messages containing patches or bundles. It then creates an entry on the web site displaying the patch, and lets developers add comments (which get forwarded to the mailing list).

Now while Bundle Buggy can track plain patches, a number of its time saving features only work for bundles:

  1. Automatic rejection of superseded patches: when working on a feature, it is common to go through a number of iterations. When going through the list of pending changes, the developers don’t want to see all the old versions. Since a bundle describes a Bazaar branch, and it is trivial to check if one branch is an extension of another though, Bundle Buggy can tell which bundles are obsolete and remove them from the list.
  2. Automatically mark merged bundles as such: the canonical way to know that a patch has been accepted is for it to be merged to mainline. Each Bazaar revision has a globally unique identifier, so we can easily check to see if the head revision of the bundle is in the ancestry of mainline. When this happens, Bundle Buggy automatically marks them as merged.

Using these techniques the list of pending bundles is kept under control.

Further Possibilities

Of course, these aren’t the only things that can be done to save time in the review process. Another useful idea is to automatically try and merge pending bundles or branches to see if they can still be merged without conflicts. This can be used as a way to put the ball back in the contributors court, obligating them to fix the problem before the branch can be reviewed.

This sort of automation is not only limited to projects using a mailing list for code review. The same techniques could be applied to a robot that scanned bug reports in the bug tracker (e.g. Bugzilla) for bundles, and updated their status accordingly.

Bazaar Bundles

This article follows on from the series of tutorials on using Bazaar that I have neglected for a while. This article is about the bundle feature of Bazaar. Bundles are to Bazaar branches what patches are to tarballs or plain source trees.

Context/unified diffs and the patch utility are arguably one of most important inventions that enable distributed development:

  • The patch is a self contained text file, making it easy to send as an email attachment or attach to a bug report.
  • The size of the patch is proportional to the size of the changes rather than the size of the source tree. So submitting a one line fix to the Linux kernel is as easy as a one line fix for a small one person project.
  • Even if the destination source tree has moved forward since the patch was created, the patch utility does a decent job of applying the changes using heuristics to match the surrounding context. Human intervention is only needed if the edits are to the same section of code.
  • As patches are human readable text files, they are a convenient form to review the code changes.

Of course, patches do have their limitations:

  • The unified diff format doesn’t convey file moves, instead showing the entire file content being removed and then added again. If the file was changed in addition to being moved, the change can easily be missed when reviewing the patch.
  • Changes to binary files are omitted from the patch. While we can’t expect such changes to be represented in a human readable form, it’d be nice for them to be represented in a way that they can be applied at the other end.
  • The patch doesn’t record any intermediate steps in the creation of the change. This can be worked around by sending a sequence of patches that each build on the previous one, but this requires a fair bit of attentiveness on the part of the patch creator.
  • If the project in question is using some form of version control, the changes in the patch will likely be attributed to the person who applied the patch rather than the person who made the patch.

Using distributed version control solves these limitations, but simply publishing a branch and telling someone to pull from it does not provide all the benefits of a patch. For one, the person reviewing the changes needs to be online to merge the branch and evaluate the changes.

Second, the contributor of the change needs somewhere to host the branch. Even though finding a place to host the branch may not be difficult (for example, anyone can host their branches on Launchpad), uploading the branch may be more effort than the contributor cares for (uploading a branch the size of the Linux kernel will take a while, for instance). That branch would need to remain available until the changes were accepted.

For Bazaar, bundles provide a solution to this problem. A bundle is effectively a “branch diff”, which can then be used to integrate a set of revisions into a repository assuming it contains the revisions from the target branch. At this point, those changes can be merged or pulled.

So how do we produce a bundle? Lets start by creating a branch of the project we want to contribute to. For this example, we’ll create a branch of Mailman to make our changes. As Mailman is using Launchpad to host its branches, I can use the shorthand implemented by the Launchpad Bazaar plugin to create my branch:

bzr branch lp:mailman mailman.jamesh
cd mailman.jamesh
# make my changes here
bzr commit

After I am happy with my changes, I can create a bundle of those changes:

bzr bundle > my-changes.diff

As mentioned earlier, a bundle is essentially a diff between two branches. As I did not specify any branch in the above command, Bazaar uses the parent branch, which in this case will be the upstream Mailman branch. If we look at my-changes.diff, we will see a text file with three general sections:

  1. A short header identifying the file as a bundle and giving the last commit message, author and date
  2. A unified diff made between the last common revision with the parent and the head of our branch (this bit is also convenient to review).
  3. Some extra book keeping data. If I’d made multiple commits, this would include data needed to reconstruct the other revisions in the bundle.

I can now submit this bundle in the same way that I’d submit a patch: as an email attachment or in the bug tracker.

To merge the bundle, a developer simply needs to save the bundle to disk and use “bzr merge” on it:

bzr merge my-changes.diff
bzr commit

This will have the same effect as if they merged a branch with those changes. The “bzr log” output will show the merged revisions and “bzr annotate” will credit the changes to the person who made them rather than the person who merged it.

So next time you want to submit a patch to a project that uses Bazaar, consider submitting a bundle instead.

Storm Released

This week at the EuroPython conference, Gustavo Niemeyer announced the release of Storm and gave a tutorial on using it.

Storm is a new object relational mapper for Python that was developed for use in some Canonical projects, and we’ve been working on moving Launchpad over to it. I’ll discuss a few of the nice features of the package:

Loose Binding Between Database Connections and Classes

Storm has a much looser binding between database connections and the classes used to represent records in particular tables. The standard way of querying the database uses a store object:

for obj in store.find(SomeClass, conditions):
    # do something with obj (which will be a SomeClass instance)

Some things to note about this syntax:

  • The class used to represent rows in the table is passed to find(), so it is possible to have multiple classes representing a single table. This can be useful with large tables where you are only interested in a few columns in some cases.
  • The class used to represent the table is not bound to a particular connection. So instances of it can come from different stores.

Lockstep Iteration

As well as iterating over a single table, a Storm result set can iterate over multiple tables together. For instance, if we have a table representing people and a table representing email addresses (where each person can have multiple email addresses), it is possible to iterate over them in lockstep:

for person, email in store.find((Person, Email), Person.id == Email.person):
    print person.name, email.address

Automatic Flushing Before Queries

One of the gotchas when using SQLObject was the way it locally cached updates to tables. This is a great way to reduce the number of updates sent to the database, but could result in unexpected results when performing subsequent SELECT queries. It was up to the programmer to remember to flush changes before doing a query.

With Storm, the store will flush pending changes automatically before performing the query.

Easy To Execute Raw SQL

An ORM can really help when developing a database driven application, but sometimes plain old SQL is a better fit. Storm makes it easy to execute raw SQL against a particular store with the store.execute() method. This method returns an object that you can iterate over to get the tuples from the result set. It also makes sure that any local changes have been flushed before executing the query.

Nice Clean Code

After working with SQLObject for a while, Storm has been a breath of fresh air. The internals are clean and nicely laid out, which makes hacking on it very easy. It was developed using test-driven development methodology, so there is an extensive test suite that makes it easy to validate changes.

gnome-vfs-obexftp 0.4

It hasn’t been long since the last gnome-vfs-obexftp release, but I thought it’d be good to get these fixes out before undertaking more invasive development. The new version is available from:

http://download.gnome.org/sources/gnome-vfs-obexftp/0.4/

The highlights of this release are:

  • If the phone does not provide free space values in the OBEX capability object, do not report this as zero free space. This fixes Nautilus file copy behaviour on a number of Sony Ericsson phones.
  • Fix date parsing when the phone returns UTC timestamps in the folder listings.
  • Add some tests for the capability object and folder listing XML parsers. Currently has sample data for Nokia 6230, Motorola KRZR K1, and Sony K800i, Z530i and Z710i phones.

These fixes should improve the user experience for owners of some Sony Ericsson phones by letting them copy files to the phone, rather than Nautilus just telling them that there is no free space. Unfortunately, if there isn’t enough free space you’ll get an error part way through the copy. This is the best that can be done with the information provided by the phone.

Test Suite

As noted in the third point, I’ve started to build up a collection of capability and folder listing XML documents produced by various phone models. This serves a dual purpose:

  1. Ensure that the capability object and folder listing XML parsers don’t regress between releases. It is impractical for me to test gnome-vfs-obexftp against all these phone models since I don’t have the hardware or time.
  2. Give an idea of what information the different phone models provide, which should be useful when planning new features.

If you have an OBEX FTP capable phone that is not already in the test suite, it’d be useful if you could collect the data and file a bug. The information can be collected using the command line “obexftp” program (part of the “obexftp” package on Ubuntu). The following commands will give the capability object and root folder listing:

obexftp --bluetooth $BDADDR --capability
obexftp --bluetooth $BDADDR --list

It’d also be useful to get a listing for one or two other directories. If there is a memory card, it’d be useful to get that folder. For example:

obexftp --bluetooth $BDADDR --list "Memory card/"

It’d be most useful if the transcript of the various commands were included as an attachment. Feel free to censor personal information if you want (e.g. the phone serial number in the capability object, some non-default file names).

In particular, I wouldn’t mind getting information on phones with brands other than than Nokia or Sony to see what info they provide.

Investigating OBEX over USB

I’ve had a number of requests for USB support in gnome-vfs-obexftp. At first I didn’t have much luck talking to my phone via USB. Running the obex_test utility from OpenOBEX gave the following results:

$ obex_test -u
Using USB transport, querying available interfaces
Interface 0:   (null)
Interface 1:   (null)
Interface 2:   (null)
Use 'obex_test -u interface_number' to run interactive OBEX test client

Trying to talk via any of these interface numbers failed. After reading up a bit, it turned out that I needed to add a udev rule to give permissions on my phone. After doing so, I got a better result:

$ obex_test -u
Using USB transport, querying available interfaces
Interface 0: Nokia Nokia 6230 (null)
Interface 1: Nokia Nokia 6230 (null)
Interface 2: Nokia Nokia 6230 (null)
Use 'obex_test -u interface_number' to run interactive OBEX test client

With the change, I was also able to access the phone using the obexftp command line client. This seemed enough to start investigating a bit further. The OpenOBEX API for setting up USB connections goes something like this:

  1. The app calls OBEX_FindInterfaces(), which returns a list of obex_interface_t structures that represent the different discovered interfaces.
  2. The app picks one of the discovered interfaces (based on the manufacturer, product and serial number strings), then connects to it using OBEX_InterfaceConnect().

There are a number of issues with this interface though.

  • If the phone doesn’t provide a serial number via its USB interface (like my 6230 doesn’t), the obex_interface_t structure is not enough to identify a particular phone.
  • If the phone exposes multiple OBEX USB interfaces for some reason, OpenOBEX lists it multiple times. In the obex_test output shown above, there was a single phone attached – not three.
  • There is no way to tell when phones are connected or disconnected. While HAL can do that job for us, there is no way to map from the device information provided by HAL to one of the discovered interfaces provided by OpenOBEX.

To sum up, it shouldn’t be difficult to hack support for USB connections into gnome-vfs-obexftp with URLs like obex://usb-N/ (where N is the number of the discovered interface), but there are a number of features I’d need to provide a good user experience:

  1. The ability to ask OpenOBEX to connect to a particular USB device, rather than having to deal with its discovery interface.
  2. A good set of udev rules to grant the needed permissions on common phones so they don’t need to find out why things only work as root.

TXT records in mDNS

Havoc: for a lot of services advertised via mDNS, the client doesn’t have the option of ignoring TXT records if it wants to behave correctly.

For example, the Bonjour Printing Specification puts the underlying print queue name in a TXT record (as multiple printers might be advertised by a single print server). While it says that the server can omit the queue name (in which case the default queue name “auto” is used), a client is not going to be able to do what the user asked without checking for the presence of the record.

Rather than thinking of TXT records as optional data, it is better to think of them as “stuff that is can not be used to perform searches”. In the printer example above, the fact that you can’t search by print queue name is not a problem because users instead pick a printer based on the human readable service name which is exposed as a DNS name.

In your example of including session and machine identifiers in TXT data, it would be enough to tell a client that two services belonged to the same machine or session, but it wouldn’t let you do searches like “find all the card game servers belonging to the same session as the guy I’m chatting with”.  For that you’d also need to advertise DNS names that include the machine identifier or session identifier.

gnome-vfs-obexftp 0.3

I’ve just released a new version of gnome-vfs-obexftp, which includes the features discussed previously. It can be downloaded from:

http://download.gnome.org/sources/gnome-vfs-obexftp/0.3/

The highlights of the release include:

  • Sync osso-gwobex and osso-gnome-vfs-extras changes from Maemo Subversion.
  • Instead of asking hcid to set up the RFCOMM device for communication, use an RFCOMM socket directly. This is both faster and doesn’t require enabling experimental hcid interfaces. Based on work from Bastien Nocera.
  • Improve free space calculation for Nokia phones with multiple memory types (e.g. phone memory and a memory card). Now the free space for the correct memory type for a given directory should be returned. This fixes various free-space dependent operations in Nautilus such as copying files.

Any bug reports should be filed in Launchpad at:

https://bugs.launchpad.net/gnome-vfs-obexftp/

New stuff in gnome-vfs-obexftp

Recently, Bastien has been doing a bit of hacking on gnome-vfs-obexftp. The changes remove the use of the org.bluez.RFCOMM interface to hcid, instead doing SDP and createing RFCOMM sockets directly. This removes the need to run hcid with its experimental interfaces enabled. This is also good because the org.bluez.RFCOMM interface has been removed in newer releases (replaced by org.bluez.serial.Manager).

I’ve now integrated that patch and made a number of further clean ups so that we don’t need any D-Bus requests to establish the connection to the remote device. Now the only D-Bus requests being made are for device enumeration used for the “obex:///” virtual folder. Should make it easier to switch over to HAL if/when it supports scanning for Bluetooth devices.

I also had a go at fixing bug #116912. Using the memory type information provided in folder listings on Nokia phones, I’ve now got the get_volume_free_space() routine to return the free space of the appropriate memory type for the folder. While this works fine in the GNOME-VFS API, unfortunately it hasn’t improved matters in Nautilus. It seems to always request the free space for the root directory, so I still see an incorrect free space value for e.g. obex://[...]/Memory%20card/ on my phone. I’m not sure how I’ll go about fixing that.

There does appear to be some problems with some Sony phones, but I don’t have any hardware to test this. Perhaps this is something to work on in the next release if I can get some help debugging the problem.

Update: looks like I spoke too soon about Nautilus not getting the correct free space value.  After restarting Nautilus again, it started showing the free space correctly, and I can copy large files to my memory card.

Stupid Patent Application

I recently received a bug report about the free space calculation in gnome-vfs-obexftp. At the moment, the code exposes a single free space value for the OBEX connection. However, some phones expose multiple volumes via the virtual file system presented via OBEX.

It turns out my own phone does this, which was useful for testing. The Nokia 6230 can store things on the phone’s memory (named DEV in the OBEX capabilities list), or the Multimedia Card (named MMC). So the fix would be to show the DEV free space when browsing folders on DEV and the MMC free space when browsing folders on MMC.

Doing a bit of investigation, I found that the information I wanted was in the folder listings:

<?xml version="1.0"?>
<!DOCTYPE folder-listing SYSTEM "obex-folder-listing.dtd"
 [ <!ATTLIST folder mem-type CDATA #IMPLIED> ]>
<folder-listing version="1.0">
    <folder name="Memory card" user-perm="RW" mem-type="MMC"/>
    <folder name="Images" created="19800101T000008" user-perm="R" mem-type="DEV"/>
    ...
</folder-listing>

I took a look through the OBEX specification, and this mem-type wasn’t defined. So it looked like a Nokia extension. Doing a quick search, the closest I came to a describing it was US patent application #20060095537.

So Nokia is effectively trying to patent an XML attribute. I’d seen a lot of bad patents, but this seemed particularly weak. The patent application even comes with a useful diagram explaining the invention:

Figure 2

As far as I can tell, using the information returned from the phone wouldn’t be covered by the patent (if it gets issued, that is). So it should be fine to use the information to calculate free space more accurately.

FM Radio in Rhythmbox – The Code

Previously, I posted about the FM radio plugin I was working on. I just posted the code to bug 168735. A few notes about the implementation:

  • The code only supports Video4Linux 2 radio tuners (since that’s the interface my device supports, and the V4L1 compatibility layer doesn’t work for it). It should be possible to port it support both protocols if someone is interested.
  • It does not pass the audio through the GStreamer pipeline. Instead, you need to configure your mixer settings to pass the audio through (e.g. unmute the Line-in source and set the volume appropriately). It plugs in a GStreamer source that generates silence to work with the rest of the Rhythmbox infrastructure. This does mean that the volume control and visualisations won’t work
  • No properties dialog yet. If you want to set titles on the stations, you’ll need to edit rhythmdb.xml directly at the moment.
  • The code assumes that the radio device is /dev/radio0.

Other than that, it all works quite well (I’ve been using it for the last few weeks).

Development

I developed this plugin in Bazaar using Jelmer‘s bzr-svn plugin. It produces a repeatable import, so I should be able to cross merge with anyone else producing branches with it.

It is also possible to use bzr-svn to merge Bazaar branches back into the original Subversion repository through the use of a lightweight checkout.

For anyone wanting to play with my Bazaar branch, it is published in Launchpad and can be grabbed with the following command:

bzr branch lp:~jamesh/rhythmbox/fmradio rhythmbox