[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFC: Make dh_makeshlibs auto-detect udebs in common cases



Cyril Brulebois:
> Niels Thykier <niels@thykier.net> (2019-07-20):
>> Thanks for the feedback. :)
>>
>> I have tested this and they do.
>>
>> Test method: I have verified this against systemd at git master[1] using
>> the following three variants/builds:
>>
>>  * Vanilla debhelper/12.2.3 with explicit --add-udeb
>>  * Patched debhelper/12.2.3 with explicit --add-udeb
>>  * Patched debhelper/12.2.3 dropping the "dh_makeshlibs -plibudev1 \
>>    --add-udeb=libudev1-udeb [...]" line[2]
>>
>> All three generate identical results (i.e. bit-for-bit identical
>> debs/udebs)[3].
> 
> Thanks, but that's just a single package.
> 

So far, I have tested systemd and cdebconf (the latter after your mail),
which have both been bit-for-bit identical with and without the --add-udeb.

I am ok with expanding my tests up to 5 packages (incl. systemd and
cdebconf, which I already tested).  This would cover about 5% of all
packages with --add-udeb.  If we limit the remaining tests to the "will
probably work"-cases below, then I will cover about 10% of that bucket.

Would that cover your concern about test coverage?

>> I will never be able to make code that works all the time given the
>> premise that people "blindly" apply changes.  However, it is not my
>> goal either. The goal for this change is to simplify udeb handling to
>> remove one papercut /in the common case/ for udeb packages in two
>> forms:
>>
>>  1) Packages that need to add a new udeb will have one less step to
>>     worry about when they follow the naming convention for udebs.
>>
>>  2) /Some/ existing udeb producing packages will be able to drop their
>>     manual overrides (assuming they follow this convention as well).
>>     An example of a package that would benefit is bind9, where we can
>>     remove about 11 lines[4].
>>
>> While most packages seem follow the convention, there are exceptions.
>> As an example, I spotted fontconfig[5] which will not benefit from this
>> change.  Instead, it (and other exceptions) will have to keep --add-udeb.
> 
> While I'm of course fine with the idea of automating what can be
> automated, I think I'd be happier to also see some hint somewhere for
> maintainers who wish to drop the --add-udeb, e.g. checking the
> resulting SHLIBS before/after the change…
> 

Sure, but I do not think it will be worth it to automate such a hint.
The target scope here is <100 packages and the scales of things are[1]:

 * ~36 will definitely be able to use it.  (~38%)
   - includes: systemd, libdebian-installer, util-linux, bind9, zlib

 * ~40 will "probably" be able to use it. (~43%)
   - includes: cdebconf, json-c, libpciaccess, libxrender, lzo2
   - Dropping --add-udeb here will *probably* work but needs test or
     a closer review.

 * ~8 will "probably" *not* be able to use it. (~8.6%)
   - includes: reiserfsprogs, harfbuzz, pcre2
   - Dropping --add-udeb here will *probably* be incorrect and testing
     will most /likely/ confirm an issue.  These are cases I suspect of
     having multiple libraries merged into one udeb.

 * ~7 will definitely *not* be able to use it. (~7.5%)
   - includes: ntfs-3g, glib2.0, e2fsprogs
   - Dropping --add-udeb here will definitely be incorrect as these
     cases fail to satisfy the basic premise for this automation to
     work.

 * ~2 special cases / non-straight forward cases. (~2.2%)

The "probably" comes from cases like "dh_makeshlibs [-a] --add-udeb ..."
where I have skimmed d/control for whether it built a single library
package or not.  The uncertainty comes from the fact that people /can*
shove shlib files into packages not starting with "lib" or do other
non-standard things and I could not check each of every one of them.

The definite cases comes from "dh_makeshlib -pX --add-udeb X-udeb" cases
or cases where the name of the deb does *not* match the udeb.  In the
latter case, the automation will definitely not work.

Disclaimer: All data is collected by manually reading through
codesearch.d.n + sources.d.o.  As such, I may have made errors in my review.

Thanks,
~Niels

[1] My manually curated list is as follows:

Applicable: 36

 * freebsd-glue
 * gtk+3.0
 * libxcb
 * libzstd
 * zlib
 * libdrm
 * libusb-1.0
 * util-linux
 * ufsutils
 * libselinux
 * vte
 * libx11
 * at-spi2-atk
 * libcap2
 * lvm2
 * libinput
 * libdebian-installer
 * pixman
 * zfsutils
 * parted
 * systemd
 * attr
 * pcre3
 * atk1.0
 * libffi
 * ncurses
 * libwacom
 * freebsd-libs
 * bind9
 * mtdev
 * libgpg-error
 * gtk+2.0
 * libthai
 * vte2.91
 * libnl3
 * libdatrie

Probably applicable: 40

 * alsa-lib
 * libxtst
 * libxcursor
 * libbsd
 * xft
 * cdebconf
 * kmod
 * freetype
 * libaio
 * libxkbfile
 * cryptsetup
 * libxfixes
 * lzo2
 * liburcu
 * libgcrypt
 * libxrender
 * libxext
 * libpng1.6
 * libepoxy
 * fribidi
 * expat
 * sysfsutils
 * libtextwrap
 * libfrontenc
 * dmraid
 * json-c
 * libxi
 * libxshmfence
 * libxfont
 * gdk-pixbuf
 * popt
 * libpciaccess
 * fuse3
 * pciutils
 * libxau
 * libevdev
 * libxinerama
 * dbus
 * libxdmcp
 * fuse

Probably *not* applicable: 7

 * argon2
 * pango1.0
 * multipath-tools
 * reiserfsprogs
 * newt
 * harfbuzz
 * pcre2

Definitely *not* applicable: 8

 * open-isns
 * ntfs-3g
 * at-spi2-core
 * libusb
 * hurd
 * fontconfig
 * glib2.0
 * e2fsprogs

Special-case: 2
 * openssl (maybe; does funky hacks)
 * cairo (1 call is not applicable, another one is)


Reply to: