[debhelper-devel] [debhelper] 01/01: dh: Do not re-run completed targets
Niels Thykier
nthykier at moszumanska.debian.org
Tue Jan 2 14:07:02 UTC 2018
This is an automated email from the git hooks/post-receive script.
nthykier pushed a commit to branch master
in repository debhelper.
commit 33788cb6ce471f5ee4b02444414a20163e428a25
Author: Niels Thykier <niels at thykier.net>
Date: Tue Jan 2 14:01:01 2018 +0000
dh: Do not re-run completed targets
This is part of 2 of the dh sequence rewrite. Part 1
is cb2caa7f67837294b0681d881f52dd23df487f33.
This change ensures that dh no longer attempts to run a target that is
already marked as complete (e.g. via the stamp file).
Signed-off-by: Niels Thykier <niels at thykier.net>
---
debian/changelog | 3 +
dh | 117 ++++++++++++++++++++++++----------
lib/Debian/Debhelper/SequencerUtil.pm | 5 +-
t/dh-sequencer.t | 37 +++++++----
4 files changed, 115 insertions(+), 47 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index b0448c2..9db0168 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -53,6 +53,9 @@ debhelper (11.1) UNRELEASED; urgency=medium
* dh_gencontrol: Deduplicate debug-ids before inserting them into
the control file. Thanks to Mattia Rizzolo for reporting the
bug. (Closes: #886038)
+ * dh: Rewrite sequence handling to ensure that dh does not "inline"
+ a subtarget of a target it recurses into. Thanks to James
+ Cowgill for reporting the bug. (Closes: #880840)
-- Niels Thykier <niels at thykier.net> Sun, 17 Dec 2017 07:59:18 +0000
diff --git a/dh b/dh
index 201baa7..537d5e7 100755
--- a/dh
+++ b/dh
@@ -454,7 +454,7 @@ $sequences{'build-indep'} = [@bd];
$sequences{'build-arch'} = [@bd];
if (! compat(8)) {
# From v9, sequences take standard rules targets into account.
- $sequences{build} = [@bd_minimal, to_rules_target("build-arch"), to_rules_target("build-indep")];
+ $sequences{build} = [to_rules_target("build-arch"), to_rules_target("build-indep")];
$sequences{'install-indep'} = [to_rules_target("build-indep"), @i];
$sequences{'install-arch'} = [to_rules_target("build-arch"), @i];
$sequences{'install'} = [to_rules_target("build"), to_rules_target("install-arch"), to_rules_target("install-indep")];
@@ -574,13 +574,27 @@ if (! exists $sequences{$sequence}) {
error "Unknown sequence $sequence (choose from: ".
join(" ", sort keys %sequences).")";
}
-# In compat <= 9, the sequences are always inlined (those versions do not
-# recurse into debian/rules anyway). In compat 10+, we never inline an
-# existing rules target.
-my @sequence = optimize_sequence(\%sequences, $sequence, (!compat(9) ? 0 : 1));
# The list of all packages that can be acted on.
my @packages=@{$dh{DOPACKAGES}};
+my @arch_packages = getpackages("arch");
+my @indep_packages = getpackages("indep");
+my %sequence2packages = (
+ 'build-arch' => \@arch_packages,
+ 'install-arch' => \@arch_packages,
+ 'binary-arch' => \@arch_packages,
+
+ 'build-indep' => \@indep_packages,
+ 'install-indep' => \@indep_packages,
+ 'binary-indep' => \@indep_packages,
+
+ 'clean' => \@packages,
+ 'build' => \@packages,
+ 'install' => \@packages,
+ 'binary' => \@packages,
+);
+
+my %completed_sequences;
# Get the options to pass to commands in the sequence.
# Filter out options intended only for this program.
@@ -593,16 +607,14 @@ if ($sequence eq 'build-arch' ||
push @options, "-a";
# as an optimisation, remove from the list any packages
# that are not arch dependent
- my %arch_packages = map { $_ => 1 } getpackages("arch");
- @packages = grep { $arch_packages{$_} } @packages;
+ @packages = @{$sequence2packages{$sequence}};
}
elsif ($sequence eq 'build-indep' ||
$sequence eq 'install-indep' ||
$sequence eq 'binary-indep') {
push @options, "-i";
# ditto optimisation for arch indep
- my %indep_packages = map { $_ => 1 } getpackages("indep");
- @packages = grep { $indep_packages{$_} } @packages;
+ @packages = @{$sequence2packages{$sequence}};
}
while (@ARGV_orig) {
my $opt=shift @ARGV_orig;
@@ -647,19 +659,44 @@ while (@ARGV_orig) {
}
# Figure out at what point in the sequence to start for each package.
-my %logged;
-my %startpoint;
-my %stamp_file;
+my (%logged, %startpoint, %stamp_file);
-if ( -f $build_stamp_file) {
+if ( -f $build_stamp_file and not compat(9)) {
open(my $fd, '<', $build_stamp_file) or error("open($build_stamp_file, ro) failed: $!");
while (my $line = <$fd>) {
chomp($line);
$stamp_file{$line} = 1;
}
close($fd);
+ my $build_indep_target_done = 1;
+ my $build_arch_target_done = 1;
+ for my $pkg (@{$sequence2packages{'build-arch'}}) {
+ if (not $stamp_file{$pkg}) {
+ $build_arch_target_done = 0;
+ last;
+ }
+ }
+ for my $pkg (@{$sequence2packages{'build-indep'}}) {
+ if (not $stamp_file{$pkg}) {
+ $build_indep_target_done = 0;
+ last;
+ }
+ }
+ $completed_sequences{'build-arch'} = 1 if $build_arch_target_done;
+ $completed_sequences{'build-indep'} = 1 if $build_indep_target_done;
+ $completed_sequences{'build'} = 1 if $build_indep_target_done and $build_arch_target_done;
}
+# In compat <= 9, the sequences are always inlined (those versions do not
+# recurse into debian/rules anyway). In compat 10+, we never inline an
+# existing rules target.
+my ($rules_targets, $full_sequence) = optimize_sequence(\%sequences,
+ $sequence,
+ (!compat(9) ? 0 : 1),
+ \%completed_sequences
+ );
+
+
# Lazy cache of the result of optimize_sequence on the "build"
# sequence
my $optimized_build_seq;
@@ -673,8 +710,8 @@ foreach my $package (@packages) {
# everything (admittedly, it is bit of an over
# approximation)
# Related bug: #851071
- my @seq = optimize_sequence(\%sequences, 'build', 1);
- $optimized_build_seq = \@seq;
+ my (undef, $seq) = optimize_sequence(\%sequences, 'build', 1);
+ $optimized_build_seq = $seq;
}
@log = @{$optimized_build_seq};
# We do not need %logged in compat 10
@@ -682,13 +719,13 @@ foreach my $package (@packages) {
if ($dh{AFTER}) {
# Run commands in the sequence that come after the
# specified command.
- $startpoint{$package}=command_pos($dh{AFTER}, @sequence) + 1;
+ $startpoint{$package} = command_pos($dh{AFTER}, @{$full_sequence}) + 1;
# Write a dummy log entry indicating that the specified
# command was, in fact, run. This handles the case where
# no commands remain to run after it, communicating to
# future dh instances that the specified command should not
# be run again.
- write_log($sequence[$startpoint{$package}-1], $package);
+ write_log($full_sequence->[$startpoint{$package}-1], $package);
}
elsif ($dh{REMAINING}) {
# Start at the beginning so all remaining commands will get
@@ -701,8 +738,8 @@ foreach my $package (@packages) {
# command is in the sequence, we're starting at the beginning..
$startpoint{$package}=0;
COMMAND: foreach my $command (reverse @log) {
- foreach my $i (0..$#sequence) {
- if ($command eq $sequence[$i]) {
+ foreach my $i (0..$#{$full_sequence}) {
+ if ($command eq $full_sequence->[$i]) {
$startpoint{$package}=$i+1;
last COMMAND;
}
@@ -712,24 +749,46 @@ COMMAND: foreach my $command (reverse @log) {
}
# Figure out what point in the sequence to go to.
-my $stoppoint=$#sequence;
+my $stoppoint=$#{$full_sequence};
if ($dh{UNTIL}) {
- $stoppoint=command_pos($dh{UNTIL}, @sequence);
+ $stoppoint = command_pos($dh{UNTIL}, @{$full_sequence});
}
elsif ($dh{BEFORE}) {
- $stoppoint=command_pos($dh{BEFORE}, @sequence) - 1;
+ $stoppoint = command_pos($dh{BEFORE}, @{$full_sequence}) - 1;
+}
+
+for my $rules_command (@{$rules_targets}) {
+ my $rules_target = extract_rules_target_name($rules_command)
+ // error("Internal error: $rules_command was not a rules target!?");
+ # Don't pass DH_ environment variables, since this is
+ # a fresh invocation of debian/rules and any sub-dh commands.
+ delete($ENV{DH_INTERNAL_OPTIONS});
+ delete($ENV{DH_INTERNAL_OVERRIDE});
+ run("debian/rules", $rules_target);
+ my $override_packages = $sequence2packages{$rules_target} // \@packages;
+ for my $package (@{$override_packages}) {
+ my (undef, $seq) = optimize_sequence(\%sequences, $rules_target, 1);
+ COMMAND: for my $c (reverse(@{$seq})) {
+ for my $j (0 .. $#{$full_sequence}) {
+ if ($c eq $full_sequence->[$j]) {
+ $startpoint{$package} = $j + 1;
+ last COMMAND;
+ }
+ }
+ }
+ }
}
# Now run the commands in the sequence.
foreach my $i (0..$stoppoint) {
- my $command=$sequence[$i];
+ my $command = $full_sequence->[$i];
# Figure out which packages need to run this command.
my @todo;
my @opts=@options;
foreach my $package (@packages) {
if ($startpoint{$package} > $i ||
- $logged{$package}{$sequence[$i]}) {
+ $logged{$package}{$full_sequence->[$i]}) {
push @opts, "-N$package";
}
else {
@@ -739,14 +798,8 @@ foreach my $i (0..$stoppoint) {
next unless @todo;
my $rules_target = extract_rules_target_name($command);
- if (defined $rules_target) {
- # Don't pass DH_ environment variables, since this is
- # a fresh invocation of debian/rules and any sub-dh commands.
- delete $ENV{DH_INTERNAL_OPTIONS};
- delete $ENV{DH_INTERNAL_OVERRIDE};
- run("debian/rules", $rules_target);
- next;
- }
+ error("Internal error: $command is a rules target, but it is not supported to be!?") if defined($rules_target);
+
if (my $stamp_file = stamp_target($command)) {
my %seen;
print " create-stamp ".escape_shell($stamp_file)."\n";
diff --git a/lib/Debian/Debhelper/SequencerUtil.pm b/lib/Debian/Debhelper/SequencerUtil.pm
index d78aa03..1aba996 100644
--- a/lib/Debian/Debhelper/SequencerUtil.pm
+++ b/lib/Debian/Debhelper/SequencerUtil.pm
@@ -32,7 +32,7 @@ sub to_rules_target {
}
sub optimize_sequence {
- my ($sequences, $sequence_name, $always_inline) = @_;
+ my ($sequences, $sequence_name, $always_inline, $completed_sequences) = @_;
my (@sequence, @targets, %seen, %non_inlineable_targets, @stack);
push(@stack, [@{$sequences->{$sequence_name}}]);
while (@stack) {
@@ -41,6 +41,7 @@ sub optimize_sequence {
while (@{$current_sequence}) {
my $command = shift(@{$current_sequence});
my $rules_target=extract_rules_target_name($command);
+ next if (defined($rules_target) and exists($completed_sequences->{$rules_target}));
if (defined($rules_target) &&
! exists($non_inlineable_targets{$rules_target}) &&
! defined(rules_explicit_target($rules_target))) {
@@ -69,7 +70,7 @@ sub optimize_sequence {
}
}
}
- return (@targets, @sequence);
+ return (\@targets, \@sequence);
}
diff --git a/t/dh-sequencer.t b/t/dh-sequencer.t
index 8f55459..6509a9b 100755
--- a/t/dh-sequencer.t
+++ b/t/dh-sequencer.t
@@ -49,7 +49,7 @@ my %sequences = (
);
-plan tests => 9;
+plan tests => 11;
# We will horse around with %EXPLICIT_TARGETS in this test; it should
# definitely not attempt to read d/rules or the test will be break.
@@ -58,40 +58,51 @@ $Debian::Debhelper::SequencerUtil::RULES_PARSED = 1;
is_deeply(
[optimize_sequence(\%sequences, 'build')],
- [@bd_minimal, @bd],
+ [[], [@bd_minimal, @bd]],
'Inlined build sequence matches build-indep/build-arch');
is_deeply(
[optimize_sequence(\%sequences, 'install')],
- [@bd_minimal, @bd, @i],
+ [[], [@bd_minimal, @bd, @i]],
'Inlined install sequence matches build-indep/build-arch + install commands');
is_deeply(
[optimize_sequence(\%sequences, 'binary-arch')],
- [@bd_minimal, @bd, @i, @ba, @b],
+ [[], [@bd_minimal, @bd, @i, @ba, @b]],
'Inlined binary-arch sequence has all the commands');
is_deeply(
[optimize_sequence(\%sequences, 'binary-indep')],
- [@bd_minimal, @bd, @i, @b],
+ [[], [@bd_minimal, @bd, @i, @b]],
'Inlined binary-indep sequence has all the commands except @bd');
is_deeply(
[optimize_sequence(\%sequences, 'binary')],
- [@bd_minimal, @bd, @i, @ba, @b],
+ [[], [@bd_minimal, @bd, @i, @ba, @b]],
'Inlined binary sequence has all the commands');
+
+is_deeply(
+ [optimize_sequence(\%sequences, 'binary', 0, { 'build' => 1, 'build-arch' => 1, 'build-indep' => 1})],
+ [[], [@i, @ba, @b]],
+ 'Inlined binary sequence with build-* done has @i, @ba and @b');
+
{
local $Debian::Debhelper::SequencerUtil::EXPLICIT_TARGETS{'build'} = 1;
is_deeply(
[optimize_sequence(\%sequences, 'binary')],
- [to_rules_target('build'), @i, @ba, @b],
+ [[to_rules_target('build')], [@i, @ba, @b]],
'Inlined binary sequence has all the commands but build target is opaque');
+ is_deeply(
+ [optimize_sequence(\%sequences, 'binary', 0, { 'build' => 1, 'build-arch' => 1, 'build-indep' => 1})],
+ [[], [@i, @ba, @b]],
+ 'Inlined binary sequence has all the commands with build-* done and not build-target');
+
is_deeply(
[optimize_sequence(\%sequences, 'build')],
- [@bd_minimal, @bd],
+ [[], [@bd_minimal, @bd]],
'build sequence is inlineable');
}
@@ -103,7 +114,7 @@ is_deeply(
[optimize_sequence(\%sequences, 'binary')],
# @bd_minimal, @bd and @i should be "-i"-only, @ba + @b should be both.
# Unfortunately, optimize_sequence cannot show that.
- [to_rules_target('install-arch'), @bd_minimal, @bd, @i, @ba, @b],
+ [[to_rules_target('install-arch')], [@bd_minimal, @bd, @i, @ba, @b]],
'Inlined binary sequence has all the commands');
}
@@ -114,11 +125,11 @@ is_deeply(
my $actual = [optimize_sequence(\%sequences, 'binary')];
# @i should be "-i"-only, @ba + @b should be both.
# Unfortunately, optimize_sequence cannot show that.
- my $expected = [to_rules_target('build'), to_rules_target('install-arch'), @i, @ba, @b];
+ my $expected = [[to_rules_target('build'), to_rules_target('install-arch')], [@i, @ba, @b]];
# Permit some fuzz on the order between build and install-arch
- if ($actual->[0] eq to_rules_target('install-arch')) {
- $expected->[0] = to_rules_target('install-arch');
- $expected->[1] = to_rules_target('build');
+ if ($actual->[0][0] eq to_rules_target('install-arch')) {
+ $expected->[0][0] = to_rules_target('install-arch');
+ $expected->[0][1] = to_rules_target('build');
}
is_deeply(
$actual,
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/debhelper/debhelper.git
More information about the debhelper-devel
mailing list