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
`-
signature.asc
Description: Digital Signature
