Bug#916924: sbuild: doesn't run autodep8 autopkgtests

2018-12-30 Thread Johannes Schauer
Hi Sean,

Quoting Sean Whitton (2018-12-30 14:52:48)
> On Fri 21 Dec 2018 at 08:18pm +0100, Johannes Schauer wrote:
> > could you check with the autopkgtest maintainers/developers about this?
> 
> Instead of doing this, I just looked at the code, of both autopkgtest
> and autodep8.
> 
> I can confirm these two facts:
> 
> 1. autopkgtest invokes autodep8 if d/tests/control is missing.
>Quoting README.package-tests.txt,
> 
> There are groups of similarly-structured packages for which the
> contents of ``debian/tests/control`` would be mostly identical, such
> as Perl or Ruby libraries. If ``debian/tests/control`` is absent,
> the ``autodep8`` tool can generate an automatic control file. If
> installed, ``autopkgtest`` will automatically use it; this can be
> disabled with the ``--no-auto-control`` option.
> 
> 2. autodep8 does not require the Testsuite: field to be present.  It
>chooses whether to generate d/tests/control based on arbitrary
>heuristics.  For example, for elpa-* packages, it generates a
>d/tests/control when either an appropriate Testsuite: value is
>present, or it actually finds runnable tests (it greps the source
>code of the package).
> 
> To summarise the bug I consider sbuild to have:
> 
> autopkgtest can run many tests suites when d/tests/control is not
> present, by invoking autodep8, but recent sbuild shortcircuits that by
> refusing to invoke autopkgtest when d/tests/control is absent, even when
> the user has passed --run-autopkgtest to sbuild.
> 
> In the absence of d/tests/control, the only way for sbuild to be sure
> whether autopkgtest is able to run the tests would be to duplicate the
> logic in autodep8 or have sbuild invoke autodep8; this seems to me like
> a layering violation.  I think that sbuild should revert to the old
> behaviour of always invoking autopkgtest when the user requests it, and
> letting it decide whether to skip.
> 
> I'm sorry that I did not include all this information in my original report.
> Thank you for your patience.

I agree with your reasoning. Given these facts sbuild should not attempt to
know better than autopkgtest/autodep8 whether tests should be run or not.

I think the check should just be completely removed and autopkgtest should
always be run unless RUN_AUTOPKGTEST is false.

Thanks!

cheers, josch


signature.asc
Description: signature


Bug#916924: sbuild: doesn't run autodep8 autopkgtests

2018-12-30 Thread Sean Whitton
Hello Johannes,

On Fri 21 Dec 2018 at 08:18pm +0100, Johannes Schauer wrote:

> could you check with the autopkgtest maintainers/developers about this?

Instead of doing this, I just looked at the code, of both autopkgtest
and autodep8.

I can confirm these two facts:

1. autopkgtest invokes autodep8 if d/tests/control is missing.
   Quoting README.package-tests.txt,

There are groups of similarly-structured packages for which the
contents of ``debian/tests/control`` would be mostly identical, such
as Perl or Ruby libraries. If ``debian/tests/control`` is absent,
the ``autodep8`` tool can generate an automatic control file. If
installed, ``autopkgtest`` will automatically use it; this can be
disabled with the ``--no-auto-control`` option.

2. autodep8 does not require the Testsuite: field to be present.  It
   chooses whether to generate d/tests/control based on arbitrary
   heuristics.  For example, for elpa-* packages, it generates a
   d/tests/control when either an appropriate Testsuite: value is
   present, or it actually finds runnable tests (it greps the source
   code of the package).

To summarise the bug I consider sbuild to have:

autopkgtest can run many tests suites when d/tests/control is not
present, by invoking autodep8, but recent sbuild shortcircuits that by
refusing to invoke autopkgtest when d/tests/control is absent, even when
the user has passed --run-autopkgtest to sbuild.

In the absence of d/tests/control, the only way for sbuild to be sure
whether autopkgtest is able to run the tests would be to duplicate the
logic in autodep8 or have sbuild invoke autodep8; this seems to me like
a layering violation.  I think that sbuild should revert to the old
behaviour of always invoking autopkgtest when the user requests it, and
letting it decide whether to skip.

I'm sorry that I did not include all this information in my original
report.  Thank you for your patience.

-- 
Sean Whitton


signature.asc
Description: PGP signature


Bug#916924: sbuild: doesn't run autodep8 autopkgtests

2018-12-21 Thread Johannes Schauer
Hi Sean,

Quoting Sean Whitton (2018-12-21 13:09:59)
> On Fri 21 Dec 2018 at 12:27pm +0100, Johannes Schauer wrote:
> > okay, but now I'm confused how autodep8 gets triggered at all. Isn't
> > autodep8 wrapping autopkgtest and not the other way round? Because sbuild
> > only executes autopkgtest itself which will fail if debian/tests/control is
> > missing.
> >
> > So how does the machinery involing autodep8 work? Can you explain in a bit 
> > more
> > detail?
> 
> I have not looked at the code, but I believe that autopkgtest invokes
> autodep8 when there is no debian/tests/control.  autodep8 then makes its
> own decision about whether to generate a debian/tests/control to pass
> back to autopkgtest.
> 
> The reason I believe this is that with older sbuild, when there is no
> d/tests/control, autopkgtest successfully runs the test suite.  So it must be
> that it is invoking autodep8.

could you check with the autopkgtest maintainers/developers about this?

Thanks!

cheers, josch


signature.asc
Description: signature


Bug#916924: sbuild: doesn't run autodep8 autopkgtests

2018-12-21 Thread Sean Whitton
Hello,

On Fri 21 Dec 2018 at 12:27pm +0100, Johannes Schauer wrote:

> okay, but now I'm confused how autodep8 gets triggered at all. Isn't autodep8
> wrapping autopkgtest and not the other way round? Because sbuild only executes
> autopkgtest itself which will fail if debian/tests/control is missing.
>
> So how does the machinery involing autodep8 work? Can you explain in a bit 
> more
> detail?

I have not looked at the code, but I believe that autopkgtest invokes
autodep8 when there is no debian/tests/control.  autodep8 then makes its
own decision about whether to generate a debian/tests/control to pass
back to autopkgtest.

The reason I believe this is that with older sbuild, when there is no
d/tests/control, autopkgtest successfully runs the test suite.  So it
must be that it is invoking autodep8.

-- 
Sean Whitton


signature.asc
Description: PGP signature


Bug#916924: sbuild: doesn't run autodep8 autopkgtests

2018-12-21 Thread Johannes Schauer
Hello Sean,

Quoting Sean Whitton (2018-12-21 11:56:55)
> On Thu 20 Dec 2018 at 08:37pm +0100, Johannes Schauer wrote:
> > Quoting Sean Whitton (2018-12-20 15:30:12)
> >> sbuild in stretch unconditionally invokes autopkgtest even if the file
> >> debian/tests/control is not present in the source package.  Newer sbuild
> >> does not invoke autopkgtest unless that file is present.
> >>
> >> Many packages do not have debian/tests/control, and simply have a
> >> Testsuite: field, which causes autopkgtest to invoke autodep8 to
> >> generate an appropriate debian/tests/control.  Under stretch, such
> >> packages test suites would be run by sbuild, but now they are not.
> >>
> >> This seems like a regression since stretch.  Perhaps, at least, there
> >> could be an option to revert to always invoking autopkgtest when
> >> --run-autopkgtest is passed?  --definitely-run-autopkgtest?
> > why not instead also check for the Testsuite field in debian/control and
> > never run autopkgtest if both the field is missing and debian/tests/control
> > is missing.
> >
> > Would that fix this problem?
> Unfortunately, not fully.  autodep8 chooses whether to generate a
> debian/tests/control based on arbitrary shell code; this may check for a
> Testsuite: field, but it may go ahead and generate a debian/tests/control
> even if there is no Testsuite: field.
> 
> For example, for elpa packages, I believe that autodep8 will trigger if
> the package has the right build-deps, even if it lacks a Testsuite: field.

okay, but now I'm confused how autodep8 gets triggered at all. Isn't autodep8
wrapping autopkgtest and not the other way round? Because sbuild only executes
autopkgtest itself which will fail if debian/tests/control is missing.

So how does the machinery involing autodep8 work? Can you explain in a bit more
detail?

Thanks!

cheers, josch


signature.asc
Description: signature


Bug#916924: sbuild: doesn't run autodep8 autopkgtests

2018-12-21 Thread Sean Whitton
Hello Johannes,

On Thu 20 Dec 2018 at 08:37pm +0100, Johannes Schauer wrote:

> Quoting Sean Whitton (2018-12-20 15:30:12)
>> sbuild in stretch unconditionally invokes autopkgtest even if the file
>> debian/tests/control is not present in the source package.  Newer sbuild
>> does not invoke autopkgtest unless that file is present.
>>
>> Many packages do not have debian/tests/control, and simply have a
>> Testsuite: field, which causes autopkgtest to invoke autodep8 to
>> generate an appropriate debian/tests/control.  Under stretch, such
>> packages test suites would be run by sbuild, but now they are not.
>>
>> This seems like a regression since stretch.  Perhaps, at least, there
>> could be an option to revert to always invoking autopkgtest when
>> --run-autopkgtest is passed?  --definitely-run-autopkgtest?
>
> why not instead also check for the Testsuite field in debian/control and never
> run autopkgtest if both the field is missing and debian/tests/control is
> missing.
>
> Would that fix this problem?

Unfortunately, not fully.  autodep8 chooses whether to generate a
debian/tests/control based on arbitrary shell code; this may check for a
Testsuite: field, but it may go ahead and generate a
debian/tests/control even if there is no Testsuite: field.

For example, for elpa packages, I believe that autodep8 will trigger if
the package has the right build-deps, even if it lacks a Testsuite:
field.

-- 
Sean Whitton


signature.asc
Description: PGP signature


Bug#916924: sbuild: doesn't run autodep8 autopkgtests

2018-12-20 Thread Johannes Schauer
Hi Sean,

Quoting Sean Whitton (2018-12-20 15:30:12)
> sbuild in stretch unconditionally invokes autopkgtest even if the file
> debian/tests/control is not present in the source package.  Newer sbuild
> does not invoke autopkgtest unless that file is present.
> 
> Many packages do not have debian/tests/control, and simply have a
> Testsuite: field, which causes autopkgtest to invoke autodep8 to
> generate an appropriate debian/tests/control.  Under stretch, such
> packages test suites would be run by sbuild, but now they are not.
> 
> This seems like a regression since stretch.  Perhaps, at least, there
> could be an option to revert to always invoking autopkgtest when
> --run-autopkgtest is passed?  --definitely-run-autopkgtest?

why not instead also check for the Testsuite field in debian/control and never
run autopkgtest if both the field is missing and debian/tests/control is
missing.

Would that fix this problem?

Thanks!

cheers, josch


signature.asc
Description: signature


Bug#916924: sbuild: doesn't run autodep8 autopkgtests

2018-12-20 Thread Sean Whitton
Package: sbuild
Version: 0.77.1-2
Severity: important
Control: fixed -1 0.73.0-4

Dear maintainer,

sbuild in stretch unconditionally invokes autopkgtest even if the file
debian/tests/control is not present in the source package.  Newer sbuild
does not invoke autopkgtest unless that file is present.

Many packages do not have debian/tests/control, and simply have a
Testsuite: field, which causes autopkgtest to invoke autodep8 to
generate an appropriate debian/tests/control.  Under stretch, such
packages test suites would be run by sbuild, but now they are not.

This seems like a regression since stretch.  Perhaps, at least, there
could be an option to revert to always invoking autopkgtest when
--run-autopkgtest is passed?  --definitely-run-autopkgtest?

Thank you.

-- 
Sean Whitton


signature.asc
Description: PGP signature