Re: new set of changes
On Wed, Jul 30, 2008 at 08:45:08PM -0500, Raphael Geissert wrote:
> Hi all,
>
> Attached is a patch making the following changes:
> * correctly parse line continuations
> * avoid trying to perform any extra check on version 1 watch files
> * know about versionmangle when checking if there is any mangling because of
> $repack
> * add line information to some of the tags
> * attempt to detect when watch file tries to access a sf.net mirror directly
> (projectfoo.sf.net/some_file-(.+) and similar of course don't cause false
> positives)
> * debian-watch-file-specifies-wrong-upstream-version
> * debian-watch-file-specifies-old-upstream-version
I would prefer if you could split that up in separate patches for the
separate issues. Also if you could submit it in git patch format or
offer a public git repository to pull from that would make applying
them easier.
> The changes to testset/scripts/debian/changelog were made to make it easier
> to notice that it *is* a !native package.
> diff --git a/checks/watch-file b/checks/watch-file
> index 0177bd9..7b01a90 100644
> --- a/checks/watch-file
> +++ b/checks/watch-file
> @@ -51,15 +51,19 @@ if ($version =~ /(dfsg|debian|ds)/) {
> # diagnose on the first time through.
> open(WATCH, '<', 'debfiles/watch') or fail("cannot open watch file: $!");
> local $_;
> -my ($watchver, $mangle, $dmangle, $nonempty);
> +my ($watchver, $mangle, $dmangle, $nonempty, %dversions);
> while (<WATCH>) {
> next if /^\s*\#/;
> next if /^\s*$/;
> s/^\s*//;
> +
> + CHOMP:
> + chomp;
> if (s/(?<!\\)\\$//) {
> # This is caught by uscan.
> last if eof(WATCH);
> $_ .= <WATCH>;
> + goto CHOMP;
White space damage.
> + if (m%(https?|ftp)://((.+\.)?dl|prdownloads|ftp\d?|downloads?)\.(sourceforge|sf)\.net%) {
> + tag 'debian-watch-file-should-use-sf-redirector', "line $.";
> + }
White space damage.
> +my $changes = $info->changelog;
> +if (defined $changes && %dversions) {
> + my $data = $changes->data;
> + my %changelog_versions;
> + my $count = 1;
> + for my $entry (@{$data}) {
> + my $uversion = $entry->Version;
> + $uversion =~ s/-[^-]+$//; # revision
> + $uversion =~ s/^\d+://; # epoch
> + $changelog_versions{'orig'}{$entry->Version} = $count;
> + # preserve the first value here, as to correctly detect old versions
> + $changelog_versions{'mangled'}{$uversion} = $count
> + unless (exists($changelog_versions{'mangled'}{$uversion}));
> + $count++;
> + }
Whitespace damage
> + while (my ($dversion, $lines) = each %dversions) {
> + next if (!defined($dversion) || $dversion eq 'debian');
I would prefer either using "or" here or using parantheses around the eq
term, just to make it clear that this is correct.
> + local $" = ', ';
Hmm, I would prefer not to use too many of Perl's special variables
(IMHO everything other than $_, $/ and $.). I would recommend either us
ng explicit joins or "use English".
> + if (!$info->native && exists($changelog_versions{'orig'}{$dversion})) {
> + tag 'debian-watch-file-specifies-wrong-upstream-version', "$dversion: @{$lines}";
> + next;
> + }
> + if (exists($changelog_versions{'mangled'}{$dversion})
> + && $changelog_versions{'mangled'}{$dversion} != 1) {
> + tag 'debian-watch-file-specifies-old-upstream-version', "$dversion: @{$lines}";
> + next;
> + }
> + }
> +}
I believe we will need to add changelog-file to Needs-Info for that to
reliably work.
Just out of curiousity: how many packages in the archive are actually
using this feature (and do not use "debian")?
> -http://qa.debian.org/watch/sf.php?project=foo scripts\.([\d.]+)\.tar\.gz
> +http://qa.debian.org/watch/sf.php?project=foo scripts\.([\d.]+)\.tar\.gz debian uupdate
>
> +version=3
> +http://ftp.sf.net/foo/foo_bar(.+)\.Z 5 uupdate
> \ No newline at end of file
Is that missing newline intentional?
Gruesse,
--
Frank Lichtenheld <djpig@debian.org>
www: http://www.djpig.de/
Reply to: