onboard has overactive dependencies

[Emmet Hikory]

Binary package hint: onboard

onboard, by depending on libxi-dev and libx11-dev pulls in a host of development libraries that are not required to run the package. The change claims to fix bug #523987 and bug #523554 , which it does, but indirectly. The package should properly only depend on libxi6 and libx11-6, but do so in a way that automatically selects the correct ABI based on the callers. It may be that 523987 and 523554 are not actually bugs in onboard, although expressed in onboard.

[Emmet Hikory (persia)]

The specific issue is that Onboard uses CDLL against the bare .so files, which are only provided as symlinks in the development libraries. Onboard should determine the appropriate libraries to open at build time, and depend on the packages providing only these libraries.

[Emmet Hikory (persia)]

Attached is a patch that addresses the issue for me. Some notes about this:

1) The Maintainer change is by request of the individual previously listed
2) The complete set of changes cannot be accomplished simply by patch application: one must also run uscan --force-download in order to accomplish the denativisation
3) No, it doesn't really need this many make variables, but doing it this avoids a) shellscripts in makefiles, and 2) keeps each line under 80 characters
4) The sed expressions happen to work on the current source: I can't be sure they really patch and unpatch for arbitrary environments : suggestions for improvements welcome

[Launchpad Janitor (janitor)]

This bug was fixed in the package onboard - 0.93.0-0ubuntu3

---------------
onboard (0.93.0-0ubuntu3) lucid; urgency=low

  * Set library dependencies at build-time with cdll:Depends (LP: #524148)
  * Patch Onboard/X11.py at build-time to use identified library files
  * Add watch file and convert to non-native
 -- Emmet Hikory <email address hidden> Fri, 19 Feb 2010 11:42:48 +0900

[marmuta (marmuta)]

Emmet, instead of patching X11.py on build, wouldn't it be preferable to permanently modify the source to reference sonames instead of linker names? I.e. changing libX11.so -> libX11.so.6 and libXi.so -> libXi.so.6 in X11.py.
That way running onboard from the project directory wouldn't require having libx11-dev and libxi-dev installed either. Also arguably X11.py expects a specific ABI already, I just missed to add the version extension.

[Emmet Hikory (persia)]

I'm not entirely sure. Both represent different classes of maintenance issues. I chose to patch the source on build because from the documentation I found I had the impression that CDLL matched a specific API, rather than a specific ABI, and doing it this way allows the ABI to change without API changes, and track it better. What I didn't do, but would be a good extension, is to write a python test-case that exercises the CDLL calls in X11.py and run that test suite during build, so that the package fails to build if the calls cannot be satisfied: this, in combination with the dynamic patching, should cause a behaviour similar to that found with native C clients of the libraries.

Another reason to choose to patch the source at runtime was to follow the upstream model: as the source does not specify any specific library version, it is presumed that it is known to work with a number of versions (this is certainly the case for a lot of C clients, so I don't know why it would be diffferent for python CDLL): using the dynamic patch stays closer to the intent of the upstream code whilst working around some of the inconveniences that this caused in the way Ubuntu packages are organised.

[marmuta (marmuta)]

An automated test-case for the CDLL-calls sounds like a good idea, if I new how to do this, I would add it to trunk too.

I should have mentioned that in this case I am part of the upstream, the ctypes piece in question was actually my (troublesome) contribution. You found quite correctly that my initial intention was to stay independent of library versions, but in hindsight that looks more like wishful thinking on my part. API as well as ABI of libX11 and libXi are more or less hard coded in X11.py since it relies on pure python with ctypes. ABI at least in form of fixed struct order and sizes and rigid function parameters. There isn't really any guarantee this would still work with the next major versions, whenever that may happen with X11 anyway. There is no way to take advantage of updated header files like in a C application. Last but not least, I missed before that the python ctypes documentation also uses sonames in it's sample.

Consequently today I have modified X11.py to use sonames libX11.so.6 and libXi.6 in onboard trunk. I've also changed onboard to fail more gracefully next time. Instead of coming down crashing only a small part of the functionality would be disabled, namely the two new mouse click buttons, which aren't even immediately visible in the default keyboard layout.

Looking at your patch it should be compatible, since clean:: removes the versioning before it is readded, but if you agree with the above it may not be necessary anymore once Francesco decides to create a new minor release. In any case this was a very necessary and rapidly done fix, so thank you very much for it :).

[Emmet Hikory (persia)]

If the decison is to hardcode the SONAMEs in the CDLL calls, then it makes sense to drop my patch, and just add the correct library dependencies to the package. If there was a test suite, it may be useful to reintroduce the dynamic detection. I suspect that for advanced hackers (not I) it would be possible to use the development headers to generate compiled python bindings and set the right dependencies at build time. Another alternative would be to work with X upstream to get generic python bindings, and then require them in onboard.

The problem with retaining the debian/rules patch if the SONAMEs are hardcoded in X11.py is that the package will silently break if there is an ABI change in the libraries, because the binary package will appear to have been updated correctly with a rebuild, but then will fail at runtime. With hardcoding, it would be good to put a different check in build/onboard:: while keeping the build-dependencies. Something like:

WRITE_TEST =[ -f /usr/lib/$(1) ];
CDLL_LIBS ?= ${shell grep CDLL Onboard/X11.py | cut -d\' -f2}
CDLL_TESTS ?= ${foreach lib,${CDLL_LIBS},${call WRITE_TEST,$(lib)}}

build/onboard::
        ${CDLL_TESTS}

    I believe this would interrupt the build if the correct libraries were not present on the system, and in combination with build-dependencies on libXi-dev and libX11-dev one would be able to verify that the deisred libraries were the default libraries (or if not, have warning that the code needed porting). I haven't tested this, and it ought be tested prior to upload, but something of this form would provide primitive protection against runtime errors.

Regardless of the treatment of the debian/rules patch, the debian/watch patch should be preserved (or modified if the launchpad download locations change).

[marmuta (marmuta)]

Hi Emmet, sorry for the delay.
> I suspect that for
> advanced hackers (not I) it would be possible to use the development
> headers to generate compiled python bindings and set the right
> dependencies at build time.
Turns out there are actually code generators for ctypes.
http://starship.python.net/crew/theller/ctypes/old/codegen.html
That might be the most prominent one, but it still appears somewhat outdated if not abandoned. Also I'm not sure if ctypes is the right choice anymore once you need to generate source. IMO the apeal is it's low barrier for accessing a handful of functions. If the code grows beyond that it's probably better to switch to a C extension anyway.

> Another alternative would be to work with X upstream to get generic
> python bindings, and then require them in onboard.
This would be great, but it is a bit out of scope for me personally. I had looked at python-xlib btw., but it doesn't have direct support for xinput or any other extension as far as I could tell.

> The problem with retaining the debian/rules patch if the SONAMEs are
> hardcoded in X11.py is that the package will silently break if there
> is an ABI change in the libraries, because the binary package will
> appear to have been updated correctly with a rebuild, but then will
> fail at runtime.
Is that necessarily the case here? I'm not exactly experienced with packaging, but can't we simply add libx11-6 and libxi6 as depends and as you suggested their -dev packages as build-depends manually and be done with it? Wouldn't that be enough to break packaging and installation in case of an ABI change? Assuming "6" in libxi6 is the same as in the soname libxi.so.6, is there still a possibility for a backwards incompatible lib to slip in under the radar?

> Regardless of the treatment of the debian/rules patch, the
> debian/watch patch should be preserved (or modified if the launchpad
> download locations change).
Yes, no objections from me there. I would need to double check with the other devs though.

[Emmet Hikory (persia)]

It is necessarily the case that the debian/rules patch should be removed if there is intentional hardcoding against specific versions of the libraries, because the patch will destroy the information provided in the upstream files, and set them to be the current system libraries, which is potentially wrong. It will also break policy because the unpatching in clean: won't restore the pristine source.

If the specific libraries are hardcoded in the source, the specific dependencies should be hardcoded in debian/control, and a note to that effect added to debian/README.source so that future maintainers know to check and adjust if updating.

[marmuta (marmuta)]

Ok, thanks, I'll do so then.

[Francesco Fumanti (frafu)]

Adding a watch file was on my ToDo list, but I did not create on yet because I did not have time to look up how it works. Consequently, I am also in favor of preserving it.