Re: Bug#904776: /usr/bin/dpkg-source: please parse d/t/control.autodep8 like d/t/control

[Guillem Jover <guillem@debian.org>]

Hi!

On Wed, 2018-08-01 at 15:58:29 +0200, Paul Gevers wrote:
> On 01-08-18 04:44, Guillem Jover wrote:
> > On Mon, 2018-07-30 at 20:03:07 +0200, Paul Gevers wrote:
> >> autopkgtest (the binary that is _an_
> >> implementation of the spec) only calls autodep8 when it doesn't find the
> >> control file as a fall back. autopkgtest doesn't know why it is called,
> >> it doesn't know about the Testsuite field, nor does anything in the CI
> >> framework except for the one that requests the test.
> > 
> > Right, but it seems autodep8 does check, and uses the Testsuite field
> > to decide whether some of the precooked checks should be used.
> 
> Yes, but it may be absent: autodep8 also works on packages that haven't
> set any Testsuite field in debian/control. It tries to figure out if the
> binary packages are of some type and cooks up the control stanza for
> those. E.g. if a source or binary package starts with python-, it gives
> us the python2.7 version, if you run autopkgtest (with auto_control
> enabled) on them. This has been very useful on the ci.d.n infrastructure
> to have large classes of packages tested. This isn't used for
> influencing migration though.

Sure.

> > Wouldn't simply changing the logic to something like the following
> > simplified stuff, do the trick?
> > 
> >   * change autopkgtest to always call autodep8 if auto_control is
> >     enabled.
> >   * change autodep8 to always cat debian/tests/control if present.
> >   * then autodep8 would cat the autogenerated tests like now based on
> >     the Testsuite field, and for backwards compat also the
> >     control.autodep8 file if present.
> 
> What is currently missing is a way for the package maintainer to say:
> DON'T add autodep8 stanza to my package (as it fails). Currently that is
> implemented by having a file called debian/tests/control. So if we want
> to go this route, we either need time to have all packages that need
> autodep8 off to be able to migrate to a still to be defined way of
> saying so (e.g. by explicitly adding "Testsuite: autopkgtest" to
> d/control, which may break a lot of packages when autodep8 adds support
> for another class) or we can have autopgktest behave differently for
> different ways of calling the test, e.g. when run on .dsc files compared
> to when run from an unbuilt source-tree. I don't like to go the route of
> different results.

The problems I see are:

  * This feels like an internal implementation detail that would get
    leaked upwards in the stack, so a layer violation.
  * The autodep8 stuff is optionally invoked so the information
    generated might not always be relevant, but the control.autodep8
    file looks like it's something that one would not want to make
    dependant on autodep8 being run or not.
  * The auto generated stanzas are output into a temporary file so
    these would not be taken into account either, which might be the
    desired intention, but it feels weird.

I guess, the logic proposed above could be changed a bit to support
something closer to the current logic, and not introduce such a
global semantic change, so perhaps:

  * change autopkgtest to check the Testsuite field in debian/control:
    - if it contains say an autopkgtest-autogen value then call autodep8,
      even if debian/tests/control exists.
    - otherwise call autodep8 only if debian/tests/control does not
      exist.
  * change autodep8 to always cat debian/tests/control if present.
  * then autodep8 would cat the autogenerated tests like now based on
    the Testsuite field, and for backwards compat also the
    control.autodep8 file if present.


So this seems fully backwards compatible, and would make things more
clear IMO. It would also make those control.autodep8 files non-optional
based on autodep8 running or not. And the transition would be smooth. Any
current package shipping a control.autodep8 would just need to rename it
to control and add the “Testsuite: autopkgtest-autogen” field. Any
package that does not have such value in the field and contains a
debian/tests/control would not get autodep8 executed so would
automatically opt-out like now.

On Sun, 2018-08-19 at 21:38:26 +0200, Paul Gevers wrote:
> This bug is still marked as moreinfo, are you still waiting for more
> input from me?

I'm just still not convinced this should be handled in dpkg. :)

Thanks,
Guillem