Re: Some upcoming dpkg changes, test and feedback welcome
Hi,
On Wed, 20 Jul 2011, Guillem Jover wrote:
> > It required some important restructuring of the source package build
> > procedure so I would be glad to have some more people running with this.
> > Also you might have some input on the new interface.
>
> It would be preferable in general to split refactoring from new
> additional features so that reviewing is actually easier.
I made the effort to split the changes in several commits,
the new series is attached (and in my pu/master branch).
Reviews and tests are welcome.
> Yes, there are some pending changes I want to merge first, some
> requirements for the other multiarch changes, but I don't want to
> upload it as long as at least the the Build-Features stuff is on
> master.
Still waiting on your feedback to my other reply on this. Unfortunately
the tech-ctte has still not yet decided on this but it will likely happen
in the upcoming days.
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Follow my Debian News ▶ http://RaphaelHertzog.com (English)
▶ http://RaphaelHertzog.fr (Français)
>From c6cfc41b5963da180c993de61c830be2a28465f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Thu, 14 Jul 2011 18:56:30 +0200
Subject: [PATCH 1/6] dpkg-source: accept "." as the directory parameter
dpkg-source should never be called from within the unpacked source
tree, the result is usually not what one would expect. Fix this
by automatically converting the directory name and by changing the
current directory to the parent one.
This is particularly interesting for the upcoming --record-changes
option.
---
scripts/dpkg-source.pl | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/scripts/dpkg-source.pl b/scripts/dpkg-source.pl
index 209368f..6279fc1 100755
--- a/scripts/dpkg-source.pl
+++ b/scripts/dpkg-source.pl
@@ -11,7 +11,7 @@
# Copyright © 2005 Brendan O'Dea <bod@debian.org>
# Copyright © 2006-2008 Frank Lichtenheld <djpig@debian.org>
# Copyright © 2006-2009 Guillem Jover <guillem@debian.org>
-# Copyright © 2008-2010 Raphaël Hertzog <hertzog@debian.org>
+# Copyright © 2008-2011 Raphaël Hertzog <hertzog@debian.org>
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -45,6 +45,8 @@ use Dpkg::Changelog::Parse;
use Dpkg::Source::Package;
use Dpkg::Vendor qw(run_vendor_hook);
+use Cwd;
+use File::Basename;
use File::Spec;
textdomain("dpkg-dev");
@@ -104,6 +106,11 @@ if (defined($options{'opmode'}) &&
if (not -d $dir) {
error(_g("directory argument %s is not a directory"), $dir);
}
+ if ($dir eq ".") {
+ # . is never correct, adjust automatically
+ $dir = basename(cwd());
+ chdir("..") || syserr(_g("unable to chdir to `%s'"), "..");
+ }
# --format options are not allowed, they would take precedence
# over real command line options, debian/source/format should be used
# instead
--
1.7.5.4
>From 547989307084abaf89c450f44a3bd404fd140158 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Thu, 14 Jul 2011 19:02:49 +0200
Subject: [PATCH 2/6] dpkg-source: uniform handling of the patch header
Formats "2.0" and "3.0 (quilt)" now generate the patch header
with the same code. Drop some useless duplication.
---
scripts/Dpkg/Source/Package/V2.pm | 39 +++++++++++++++++++---
scripts/Dpkg/Source/Package/V3/quilt.pm | 52 +-----------------------------
2 files changed, 35 insertions(+), 56 deletions(-)
diff --git a/scripts/Dpkg/Source/Package/V2.pm b/scripts/Dpkg/Source/Package/V2.pm
index 7a1880a..8be3312 100644
--- a/scripts/Dpkg/Source/Package/V2.pm
+++ b/scripts/Dpkg/Source/Package/V2.pm
@@ -1,4 +1,4 @@
-# Copyright © 2008 Raphaël Hertzog <hertzog@debian.org>
+# Copyright © 2008-2011 Raphaël Hertzog <hertzog@debian.org>
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -30,6 +30,9 @@ use Dpkg::Source::Archive;
use Dpkg::Source::Patch;
use Dpkg::Exit;
use Dpkg::Source::Functions qw(erasedir is_binary fs_time);
+use Dpkg::Vendor qw(run_vendor_hook);
+use Dpkg::Control;
+use Dpkg::Changelog::Parse;
use POSIX;
use File::Basename;
@@ -526,7 +529,7 @@ sub do_build {
}
sub get_patch_header {
- my ($self, $dir, $previous) = @_;
+ my ($self, $dir) = @_;
my $ph = File::Spec->catfile($dir, "debian", "source", "local-patch-header");
unless (-f $ph) {
$ph = File::Spec->catfile($dir, "debian", "source", "patch-header");
@@ -538,10 +541,34 @@ sub get_patch_header {
close(PH);
return $text;
}
- return "Description: Undocumented upstream changes
- This patch has been created by dpkg-source during the package build
- but it might have accumulated changes from several uploads. Please
- check the changelog to (hopefully) learn more on those changes.\n\n";
+ my $ch_info = changelog_parse(offset => 0, count => 1,
+ file => File::Spec->catfile($dir, "debian", "changelog"));
+ return '' if not defined $ch_info;
+ my $header = Dpkg::Control->new(type => CTRL_UNKNOWN);
+ $header->{'Description'} = "<short summary of the patch>\n";
+ $header->{'Description'} .=
+"TODO: Put a short summary on the line above and replace this paragraph
+with a longer explanation of this change. Complete the meta-information
+with other relevant fields (see below for details). To make it easier, the
+information below has been extracted from the changelog. Adjust it or drop
+it.\n";
+ $header->{'Description'} .= $ch_info->{'Changes'} . "\n";
+ $header->{'Author'} = $ch_info->{'Maintainer'};
+ $text = "$header";
+ run_vendor_hook("extend-patch-header", \$text, $ch_info);
+ $text .= "\n---
+The information above should follow the Patch Tagging Guidelines, please
+checkout http://dep.debian.net/deps/dep3/ to learn about the format. Here
+are templates for supplementary fields that you might want to add:
+
+Origin: <vendor|upstream|other>, <url of original patch>
+Bug: <url in upstream bugtracker>
+Bug-Debian: http://bugs.debian.org/<bugnumber>
+Bug-Ubuntu: https://launchpad.net/bugs/<bugnumber>
+Forwarded: <no|not-needed|url proving that it has been forwarded>
+Reviewed-By: <name and email of someone who approved the patch>
+Last-Update: <YYYY-MM-DD>\n\n";
+ return $text;
}
sub register_autopatch {
diff --git a/scripts/Dpkg/Source/Package/V3/quilt.pm b/scripts/Dpkg/Source/Package/V3/quilt.pm
index eb201c1..42064e2 100644
--- a/scripts/Dpkg/Source/Package/V3/quilt.pm
+++ b/scripts/Dpkg/Source/Package/V3/quilt.pm
@@ -1,4 +1,4 @@
-# Copyright © 2008-2009 Raphaël Hertzog <hertzog@debian.org>
+# Copyright © 2008-2011 Raphaël Hertzog <hertzog@debian.org>
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -29,9 +29,7 @@ use Dpkg::ErrorHandling;
use Dpkg::Source::Patch;
use Dpkg::Source::Functions qw(erasedir fs_time);
use Dpkg::IPC;
-use Dpkg::Vendor qw(get_current_vendor run_vendor_hook);
-use Dpkg::Control;
-use Dpkg::Changelog::Parse;
+use Dpkg::Vendor qw(get_current_vendor);
use POSIX;
use File::Basename;
@@ -386,51 +384,5 @@ sub register_autopatch {
}
}
-sub get_patch_header {
- my ($self, $dir, $previous) = @_;
- my $ph = File::Spec->catfile($dir, "debian", "source", "local-patch-header");
- unless (-f $ph) {
- $ph = File::Spec->catfile($dir, "debian", "source", "patch-header");
- }
- my $text;
- if (-f $ph) {
- open(PH, "<", $ph) || syserr(_g("cannot read %s"), $ph);
- $text = join("", <PH>);
- close(PH);
- return $text;
- }
- my $ch_info = changelog_parse(offset => 0, count => 1,
- file => File::Spec->catfile($dir, "debian", "changelog"));
- return '' if not defined $ch_info;
- return $self->SUPER::get_patch_header($dir, $previous)
- if $self->{'options'}{'single-debian-patch'};
- my $header = Dpkg::Control->new(type => CTRL_UNKNOWN);
- $header->{'Description'} = "Upstream changes introduced in version " .
- $ch_info->{'Version'} . "\n";
- $header->{'Description'} .=
-"This patch has been created by dpkg-source during the package build.
-Here's the last changelog entry, hopefully it gives details on why
-those changes were made:\n";
- $header->{'Description'} .= $ch_info->{'Changes'} . "\n";
- $header->{'Description'} .=
-"\nThe person named in the Author field signed this changelog entry.\n";
- $header->{'Author'} = $ch_info->{'Maintainer'};
- $text = "$header";
- run_vendor_hook("extend-patch-header", \$text, $ch_info);
- $text .= "\n---
-The information above should follow the Patch Tagging Guidelines, please
-checkout http://dep.debian.net/deps/dep3/ to learn about the format. Here
-are templates for supplementary fields that you might want to add:
-
-Origin: <vendor|upstream|other>, <url of original patch>
-Bug: <url in upstream bugtracker>
-Bug-Debian: http://bugs.debian.org/<bugnumber>
-Bug-Ubuntu: https://launchpad.net/bugs/<bugnumber>
-Forwarded: <no|not-needed|url proving that it has been forwarded>
-Reviewed-By: <name and email of someone who approved the patch>
-Last-Update: <YYYY-MM-DD>\n\n";
- return $text;
-}
-
# vim:et:sw=4:ts=8
1;
--
1.7.5.4
>From 06fdcc4d20a58e78dc1024480df4e931cd2249b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Thu, 28 Jul 2011 15:10:43 +0200
Subject: [PATCH 3/6] Dpkg::Source::Package: replace register_autopatch() with
register_patch()
While register_autopatch() is only able to register a patch as the
automatic patch, register_patch() can register a patch under any
desired patch name.
Also it doesn't not drop the input patch file, leaving that responsibility
to whoever called it. However if the input patch file is empty, it will
remove the target patch from the debian source package.
---
scripts/Dpkg/Source/Package/V2.pm | 40 ++++++++++++++----------------
scripts/Dpkg/Source/Package/V3/quilt.pm | 36 +++++++++++++++++----------
2 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/scripts/Dpkg/Source/Package/V2.pm b/scripts/Dpkg/Source/Package/V2.pm
index 8be3312..4049781 100644
--- a/scripts/Dpkg/Source/Package/V2.pm
+++ b/scripts/Dpkg/Source/Package/V2.pm
@@ -40,6 +40,7 @@ use File::Temp qw(tempfile tempdir);
use File::Path;
use File::Spec;
use File::Find;
+use File::Copy;
our $CURRENT_MINOR_VERSION = "0";
@@ -471,31 +472,22 @@ sub do_build {
%{$self->{'diff_options'}}, handle_binary_func => $handle_binary,
order_from => $autopatch);
error(_g("unrepresentable changes to source")) if not $diff->finish();
- # The previous auto-patch must be removed, it has not been used and it
- # will be recreated if it's still needed
- if (-e $autopatch) {
- unlink($autopatch) || syserr(_g("cannot remove %s"), $autopatch);
- }
# Install the diff as the new autopatch
- if (not -s $tmpdiff) {
- unlink($tmpdiff) || syserr(_g("cannot remove %s"), $tmpdiff);
- } else {
- mkpath(File::Spec->catdir($dir, "debian", "patches"));
+ mkpath(File::Spec->catdir($dir, "debian", "patches"));
+ $autopatch = $self->register_patch($dir, $tmpdiff,
+ $self->get_autopatch_name());
+ if (-s $autopatch) {
info(_g("local changes stored in %s, the modified files are:"), $autopatch);
my $analysis = $diff->analyze($dir, verbose => 0);
foreach my $fn (sort keys %{$analysis->{'filepatched'}}) {
print " $fn\n";
}
- rename($tmpdiff, $autopatch) ||
- syserr(_g("cannot rename %s to %s"), $tmpdiff, $autopatch);
- chmod(0666 & ~ umask(), $autopatch) ||
- syserr(_g("unable to change permission of `%s'"), $autopatch);
}
- $self->register_autopatch($dir);
if (-e $autopatch and $self->{'options'}{'abort_on_upstream_changes'}) {
error(_g("aborting due to --abort-on-upstream-changes"));
}
rmdir(File::Spec->catdir($dir, "debian", "patches")); # No check on purpose
+ unlink($tmpdiff) || syserr(_g("cannot remove %s"), $tmpdiff);
pop @Dpkg::Exit::handlers;
# Remove the temporary directory
@@ -571,16 +563,22 @@ Last-Update: <YYYY-MM-DD>\n\n";
return $text;
}
-sub register_autopatch {
- my ($self, $dir) = @_;
- my $autopatch = File::Spec->catfile($dir, "debian", "patches",
- $self->get_autopatch_name());
- if (-e $autopatch) {
+sub register_patch {
+ my ($self, $dir, $patch_file, $patch_name) = @_;
+ my $patch = File::Spec->catfile($dir, "debian", "patches", $patch_name);
+ if (-s $patch_file) {
+ copy($patch_file, $patch) ||
+ syserr(_g("failed to copy %s to %s"), $patch_file, $patch);
+ chmod(0666 & ~ umask(), $patch) ||
+ syserr(_g("unable to change permission of `%s'"), $patch);
my $applied = File::Spec->catfile($dir, "debian", "patches", ".dpkg-source-applied");
open(APPLIED, '>>', $applied) || syserr(_g("cannot write %s"), $applied);
- print APPLIED ($self->get_autopatch_name() . "\n");
- close(APPLIED);
+ print APPLIED "$patch\n";
+ close(APPLIED) || syserr(_g("cannot close %s"), $applied);
+ } elsif (-e $patch) {
+ unlink($patch) || syserr(_g("cannot remove %s"), $patch);
}
+ return $patch;
}
# vim:et:sw=4:ts=8
diff --git a/scripts/Dpkg/Source/Package/V3/quilt.pm b/scripts/Dpkg/Source/Package/V3/quilt.pm
index 42064e2..4db776b 100644
--- a/scripts/Dpkg/Source/Package/V3/quilt.pm
+++ b/scripts/Dpkg/Source/Package/V3/quilt.pm
@@ -35,6 +35,7 @@ use POSIX;
use File::Basename;
use File::Spec;
use File::Path;
+use File::Copy;
our $CURRENT_MINOR_VERSION = "0";
@@ -330,8 +331,8 @@ sub check_patches_applied {
}
}
-sub register_autopatch {
- my ($self, $dir) = @_;
+sub register_patch {
+ my ($self, $dir, $tmpdiff, $patch_name) = @_;
sub add_line {
my ($file, $line) = @_;
@@ -350,38 +351,47 @@ sub register_autopatch {
close(FILE);
}
- my $auto_patch = $self->get_autopatch_name();
my @patches = $self->get_patches($dir);
- my $has_patch = (grep { $_ eq $auto_patch } @patches) ? 1 : 0;
+ my $has_patch = (grep { $_ eq $patch_name } @patches) ? 1 : 0;
my $series = $self->get_series_file($dir);
$series ||= File::Spec->catfile($dir, "debian", "patches", "series");
my $applied = File::Spec->catfile($dir, ".pc", "applied-patches");
- my $patch = File::Spec->catfile($dir, "debian", "patches", $auto_patch);
+ my $patch = File::Spec->catfile($dir, "debian", "patches", $patch_name);
+
+ if (-s $tmpdiff) {
+ copy($tmpdiff, $patch) ||
+ syserr(_g("failed to copy %s to %s"), $tmpdiff, $patch);
+ chmod(0666 & ~ umask(), $patch) ||
+ syserr(_g("unable to change permission of `%s'"), $patch);
+ } elsif (-e $patch) {
+ unlink($patch) || syserr(_g("cannot remove %s"), $patch);
+ }
if (-e $patch) {
$self->create_quilt_db($dir);
- # Add auto_patch to series file
+ # Add patch to series file
if (not $has_patch) {
- add_line($series, $auto_patch);
- add_line($applied, $auto_patch);
+ add_line($series, $patch_name);
+ add_line($applied, $patch_name);
}
# Ensure quilt meta-data are created and in sync with some trickery:
# reverse-apply the patch, drop .pc/$patch, re-apply it
# with the correct options to recreate the backup files
my $patch_obj = Dpkg::Source::Patch->new(filename => $patch);
$patch_obj->apply($dir, add_options => ['-R', '-E'], verbose => 0);
- erasedir(File::Spec->catdir($dir, ".pc", $auto_patch));
- $self->apply_quilt_patch($dir, $auto_patch);
+ erasedir(File::Spec->catdir($dir, ".pc", $patch_name));
+ $self->apply_quilt_patch($dir, $patch_name);
} else {
# Remove auto_patch from series
if ($has_patch) {
- drop_line($series, $auto_patch);
- drop_line($applied, $auto_patch);
- erasedir(File::Spec->catdir($dir, ".pc", $auto_patch));
+ drop_line($series, $patch_name);
+ drop_line($applied, $patch_name);
+ erasedir(File::Spec->catdir($dir, ".pc", $patch_name));
}
# Clean up empty series
unlink($series) if not -s $series;
}
+ return $patch;
}
# vim:et:sw=4:ts=8
--
1.7.5.4
>From af32d4fd4fca74daf44260f901bbfe603f31d03a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Thu, 28 Jul 2011 16:26:51 +0200
Subject: [PATCH 4/6] Dpkg::Source::Package::V2: move logic to create patches
in a separate function
This commit extracts the logic to create automatic patches in a new
generate_patch() method. It's expected that this function will be
reused to implement dpkg-source --commit.
The code is mainly moved around as-is to simplify reviews. All
desired changes have been left for further commits.
---
scripts/Dpkg/Source/Package/V2.pm | 91 ++++++++++++++++++++++---------------
1 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/scripts/Dpkg/Source/Package/V2.pm b/scripts/Dpkg/Source/Package/V2.pm
index 4049781..f846bc8 100644
--- a/scripts/Dpkg/Source/Package/V2.pm
+++ b/scripts/Dpkg/Source/Package/V2.pm
@@ -312,23 +312,12 @@ sub check_patches_applied {
}
}
-sub do_build {
- my ($self, $dir) = @_;
+sub generate_patch {
+ my ($self, $dir, %opts) = @_;
my ($dirname, $updir) = fileparse($dir);
- my @argv = @{$self->{'options'}{'ARGV'}};
- if (scalar(@argv)) {
- usageerr(_g("-b takes only one parameter with format `%s'"),
- $self->{'fields'}{'Format'});
- }
- $self->prepare_build($dir);
-
- my $include_binaries = $self->{'options'}{'include_binaries'};
- my @tar_ignore = map { "--exclude=$_" } @{$self->{'options'}{'tar_ignore'}};
-
my $sourcepackage = $self->{'fields'}{'Source'};
my $basenamerev = $self->get_basename(1);
- my $basename = $self->get_basename();
- my $basedirname = $basename;
+ my $basedirname = $self->get_basename();
$basedirname =~ s/_/-/;
# Identify original tarballs
@@ -379,6 +368,53 @@ sub do_build {
# Apply all patches except the last automatic one
$self->apply_patches($tmp, skip_auto => 1, usage => 'build');
+ # Create a patch
+ my $autopatch = File::Spec->catfile($dir, "debian", "patches",
+ $self->get_autopatch_name());
+ my ($difffh, $tmpdiff) = tempfile("$basenamerev.diff.XXXXXX",
+ DIR => $updir, UNLINK => 0);
+ push @Dpkg::Exit::handlers, sub { unlink($tmpdiff) };
+ my $diff = Dpkg::Source::Patch->new(filename => $tmpdiff,
+ compression => "none");
+ $diff->create();
+ $diff->set_header($self->get_patch_header($dir, $autopatch));
+ $diff->add_diff_directory($tmp, $dir, basedirname => $basedirname,
+ %{$self->{'diff_options'}},
+ handle_binary_func => $opts{'handle_binary'},
+ order_from => $opts{'order_from'});
+ error(_g("unrepresentable changes to source")) if not $diff->finish();
+
+ if (-s $autopatch) {
+ info(_g("local changes stored in %s, the modified files are:"), $autopatch);
+ my $analysis = $diff->analyze($dir, verbose => 0);
+ foreach my $fn (sort keys %{$analysis->{'filepatched'}}) {
+ print " $fn\n";
+ }
+ }
+
+ # Remove the temporary directory
+ erasedir($tmp);
+ pop @Dpkg::Exit::handlers;
+ pop @Dpkg::Exit::handlers;
+
+ return $tmpdiff;
+}
+
+sub do_build {
+ my ($self, $dir) = @_;
+ my @argv = @{$self->{'options'}{'ARGV'}};
+ if (scalar(@argv)) {
+ usageerr(_g("-b takes only one parameter with format `%s'"),
+ $self->{'fields'}{'Format'});
+ }
+ $self->prepare_build($dir);
+
+ my $include_binaries = $self->{'options'}{'include_binaries'};
+ my @tar_ignore = map { "--exclude=$_" } @{$self->{'options'}{'tar_ignore'}};
+
+ my $sourcepackage = $self->{'fields'}{'Source'};
+ my $basenamerev = $self->get_basename(1);
+
# Prepare handling of binary files
my %auth_bin_files;
my $incbin_file = File::Spec->catfile($dir, "debian", "source", "include-binaries");
@@ -461,28 +497,15 @@ sub do_build {
# Create a patch
my $autopatch = File::Spec->catfile($dir, "debian", "patches",
$self->get_autopatch_name());
- my ($difffh, $tmpdiff) = tempfile("$basenamerev.diff.XXXXXX",
- DIR => $updir, UNLINK => 0);
+ my $tmpdiff = $self->generate_patch($dir, order_from => $autopatch,
+ handle_binary => $handle_binary,
+ usage => 'build');
push @Dpkg::Exit::handlers, sub { unlink($tmpdiff) };
- my $diff = Dpkg::Source::Patch->new(filename => $tmpdiff,
- compression => "none");
- $diff->create();
- $diff->set_header($self->get_patch_header($dir, $autopatch));
- $diff->add_diff_directory($tmp, $dir, basedirname => $basedirname,
- %{$self->{'diff_options'}}, handle_binary_func => $handle_binary,
- order_from => $autopatch);
- error(_g("unrepresentable changes to source")) if not $diff->finish();
+
# Install the diff as the new autopatch
mkpath(File::Spec->catdir($dir, "debian", "patches"));
$autopatch = $self->register_patch($dir, $tmpdiff,
$self->get_autopatch_name());
- if (-s $autopatch) {
- info(_g("local changes stored in %s, the modified files are:"), $autopatch);
- my $analysis = $diff->analyze($dir, verbose => 0);
- foreach my $fn (sort keys %{$analysis->{'filepatched'}}) {
- print " $fn\n";
- }
- }
if (-e $autopatch and $self->{'options'}{'abort_on_upstream_changes'}) {
error(_g("aborting due to --abort-on-upstream-changes"));
}
@@ -490,10 +513,6 @@ sub do_build {
unlink($tmpdiff) || syserr(_g("cannot remove %s"), $tmpdiff);
pop @Dpkg::Exit::handlers;
- # Remove the temporary directory
- erasedir($tmp);
- pop @Dpkg::Exit::handlers;
-
# Update debian/source/include-binaries if needed
if (scalar(@binary_files) and $include_binaries) {
mkpath(File::Spec->catdir($dir, "debian", "source"));
@@ -509,7 +528,7 @@ sub do_build {
# Create the debian.tar
my $debianfile = "$basenamerev.debian.tar." . $self->{'options'}{'comp_ext'};
info(_g("building %s in %s"), $sourcepackage, $debianfile);
- $tar = Dpkg::Source::Archive->new(filename => $debianfile);
+ my $tar = Dpkg::Source::Archive->new(filename => $debianfile);
$tar->create(options => \@tar_ignore, 'chdir' => $dir);
$tar->add_directory("debian");
foreach my $binary (@binary_files) {
--
1.7.5.4
>From e601fb8fe9277c6236d8ecde22b689572fa4ec6f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Thu, 28 Jul 2011 17:14:55 +0200
Subject: [PATCH 5/6] Dpkg::Source::Package::V2: cleanup generate_patch()
Drop all references to $autopatch which has nothing to do with
generating a patch. Move the message explaining where the
changes have been recorded outside of the function.
Drop unused parameter to get_patch_header().
Drop intermediary variables which are only used once.
---
scripts/Dpkg/Source/Package/V2.pm | 11 ++++-------
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/scripts/Dpkg/Source/Package/V2.pm b/scripts/Dpkg/Source/Package/V2.pm
index f846bc8..9a16cc6 100644
--- a/scripts/Dpkg/Source/Package/V2.pm
+++ b/scripts/Dpkg/Source/Package/V2.pm
@@ -315,8 +315,6 @@ sub check_patches_applied {
sub generate_patch {
my ($self, $dir, %opts) = @_;
my ($dirname, $updir) = fileparse($dir);
- my $sourcepackage = $self->{'fields'}{'Source'};
- my $basenamerev = $self->get_basename(1);
my $basedirname = $self->get_basename();
$basedirname =~ s/_/-/;
@@ -369,23 +367,21 @@ sub generate_patch {
$self->apply_patches($tmp, skip_auto => 1, usage => 'build');
# Create a patch
- my $autopatch = File::Spec->catfile($dir, "debian", "patches",
- $self->get_autopatch_name());
my ($difffh, $tmpdiff) = tempfile("$basenamerev.diff.XXXXXX",
DIR => $updir, UNLINK => 0);
push @Dpkg::Exit::handlers, sub { unlink($tmpdiff) };
my $diff = Dpkg::Source::Patch->new(filename => $tmpdiff,
compression => "none");
$diff->create();
- $diff->set_header($self->get_patch_header($dir, $autopatch));
+ $diff->set_header($self->get_patch_header($dir));
$diff->add_diff_directory($tmp, $dir, basedirname => $basedirname,
%{$self->{'diff_options'}},
handle_binary_func => $opts{'handle_binary'},
order_from => $opts{'order_from'});
error(_g("unrepresentable changes to source")) if not $diff->finish();
- if (-s $autopatch) {
- info(_g("local changes stored in %s, the modified files are:"), $autopatch);
+ if (-s $tmpdiff) {
+ info(_g("local changes detected, the modified files are:"));
my $analysis = $diff->analyze($dir, verbose => 0);
foreach my $fn (sort keys %{$analysis->{'filepatched'}}) {
print " $fn\n";
@@ -509,6 +505,7 @@ sub do_build {
if (-e $autopatch and $self->{'options'}{'abort_on_upstream_changes'}) {
error(_g("aborting due to --abort-on-upstream-changes"));
}
+ info(_g("local changes have been recorded in a new patch: %s"), $autopatch);
rmdir(File::Spec->catdir($dir, "debian", "patches")); # No check on purpose
unlink($tmpdiff) || syserr(_g("cannot remove %s"), $tmpdiff);
pop @Dpkg::Exit::handlers;
--
1.7.5.4
>From 47f2b6f03cead378b8657cdfe14af093b026707c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Thu, 14 Jul 2011 20:31:33 +0200
Subject: [PATCH 6/6] dpkg-source: implement --commit and fail with unrecorded
changes
Formats "2.0" and "3.0 (quilt)" now fail by default in presence of
changes to upstream files that are not managed by their respective patch
system. The user is invited to run dpkg-source --commit if he
wants to keep the changes.
This will avoid that maintainers upload packages with unexpected changes.
The old behaviour can be kept with the option --auto-commit. The option
--abort-on-upstream-changes is now useless with formats "2.0" and "3.0
(quilt)" except to cancel the effect of a former --auto-commit.
See http://lists.debian.org/20110529085303.GA17707@rivendell.home.ouaza.com
for the discussion that enterined this change.
---
debian/changelog | 6 +++
man/dpkg-source.1 | 29 ++++++++++++++--
scripts/Dpkg/Source/Package.pm | 8 ++++-
scripts/Dpkg/Source/Package/V2.pm | 70 +++++++++++++++++++++++++++++++-----
scripts/dpkg-source.pl | 22 ++++++++---
5 files changed, 115 insertions(+), 20 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index 293e0f5..6b4f4d5 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -85,6 +85,12 @@ dpkg (1.16.1) UNRELEASED; urgency=low
Closes: #606839
* Fix the dpkg-divert test-suite to also skip test that would fail if run
under root. Closes: #634961
+ * With source format 2.0 and 3.0 (quilt), dpkg-source now fails by default
+ when upstream changes have not been recorded in a quilt patch. The new
+ option --commit can be used to properly record the changes before-hand.
+ LP: #797839
+ And it fails before installing the automatic patch in debian/patches/
+ Closes: #615899
[ Guillem Jover ]
* Install deb-src-control(5) man pages in dpkg-dev. Closes: #620520
diff --git a/man/dpkg-source.1 b/man/dpkg-source.1
index 025f7db..1e45974 100644
--- a/man/dpkg-source.1
+++ b/man/dpkg-source.1
@@ -1,5 +1,5 @@
.\" Authors: Ian Jackson, Raphaël Hertzog
-.TH dpkg\-source 1 "2011-07-04" "Debian Project" "dpkg utilities"
+.TH dpkg\-source 1 "2011-07-12" "Debian Project" "dpkg utilities"
.SH NAME
dpkg\-source \- Debian source package (.dsc) manipulation tool
.
@@ -88,6 +88,12 @@ all source formats implement something in this hook, and those that do
usually use it to undo what \fB\-\-before\-build\fP has done.
.TP
+.RI "\fB\-\-commit\fP [" directory "] ..."
+Record changes in the source tree unpacked in \fIdirectory\fP. This
+command can take supplementary parameters depending on the source format.
+It will error out for formats where this operation doesn't mean anything.
+
+.TP
.BR \-h ", " \-\-help
Show the usage message and exit.
.TP
@@ -437,7 +443,9 @@ debian directory is copied over in the temporary directory, and all
patches except the automatic patch (\fBdebian-changes-\fP\fIversion\fP
or \fBdebian-changes\fP, depending on \fB\-\-single\-debian\-patch\fP) are
applied. The temporary directory is compared to the source package
-directory and the diff (if non-empty) is stored in the automatic patch.
+directory. When the diff is non-empty, the build fails unless
+\fB\-\-single\-debian\-patch\fP or \fB\-\-auto\-commit\fP
+has been used. In the latter case, the diff is stored in the automatic patch.
If the automatic patch is created/deleted, it's added/removed from the
series file and from the quilt metadata.
@@ -466,6 +474,17 @@ unapplied patches (they are listed in the \fBseries\fP file but not in
applied without errors, it will apply them all. The option
\fB\-\-no\-preparation\fP can be used to disable this
behavior.
+.PP Recording changes
+.RI "\fB\-\-commit " [ directory "] [" patch-name "] [" patch-file ]
+Generates a patch corresponding to the local changes that are not managed
+by the quilt patch system and integrates it in the patch system under
+the name \fIpatch-name\fP. If the name is missing, it will be asked
+interactively. If \fIpatch-file\fP is given, it is used as the patch
+corresponding to the local changes to integrate. This is mainly useful
+after a build failure that pre-generated this file. Once integrated, an
+editor is launched so that you can edit the meta-information in the patch
+header.
+
.PP
.B Build options
.TP
@@ -523,6 +542,10 @@ can be used to ensure that all changes were properly recorded in separate
quilt patches prior to the source package build. This option is not
allowed in \fBdebian/source/options\fP but can be used in
\fBdebian/source/local\-options\fP.
+.TP
+.B \-\-auto\-commit
+The process doesn't fail if an automatic patch has been generated, instead
+it's immediately recorded in the quilt series.
.PP
.B Extract options
@@ -704,7 +727,7 @@ Copyright \(co 1995-1996 Ian Jackson
.br
Copyright \(co 2000 Wichert Akkerman
.br
-Copyright \(co 2008-2010 Rapha\[:e]l Hertzog
+Copyright \(co 2008-2011 Rapha\[:e]l Hertzog
.sp
This is free software; see the GNU General Public Licence version 2 or later
for copying conditions. There is NO WARRANTY.
diff --git a/scripts/Dpkg/Source/Package.pm b/scripts/Dpkg/Source/Package.pm
index 21089a2..94e6b8f 100644
--- a/scripts/Dpkg/Source/Package.pm
+++ b/scripts/Dpkg/Source/Package.pm
@@ -1,4 +1,4 @@
-# Copyright © 2008 Raphaël Hertzog <hertzog@debian.org>
+# Copyright © 2008-2011 Raphaël Hertzog <hertzog@debian.org>
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -512,6 +512,12 @@ sub add_file {
use_files_for_md5 => 1);
}
+sub commit {
+ my ($self, $dir) = @_;
+ info(_g("'%s' is not supported by the source format '%s'"),
+ "dpkg-source --commit", $self->{'fields'}{'Format'});
+}
+
sub write_dsc {
my ($self, %opts) = @_;
my $fields = $self->{'fields'};
diff --git a/scripts/Dpkg/Source/Package/V2.pm b/scripts/Dpkg/Source/Package/V2.pm
index 9a16cc6..1ea18bd 100644
--- a/scripts/Dpkg/Source/Package/V2.pm
+++ b/scripts/Dpkg/Source/Package/V2.pm
@@ -63,8 +63,8 @@ sub init_options {
unless exists $self->{'options'}{'skip_debianization'};
$self->{'options'}{'create_empty_orig'} = 0
unless exists $self->{'options'}{'create_empty_orig'};
- $self->{'options'}{'abort_on_upstream_changes'} = 0
- unless exists $self->{'options'}{'abort_on_upstream_changes'};
+ $self->{'options'}{'auto_commit'} = 0
+ unless exists $self->{'options'}{'auto_commit'};
}
sub parse_cmdline_option {
@@ -94,7 +94,10 @@ sub parse_cmdline_option {
$self->{'options'}{'create_empty_orig'} = 1;
return 1;
} elsif ($opt =~ /^--abort-on-upstream-changes$/) {
- $self->{'options'}{'abort_on_upstream_changes'} = 1;
+ $self->{'options'}{'auto_commit'} = 0;
+ return 1;
+ } elsif ($opt =~ /^--auto-commit$/) {
+ $self->{'options'}{'auto_commit'} = 1;
return 1;
}
return 0;
@@ -340,8 +343,10 @@ sub generate_patch {
error(_g("no upstream tarball found at %s"),
$self->upstream_tarball_template()) unless $tarfile;
- info(_g("building %s using existing %s"),
- $sourcepackage, "@origtarballs");
+ if ($opts{'usage'} eq "build") {
+ info(_g("building %s using existing %s"),
+ $self->{'fields'}{'Source'}, "@origtarballs");
+ }
# Unpack a second copy for comparison
my $tmp = tempdir("$dirname.orig.XXXXXX", DIR => $updir);
@@ -367,8 +372,8 @@ sub generate_patch {
$self->apply_patches($tmp, skip_auto => 1, usage => 'build');
# Create a patch
- my ($difffh, $tmpdiff) = tempfile("$basenamerev.diff.XXXXXX",
- DIR => $updir, UNLINK => 0);
+ my ($difffh, $tmpdiff) = tempfile($self->get_basename(1) . ".diff.XXXXXX",
+ DIR => File::Spec->tmpdir(), UNLINK => 0);
push @Dpkg::Exit::handlers, sub { unlink($tmpdiff) };
my $diff = Dpkg::Source::Patch->new(filename => $tmpdiff,
compression => "none");
@@ -497,14 +502,18 @@ sub do_build {
handle_binary => $handle_binary,
usage => 'build');
push @Dpkg::Exit::handlers, sub { unlink($tmpdiff) };
+ unless (not -s $tmpdiff or $self->{'options'}{'single_debian_patch'}
+ or $self->{'options'}{'auto_commit'}) {
+ info(_g("you can integrate the local changes with %s"),
+ "dpkg-source --commit . '' $tmpdiff");
+ error(_g("aborting due to unexpected upstream changes, see %s"),
+ $tmpdiff);
+ }
# Install the diff as the new autopatch
mkpath(File::Spec->catdir($dir, "debian", "patches"));
$autopatch = $self->register_patch($dir, $tmpdiff,
$self->get_autopatch_name());
- if (-e $autopatch and $self->{'options'}{'abort_on_upstream_changes'}) {
- error(_g("aborting due to --abort-on-upstream-changes"));
- }
info(_g("local changes have been recorded in a new patch: %s"), $autopatch);
rmdir(File::Spec->catdir($dir, "debian", "patches")); # No check on purpose
unlink($tmpdiff) || syserr(_g("cannot remove %s"), $tmpdiff);
@@ -597,5 +606,46 @@ sub register_patch {
return $patch;
}
+sub commit {
+ my ($self, $dir) = @_;
+ my ($patch_name, $tmpdiff) = @{$self->{'options'}{'ARGV'}};
+
+ sub bad_patch_name {
+ my ($dir, $patch_name) = @_;
+ return 1 if not defined($patch_name);
+ return 1 if not length($patch_name);
+ my $patch = File::Spec->catfile($dir, "debian", "patches", $patch_name);
+ if (-e $patch) {
+ warning(_g("cannot register changes in %s, this patch already exists"), $patch);
+ return 1;
+ }
+ return 0;
+ }
+
+ $self->prepare_build($dir);
+
+ unless ($tmpdiff && -e $tmpdiff) {
+ $tmpdiff = $self->generate_patch($dir, usage => "commit");
+ }
+ push @Dpkg::Exit::handlers, sub { unlink($tmpdiff) };
+ unless (-s $tmpdiff) {
+ unlink($tmpdiff) || syserr(_g("cannot remove %s"), $tmpdiff);
+ info(_g("there are no local changes to record"));
+ return;
+ }
+ while (bad_patch_name($dir, $patch_name)) {
+ # Ask the patch name interactively
+ print STDOUT _g("Enter the desired patch name: ");
+ chomp($patch_name = <STDIN>);
+ $patch_name =~ s/\s+/-/g;
+ $patch_name =~ s/\///g;
+ }
+ my $patch = $self->register_patch($dir, $tmpdiff, $patch_name);
+ system("sensible-editor", $patch);
+ unlink($tmpdiff) || syserr(_g("cannot remove %s"), $tmpdiff);
+ pop @Dpkg::Exit::handlers;
+ info(_g("local changes have been recorded in a new patch: %s"), $patch);
+}
+
# vim:et:sw=4:ts=8
1;
diff --git a/scripts/dpkg-source.pl b/scripts/dpkg-source.pl
index 6279fc1..1b90491 100755
--- a/scripts/dpkg-source.pl
+++ b/scripts/dpkg-source.pl
@@ -87,6 +87,8 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
setopmode('-x');
} elsif (m/^--(before|after)-build$/) {
setopmode($_);
+ } elsif (m/^--commit$/) {
+ setopmode($_);
} elsif (m/^--print-format$/) {
setopmode('--print-format');
report_options(info_fh => \*STDERR); # Avoid clutter on STDOUT
@@ -97,11 +99,14 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
my $dir;
if (defined($options{'opmode'}) &&
- $options{'opmode'} =~ /^(-b|--print-format|--before-build|--after-build)$/) {
+ $options{'opmode'} =~ /^(-b|--print-format|--(before|after)-build|--commit)$/) {
if (not scalar(@ARGV)) {
- usageerr(_g("%s needs a directory"), $options{'opmode'});
+ usageerr(_g("%s needs a directory"), $options{'opmode'})
+ unless $1 eq "--commit";
+ $dir = ".";
+ } else {
+ $dir = File::Spec->catdir(shift(@ARGV));
}
- $dir = File::Spec->catdir(shift(@ARGV));
stat($dir) || syserr(_g("cannot stat directory %s"), $dir);
if (not -d $dir) {
error(_g("directory argument %s is not a directory"), $dir);
@@ -206,10 +211,10 @@ while (@options) {
}
unless (defined($options{'opmode'})) {
- usageerr(_g("need a command (-x, -b, --before-build, --after-build, --print-format)"));
+ usageerr(_g("need a command (-x, -b, --before-build, --after-build, --print-format, --commit)"));
}
-if ($options{'opmode'} =~ /^(-b|--print-format|--(before|after)-build)$/) {
+if ($options{'opmode'} =~ /^(-b|--print-format|--(before|after)-build|--commit)$/) {
$options{'ARGV'} = \@ARGV;
@@ -363,6 +368,9 @@ if ($options{'opmode'} =~ /^(-b|--print-format|--(before|after)-build)$/) {
} elsif ($options{'opmode'} eq "--after-build") {
$srcpkg->after_build($dir);
exit(0);
+ } elsif ($options{'opmode'} eq "--commit") {
+ $srcpkg->commit($dir);
+ exit(0);
}
# Verify pre-requisites are met
@@ -466,7 +474,9 @@ Commands:
extract source package.
-b <dir> build source package.
--print-format <dir> print the source format that would be
- used to build the source package.")
+ used to build the source package.
+ --commit [<dir> [<patch-name>]]
+ store upstream changes in a new patch.")
. "\n\n" . _g(
"Build options:
-c<controlfile> get control info from this file.
--
1.7.5.4
Reply to: