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

Re: knotifications_5.45.0-2_source.changes ACCEPTED into unstable



In data domenica 13 maggio 2018 10:10:56 CEST, Maximiliano Curia ha scritto:
> ¡Hola Pino!
> 
> El 2018-05-13 a las 09:12 +0200, Pino Toscano escribió:
> > In data domenica 13 maggio 2018 09:01:18 CEST, Maximiliano Curia ha scritto:
> >> ¡Hola Debian!
> 
> >> El 2018-05-13 a las 06:05 +0000, Debian FTP Masters escribió:
> >>>   * Run the test suite during the build:
> >>>     - add the dbus-x11, xauth, and xvfb build dependencies
> >>>     - run xvfb-run, using a fake home directory for it, running the test suite
> >>>       inside a dbus session
> 
> >> Interesting, that used to fail in the past.
> 
> >>>   * Remove the unused, and now no more useful autopkgtest stuff.
> 
> >> You also dropped the acc tests with this change, the acc test builds the
> >> installed headers and that helps checking that the -dev package is not missing
> >> a dependency and that the installed headers are not include a header that's
> >> not being installed.
> 
> > How does running a tool ('acc') that is supposed to check for ABI
> > compatibility do that? From what I see, it is supposed to be used with
> > existing data as baseline, and none of the sources ship that.
> 
> The abi generated files are too large to ship them in our packages, 
> way more unstable than symbols, and not catching things like changes in the 
> struct size. But while doing these tests building the headers help finding 
> quite a number of missing headers/missing dependencies, etc.

Still, this is misusing a tool for not what is designed for.  The ABI
compliance checker is, well, a tool for checking ABI, not for "checking
headers", and this means that the tool can change its way of working
any day, breaking the way it is (mis)used in this case.

Also, neither dh-acc, acc, or any of the acc autopkgtest in sources
depend on a g++ compiler, so I don't see how this even works (unless
something else pulls it as side dependency, or a chroot with
build-essential is by chance reused for the test).

Furthermore, just parsing the headers is not a real-world test: acc
seems to discover the location of the headers used on its own, while
when using cmake/qmake the actual headers path will be added using
targets and/or variables, which is what needs to be tested (see
#877351 for example).

Another point is that acc seems to use a C compiler when parsing the
headers, which means that it basically skips all the content of KF5
headers, demoting the acc run to a mere "do all the #include's work",
and nothing else.

Again: acc is the wrong tool for this job.

> > Also, if we want to check that a -dev does not miss dependencies, then
> > there are better ways to do that, rather than relying on the indirect
> > results of a tool that does something else.
> 
> > I have a small utility locally in my disk that does exactly this, i.e.
> > run cmake on an auto-generated temporary CMakeLists.txt with
> > find_package(...) calls for modules specified on command line -- i.e.
> >  $ pkgkde-test-cmake --output-on-failure KF5Foo Grantlee5
> > (the name can be changed, of course)
> > This makes it easy to integrate it as autopkgtest, as it does not even
> > need an helper script -- i.e. just add in debian/tests/control:
> >  Test-Command: pkgkde-test-cmake --output-on-failure KF5Foo
> >  Depends: build-essential, cmake, libkf5foo-dev
> 
> > In the same fashion, something similar for qmake .pro files can be
> > created too.
> 
> > Also, for pkg-config files a better test IMHO is:
> >  Test-Command: pkg-config --cflags --libs Qt5WebKit
> >  Depends: pkg-config, libqt5webkit5-dev
> 
> > Would the above suggestions be better options for you?
> 
> Sounds interesting and I'm sure this could be of use, but from what I can 
> infer these scripts don't actually build the installed headers, so this would 
> be an additional test, not a replacement.

Then let's have a _proper_ build test: add a sample source code (or
uses the examples, if the sources ship any), and build it with
cmake/qmake. This will be way more close to how libraries are actually
used, rather than just parsing their headers.

-- 
Pino Toscano

Attachment: signature.asc
Description: This is a digitally signed message part.


Reply to: