Package: dpkg-dev Version: 1.15.4 Severity: normal Tags: patch User: ubuntu-de...@lists.ubuntu.com Usertags: origin-ubuntu ubuntu-patch karmic
dpkg-parsechangelog doesn't emit the Launchpad-Bugs-Fixed field when the vendor is Ubuntu. In general I think handling this in dpkg-genchanges is just the wrong place for it; other scripts mention Closes and so need to handle Launchpad-Bugs-Fixed as well. The attached patch series makes things work much better, and adds a few tests (although at the moment you need to set DEB_VENDOR to various things in order to exercise all the possibilities). I'm not entirely happy with the fact that I've had to mention Launchpad-Bugs-Fixed outside vendor hooks in various places, although those mentions are a no-op if the Ubuntu vendor hook isn't run so I think it's a tolerable situation; let me know if you can think of a cleaner approach. I didn't want to add a proliferation of hooks just for a single feature! Thanks, -- Colin Watson [cjwat...@ubuntu.com]
>From 0499622e2ad7a71f6229696f31b956b7bb4cda5b Mon Sep 17 00:00:00 2001 From: Colin Watson <cjwat...@ubuntu.com> Date: Tue, 7 Jul 2009 10:46:26 +0100 Subject: [PATCH 1/4] Localise $_ in Dpkg::Cdata::parsecdata to avoid confusing callers. --- debian/changelog | 3 +++ scripts/Dpkg/Cdata.pm | 1 + 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/debian/changelog b/debian/changelog index a78a768..5218767 100644 --- a/debian/changelog +++ b/debian/changelog @@ -21,6 +21,9 @@ dpkg (1.15.4) UNRELEASED; urgency=low sub-directories. Closes: #535138 * Upgrade Standards-Version to 3.8.2 (no changes). + [ Colin Watson ] + * Localise $_ in Dpkg::Cdata::parsecdata to avoid confusing callers. + [ Updated dpkg translations ] * Asturian (Marcos Alvarez Costales). Closes: #535327 * French (Christian Perrier). diff --git a/scripts/Dpkg/Cdata.pm b/scripts/Dpkg/Cdata.pm index 58fdc3a..66e4dc5 100644 --- a/scripts/Dpkg/Cdata.pm +++ b/scripts/Dpkg/Cdata.pm @@ -54,6 +54,7 @@ can be used to access the various fields. sub parsecdata { my ($input, $file, %options) = @_; + local $_; $options{allow_pgp} = 0 unless exists $options{allow_pgp}; $options{allow_duplicate} = 0 unless exists $options{allow_duplicate}; -- 1.6.3.3
>From 2bbea1f9ac6af17d8b801cd32bddc2da75224adb Mon Sep 17 00:00:00 2001 From: Colin Watson <cjwat...@ubuntu.com> Date: Tue, 7 Jul 2009 11:05:45 +0100 Subject: [PATCH 2/4] Fix changelog parsing when Ubuntu vendor class is in use The relevant hook is now called from Dpkg::Changelog::Debian rather than dpkg-genchanges, so that it can work for dpkg-parsechangelog etc. as well. Since the hook interface changed in the process, I've renamed it from before-changes-creation to parse-changelog. --- debian/changelog | 5 +++++ scripts/Dpkg/Changelog/Debian.pm | 2 ++ scripts/Dpkg/Vendor/Debian.pm | 4 ++-- scripts/Dpkg/Vendor/Default.pm | 12 ++++++------ scripts/Dpkg/Vendor/Ubuntu.pm | 9 +++------ scripts/dpkg-genchanges.pl | 1 - 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/debian/changelog b/debian/changelog index 5218767..c875d44 100644 --- a/debian/changelog +++ b/debian/changelog @@ -23,6 +23,11 @@ dpkg (1.15.4) UNRELEASED; urgency=low [ Colin Watson ] * Localise $_ in Dpkg::Cdata::parsecdata to avoid confusing callers. + * Fix changelog parsing when Ubuntu vendor class is in use. The relevant + hook is now called from Dpkg::Changelog::Debian rather than + dpkg-genchanges, so that it can work for dpkg-parsechangelog etc. as + well. Since the hook interface changed in the process, I've renamed it + from before-changes-creation to parse-changelog. [ Updated dpkg translations ] * Asturian (Marcos Alvarez Costales). Closes: #535327 diff --git a/scripts/Dpkg/Changelog/Debian.pm b/scripts/Dpkg/Changelog/Debian.pm index c702b79..9020354 100644 --- a/scripts/Dpkg/Changelog/Debian.pm +++ b/scripts/Dpkg/Changelog/Debian.pm @@ -68,6 +68,7 @@ use Date::Parse; use Dpkg; use Dpkg::Gettext; use Dpkg::Changelog qw( :util ); +use Dpkg::Vendor qw(run_vendor_hook); use base qw(Dpkg::Changelog); =pod @@ -145,6 +146,7 @@ sub parse { } unless ($entry->is_empty) { $entry->{'Closes'} = find_closes( $entry->{Changes} ); + run_vendor_hook('parse-changelog', $entry); # print STDERR, Dumper($entry); push @{$self->{data}}, $entry; $entry = new Dpkg::Changelog::Entry; diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm index b121852..d2b696f 100644 --- a/scripts/Dpkg/Vendor/Debian.pm +++ b/scripts/Dpkg/Vendor/Debian.pm @@ -39,8 +39,8 @@ sub run_hook { if ($hook eq "before-source-build") { my $srcpkg = shift @params; - } elsif ($hook eq "before-changes-creation") { - my $fields = shift @params; + } elsif ($hook eq "parse-changelog") { + my $entry = shift @params; } elsif ($hook eq "keyrings") { return ('/usr/share/keyrings/debian-keyring.gpg', '/usr/share/keyrings/debian-maintainers.gpg'); diff --git a/scripts/Dpkg/Vendor/Default.pm b/scripts/Dpkg/Vendor/Default.pm index 43d40cd..ba88dae 100644 --- a/scripts/Dpkg/Vendor/Default.pm +++ b/scripts/Dpkg/Vendor/Default.pm @@ -73,11 +73,11 @@ supported hooks are: The first parameter is a Dpkg::Source::Package object. The hook is called just before the execution of $srcpkg->build(). -=item before-changes-creation ($fields) +=item parse-changelog ($entry) -The hook is called just before the content of .changes file is output -by dpkg-genchanges. The first parameter is a Dpkg::Fields::Object -representing all the fields that are going to be output. +The hook is called while parsing each non-empty changelog entry. The first +parameter is a Dpkg::Changelog::Entry object containing the changelog entry +being parsed. =item keyrings () @@ -94,8 +94,8 @@ sub run_hook { if ($hook eq "before-source-build") { my $srcpkg = shift @params; - } elsif ($hook eq "before-changes-creation") { - my $fields = shift @params; + } elsif ($hook eq "parse-changelog") { + my $entry = shift @params; } elsif ($hook eq "keyrings") { return (); } diff --git a/scripts/Dpkg/Vendor/Ubuntu.pm b/scripts/Dpkg/Vendor/Ubuntu.pm index 2f31a8b..1df5126 100644 --- a/scripts/Dpkg/Vendor/Ubuntu.pm +++ b/scripts/Dpkg/Vendor/Ubuntu.pm @@ -60,14 +60,11 @@ sub run_hook { } } - } elsif ($hook eq "before-changes-creation") { - my $fields = shift @params; + } elsif ($hook eq "parse-changelog") { + my $entry = shift @params; # Add Launchpad-Bugs-Fixed field - my $bugs = find_launchpad_closes($fields->{"Changes"} || ""); - if (scalar(@{$bugs})) { - $fields->{"Launchpad-Bugs-Fixed"} = join(" ", @{$bugs}); - } + $entry->{'Launchpad-Bugs-Fixed'} = find_launchpad_closes($entry->{Changes} || ""); } elsif ($hook eq "keyrings") { my @keyrings = $self->SUPER::run_hook($hook); diff --git a/scripts/dpkg-genchanges.pl b/scripts/dpkg-genchanges.pl index 5c28859..a3ca95f 100755 --- a/scripts/dpkg-genchanges.pl +++ b/scripts/dpkg-genchanges.pl @@ -539,6 +539,5 @@ for my $f (keys %remove) { } tied(%{$fields})->set_field_importance(@changes_fields); -run_vendor_hook('before-changes-creation', $fields); tied(%{$fields})->output(\*STDOUT); # Note: no substitution of variables -- 1.6.3.3
>From e4493b25885d3211f11764948bbfb0bd44e41a0d Mon Sep 17 00:00:00 2001 From: Colin Watson <cjwat...@ubuntu.com> Date: Tue, 7 Jul 2009 11:06:54 +0100 Subject: [PATCH 3/4] Add Launchpad-Bugs-Fixed to various places that mention Closes This arranges that the Launchpad-Bugs-Fixed field added by the Ubuntu vendor hook is properly aggregated from multiple changelog versions, is sorted better, doesn't trigger unknown-field warnings, etc. --- debian/changelog | 3 +++ scripts/Dpkg/Changelog.pm | 19 ++++++++++++++++++- scripts/dpkg-genchanges.pl | 6 +++--- scripts/dpkg-gencontrol.pl | 2 +- scripts/dpkg-source.pl | 2 +- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/debian/changelog b/debian/changelog index c875d44..e75efb1 100644 --- a/debian/changelog +++ b/debian/changelog @@ -28,6 +28,9 @@ dpkg (1.15.4) UNRELEASED; urgency=low dpkg-genchanges, so that it can work for dpkg-parsechangelog etc. as well. Since the hook interface changed in the process, I've renamed it from before-changes-creation to parse-changelog. + * Add Launchpad-Bugs-Fixed to various places that mention Closes, so that + the field is properly aggregated from multiple changelog versions, is + sorted better, doesn't trigger unknown-field warnings, etc. [ Updated dpkg translations ] * Asturian (Marcos Alvarez Costales). Closes: #535327 diff --git a/scripts/Dpkg/Changelog.pm b/scripts/Dpkg/Changelog.pm index 7c4b1ca..6cbddaf 100644 --- a/scripts/Dpkg/Changelog.pm +++ b/scripts/Dpkg/Changelog.pm @@ -465,6 +465,10 @@ date of the (first) entry bugs closed by the entry/entries, sorted by bug number +=item Launchpad-Bugs-Fixed + +bugs closed by the entry/entries, sorted by bug number (Ubuntu vendor hook) + =item Changes content of the the entry/entries @@ -487,7 +491,8 @@ our ( @CHANGELOG_FIELDS, %CHANGELOG_FIELDS ); our ( @URGENCIES, %URGENCIES ); BEGIN { @CHANGELOG_FIELDS = qw(Source Version Distribution - Urgency Maintainer Date Closes Changes + Urgency Maintainer Date + Closes Launchpad-Bugs-Fixed Changes Timestamp Header Items Trailer Urgency_comment Urgency_lc); tie %CHANGELOG_FIELDS, 'Dpkg::Fields::Object'; @@ -518,6 +523,9 @@ sub dpkg { $f->{Changes} = get_dpkg_changes( $data->[0] ); $f->{Closes} = [ @{$data->[0]{Closes}} ]; + if (exists $data->[0]{'Launchpad-Bugs-Fixed'}) { + $f->{'Launchpad-Bugs-Fixed'} = [ @{$data->[0]{'Launchpad-Bugs-Fixed'}} ]; + } my $first = 1; my $urg_comment = ''; foreach my $entry (@$data) { @@ -532,6 +540,9 @@ sub dpkg { $f->{Changes} .= "\n .".get_dpkg_changes( $entry ); push @{$f->{Closes}}, @{$entry->{Closes}}; + if (exists $entry->{'Launchpad-Bugs-Fixed'}) { + push @{$f->{'Launchpad-Bugs-Fixed'}}, @{$entry->{'Launchpad-Bugs-Fixed'}}; + } # handle unknown fields foreach my $field (keys %$entry) { @@ -542,6 +553,9 @@ sub dpkg { } $f->{Closes} = join " ", sort { $a <=> $b } @{$f->{Closes}}; + if (exists $f->{'Launchpad-Bugs-Fixed'}) { + $f->{'Launchpad-Bugs-Fixed'} = join " ", sort { $a <=> $b } @{$f->{'Launchpad-Bugs-Fixed'}}; + } $f->{Urgency} .= $urg_comment; return %$f if wantarray; @@ -594,6 +608,9 @@ sub rfc822 { $f->{Urgency} .= $entry->{Urgency_Comment}; $f->{Changes} = get_dpkg_changes( $entry ); $f->{Closes} = join " ", sort { $a <=> $b } @{$entry->{Closes}}; + if (exists $entry->{'Launchpad-Bugs-Fixed'}) { + $f->{'Launchpad-Bugs-Fixed'} = join " ", sort { $a <=> $b } @{$entry->{'Launchpad-Bugs-Fixed'}}; + } # handle unknown fields foreach my $field (keys %$entry) { diff --git a/scripts/dpkg-genchanges.pl b/scripts/dpkg-genchanges.pl index a3ca95f..1d1b0d8 100755 --- a/scripts/dpkg-genchanges.pl +++ b/scripts/dpkg-genchanges.pl @@ -25,8 +25,8 @@ textdomain("dpkg-dev"); my @changes_fields = qw(Format Date Source Binary Architecture Version Distribution Urgency Maintainer Changed-By - Description Closes Changes Checksums-Md5 - Checksums-Sha1 Checksums-Sha256 Files); + Description Closes Launchpad-Bugs-Fixed Changes + Checksums-Md5 Checksums-Sha1 Checksums-Sha256 Files); my $controlfile = 'debian/control'; my $changelogfile = 'debian/changelog'; @@ -336,7 +336,7 @@ foreach $_ (keys %{$changelog}) { set_source_package($v); } elsif (m/^Maintainer$/i) { $fields->{"Changed-By"} = $v; - } elsif (m/^(Version|Changes|Urgency|Distribution|Date|Closes)$/i) { + } elsif (m/^(Version|Changes|Urgency|Distribution|Date|Closes|Launchpad-Bugs-Fixed)$/i) { $fields->{$_} = $v; } elsif (s/^X[BS]*C[BS]*-//i) { $fields->{$_} = $v; diff --git a/scripts/dpkg-gencontrol.pl b/scripts/dpkg-gencontrol.pl index 73c9dc9..95c8f4d 100755 --- a/scripts/dpkg-gencontrol.pl +++ b/scripts/dpkg-gencontrol.pl @@ -206,7 +206,7 @@ foreach $_ (keys %{$changelog}) { } elsif (m/^Version$/) { $sourceversion = $v; $fields->{$_} = $v unless defined($forceversion); - } elsif (m/^(Maintainer|Changes|Urgency|Distribution|Date|Closes)$/) { + } elsif (m/^(Maintainer|Changes|Urgency|Distribution|Date|Closes|Launchpad-Bugs-Fixed)$/) { } elsif (s/^X[CS]*B[CS]*-//i) { $fields->{$_} = $v; } elsif (!m/^X[CS]+-/i) { diff --git a/scripts/dpkg-source.pl b/scripts/dpkg-source.pl index cf13936..fa086ca 100755 --- a/scripts/dpkg-source.pl +++ b/scripts/dpkg-source.pl @@ -237,7 +237,7 @@ if ($options{'opmode'} eq 'build') { $fields->{$_} = $v; } elsif (s/^X[BS]*C[BS]*-//i) { $fields->{$_} = $v; - } elsif (m/^(Maintainer|Changes|Urgency|Distribution|Date|Closes)$/i || + } elsif (m/^(Maintainer|Changes|Urgency|Distribution|Date|Closes|Launchpad-Bugs-Fixed)$/i || m/^X[BS]+-/i) { } else { unknown($_, _g("parsed version of changelog")); -- 1.6.3.3
>From da3bf9b54b4e680ccbb1d677961f364d43f31627 Mon Sep 17 00:00:00 2001 From: Colin Watson <cjwat...@ubuntu.com> Date: Tue, 7 Jul 2009 11:30:16 +0100 Subject: [PATCH 4/4] Add tests for Launchpad-Bugs-Fixed handling --- scripts/t/600_Dpkg_Changelog.t | 35 ++++++++++++++++++++++++++++++++--- scripts/t/600_Dpkg_Changelog/fields | 2 ++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/scripts/t/600_Dpkg_Changelog.t b/scripts/t/600_Dpkg_Changelog.t index 7182718..fdc458a 100644 --- a/scripts/t/600_Dpkg_Changelog.t +++ b/scripts/t/600_Dpkg_Changelog.t @@ -12,7 +12,7 @@ BEGIN { + $no_err_examples * 2 + 24 # countme + 2 # fields - + 24; + + 26; require Test::More; import Test::More tests => $no_tests; @@ -20,6 +20,7 @@ BEGIN { BEGIN { use_ok('Dpkg::Changelog'); use_ok('Dpkg::Changelog::Debian'); + use_ok('Dpkg::Vendor'); }; my $srcdir = $ENV{srcdir} || '.'; @@ -31,6 +32,21 @@ my $test = Dpkg::Changelog::Debian->init( { infile => '/nonexistant', quiet => 1 } ); ok( !defined($test), "fatal parse errors lead to init() returning undef"); +my $expect_launchpad_bugs_fixed; +my $info = Dpkg::Vendor::get_vendor_info(Dpkg::Vendor::get_current_vendor()); +ok( $info, "can retrieve vendor info" ); +if ($info->{Vendor} eq 'Ubuntu') { + $expect_launchpad_bugs_fixed = 1; +} else { + while (defined($info) && exists $info->{Parent}) { + $info = Dpkg::Vendor::get_vendor_info($info->{Parent}); + if ($info->{Vendor} eq 'Ubuntu') { + $expect_launchpad_bugs_fixed = 1; + last; + } + } +} + my $save_data; foreach my $file ("$srcdir/countme", "$srcdir/shadow", "$srcdir/fields", "$srcdir/regressions") { @@ -169,14 +185,21 @@ Urgency: high Maintainer: Frank Lichtenheld <fr...@lichtenheld.de> Date: Sun, 13 Jan 2008 15:49:19 +0100 Closes: 1000000 1111111 1111111 2222222 2222222 -Changes: +'; + if ($expect_launchpad_bugs_fixed) { + $expected .= 'Launchpad-Bugs-Fixed: 12345 54321 424242 2424242 +'; + } + $expected .= 'Changes: fields (2.0-0etch1) stable; urgency=low . * Upload to stable (Closes: #1111111, #2222222) + * Fix more stuff. (LP: #54321, #2424242) . fields (2.0-1) unstable; urgency=medium . * Upload to unstable (Closes: #1111111, #2222222) + * Fix stuff. (LP: #12345, #424242) . fields (2.0~b1-1) unstable; urgency=low,xc-userfield=foobar . @@ -197,10 +220,16 @@ Urgency: medium Maintainer: Frank Lichtenheld <dj...@debian.org> Date: Sun, 12 Jan 2008 15:49:19 +0100 Closes: 1111111 2222222 -Changes: +'; + if ($expect_launchpad_bugs_fixed) { + $expected .= 'Launchpad-Bugs-Fixed: 12345 424242 +'; + } + $expected .= 'Changes: fields (2.0-1) unstable; urgency=medium . * Upload to unstable (Closes: #1111111, #2222222) + * Fix stuff. (LP: #12345, #424242) . fields (2.0~b1-1) unstable; urgency=low,xc-userfield=foobar . diff --git a/scripts/t/600_Dpkg_Changelog/fields b/scripts/t/600_Dpkg_Changelog/fields index 5f08b71..5072755 100644 --- a/scripts/t/600_Dpkg_Changelog/fields +++ b/scripts/t/600_Dpkg_Changelog/fields @@ -1,12 +1,14 @@ fields (2.0-0etch1) stable; urgency=low * Upload to stable (Closes: #1111111, #2222222) + * Fix more stuff. (LP: #54321, #2424242) -- Frank Lichtenheld <fr...@lichtenheld.de> Sun, 13 Jan 2008 15:49:19 +0100 fields (2.0-1) unstable; urgency=medium * Upload to unstable (Closes: #1111111, #2222222) + * Fix stuff. (LP: #12345, #424242) -- Frank Lichtenheld <dj...@debian.org> Sun, 12 Jan 2008 15:49:19 +0100 -- 1.6.3.3