(It’s a new day, it’s a new bug…)

A regression appeared in the synaptics input driver, between the 1.4.1 and the 1.5.0 release, as reported in Debian #649002 by many users. Basically, no more touchpad on PowerPC (hi, iBook G4 users!). :-(

What follows is how I tracked it down. The upstream bug report is fdo#43728. Anyone could have done so, there’s no black magic involved. A little hint maybe: git bisect is really easy on X drivers, since one can build and test without the Debian packaging bits.

1st step: Getting started

You need sources and build dependencies, nothing fancy. Since packages might be named a bit differently, there’s a reference to the relevant upstream repositories in debian/watch for X packages.

# You need git here:
debcheckout xserver-xorg-input-synaptics synaptics.git
cd synaptics.git
# Let’s get the upstream tags and branches:
cat debian/watch
git remote add upstream git://anongit.freedesktop.org/xorg/driver/xf86-input-synaptics
git fetch --all
# Let’s install build dependencies:
sudo apt-get build-dep xserver-xorg-input-synaptics

2nd step: Reproducing

It was said 1.5.0 was the first known buggy version, so let’s make sure:

# Get the appropriate tag:
git checkout xf86-input-synaptics-1.5.0
# Prepare the build system:
autoreconf -vfi
# X finds its modules under /usr, not /usr/local:
./configure --prefix=/usr
# Build and install:
make && sudo make install

Now time to restart the display manager, and confirm that the pointer is not moving → bug reproduced.

Now let’s check the 1.4.1 release isn’t affected:

# Clean everything:
git clean -xdf
git checkout xf86-input-synaptics-1.4.1
autoreconf -vfi
./configure --prefix=/usr
make && sudo make install

And after a display manager restart, the pointer is moving again, good! For reference, the log has very different lines about appletouch, so it can be used to programmatically learn about the status (good or bad).

3rd step: Narrowing it down

So, we have a known good version and a known bad version. Time for a binary search to find the guilty commit which introduced that regression!

git bisect start
git bisect good xf86-input-synaptics-1.4.1
git bisect bad  xf86-input-synaptics-1.5.0

You don’t even need to know that 1.4.x and 1.5.x live in different branches, git figures that out for you:

Bisecting: a merge base must be tested
[de0dfb76444ad4160468d00515876c91a9fa20bf] synaptics 1.4.0

It’s just a matter of running autoreconf … && ./configure … && make && sudo make install && sudo /etc/init.d/xdm restart every step of the way, so it’s pretty easy.

No wonder, 1.4.0 was working OK, so it can be recorded as such through git bisect good, and the binary search goes between that commit and 1.5.0. After a few iterations of git bisect good and git bisect bad, we get to the commit reported on the upstream bug.

As an extra bonus, mentioning the upstream bug number/URL in the Debian bug means it can be marked as forwarded there and we’ll receive automatic notifications when its status changes, thanks to bts-link.

Now, if you want to provide a patch for this bug, you may want to try and revert this commit.

4th step: Providing a patch

Let’s go back to the affected release and try to revert the guilty commit:

git checkout xf86-input-synaptics-1.5.0
git revert 13543b156d78bc4d01a19844a5ee8f283269621b

Unfortunately, some later commits modified the affected areas so we’re running into conflicts:

error: could not revert 13543b1... eventcomm: replace synaptics-custom TEST_BIT with server's BitIsOn.
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

Let’s try to understand what the offending commit did: it replaced a set of macros with a single one, taken from the server. Both the old TEST_BIT() and the new BitIsOn() take 2 arguments, but the order is reversed. Reverting it should only be a matter of introducing the TEST_BIT() macro again (along with the two other ones it needs), and using it.

The problem is replacing all BitIsOn(foo,bar) with TEST_BIT(bar,foo): foo and bar might not be trivial, they could be complex C expressions, and a replacement in your favourite editor might not get all occurrences right.

Thankfully, there’s a nice tool to handle such things, called coccinelle. I won’t go into details, just show how it can help here. Basically you just need to craft a tiny patch, called a semantic patch, which describes what I wrote in the previous paragraph. You only need to declare two expressions, say a and b, and declare that all BitIsOn(a,b) must be replaced with TEST_BIT(b,a). Here’s the syntax:

@@
expression a,b;
@@
-BitIsOn(a,b)
+TEST_BIT(b,a)

Save it under testbit.cocci and apply it through spatch (from the coccinelle package), asking it to perform in-place replacement (we’re in a git checkout copy, remember):

# Apply:
spatch -sp_file testbit.cocci -in_place .
# Profit:
git diff

Then you can run git commit -s, write a nice commit message, send it upstream, and enjoy the rest of the night.

Coming soon to a mirror near you: fixing commit. As mentioned on the upstream bug report, I got the Debian reference wrong in the commit message, I really meant #649002.