Hi Guillem, On 01-08-18 04:44, Guillem Jover wrote: > [ Please excuse some of the probably obvious questions, I probably > just do not have a clear picture of how the pieces fit together. ] Thanks for being critical. Some of these pieces are also looked upon by me for the first time. > On Mon, 2018-07-30 at 20:03:07 +0200, Paul Gevers wrote: >> On 30-07-18 05:42, Guillem Jover wrote: >>>> It would be great if dpkg-source would also parse debian/tests/control.autodep8 >>>> in the same way as it does debian/tests/control, except I guess it should not >>>> set the Testsuite field (as dpkg-source doesn't know which type of autopkgtest >>>> this is). >>> >>> Hmm, why did we get this new file at all? >> >> It's more than two years old. It was added in response to bug 823931. > > Would you be open to revisit that decision? Sure, if we find a good alternative. > The current interface > feels a bit like a wart to me. It didn't look like there were many > packages involved, when I checked the other day. I didn't check. https://codesearch.debian.net/search?q=Test+path%3Adebian%2Ftests%2Fcontrol.autodep8 gives 49 packages. >>> Why can't we use >>> debian/tests/control instead? The Testsuite field would be used to >>> specify that we still want autodep8 to be run, no? >> >> Looking at the bug, I can only assume it just wasn't considered to >> change autopkgtest, e.g. IIAC the maintainer of autodep8 wasn't a >> developer of autopkgtest at the time. > > Well, autodep8 seems to be called from autopkgtest, so I'm not sure I > understand that reason? :) Well, it would need changes in autopkgtest (on top of changes to autodep8). So there was the "out of my control" argument I guess. But as said, I am guessing. Anyways, I don't think it is relevant for the discussion. >> Also, the current implementation works without the spec needing to >> mention the debian/control file. > > Isn't it implicitly mentioned in the “Source package header” section? Right, missed that (I was doing it from memory, not looking up the file). But the ironic thing is that this paragraph is actually irrelevant for autopkgtest itself. autopkgtest doesn't care at all. So only autodep8 is using the Testsuite field, and then only from the debian/control file (not from the .dsc file). The d/contol file only needs to mention it when maintainers opt-in for autodep8 >> 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. > 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. >> If we would want to add support for this adding autodep8 on top of an >> existing debian/tests/control file, apart from adapting autopkgtest with >> a new option, also multiple interface would need to be adapted to pass >> on this info: >> britney <-> debci <-> autopkgtest > > Why is that, what information do these require? But rethinking this now it doesn't make sense anymore. The same argument applies: autopkgtest would behave different for different ways of calling it on the same source. Paul
Attachment:
signature.asc
Description: OpenPGP digital signature