Bug#1008267: pkg-perl-autopkgtest: use.t: reading debian/tests/pkg-perl/use-whitelist too restrained?

[gregor herrmann]

On Sat, 26 Mar 2022 16:22:29 +0200, Niko Tyni wrote:

> > I just had an issue with our use.t and
> > debian/tests/pkg-perl/use-whitelist,
> Reading #845771 I think it was intentionally just one line. OTOH the
> documentation in autopkgtest.pod has always said "expected warnings can
> be listed as regular expressions", where the plural does not correspond
> to reality.  So there is a bug, we just need to decide where :)

:)
Right. And while taking only one line is not my main concern (I
stumbled across this for the first time yesterday) …
 
> I suppose all use cases could be expressed in one regexp with '|'
> alternation, but allowing multiple lines with separate comments gives
> a much cleaner and self documenting file. So I'm all for that.

… this totally makse sense to me.
 
> > Now even if only reading one line is ok, as this is used as a regexp
> > in line 106, /(\S+)/ seems a bit, hm, minimalistic.
> I assume you're referring to not allowing whitespace in the pattern?
> Yeah I think that's a bug. I think I'd prefer to keep all whitespace
> and any other words on the same line.

Yes, I mean the "cutoff" at the first whitespace.

I looked at the existing debian/tests/pkg-perl/use-whitelist files
again now, and only very few contain something regexp-y, most are the
warning verbatim, e.g. picking a random example (occuring several
times):

    Any::Moose is deprecated. Please use Moo instead

In this case the current code just matches 'Any::Moose' and will
accept all warnings with this word.

Or

    Name "FFI::Platypus::TypeParser::basic_type" used only once

will match everything with "Name".
 
> Not sure how much we need to worry about compatibility with existing
> files with currently ignored whitespace or multiple words.

Yes, we might see some fallout, but we only have 26
debian/tests/pkg-perl/use-whitelist files, and having use.t work more
as expected seems worth the potential need to fix one or another.
 
> Also, while looking at the code, I can't see any reason for limiting
> use-name to just one name either. The main test already handles multiple
> modules if they come from @ARGV.

Nice catch.
 
> Similarly for whitespace in use-name. The name is interpolated into
> a shell command, but \S doesn't protect from anything and the source
> package can run arbitrary code inside the testbed by design.

Ack.
 
> I've filed MR !16 with a lightly tested fix. Please review and feel
> free to merge if it looks good.
>  https://salsa.debian.org/perl-team/modules/packages/pkg-perl-tools/-/merge_requests/16

Looks good to me, thanks!

I also did a brief test with my "problem package" from yesterday, and
the new use.t works with the old debian/tests/pkg-perl/use-whitelist
as well as with the version I wanted to write originally :) (i.e. 2
lines with complete warnings). Putting 2 module names in use-name
worked as well, so I guess this can be merged and shipped.

I'll leave it a bit to give others a chance to check as well.


Cheers,
gregor 

-- 
 .''`.  https://info.comodo.priv.at -- Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
   `-   
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 963 bytes
Desc: Digital Signature
URL: <http://alioth-lists.debian.net/pipermail/pkg-perl-maintainers/attachments/20220326/0b8ea797/attachment.sig>