Package: dpkg
Version: 1.15.8.6
Severity: normal
Tags: patch
User: [email protected]
Usertags: origin-ubuntu ubuntu-patch natty
The plymouth package in Ubuntu is maintained using 3.0 (quilt) and
--single-debian-patch. (This isn't my personal preference - I prefer
separate patches, not to mention that Debian's package uses separate
patches and we ought to rebase on top of that - but another developer
set it up and I haven't got around to arguing yet. :-) )
debian/patches/debian-changes is stored in bzr. I've noticed over time
that when people upload changes to plymouth without explicitly using
'quilt refresh', i.e. relying on dpkg-source to regenerate the patch
file for them, the ordering of files in the patch is shuffled. This is
annoying as it produces enormous but almost content-free deltas in
revision control.
'quilt refresh' does not have this problem. It sorts files in the
resulting patch as follows:
* Any files in the existing patch come first, in the order in which
they appear in the existing patch.
* New files follow, sorted lexicographically.
(See the files_in_patch_ordered function in
/usr/share/quilt/scripts/patchfns.)
This seems pretty reasonable, and I think dpkg-source should do the
same. The attached patch implements this.
Thanks,
--
Colin Watson [[email protected]]
>From 278a62058b132fa2e239e1dade8d60e5220ef910 Mon Sep 17 00:00:00 2001
From: Colin Watson <[email protected]>
Date: Mon, 6 Dec 2010 09:42:17 +0000
Subject: [PATCH] Keep file order stable when regenerating autopatches.
---
scripts/Dpkg/Source/Package/V2.pm | 3 +-
scripts/Dpkg/Source/Patch.pm | 82 +++++++++++++++++++++++++-----------
2 files changed, 59 insertions(+), 26 deletions(-)
diff --git a/scripts/Dpkg/Source/Package/V2.pm b/scripts/Dpkg/Source/Package/V2.pm
index 8f6618e..13552a7 100644
--- a/scripts/Dpkg/Source/Package/V2.pm
+++ b/scripts/Dpkg/Source/Package/V2.pm
@@ -432,7 +432,8 @@ sub do_build {
$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);
+ %{$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
diff --git a/scripts/Dpkg/Source/Patch.pm b/scripts/Dpkg/Source/Patch.pm
index efeef5d..6c6fbdf 100644
--- a/scripts/Dpkg/Source/Patch.pm
+++ b/scripts/Dpkg/Source/Patch.pm
@@ -154,6 +154,7 @@ sub add_diff_directory {
$diff_ignore = sub { return 0 };
}
+ my @diff_files;
my %files_in_new;
my $scan_new = sub {
my $fn = (length > length($new)) ? substr($_, length($new) + 1) : '.';
@@ -189,27 +190,8 @@ sub add_diff_directory {
if ($opts{'use_dev_null'}) {
$label_old = $old_file if $old_file eq '/dev/null';
}
- my $success = $self->add_diff_file($old_file, "$new/$fn",
- label_old => $label_old,
- label_new => "$basedir/$fn",
- %opts);
-
- if ($success and ($old_file eq "/dev/null")) {
- if (not $size) {
- warning(_g("newly created empty file '%s' will not " .
- "be represented in diff"), $fn);
- } else {
- if ($mode & (S_IXUSR | S_IXGRP | S_IXOTH)) {
- warning(_g("executable mode %04o of '%s' will " .
- "not be represented in diff"), $mode, $fn)
- unless $fn eq 'debian/rules';
- }
- if ($mode & (S_ISUID | S_ISGID | S_ISVTX)) {
- warning(_g("special mode %04o of '%s' will not " .
- "be represented in diff"), $mode, $fn);
- }
- }
- }
+ push @diff_files, [$fn, $mode, $size, $old_file, "$new/$fn",
+ $label_old, "$basedir/$fn"];
} elsif (-p _) {
unless (-p "$old/$fn") {
$self->_fail_not_same_type("$old/$fn", "$new/$fn");
@@ -235,10 +217,8 @@ sub add_diff_directory {
lstat("$old/$fn") || syserr(_g("cannot stat file %s"), "$old/$fn");
if (-f _) {
if ($inc_removal) {
- $self->add_diff_file("$old/$fn", "/dev/null",
- label_old => "$basedir.orig/$fn",
- label_new => "/dev/null",
- %opts);
+ push @diff_files, [$fn, 0, 0, "$old/$fn", "/dev/null",
+ "$basedir.orig/$fn", "/dev/null"];
} else {
warning(_g("ignoring deletion of file %s"), $fn);
}
@@ -253,6 +233,55 @@ sub add_diff_directory {
find({ wanted => $scan_new, no_chdir => 1 }, $new);
find({ wanted => $scan_old, no_chdir => 1 }, $old);
+
+ if ($opts{"order_from"} and -e $opts{"order_from"}) {
+ my $order_from = Dpkg::Source::Patch->new(
+ filename => $opts{"order_from"});
+ my $analysis = $order_from->analyze($basedir);
+ my %patchorder;
+ my $i = 0;
+ for my $fn (@{$analysis->{"patchorder"}}) {
+ $fn =~ s{^[^/]+/}{};
+ $patchorder{$fn} = $i++;
+ }
+ # 'quilt refresh' sorts files as follows:
+ # - Any files in the existing patch come first, in the order in
+ # which they appear in the existing patch.
+ # - New files follow, sorted lexicographically.
+ # This seems a reasonable policy to follow, and avoids autopatches
+ # being shuffled when they are regenerated.
+ for my $diff_file (sort { $a->[0] cmp $b->[0] } @diff_files) {
+ my $fn = $diff_file->[0];
+ $patchorder{$fn} = $i++ unless exists $patchorder{$fn};
+ }
+ @diff_files = map { $_->[0] } sort { $a->[1] <=> $b->[1] }
+ map { [$_, $patchorder{$_->[0]}] } @diff_files;
+ }
+
+ for my $diff_file (@diff_files) {
+ my ($fn, $mode, $size,
+ $old_file, $new_file, $label_old, $label_new) = @$diff_file;
+ my $success = $self->add_diff_file($old_file, $new_file,
+ label_old => $label_old,
+ label_new => $label_new, %opts);
+ if ($success and
+ $old_file eq "/dev/null" and $new_file ne "/dev/null") {
+ if (not $size) {
+ warning(_g("newly created empty file '%s' will not " .
+ "be represented in diff"), $fn);
+ } else {
+ if ($mode & (S_IXUSR | S_IXGRP | S_IXOTH)) {
+ warning(_g("executable mode %04o of '%s' will " .
+ "not be represented in diff"), $mode, $fn)
+ unless $fn eq 'debian/rules';
+ }
+ if ($mode & (S_ISUID | S_ISGID | S_ISVTX)) {
+ warning(_g("special mode %04o of '%s' will not " .
+ "be represented in diff"), $mode, $fn);
+ }
+ }
+ }
+ }
}
sub finish {
@@ -287,6 +316,7 @@ sub analyze {
my $diff = $self->get_filename();
my %filepatched;
my %dirtocreate;
+ my @patchorder;
my $diff_count = 0;
sub getline {
@@ -384,6 +414,7 @@ sub analyze {
error(_g("diff `%s' patches file %s twice"), $diff, $fn);
}
$filepatched{$fn} = 1;
+ push @patchorder, $fn;
# read hunks
my $hunk = 0;
@@ -424,6 +455,7 @@ sub analyze {
}
*$self->{'analysis'}{$destdir}{"dirtocreate"} = \%dirtocreate;
*$self->{'analysis'}{$destdir}{"filepatched"} = \%filepatched;
+ *$self->{'analysis'}{$destdir}{"patchorder"} = \...@patchorder;
return *$self->{'analysis'}{$destdir};
}
--
1.7.2.3
