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

Reply via email to