I discovered that in several of my autopkgtest scripts, and in various
other packages in the archive, the following pattern appears:

...
cd somewhere
...
for py in $(py3versions -r 2>/dev/null)
...

Unfortunately, this silently fails, as no python versions are returned
when py3versions -r is run from anywhere other than the top directory
of an unpacked source, and only the error message:

py3versions: error parsing Python3-Version attribute

is given.  The "2>/dev/null" is nevertheless usually required even
when run from the correct directory to silence the warning:

py3versions: no X-Python3-Version in control file, using supported versions

which appears on most packages.

The corrected script should read something like:

...
for py in $(py3versions -r 2>/dev/null); do
    cd somewhere
...

or

...
python3_versions=$(py3versions -r 2>/dev/null)
cd somewhere
...
for py in $python3_versions; do
   ...

A regex such as /\bcd\b.*py3versions\s+-r/s applied to the entire
content of debian/tests/control and every other file in debian/tests
should catch this issue.

Best wishes,

   Julian

Hi Julian,

I think (although I'm not an authoritative voice of any kind here) that
you you are using the wrong option altogether actually.

Of course it's silent.  you are asking something and then ignoring the
output…

You should use `-s` instead of `-r` in those cases, and drop the
2>/dev/null.
Besides, even if you keep the -r it wouldn't be much of a problem: $()
only captures stdout, stderr just gets printed and doesn't interfere
with the for loop or such, so why are you doing this?


And then, `py3versions -s` will "just work" wherever you are, no need to
do this fairly complicated check.

Yeah sure, and then what about pushd ?  Doing this kind of check in
lintian is fraught with false positives, so I recommend the lintian
maintainers don't try to do this.


However, instead, I'd suggest that, after checking with the
debian-python@ lists, we could tell people to use -s if and only if
X-Python3-Version is not defined (conversely, we should warn if packages
use -s if X-Python3-Version *is* defined, probably).

Hi Mattia,

That may or may not be true.  What I am observing, though, is that
there are a number of packages that have this bug.  Not many, but a
moderate number nonetheless.
set, py3versions -r prints a warning message to stderr, and we want to
suppress this so that autopkgtest doesn't fail for no reason (adding
Restrictions: allow-stderr for a single expected warning is not a good
thing to do).  However, this doesn't separate from the case where
py3versions -r prints an error message and exits with an error status.

You may well be correct.  That doesn't change the reality that this is
what is happening in a number of packages.

I'm not convinced; if a X-Python3-Version is added later to a package
using -s, then things might go wrong, whereas -r will always do the
right thing as it falls back to -s.  Or if copying boilerplate testing
code, -r always works but -s doesn't.  A better possibility might be
for py3versions to introduce an additional option -q/--quiet to
suppress the warning message when -r falls back to -s; this is vastly
superior to 2>/dev/null as it will let the true failure remain
visible (and possibly cause autopkgtest to correctly fail).

Because any output to stderr causes autopkgtest to fail unless it is
specifically instructed to allow-stderr.

You may indeed be right.

True.

I don't think I've yet found a case where pushd has been incorrectly
used (and I've so far checked all source packages beginning with a-f
and all source packages with "python" in their name).
one check that the advice is being followed?  Hence my suggestion to
have a lintian check for this.

But it may be a moot point: if it turns out that only a handful of
packages have this issue (and I'm currently filing bug reports on
them), then once they are fixed, maintainers who copy such scripts are
unlikely to reintroduce the issue.

Best wishes,

   Julian

Hi Mattia,

Here's an update.

I've been through the archive, and the figures are approximately the
following (only approximate, as some packages have multiple versions
in unstable Sources, and I wasn't careful to only include one of
each):

829 packages call py3versions in their debian/tests/* scripts
337 packages call py3versions -r
438 packages call py3versions -s
54 packages use other options: some use -i, some use -d, some use
  --supported (= -s), one uses -r

So this is a *lot* of packages using -r, almost as many as the number
using -s!

After I fixed my broken packages, there are only a handful of packages
incorrectly doing a cd before py3versions -r (3 or 4 such packages),
so my suggested check is probably unwarranted.  I've filed bug reports
against those packages.

Almost every package using py3versions -r redirects 2>/dev/null; the 5
or so which don't have "Restrictions: allow-stderr" in their
tests/control file.

I hope this is helpful.

Best wishes,

   Julian

Thanks for passing this through NEW!

The question of py3versions -r versus py3versions -s is not so
straightforward, IMHO.  Using py3versions -r protects against
the minor error of later introducing an X-Python3-Version field and
forgetting to change -s to -r.  With py3versions -r 2>/dev/null, the
correct behaviour is always obtained, as -r falls back to -s when
there is no X-Python3-Version field present.  Perhaps better would be
for py3versions to provide a -q flag to suppress the warning message
in this case; this would also protect against the error of using
py3versions incorrectly.

Furthermore, this idiom appears throughout the archive.
X-Python3-Version is quite rare: there are only about 194 occurrences
of it across the current unstable main archive, of which only 6 are
currently needed (those which specify X-Python3-Version: 3.9; all the
rest specify >= 3.x, with x at most 9).  On the other hand, there are
over 800 packages that use py3versions in their debian/test/* scripts,
but py3versions -r is very common among them, being used in a little
under half of the cases.  Only 4 packages have py3versions -r together
with X-Python3-Version (namely formiko, pyferret, python-rpaths and
sphinxcontrib-doxylink, though all of their X-Python3-Version fields
are now unnecessary).  There are also 4 packages which use py3versions
-s together with X-Python3-Version, and these work only because the
X-Python3-Version is now redundant (they are: mkosi, pyrlp,
python-ecdsa, python-libzim).

See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1001677 for a
little more detail (copied into this reply).

*If* the consensus is that py3versions -r is wrong, then we should
probably have a lintian check for it: py3versions -r (and variants
such as -rv and -vr) without a corresponding X-Python3-Version field
should give a lintian warning, and py3versions -s with an
X-Python3-Version field should do so likewise.

Best wishes,

   Julian

Hi,

For reference, here is Lintian's current code examining the use of
'py3versions' in tests. [1]

Kind regards
Felix Lechner

[1] https://salsa.debian.org/lintian/lintian/-/blob/master/lib/Lintian/Check/Testsuite.pm#L289-309

Thanks Felix, that's really helpful!

So perhaps another clause or two, something along the lines of the
following, would be good?  (This is not working code; I don't know all
the ins and outs of lintian's library to make it work.  Also, these
should probably be warnings rather than hints.)

    $x_python3_depend_field_present = [boolean specifying whether
        X-Python3-Depend field is present in debian/control];

    $self->pointed_hint(
        'runtime-test-file-uses-requested-python-versions-without-x-python3-depend',
        $pointer,
        $command
    )
    if $options =~ /\s(?:-\w*r|--requested)/
    && !$x_python3_depend_field_present;

    $self->pointed_hint(
        'runtime-test-file-uses-supported-python-versions-with-x-python3-depend',
        $pointer,
        $command
    )
    if $options =~ /\s(?:-\w*s|--supported)/
    && $x_python3_depend_field_present;

There are obviously still a bunch of debian/test/* files which have
different options:

* some use -d (which does make sense, but may not be the best thing to
  do in a debian/test/* file, as it will miss advanced warning of
  breakages on newer versions of python)
* some use -i (which is picked up by the existing tests)
* lots use -r (see earlier discussion)
* lots use -s (which is generally fine), though some of these also use
  2>/dev/null, which is not a good idea

And a couple don't follow the standard coding pattern:

* tap.py 3.1-1 has:
: ${PYVERSIONS:=py3versions}
for python in $($PYVERSIONS --supported); do

I'm unclear what is gained by thinking that the environment might
override PYVERSIONS.

* csound 6.16.2~dfsg-1 has:
    # By default, test on all versions
    versions="-r"
    if [ "$2" = default ] ; then
        versions="-d"
    fi
    for py in $(py3versions $versions 2>/dev/null) ; do

Finally, confget 4.1.0-1 tests for the presence of py3versions before
using it, which is just unnecessary (but thankfully doesn't mess up
the existing lintian test).

Best wishes,

   Julian

Hi Julian,

Your suggestion was implemented in:

https://salsa.debian.org/lintian/lintian/-/commit/d228765ce4db8057fd0c9780b2312c0e26914434

Thank you for making Lintian better for everyone!

Kind regards
Felix Lechner

Wow, fast work!  Thanks Felix!

I've just made one small suggestion on the commit.

Best wishes,

   Julian