(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.