dh: Implement conditional sequence add-ons

Signed-off-by: Niels Thykier <niels@thykier.net>

2 files changed, 196 insertions, 31 deletions

diff --git a/dh b/dh
index 9d3cdd29..b86e525d 100755
--- a/dh
+++ b/dh
@@ -273,6 +273,7 @@ option to ensure they only work on architecture dependent packages.
 # Stash this away before init modifies it.
 my @ARGV_orig=@ARGV;
 my (@addons, @addon_requests);
+our ($DH_INTERNAL_ADDON_TYPE, $DH_INTERNAL_ADDON_NAME);
 
 inhibit_log();
 
@@ -435,13 +436,111 @@ $sequences{binary} = [to_rules_target("install"), to_rules_target("binary-arch")
 
 # Additional command options
 my %command_opts;
+my %commands_added_by_addon;
+
+sub _assert_not_conditional_sequence_addon {
+	my ($feature) = @_;
+	return if $DH_INTERNAL_ADDON_TYPE eq 'both';
+	warning("The add-on ${DH_INTERNAL_ADDON_NAME} relies on a feature (${feature}) (possibly indirectly), which is "
+	    . 'not supported for conditional debhelper sequence add-ons.');
+	warning("Hint: You may have to move the build-dependency for dh-sequence-${DH_INTERNAL_ADDON_NAME} to "
+		. 'Build-Depends to avoid this error assuming it is possible to use the sequence unconditionally.');
+	die("${feature} is not supported for conditional dh sequence add-ons.\n");
+}
+
+sub _filter_sequences_for_conditional_add_ons {
+	my @sequences = @_;
+	# If it is unconditional, then there is no issues.
+	return @sequences if $DH_INTERNAL_ADDON_TYPE eq 'both' or not @sequences;
+	# Typically, if you add a command to a sequence, then you will in fact add it to two. E.g.
+	# Adding dh_foo after dh_installdocs will affect both install-arch AND install-indep.  We want
+	# this to "just work(tm)" with a conditional add-on to avoid too much hassle (i.e. only affect
+	# the relevant sequence).  At the same time, we must abort if a sequence like "clean" is
+	# affected.
+	my (%sequence_stems, @filtered);
+
+	for my $sequence_name (@sequences) {
+		my $sequence_type = _sequence_type($sequence_name);
+		my $sequence_stem = $sequence_name;
+		if ($sequence_type ne 'both') {
+			$sequence_stem =~ s/-\Q${sequence_type}//;
+		}
+		if ($sequence_type eq $DH_INTERNAL_ADDON_TYPE) {
+			push(@filtered, $sequence_name);
+			$sequence_stems{$sequence_stem} = 1;
+		}
+		elsif (not exists($sequence_stems{$sequence_stem})) {
+			$sequence_stems{$sequence_stem} = 0;
+		}
+	}
+	for my $sequence_name (@sequences) {
+		my $sequence_type = _sequence_type($sequence_name);
+		my $sequence_stem = $sequence_name;
+		if ($sequence_type ne 'both') {
+			$sequence_stem =~ s/-\Q${sequence_type}//;
+		}
+		if (not $sequence_stems{$sequence_stem}) {
+			warning("The add-on ${DH_INTERNAL_ADDON_NAME} attempted to modify the sequence ${sequence_name} (possibly "
+				. "indirectly) but the add-on is conditional for \"*-${DH_INTERNAL_ADDON_TYPE}\" targets");
+			warning("Hint: You may have to move the build-dependency for dh-sequence-${DH_INTERNAL_ADDON_NAME} to "
+				. 'Build-Depends to avoid this error assuming it is possible to use the sequence unconditionally.');
+			die("The add-on ${DH_INTERNAL_ADDON_NAME} cannot be use conditionally for \"*-${DH_INTERNAL_ADDON_TYPE}\""
+				. " targets\n");
+		}
+	}
+	return @filtered;
+}
+
+sub _register_cmd_added_by_addon {
+	my ($cmd) = @_;
+	my $existing = $commands_added_by_addon{$cmd};
+	if ($existing) {
+		if ($existing->{'addon-type'} ne $DH_INTERNAL_ADDON_TYPE) {
+			my $old_addon_name = $existing->{'addon-name'};
+			my $old_addon_type = $existing->{'addon-type'};
+			# Technically, "both" could be made compatible with "indep" OR "arch" (but not both at the same time).
+			# Implement if it turns out to be relevant.
+			warning("Both dh sequence add-ons ${DH_INTERNAL_ADDON_NAME} and ${old_addon_name} have attempted to add "
+			    . "the command $cmd (possibly indirectly).");
+			warning("However, the two add-ons do not have compatible constraints (${DH_INTERNAL_ADDON_TYPE} vs. "
+			    . "${old_addon_type}).");
+			warning("Hint: You may have to move the build-dependency for dh-sequence-<X> to "
+				. ' the same build-dependency field to avoid this error assuming it is possible.');
+			die("Multiple sequences have conflicting requests for $cmd.\n");
+		}
+		return;
+	}
+
+	$commands_added_by_addon{$cmd} = {
+		'addon-name' => $DH_INTERNAL_ADDON_NAME,
+		'addon-type' => $DH_INTERNAL_ADDON_TYPE,
+	};
+	return;
+}
+
+sub _sequences_containing_cmd {
+	my ($cmd) = @_;
+	my @sequences;
+	foreach my $sequence_name (keys %sequences) {
+		for my $scmd (@{$sequences{$sequence_name}}) {
+			if ($scmd eq $cmd) {
+				push(@sequences, $sequence_name);
+				last;
+			}
+		}
+	}
+	return @sequences;
+}
 
 # sequence addon interface
 sub _insert {
 	my ($offset, $existing, $new) = @_;
-	foreach my $sequence (keys %sequences) {
+	my @affected_sequences = _sequences_containing_cmd($existing);
+	@affected_sequences = _filter_sequences_for_conditional_add_ons(@affected_sequences);
+	return if not @affected_sequences;
+	_register_cmd_added_by_addon($new);
+	foreach my $sequence (@affected_sequences) {
 		my @list=@{$sequences{$sequence}};
-		next unless grep $existing, @list;
 		my @new;
 		foreach my $command (@list) {
 			if ($command eq $existing) {
@@ -464,25 +563,43 @@ sub insert_after {
 }
 sub remove_command {
 	my ($command) = @_;
-	foreach my $sequence (keys %sequences) {
+	# Implement if actually needed (I *think* it basically means to transform dh_foo to dh_foo -a/-i)
+	_assert_not_conditional_sequence_addon('remove_command');
+	my @affected_sequences = _sequences_containing_cmd($command);
+	@affected_sequences = _filter_sequences_for_conditional_add_ons(@affected_sequences);
+	return if not @affected_sequences;
+	foreach my $sequence (@affected_sequences) {
 		$sequences{$sequence}=[grep { $_ ne $command } @{$sequences{$sequence}}];
 	}
-
+	return;
 }
 sub add_command {
 	my ($command, $sequence) = @_;
+	_filter_sequences_for_conditional_add_ons($sequence);
+	_register_cmd_added_by_addon($command);
 	unshift @{$sequences{$sequence}}, $command;
+	return;
 }
 sub add_command_at_end {
 	my ($command, $sequence) = @_;
+	_filter_sequences_for_conditional_add_ons($sequence);
+	_register_cmd_added_by_addon($command);
 	push(@{$sequences{$sequence}}, $command);
+	return;
 }
 sub add_command_options {
 	my $command=shift;
+	# Implement if actually needed (Complicated as dh_foo becomes dh_foo -a && dh_foo -i <extra_options>
+	# and that implies smarter deduplication logic)
+	_assert_not_conditional_sequence_addon('add_command_options');
 	push @{$command_opts{$command}}, @_;
+	return;
 }
 sub remove_command_options {
 	my ($command, @cmd_options) = @_;
+	# Implement if actually needed (Complicated as dh_foo <extra_options> becomes
+	#   dh_foo -a  <extra_options> && dh_foo -i and that implies smarter deduplication logic)
+	_assert_not_conditional_sequence_addon('remove_command_options');
 	if (@cmd_options) {
 		# Remove only specified options
 		if (my $opts = $command_opts{$command}) {
@@ -496,6 +613,7 @@ sub remove_command_options {
 		# Clear all additional options
 		delete $command_opts{$command};
 	}
+	return;
 }
 
 sub list_addons {
@@ -522,9 +640,10 @@ sub list_addons {
 }
 
 sub _compute_addons {
-	my (@addon_requests_from_args) = @_;
-	my (@enabled_addons, %disabled_addons, %enabled);
+	my ($sequence_name, @addon_requests_from_args) = @_;
+	my (@enabled_addons, %disabled_addons, %enabled, $bd_dh_sequences_ref);
 	my @addon_requests;
+	my $sequence_type = _sequence_type($sequence_name);
 
 	# Order is important; DH_EXTRA_ADDONS must come before everything
 	# else; then comes built-in and finally argument provided add-ons
@@ -541,7 +660,14 @@ sub _compute_addons {
 		push(@addon_requests, '+systemd') if compat(10, 1);
 		push(@addon_requests, '+build-stamp');
 	}
-	push(@addon_requests, map { "+${_}" } Debian::Debhelper::Dh_Lib::bd_dh_sequences());
+	$bd_dh_sequences_ref = Debian::Debhelper::Dh_Lib::bd_dh_sequences();
+	for my $addon_name (sort(keys(%{$bd_dh_sequences_ref}))) {
+		my $addon_type = $bd_dh_sequences_ref->{$addon_name};
+		if ($addon_type eq 'both' or $sequence_type eq 'both' or $addon_type eq $sequence_type) {
+			push(@addon_requests, "+${addon_name}");
+		}
+	}
+
 	push(@addon_requests, @addon_requests_from_args);
 
 	# Removing disabled add-ons are expensive (O(N) per time), so we
@@ -568,24 +694,12 @@ sub _compute_addons {
 	}
 
 	$flush_disable_cache->() if %disabled_addons;
-	return @enabled_addons;
-}
-
-@addons = _compute_addons(@addon_requests);
-
-# Load addons, which can modify sequences.
-foreach my $addon (@addons) {
-	my $mod="Debian::Debhelper::Sequence::$addon";
-	$mod=~s/-/_/g;
-	eval "use $mod";
-	if ($@) {
-		error("unable to load addon $addon: $@");
-	}
-}
-
-if (! exists $sequences{$sequence}) {
-	error "Unknown sequence $sequence (choose from: ".
-		join(" ", sort keys %sequences).")";
+	return map {
+		{
+			'name' => $_,
+			'addon-type' => $bd_dh_sequences_ref->{$_} // 'both',
+		}
+	} @enabled_addons;
 }
 
 # The list of all packages that can be acted on.
@@ -629,6 +743,39 @@ elsif ($sequence eq 'build-indep' ||
 	# ditto optimisation for arch indep
 	@packages = @{$sequence2packages{$sequence}};
 }
+
+@addons = _compute_addons($sequence, @addon_requests);
+
+# Load addons, which can modify sequences.
+foreach my $addon (@addons) {
+	my $addon_name = $addon->{'name'};
+	my $addon_type = $addon->{'addon-type'};
+	my $mod="Debian::Debhelper::Sequence::${addon_name}";
+	$mod=~s/-/_/g;
+	local $DH_INTERNAL_ADDON_NAME = $addon_name;
+	local $DH_INTERNAL_ADDON_TYPE = $addon_type;
+	eval "use $mod";
+	if ($@) {
+		error("unable to load addon ${addon_name}: $@");
+	}
+}
+if (%commands_added_by_addon) {
+	while (my ($cmd, $addon) = each(%commands_added_by_addon)) {
+		my $addon_type = $addon->{'addon-type'};
+		if ($addon_type eq 'indep') {
+			unshift(@{$command_opts{$cmd}}, '-i');
+		} elsif ($addon_type eq 'arch') {
+			unshift(@{$command_opts{$cmd}}, '-a');
+		}
+	}
+}
+
+
+if (! exists $sequences{$sequence}) {
+	error "Unknown sequence $sequence (choose from: ".
+		join(" ", sort keys %sequences).")";
+}
+
 while (@ARGV_orig) {
 	my $opt=shift @ARGV_orig;
 	if ($opt =~ /^--?(after|until|before|with|without)$/) {
@@ -1018,6 +1165,15 @@ sub can_skip {
 	return 1;
 }
 
+sub _sequence_type {
+	my ($sequence_name) = @_;
+	if ($sequence_name =~ m/-indep$/) {
+		return 'indep';
+	} elsif ($sequence_name =~ m/-arch/) {
+		return 'arch';
+	}
+	return 'both';
+}
 
 =head1 SEE ALSO
 
diff --git a/lib/Debian/Debhelper/Dh_Lib.pm b/lib/Debian/Debhelper/Dh_Lib.pm
index bb4ec646..7e6d0976 100644
--- a/lib/Debian/Debhelper/Dh_Lib.pm
+++ b/lib/Debian/Debhelper/Dh_Lib.pm
@@ -1647,6 +1647,11 @@ sub getpackages {
 	error("could not find Source: line in control file.") if not defined($sourcepackage);
 	if (%bd_fields) {
 		my ($dh_compat_bd, $final_level);
+		my %field2addon_type = (
+			'build-depends' => 'both',
+			'build-depends-arch' => 'arch',
+			'build-depends-indep' => 'indep',
+		);
 		for my $field (sort(keys(%bd_fields))) {
 			my $value = join(' ', @{$bd_fields{$field}});
 			$value =~ s/^\s*//;
@@ -1688,12 +1693,16 @@ sub getpackages {
 				# Build-Depends on dh-sequence-<foo> OR dh-sequence-<foo> (<op> <version>)
 				if ($dep =~ m/^dh-sequence-(${PKGNAME_REGEX})\s*(?:[(]\s*(?:[<>]?=|<<|>>)\s*(${PKGVERSION_REGEX})\s*[)])?$/) {
 					my $sequence = $1;
-					if ($field ne 'build-depends') {
-						warning("Ignoring dh sequence add-on request for sequenece ${sequence} via ${field}: Please move it to the Build-Depends field");
-						warning("The relation that triggered this warning was: ${dep} (from the ${field} field)");
-						next;
+					my $addon_type = $field2addon_type{$field};
+					if (not defined($field)) {
+						warning("Cannot map ${field} to an add-on type (like \"both\", \"indep\" or \"arch\")");
+						error("Internal error: Cannot satisfy dh sequence add-on request for sequence ${sequence} via ${field}.");
+					}
+					if (defined($dh_bd_sequences{$sequence})) {
+						error("Saw $dep multiple times (last time in $field).  However dh only support that build-"
+							. 'dependency at most once across all Build-Depends(-Arch|-Indep) fields');
 					}
-					$dh_bd_sequences{$sequence} = 1;
+					$dh_bd_sequences{$sequence} = $addon_type;
 				}
 			}
 		}
@@ -1986,7 +1995,7 @@ sub bd_dh_sequences {
 	# Use $sourcepackage as check because %dh_bd_sequence can be empty
 	# after running getpackages().
 	getpackages() if not defined($sourcepackage);
-	return sort(keys(%dh_bd_sequences));
+	return \%dh_bd_sequences;
 }
 
 sub _concat_slurp_script_files {