From cba2a8a6ea64773e61ab41c218853ee729656650 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sun, 13 May 2018 15:17:39 +0000 Subject: Make dh_install{init,systemd}'s autoscripts independent [c12] Have dh_installinit use the new --skip-systemd-native parameter and let dh_installsystemd always generate autoscript snippets for systemd services. This ensures that dh_installsystemd's snippet will be used for starting the services and will among other ensure that services are properly unmasked before started. Signed-off-by: Niels Thykier --- t/dh_installsystemd/dh_installsystemd.t | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/dh_installsystemd/dh_installsystemd.t b/t/dh_installsystemd/dh_installsystemd.t index 7029c615..1faa1868 100755 --- a/t/dh_installsystemd/dh_installsystemd.t +++ b/t/dh_installsystemd/dh_installsystemd.t @@ -8,7 +8,7 @@ use Test::DH; use File::Path qw(remove_tree make_path); use Debian::Debhelper::Dh_Lib qw(!dirname); -plan(tests => 2); +plan(tests => 4); sub write_file { my ($path, $content) = @_; @@ -196,7 +196,9 @@ each_compat_subtest { unit_is_enabled('foo', 'source', 0); unit_is_enabled('foo', 'target', 1); ok(run_dh_tool('dh_clean')); +}; +each_compat_up_to_and_incl_subtest(11, sub { make_path('debian/foo/lib/systemd/system/'); make_path('debian/foo/etc/init.d/'); install_file('debian/foo.service', 'debian/foo/lib/systemd/system/target.service'); @@ -211,4 +213,20 @@ each_compat_subtest { unit_is_started('foo', 'source', 0); unit_is_started('foo', 'target', 0); ok(run_dh_tool('dh_clean')); -}; +}); + +each_compat_from_and_above_subtest(12, sub { + make_path('debian/foo/lib/systemd/system/'); + make_path('debian/foo/etc/init.d/'); + install_file('debian/foo.service', 'debian/foo/lib/systemd/system/target.service'); + make_symlink_raw_target('target.service', 'debian/foo/lib/systemd/system/source.service'); + write_file('debian/foo/etc/init.d/source', '# something'); + ok(run_dh_tool('dh_installsystemd')); + unit_is_enabled('foo', 'foo', 1); + # Alias= realized by symlinks are not enabled in maintaner scripts + unit_is_enabled('foo', 'source', 0); + unit_is_enabled('foo', 'target', 1); + unit_is_started('foo', 'source', 0); + unit_is_started('foo', 'target', 1); + ok(run_dh_tool('dh_clean')); +}); -- cgit v1.2.3 From 62b6840b0ed652f31c3cb542444675fec792f572 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 15 Sep 2018 17:20:31 -0700 Subject: Replace all calls to Cwd::cwd with Cwd::getcwd The former calls /bin/pwd, while the latter uses the getcwd syscall directly. This eliminates some forks and execs from every build. --- dh_auto_install | 2 +- lib/Debian/Debhelper/Buildsystem/meson.pm | 2 +- t/Test/DH.pm | 4 ++-- t/debhelper-compat/syntax.t | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) (limited to 't') diff --git a/dh_auto_install b/dh_auto_install index 1b692230..2ca13775 100755 --- a/dh_auto_install +++ b/dh_auto_install @@ -83,7 +83,7 @@ if (!$destdir) { $destdir=tmpdir($dh{MAINPACKAGE}); } } -$destdir = File::Spec->rel2abs($destdir, cwd()); +$destdir = File::Spec->rel2abs($destdir, getcwd()); if (compat(10)) { # Ensure that all debian/ directories exist diff --git a/lib/Debian/Debhelper/Buildsystem/meson.pm b/lib/Debian/Debhelper/Buildsystem/meson.pm index 2ddd0c37..a90fb703 100644 --- a/lib/Debian/Debhelper/Buildsystem/meson.pm +++ b/lib/Debian/Debhelper/Buildsystem/meson.pm @@ -84,7 +84,7 @@ sub configure { # Make the file name absolute as meson will be called from the build dir. require Cwd; $cross_file =~ s{^\./}{}; - $cross_file = Cwd::cwd() . "/${cross_file}"; + $cross_file = Cwd::getcwd() . "/${cross_file}"; } push(@opts, '--cross-file', $cross_file); } diff --git a/t/Test/DH.pm b/t/Test/DH.pm index 20e6e9f6..5ca9515d 100644 --- a/t/Test/DH.pm +++ b/t/Test/DH.pm @@ -5,7 +5,7 @@ use warnings; use Test::More; -use Cwd qw(cwd realpath); +use Cwd qw(getcwd realpath); use Errno qw(EEXIST); use Exporter qw(import); @@ -44,7 +44,7 @@ our @EXPORT = qw( our ($TEST_DH_COMPAT, $ROOT_OK, $ROOT_CMD); -my $START_DIR = cwd(); +my $START_DIR = getcwd(); my $TEST_DIR; sub run_dh_tool { diff --git a/t/debhelper-compat/syntax.t b/t/debhelper-compat/syntax.t index 9c0987d6..7fe0307a 100755 --- a/t/debhelper-compat/syntax.t +++ b/t/debhelper-compat/syntax.t @@ -31,7 +31,7 @@ sub test_build_depends { error("close $dir/debian/control failed: $!"); close($in); - my $start_dir = Test::DH::cwd(); + my $start_dir = Test::DH::getcwd(); chdir($dir) or error("chdir($dir): $!"); plan(tests => 5); -- cgit v1.2.3 From 9076fc79b27a7b7aa5f8a1e3b1e0f54a781aefba Mon Sep 17 00:00:00 2001 From: Daniele Nicolodi Date: Tue, 12 Jun 2018 19:47:20 -0600 Subject: dh_installsystemduser: New helper to handle systemd user instance units Add a new 'dh_installsystemduser' helper responsible for istalling package maintainer supplied systemd user instance units and to produce postinst and postrm maintiner scripts code blocks to appropriately enable, mask and disable units when the package is installed, upgraded, or removed. --- autoscripts/postinst-systemd-user-dont-enable | 15 ++ autoscripts/postinst-systemd-user-enable | 15 ++ autoscripts/postrm-systemd-user | 12 ++ debhelper.pod | 5 + debian/changelog | 11 + dh | 19 +- dh_installsystemduser | 270 ++++++++++++++++++++++++ t/dh_installsystemduser/debian/baz.user.service | 8 + t/dh_installsystemduser/debian/changelog | 5 + t/dh_installsystemduser/debian/control | 10 + t/dh_installsystemduser/debian/foo.user.service | 8 + t/dh_installsystemduser/dh_installsystemduser.t | 75 +++++++ 12 files changed, 444 insertions(+), 9 deletions(-) create mode 100644 autoscripts/postinst-systemd-user-dont-enable create mode 100644 autoscripts/postinst-systemd-user-enable create mode 100644 autoscripts/postrm-systemd-user create mode 100755 dh_installsystemduser create mode 100644 t/dh_installsystemduser/debian/baz.user.service create mode 100644 t/dh_installsystemduser/debian/changelog create mode 100644 t/dh_installsystemduser/debian/control create mode 100644 t/dh_installsystemduser/debian/foo.user.service create mode 100644 t/dh_installsystemduser/dh_installsystemduser.t (limited to 't') diff --git a/autoscripts/postinst-systemd-user-dont-enable b/autoscripts/postinst-systemd-user-dont-enable new file mode 100644 index 00000000..0837257f --- /dev/null +++ b/autoscripts/postinst-systemd-user-dont-enable @@ -0,0 +1,15 @@ +if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then + if deb-systemd-helper --user debian-installed #UNITFILE# ; then + # This will only remove masks created by d-s-h on package removal. + deb-systemd-helper --user unmask #UNITFILE# >/dev/null || true + + if deb-systemd-helper --quiet --user was-enabled #UNITFILE# ; then + # Create new symlinks, if any. + deb-systemd-helper --user enable #UNITFILE# >/dev/null || true + fi + fi + + # Update the statefile to add new symlinks (if any), which need to be cleaned + # up on purge. Also remove old symlinks. + deb-systemd-helper --user update-state #UNITFILE# >/dev/null || true +fi diff --git a/autoscripts/postinst-systemd-user-enable b/autoscripts/postinst-systemd-user-enable new file mode 100644 index 00000000..b16f61fb --- /dev/null +++ b/autoscripts/postinst-systemd-user-enable @@ -0,0 +1,15 @@ +if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then + # This will only remove masks created by d-s-h on package removal. + deb-systemd-helper --user unmask #UNITFILE# >/dev/null || true + + # was-enabled defaults to true, so new installations run enable. + if deb-systemd-helper --quiet --user was-enabled #UNITFILE# ; then + # Enables the unit on first installation, creates new + # symlinks on upgrades if the unit file has changed. + deb-systemd-helper --user enable #UNITFILE# >/dev/null || true + else + # Update the statefile to add new symlinks (if any), which need to be + # cleaned up on purge. Also remove old symlinks. + deb-systemd-helper --user update-state #UNITFILE# >/dev/null || true + fi +fi diff --git a/autoscripts/postrm-systemd-user b/autoscripts/postrm-systemd-user new file mode 100644 index 00000000..bf48d1a3 --- /dev/null +++ b/autoscripts/postrm-systemd-user @@ -0,0 +1,12 @@ +if [ "$1" = "remove" ]; then + if [ -x "/usr/bin/deb-systemd-helper" ] ; then + deb-systemd-helper --user mask #UNITFILES# >/dev/null || true + fi +fi + +if [ "$1" = "purge" ]; then + if [ -x "/usr/bin/deb-systemd-helper" ] ; then + deb-systemd-helper --user purge #UNITFILES# >/dev/null || true + deb-systemd-helper --user unmask #UNITFILES# >/dev/null || true + fi +fi diff --git a/debhelper.pod b/debhelper.pod index 177d2f8f..c758a72b 100644 --- a/debhelper.pod +++ b/debhelper.pod @@ -915,6 +915,11 @@ packages and not only during the building process. Please set B to false to revert to the previous behaviour. See B for details and examples. +=item - + +B is now included in the B standard +sequence by default. + =back =back diff --git a/debian/changelog b/debian/changelog index 6b3e3605..f25e531c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,14 @@ +debhelper (11.5) UNRELEASED; urgency=medium + + [ Daniele Nicolodi ] + * dh_installsystemduser: New helper responsible for istalling package + maintainer supplied systemd user instance units and to produce + postinst and postrm maintiner scripts code blocks to appropriately + enable, mask and disable units when the package is installed, + upgraded, or removed. (Closes: #764678) + + -- Daniele Nicolodi Sun, 14 Oct 2018 10:13:17 -0600 + debhelper (11.4.1) unstable; urgency=medium [ Niels Thykier ] diff --git a/dh b/dh index 0fea4422..10762133 100755 --- a/dh +++ b/dh @@ -115,10 +115,10 @@ easy way to do with is by adding an override target for that command. #!/usr/bin/make -f %: dh $@ - + override_dh_strip: dh_strip -Xfoo - + override_dh_auto_configure: dh_auto_configure -- --with-foo --disable-bar @@ -199,7 +199,7 @@ want it to run, by defining empty override targets for each command. #!/usr/bin/make -f %: dh $@ - + # Commands not to run: override_dh_auto_test override_dh_compress override_dh_fixperms: @@ -210,7 +210,7 @@ These will be skipped when running build-arch and binary-arch sequences. #!/usr/bin/make -f %: dh $@ - + override_dh_auto_build-indep: $(MAKE) -C docs @@ -272,7 +272,7 @@ option to ensure they only work on architecture dependent packages. =head1 DEPRECATED OPTIONS -The following options are deprecated. It's much +The following options are deprecated. It's much better to use override targets instead. They are B available in compat 10. @@ -308,7 +308,7 @@ my @ARGV_orig=@ARGV; my (@addons, @addon_requests); inhibit_log(); - + init(options => { "until=s" => \$dh{UNTIL}, "after=s" => \$dh{AFTER}, @@ -325,7 +325,7 @@ init(options => { "l" => \&list_addons, "list" => \&list_addons, }, - # Disable complaints about unknown options; they are passed on to + # Disable complaints about unknown options; they are passed on to # the debhelper commands. ignore_unknown_options => 1, # Bundling does not work well since there are unknown options. @@ -405,6 +405,7 @@ my @i = (qw{ dh_installinit }, (!compat(10) ? qw(dh_installsystemd) : qw()), + (compat(12) ? qw(dh_installsystemduser) : qw()), qw{ dh_installmenu @@ -499,7 +500,7 @@ sub remove_command { foreach my $sequence (keys %sequences) { $sequences{$sequence}=[grep { $_ ne $command } @{$sequences{$sequence}}]; } - + } sub add_command { my ($command, $sequence) = @_; @@ -864,7 +865,7 @@ foreach my $i (0..$stoppoint) { close($fd) or error("close($stamp_file) failed: $!"); next; } - + # Check for override targets in debian/rules, and run instead of # the usual command. (The non-arch-specific override is tried first, # for simplest semantics; mixing it with arch-specific overrides diff --git a/dh_installsystemduser b/dh_installsystemduser new file mode 100755 index 00000000..bc1d0368 --- /dev/null +++ b/dh_installsystemduser @@ -0,0 +1,270 @@ +#!/usr/bin/perl -w + +=head1 NAME + +dh_installsystemduser - install systemd unit files + +=cut + +use strict; +use warnings; +use Debian::Debhelper::Dh_Lib; + +our $VERSION = DH_BUILTIN_VERSION; + +=head1 SYNOPSIS + +B [S>] [B<--no-enable>] [B<--name=>I] [S ...>] + +=head1 DESCRIPTION + +B finds the systemd user instance service files +installed by a package and generates F, and F code +blocks for enabling and disabling the corresponding systemd user +instance services, when the package is installed, updated, or +removed. These snippets are added to the maintainer scripts by +L. + +L is used to enable and disable the systemd +units, thus it is not necessary that the machine actually runs systemd +during package installation time, enabling happens on all machines. + +B operates on all user instance unit files +installed by a package. For only generating blocks for specific unit +files, pass them as arguments. Specific unit files can be excluded +from processing using the B<-X> common L option. + +=head1 FILES + +=over 4 + +=item debian/I.user.path, + debian/I@.user.path, + debian/I.user.service, + debian/I@.user.service, + debian/I.user.socket, + debian/I@.user.socket, + debian/I.user.target, + debian/I@.user.target, + debian/I.user.timer, + debian/I@.user.timer + +If any of those files exists, they are installed into +F in the package build directory removing the +F<.user> file name part. + +=back + +=head1 OPTIONS + +=over 4 + +=item B<--name=>I + +Install the service file as I instead of the default +filename I. When this parameter is used, +B looks for and installs files named +F instead of the usual +F. Moreover, maintainer scripts are only +generated for units that match the given I. + +=item B<--no-enable> + +Disable the service(s) on purge, but do not enable them on install. + +=back + +=head1 NOTES + +This command is not idempotent. L should be called between +invocations of this command (with the same arguments). Otherwise, it +may cause multiple instances of the same text to be added to +maintainer scripts. + +=cut + +# PROMISE: DH NOOP WITHOUT tmp(usr/lib/systemd/user) user.service + +init(options => { + "no-enable" => \$dh{NO_ENABLE}, +}); + +sub quote { + # Add single quotes around the argument. + return '\'' . $_[0] . '\''; +} + +sub uniq { + my %seen; + return grep { !$seen{$_}++ } @_; +} + +sub contains_install_section { + my ($unit_path) = @_; + + open(my $fh, '<', $unit_path) or error("Cannot open($unit_path) to check for [Install]: $!"); + + while (my $line = <$fh>) { + chomp($line); + return 1 if $line =~ /^\s*\[Install\]$/i; + } + close($fh); + return 0; +} + +sub install_user_unit { + my ($package, $name, $suffix) = @_; + + my $tmpdir = tmpdir($package); + my $path = "$tmpdir/usr/lib/systemd/user"; + my $unit = pkgfile($package, "user.$suffix"); + return if $unit eq ''; + + install_dir($path); + install_file($unit, "$path/$name.$suffix"); +} + +# Extracts the directive values from a unit file. Handles repeated +# directives in the same unit file. Assumes values can only be +# composed of lists of unit names. This is good enough for the 'Also=' +# and 'Alias=' directives handled here. +sub extract_key { + my ($unit_path, $key) = @_; + my @values; + + open(my $fh, '<', $unit_path) or error("Cannot open($unit_path): $!"); + + while (my $line = <$fh>) { + chomp($line); + + # Since unit names can't have whitespace in systemd, simply + # use split and strip any leading/trailing quotes. See + # systemd-escape(1) for examples of valid unit names. + if ($line =~ /^\s*$key=(.+)$/i) { + for my $value (split(/\s+/, $1)) { + $value =~ s/^(["'])(.*)\g1$/$2/; + push @values, $value; + } + } + } + + close($fh); + return @values; +} + +sub list_installed_user_units { + my ($tmpdir, $aliases) = @_; + + my $lib_systemd_user = "$tmpdir/usr/lib/systemd/user"; + my @installed; + + return unless -d $lib_systemd_user; + opendir(my $dh, $lib_systemd_user) or error("Cannot opendir($lib_systemd_user): $!"); + + foreach my $name (readdir($dh)) { + my $path = "$lib_systemd_user/$name"; + next unless -f $path; + if (-l $path) { + my $dest = basename(readlink($path)); + $aliases->{$dest} //= [ ]; + push @{$aliases->{$dest}}, $name; + } else { + push @installed, $name; + } + } + + closedir($dh); + return @installed; +} + +# Install package maintainer provided unit files. +foreach my $package (@{$dh{DOPACKAGES}}) { + my $tmpdir = tmpdir($package); + + # unit file name + my $name = $dh{NAME} // $package; + + for my $type (qw(service target socket path timer)) { + install_user_unit($package, $name, $type); + install_user_unit("${package}@", "${name}@", $type); + } +} + +# Generate postinst and prerm code blocks to enable and disable units +foreach my $package (@{$dh{DOPACKAGES}}) { + my (@args, @enable_units, %aliases); + + my $tmpdir = tmpdir($package); + my @units = list_installed_user_units($tmpdir, \%aliases); + + # Handle either only the unit files which were passed as arguments + # or all unit files that are installed in this package. + if (@ARGV) { + @args = @ARGV; + } + elsif ($dh{NAME}) { + # Treat --name flag as if the corresponding units were passed + # in the command line. + @args = grep /(^|\/)$dh{NAME}\.(service|target|socket|path|timer)$/, @units; + } + else { + @args = @units; + } + + # Support excluding units via the -X debhelper common option. + foreach my $x (@{$dh{EXCLUDE}}) { + @args = grep !/(^|\/)$x$/, @args; + } + + # This hash prevents us from looping forever in the following + # while loop. An actual real-world example of such a loop is + # systemd's systemd-readahead-drop.service, which contains + # Also=systemd-readahead-collect.service, and that file in turn + # contains Also=systemd-readahead-drop.service, thus forming an + # endless loop. + my %seen; + + # Must use while and shift because the loop alters the list. + while (@args) { + my $name = shift @args; + my $path = "${tmpdir}/usr/lib/systemd/user/${name}"; + + error("User unit file \"$name\" not found in package \"$package\".") if ! -f $path; + + # Skip template service files. Enabling or disabling those + # services without specifying the instance is not useful. + next if ($name =~ /\@/); + + # Handle all unit files specified via Also= explicitly. This + # is not necessary for enabling, but for disabling, as we + # cannot read the unit file when disabling as it has already + # been deleted. + push @args, $_ for grep { !$seen{$_}++ } extract_key($path, 'Also'); + + push @enable_units, $name if contains_install_section($path); + } + + @enable_units = map { quote($_) } sort uniq @enable_units; + + if (@enable_units) { + # The generated maintainer script code blocks use the --user + # option that was added to deb-systemd-helper in version 1.52. + addsubstvar($package, 'misc:Depends', 'init-system-helpers', ">= 1.52"); + + my $postinst = $dh{NO_ENABLE} ? 'postinst-systemd-user-dont-enable' : 'postinst-systemd-user-enable'; + foreach my $unit (@enable_units) { + autoscript($package, 'postinst', $postinst, { 'UNITFILE' => $unit }); + } + autoscript($package, 'postrm', 'postrm-systemd-user', { 'UNITFILES' => join(' ', @enable_units) }); + } +} + +=head1 SEE ALSO + +L, L, L + +=head1 AUTHORS + +pkg-systemd-maintainers@lists.alioth.debian.org + +=cut diff --git a/t/dh_installsystemduser/debian/baz.user.service b/t/dh_installsystemduser/debian/baz.user.service new file mode 100644 index 00000000..3af041de --- /dev/null +++ b/t/dh_installsystemduser/debian/baz.user.service @@ -0,0 +1,8 @@ +[Unit] +Description=Baz User Unit + +[Service] +ExecStart=/bin/true + +[Install] +WantedBy=default.target diff --git a/t/dh_installsystemduser/debian/changelog b/t/dh_installsystemduser/debian/changelog new file mode 100644 index 00000000..5850f0e2 --- /dev/null +++ b/t/dh_installsystemduser/debian/changelog @@ -0,0 +1,5 @@ +foo (1.0-1) unstable; urgency=low + + * Initial release. (Closes: #XXXXXX) + + -- Test Mon, 11 Jul 2016 18:10:59 +0200 diff --git a/t/dh_installsystemduser/debian/control b/t/dh_installsystemduser/debian/control new file mode 100644 index 00000000..adccbc63 --- /dev/null +++ b/t/dh_installsystemduser/debian/control @@ -0,0 +1,10 @@ +Source: foo +Section: misc +Priority: optional +Maintainer: Test +Standards-Version: 3.9.8 + +Package: foo +Architecture: all +Description: package foo + Package foo diff --git a/t/dh_installsystemduser/debian/foo.user.service b/t/dh_installsystemduser/debian/foo.user.service new file mode 100644 index 00000000..7ef597d4 --- /dev/null +++ b/t/dh_installsystemduser/debian/foo.user.service @@ -0,0 +1,8 @@ +[Unit] +Description=Foo User Unit + +[Service] +ExecStart=/bin/true + +[Install] +WantedBy=default.target diff --git a/t/dh_installsystemduser/dh_installsystemduser.t b/t/dh_installsystemduser/dh_installsystemduser.t new file mode 100644 index 00000000..894eeb7a --- /dev/null +++ b/t/dh_installsystemduser/dh_installsystemduser.t @@ -0,0 +1,75 @@ +#!/usr/bin/perl +use strict; +use Test::More; +use File::Path qw(make_path); +use File::Basename qw(dirname); +use lib dirname(dirname(__FILE__)); +use Test::DH; +use Debian::Debhelper::Dh_Lib qw(!dirname); + +plan(tests => 1); + +our @TEST_DH_EXTRA_TEMPLATE_FILES = (qw( + debian/changelog + debian/control + debian/foo.user.service + debian/baz.user.service +)); + + +sub read_script { + my ($package, $name) = @_; + my @lines; + + foreach my $script (find_script($package, $name)) { + open(my $fh, '<', $script) or die("open($script): $!"); + push @lines, $_ for <$fh>; + close($fh); + } + + return @lines; +} + +sub _unit_check_user_enabled { + my ($package, $unit, $enabled) = @_; + my $verb = $enabled ? "is" : "isnt"; + my $matches; + + my @postinst = read_script($package, 'postinst'); + # Match exactly one tab character. The "dont-enable" script has + # an "enable" line for re-enabling the service if the admin had it + # enabled, but we do not want to include that in our count. + $matches = grep { m{^\tif deb-systemd-helper( --\w+)* --user was-enabled.*'\Q$unit'} } @postinst; + is($matches, $enabled, "$unit $verb enabled"); + + my @postrm = read_script($package, 'postrm'); + $matches = grep { m{deb-systemd-helper( --\w+)* --user mask.*'\Q$unit'} } @postrm; + is($matches, $enabled, "$unit $verb masked"); +} + +sub is_enabled { _unit_check_user_enabled(@_, 1); } +sub isnt_enabled { _unit_check_user_enabled(@_, 0); } + +each_compat_subtest { + make_path('debian/foo/usr/lib/systemd/user/'); + install_file('debian/foo.user.service', 'debian/foo/usr/lib/systemd/user/bar.service'); + ok(run_dh_tool('dh_installsystemduser')); + ok(-e 'debian/foo/usr/lib/systemd/user/foo.service'); + is_enabled('foo', 'foo.service'); + is_enabled('foo', 'bar.service'); + ok(run_dh_tool('dh_clean')); + + ok(run_dh_tool('dh_installsystemduser')); + ok(-e 'debian/foo/usr/lib/systemd/user/foo.service'); + ok(! -e 'debian/foo/usr/lib/systemd/user/baz.service'); + is_enabled('foo', 'foo.service'); + isnt_enabled('foo', 'baz.service'); + ok(run_dh_tool('dh_clean')); + + ok(run_dh_tool('dh_installsystemduser', '--name', 'baz')); + ok(! -e 'debian/foo/usr/lib/systemd/user/foo.service'); + ok(-e 'debian/foo/usr/lib/systemd/user/baz.service'); + isnt_enabled('foo', 'foo.service'); + is_enabled('foo', 'baz.service'); + ok(run_dh_tool('dh_clean')); +}; -- cgit v1.2.3 From 5e2f5a7777cffe4bc7a89fa20dc958954aeda809 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Mon, 25 Feb 2019 07:22:44 +0000 Subject: Dh_Lib: Ensure error uses a well-defined stable error code Signed-off-by: Niels Thykier --- debian/changelog | 8 ++++++++ lib/Debian/Debhelper/Dh_Lib.pm | 5 +++-- t/dh_missing/02-fail-on-missing.t | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) (limited to 't') diff --git a/debian/changelog b/debian/changelog index 2526061d..ae1f6870 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +debhelper (12.1.2) UNRELEASED; urgency=medium + + * Dh_Lib: Ensure the error function always triggers the same + exit code on termination. Previously, it depended on the + value of the last error (if any). + + -- Niels Thykier Mon, 25 Feb 2019 07:23:03 +0000 + debhelper (12.1.1) unstable; urgency=medium * dh_installinitramfs: Install hooks as executable rather than diff --git a/lib/Debian/Debhelper/Dh_Lib.pm b/lib/Debian/Debhelper/Dh_Lib.pm index 00703214..c707197c 100644 --- a/lib/Debian/Debhelper/Dh_Lib.pm +++ b/lib/Debian/Debhelper/Dh_Lib.pm @@ -761,8 +761,9 @@ sub nonquiet_print { # Output an error message and die (can be caught). sub error { - my $message=shift; - + my ($message) = @_; + # ensure the error code is well defined. + $! = 255; die basename($0).": $message\n"; } diff --git a/t/dh_missing/02-fail-on-missing.t b/t/dh_missing/02-fail-on-missing.t index 711ed984..e97b6eba 100755 --- a/t/dh_missing/02-fail-on-missing.t +++ b/t/dh_missing/02-fail-on-missing.t @@ -28,6 +28,6 @@ each_compat_subtest { isnt($?, -1, 'dh_missing was executed'); ok(! ($? & 127), 'dh_missing did not die due to a signal'); my $exitcode = ($? >> 8); - is($exitcode, 2, 'dh_missing exited with exit code 2'); + is($exitcode, 255, 'dh_missing exited with exit code 255'); }; -- cgit v1.2.3 From e5fc959e3b97a0d3821aec43c8a4d2aed212dae6 Mon Sep 17 00:00:00 2001 From: Dmitry Bogatov Date: Thu, 17 Jan 2019 02:19:27 +0000 Subject: dh_installinit: --name implies, that init script is present Previously, `dh_installinit' silently did nothing, when --name option was passed, but initscript debian/..init was not found. In almost all cases, explicit --name means that package maintainer meant to install init script. If it is not present, it is bug, and must not be hidden. Now, error is reported in this case. (Closes: #462389) Signed-off-by: Niels Thykier --- debian/changelog | 5 +++++ dh_installinit | 4 ++++ t/dh_installinit/dh_installinit.t | 1 + 3 files changed, 10 insertions(+) (limited to 't') diff --git a/debian/changelog b/debian/changelog index cef4d571..57ffa3c5 100644 --- a/debian/changelog +++ b/debian/changelog @@ -50,6 +50,11 @@ debhelper (12.1.2) UNRELEASED; urgency=medium * Buildsystem/cmake: Fix CMAKE_SYSTEM_PROCESSOR for mips64el. (Closes: #926815) + [ Dmitry Bogatov ] + * dh_installinit: Fail with an error if --name is given but + there is no matching init script. Thanks to A Mennucc + for reporting the issue. (Closes: #462389) + [ Translations ] * Update Portuguese translation (Américo Monteiro) (Closes: #886279) diff --git a/dh_installinit b/dh_installinit index fca0a8af..6a490370 100755 --- a/dh_installinit +++ b/dh_installinit @@ -311,6 +311,10 @@ foreach my $package (@{$dh{DOPACKAGES}}) { my $init=pkgfile($package,$scriptsrc) || pkgfile($package,"init") || pkgfile($package,"init.d"); + if (!$init && defined $dh{NAME}) { + error("--name=$dh{NAME} option specified, but init script not found"); + } + if ($init ne '' && ! $dh{ONLYSCRIPTS}) { install_dir("$tmp/etc/init.d"); install_prog($init,"$tmp/etc/init.d/$script"); diff --git a/t/dh_installinit/dh_installinit.t b/t/dh_installinit/dh_installinit.t index b20caa58..afe3821f 100755 --- a/t/dh_installinit/dh_installinit.t +++ b/t/dh_installinit/dh_installinit.t @@ -29,6 +29,7 @@ each_compat_from_and_above_subtest(11, sub { make_path(qw(debian/foo debian/bar debian/baz)); ok(run_dh_tool('dh_installinit')); + ok(! run_dh_tool({'quiet' => 1}, 'dh_installinit', '--name=missing')); ok(! -e "debian/foo/lib/systemd/system/foo.service"); ok(!find_script('foo', 'postinst')); ok(run_dh_tool('dh_clean')); -- cgit v1.2.3 From 789eb18ba0e09f87fefc33c2ffd1e6f6e5654ce7 Mon Sep 17 00:00:00 2001 From: Dmitry Bogatov Date: Mon, 15 Jul 2019 16:08:01 +0000 Subject: Fix logic error in --name sanity check Commit [e5fc959e], resolving #462389 changed behaviour of `--name' option of dh_installinit. Before this change, if `debian/{package}.{name}.init' is missing, this option was silently ignored. This change made it error. This change was incorrect, since it demanded presence of `debian/{package}.{name}.init' file for /every/ binary package. This commit instead throws error only if `debian/{package}.{name}.init' does not exist for /all/ binary {package} names. Regression test is added. Closes: #932073 Signed-off-by: Niels Thykier --- dh_installinit | 10 ++++++---- t/dh_installinit/debian/bar.other.init | 4 ++++ t/dh_installinit/dh_installinit.t | 2 ++ 3 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 t/dh_installinit/debian/bar.other.init (limited to 't') diff --git a/dh_installinit b/dh_installinit index 6a490370..cd038af1 100755 --- a/dh_installinit +++ b/dh_installinit @@ -232,6 +232,12 @@ init(options => { my %snippet_options = ('snippet-order' => 'service'); +if (my $name = $dh{NAME}) { + if (!grep { -f "debian/$_.${name}.init" } @{$dh{DOPACKAGES}}) { + error("--name=$dh{NAME} option specified, but init script not found"); + } +} + foreach my $package (@{$dh{DOPACKAGES}}) { my $tmp=tmpdir($package); @@ -311,10 +317,6 @@ foreach my $package (@{$dh{DOPACKAGES}}) { my $init=pkgfile($package,$scriptsrc) || pkgfile($package,"init") || pkgfile($package,"init.d"); - if (!$init && defined $dh{NAME}) { - error("--name=$dh{NAME} option specified, but init script not found"); - } - if ($init ne '' && ! $dh{ONLYSCRIPTS}) { install_dir("$tmp/etc/init.d"); install_prog($init,"$tmp/etc/init.d/$script"); diff --git a/t/dh_installinit/debian/bar.other.init b/t/dh_installinit/debian/bar.other.init new file mode 100644 index 00000000..ea948c58 --- /dev/null +++ b/t/dh_installinit/debian/bar.other.init @@ -0,0 +1,4 @@ +#!/bin/sh +cat << 'EOF' +I am init script to be installed into package "bar" into /etc/init.d/other path. +EOF \ No newline at end of file diff --git a/t/dh_installinit/dh_installinit.t b/t/dh_installinit/dh_installinit.t index afe3821f..3b6bc038 100755 --- a/t/dh_installinit/dh_installinit.t +++ b/t/dh_installinit/dh_installinit.t @@ -12,6 +12,7 @@ our @TEST_DH_EXTRA_TEMPLATE_FILES = (qw( debian/changelog debian/control debian/foo.service + debian/bar.other.init )); plan(tests => 2); @@ -30,6 +31,7 @@ each_compat_from_and_above_subtest(11, sub { ok(run_dh_tool('dh_installinit')); ok(! run_dh_tool({'quiet' => 1}, 'dh_installinit', '--name=missing')); + ok( run_dh_tool({'queit' => 1}, 'dh_installinit', '--name=other')); ok(! -e "debian/foo/lib/systemd/system/foo.service"); ok(!find_script('foo', 'postinst')); ok(run_dh_tool('dh_clean')); -- cgit v1.2.3 From 38fcef8ad314aeedd1c698d8bb697158286cf989 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Fri, 19 Jul 2019 18:29:26 +0000 Subject: dh_installinit: Revert check to ensure --name always matches a file Signed-off-by: Niels Thykier --- debian/changelog | 8 ++++++++ dh_installinit | 6 ------ t/dh_installinit/dh_installinit.t | 1 - 3 files changed, 8 insertions(+), 7 deletions(-) (limited to 't') diff --git a/debian/changelog b/debian/changelog index 77869569..f6cddf42 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +debhelper (12.2.3) UNRELEASED; urgency=medium + + * dh_installinit: Revert "Fail with an error if --name is given but + there is no matching init script.". (Closes: #932073, + Reopens: #462389) + + -- Niels Thykier Fri, 19 Jul 2019 18:27:41 +0000 + debhelper (12.2.2) unstable; urgency=medium * dh_shlibdeps: Remove regex anchor when parsing file(1) to aovid diff --git a/dh_installinit b/dh_installinit index 5aecf25b..fca0a8af 100755 --- a/dh_installinit +++ b/dh_installinit @@ -232,12 +232,6 @@ init(options => { my %snippet_options = ('snippet-order' => 'service'); -if (my $name = $dh{NAME}) { - if (!grep { -f "debian/$_.${name}.init" || -f "debian/$_.${name}.default" } @{$dh{DOPACKAGES}}) { - error("--name=$dh{NAME} option specified, but init script nor default file not found"); - } -} - foreach my $package (@{$dh{DOPACKAGES}}) { my $tmp=tmpdir($package); diff --git a/t/dh_installinit/dh_installinit.t b/t/dh_installinit/dh_installinit.t index 3b6bc038..cfc154a7 100755 --- a/t/dh_installinit/dh_installinit.t +++ b/t/dh_installinit/dh_installinit.t @@ -30,7 +30,6 @@ each_compat_from_and_above_subtest(11, sub { make_path(qw(debian/foo debian/bar debian/baz)); ok(run_dh_tool('dh_installinit')); - ok(! run_dh_tool({'quiet' => 1}, 'dh_installinit', '--name=missing')); ok( run_dh_tool({'queit' => 1}, 'dh_installinit', '--name=other')); ok(! -e "debian/foo/lib/systemd/system/foo.service"); ok(!find_script('foo', 'postinst')); -- cgit v1.2.3 From 819885e6590a626fca56aaf2b2817cee273fa296 Mon Sep 17 00:00:00 2001 From: Felix Lechner Date: Fri, 19 Jul 2019 18:35:31 +0000 Subject: dh_installinit.t: Fix typo of quiet Signed-off-by: Niels Thykier --- t/dh_installinit/dh_installinit.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't') diff --git a/t/dh_installinit/dh_installinit.t b/t/dh_installinit/dh_installinit.t index cfc154a7..d290df21 100755 --- a/t/dh_installinit/dh_installinit.t +++ b/t/dh_installinit/dh_installinit.t @@ -30,7 +30,7 @@ each_compat_from_and_above_subtest(11, sub { make_path(qw(debian/foo debian/bar debian/baz)); ok(run_dh_tool('dh_installinit')); - ok( run_dh_tool({'queit' => 1}, 'dh_installinit', '--name=other')); + ok(run_dh_tool({'quiet' => 1}, 'dh_installinit', '--name=other')); ok(! -e "debian/foo/lib/systemd/system/foo.service"); ok(!find_script('foo', 'postinst')); ok(run_dh_tool('dh_clean')); -- cgit v1.2.3 From 2fd94649173464628cd732ee20834bb634fc62a2 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Tue, 13 Aug 2019 15:20:54 +0000 Subject: Rewrite sequence handling to ensure add-on commands are ordered correctly Previously, we relied on a command being inserted in the -arch sequence to be ordered correctly. With this rewrite, a command added only to the -indep sequence will still appear in the right order. Signed-off-by: Niels Thykier --- dh | 154 +++++++++++++++++---------------- lib/Debian/Debhelper/Sequence.pm | 127 +++++++++++++++++++++++++++ lib/Debian/Debhelper/SequencerUtil.pm | 143 +++++++++++++++++++++++++++---- t/dh-sequencer.t | 157 +++++++++++++++++++++++++++------- 4 files changed, 456 insertions(+), 125 deletions(-) create mode 100644 lib/Debian/Debhelper/Sequence.pm (limited to 't') diff --git a/dh b/dh index b86e525d..a44bd12d 100755 --- a/dh +++ b/dh @@ -12,6 +12,7 @@ use constant { 'UNSKIPPABLE_CLI_OPTIONS_BUILD_SYSTEM' => q(-S|--buildsystem|-D|--sourcedir(?:ectory)?|-B|--builddir(?:ectory)?), }; use Debian::Debhelper::Dh_Lib; +use Debian::Debhelper::Sequence; use Debian::Debhelper::SequencerUtil; our $VERSION = DH_BUILTIN_VERSION; @@ -404,13 +405,19 @@ qw{ dh_fixperms dh_missing }); -my @ba=( +my @ba=(map { + { + 'command' => $_, + 'command-options' => [], + 'sequence-limitation' => SEQUENCE_TYPE_ARCH_ONLY, + } +} ( (!compat(11) ? qw(dh_dwz) : qw()), qw{ dh_strip dh_makeshlibs dh_shlibdeps -}); +})); if (! getpackages("arch")) { @ba=(); } @@ -420,19 +427,28 @@ my @b=qw{ dh_md5sums dh_builddeb }; -$sequences{clean} = [@bd_minimal, qw{ + +sub _add_sequence { + my @args = @_; + my $seq = Debian::Debhelper::Sequence->new(@args); + my $name = $seq->name; + $sequences{$name} = $seq; + if ($seq->allowed_subsequences eq SEQUENCE_ARCH_INDEP_SUBSEQUENCES) { + for my $subseq ((SEQUENCE_TYPE_ARCH_ONLY, SEQUENCE_TYPE_INDEP_ONLY)) { + my $subname = "${name}-${subseq}"; + $sequences{$subname} = $seq; + } + } + return; +} + +_add_sequence('build', SEQUENCE_ARCH_INDEP_SUBSEQUENCES, @bd); +_add_sequence('install', SEQUENCE_ARCH_INDEP_SUBSEQUENCES, to_rules_target("build"), @i); +_add_sequence('binary', SEQUENCE_ARCH_INDEP_SUBSEQUENCES, to_rules_target("install"), @ba, @b); +_add_sequence('clean', SEQUENCE_NO_SUBSEQUENCES, @bd_minimal, qw{ dh_auto_clean dh_clean -}]; -$sequences{'build-indep'} = [@bd]; -$sequences{'build-arch'} = [@bd]; -$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")]; -$sequences{'binary-indep'} = [to_rules_target("install-indep"), @b]; -$sequences{'binary-arch'} = [to_rules_target("install-arch"), @ba, @b]; -$sequences{binary} = [to_rules_target("install"), to_rules_target("binary-arch"), to_rules_target("binary-indep")]; +}); # Additional command options my %command_opts; @@ -452,34 +468,18 @@ 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}) { + for my $seq (@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. + # + # We solve the above by checking if the sequence has an -arch + an -indep variant and then + # insert the command only for that sequence variant. + + if ($seq->allowed_subsequences ne SEQUENCE_ARCH_INDEP_SUBSEQUENCES) { + my $sequence_name = $seq->name; 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 " @@ -488,7 +488,7 @@ sub _filter_sequences_for_conditional_add_ons { . " targets\n"); } } - return @filtered; + return @sequences; } sub _register_cmd_added_by_addon { @@ -522,9 +522,10 @@ 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); + my $seq = $sequences{$sequence_name}; + for my $scmd (@{$seq->{'_cmds'}}) { + if ($scmd->{'command'} eq $cmd) { + push(@sequences, $seq); last; } } @@ -532,6 +533,15 @@ sub _sequences_containing_cmd { return @sequences; } +sub _seq_cmd { + my ($cmd_name) = @_; + return { + 'command' => $cmd_name, + 'command-options' => [], + 'sequence-limitation' => $DH_INTERNAL_ADDON_TYPE, + }; +} + # sequence addon interface sub _insert { my ($offset, $existing, $new) = @_; @@ -539,20 +549,8 @@ sub _insert { @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}}; - my @new; - foreach my $command (@list) { - if ($command eq $existing) { - push @new, $new if $offset < 0; - push @new, $command; - push @new, $new if $offset > 0; - } - else { - push @new, $command; - } - } - $sequences{$sequence}=\@new; + for my $seq (@affected_sequences) { + $seq->_insert($offset, $existing, _seq_cmd($new)); } } sub insert_before { @@ -568,25 +566,38 @@ sub 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}}]; + for my $seq (@affected_sequences) { + $seq->remove_command($command); } return; } sub add_command { my ($command, $sequence) = @_; - _filter_sequences_for_conditional_add_ons($sequence); + _assert_not_conditional_sequence_addon('add_command'); _register_cmd_added_by_addon($command); - unshift @{$sequences{$sequence}}, $command; + if (not exists($sequences{$sequence})) { + _add_sequence($sequence, SEQUENCE_NO_SUBSEQUENCES, _seq_cmd($command)); + } else { + my $seq = $sequences{$sequence}; + _filter_sequences_for_conditional_add_ons($seq); + $seq->add_command_at_start(_seq_cmd($command)) + } return; } sub add_command_at_end { my ($command, $sequence) = @_; - _filter_sequences_for_conditional_add_ons($sequence); + _assert_not_conditional_sequence_addon('add_command'); _register_cmd_added_by_addon($command); - push(@{$sequences{$sequence}}, $command); + if (not exists($sequences{$sequence})) { + _add_sequence($sequence, SEQUENCE_NO_SUBSEQUENCES, _seq_cmd($command)); + } else { + my $seq = $sequences{$sequence}; + _filter_sequences_for_conditional_add_ons($seq); + $seq->add_command_at_end(_seq_cmd($command)) + } return; } + sub add_command_options { my $command=shift; # Implement if actually needed (Complicated as dh_foo becomes dh_foo -a && dh_foo -i @@ -595,6 +606,7 @@ sub add_command_options { push @{$command_opts{$command}}, @_; return; } + sub remove_command_options { my ($command, @cmd_options) = @_; # Implement if actually needed (Complicated as dh_foo becomes @@ -643,7 +655,7 @@ sub _compute_addons { 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); + 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 @@ -1165,16 +1177,6 @@ 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 L diff --git a/lib/Debian/Debhelper/Sequence.pm b/lib/Debian/Debhelper/Sequence.pm new file mode 100644 index 00000000..fc857055 --- /dev/null +++ b/lib/Debian/Debhelper/Sequence.pm @@ -0,0 +1,127 @@ +#!/usr/bin/perl +# +# Internal library functions for the dh(1) command + +package Debian::Debhelper::Sequence; +use strict; +use warnings; + +use Exporter qw(import); + +use Debian::Debhelper::SequencerUtil qw(extract_rules_target_name sequence_type SEQUENCE_NO_SUBSEQUENCES + SEQUENCE_ARCH_INDEP_SUBSEQUENCES SEQUENCE_TYPE_ARCH_ONLY SEQUENCE_TYPE_INDEP_ONLY SEQUENCE_TYPE_BOTH); + + +sub _as_command { + my ($input) = @_; + if (ref($input) eq 'HASH') { + return $input; + } + my $rules_target = extract_rules_target_name($input); + if (defined($rules_target)) { + my $sequence_type = sequence_type($rules_target); + return { + 'command' => $input, + 'command-options' => [], + 'sequence-limitation' => $sequence_type, + } + } + return { + 'command' => $input, + 'command-options' => [], + 'sequence-limitation' => SEQUENCE_TYPE_BOTH, + } +} + +sub new { + my ($class, $name, $sequence_type, @cmds) = @_; + return bless({ + '_name' => $name, + '_subsequences' => $sequence_type, + '_cmds' => [map {_as_command($_)} @cmds], + }, $class); +} + +sub name { + my ($this) = @_; + return $this->{'_name'}; +} + +sub allowed_subsequences { + my ($this) = @_; + return $this->{'_subsequences'}; +} + +sub _insert { + my ($this, $offset, $existing, $new) = @_; + my @list = @{$this->{'_cmds'}}; + my @new; + my $new_cmd = _as_command($new); + foreach my $command (@list) { + if ($command->{'command'} eq $existing) { + push(@new, $new_cmd) if $offset < 0; + push(@new, $command); + push(@new, $new_cmd) if $offset > 0; + } else { + push(@new, $command); + } + } + $this->{'_cmds'} = \@new; + return; +} + +sub remove_command { + my ($this, $command) = @_; + $this->{'_cmds'} = [grep { $_->{'command'} ne $command } @{$this->{'_cmds'}}]; + return; +} + +sub add_command_at_start { + my ($this, $command) = @_; + unshift(@{$this->{'_cmds'}}, _as_command($command)); + return; +} + +sub add_command_at_end { + my ($this, $command) = @_; + push(@{$this->{'_cmds'}}, _as_command($command)); + return; +} + +sub rules_target_name { + my ($this, $sequence_type) = @_; + die("Internal error: Invalid sequence type $sequence_type") if $sequence_type eq SEQUENCE_NO_SUBSEQUENCES; + my $name = $this->{'_name'}; + my $allowed_sequence_type = $this->{'_subsequences'}; + if ($sequence_type ne SEQUENCE_TYPE_BOTH and $allowed_sequence_type eq SEQUENCE_NO_SUBSEQUENCES) { + die("Internal error: Requested subsequence ${sequence_type} of sequence ${name}, but it has no subsequences"); + } + if ($sequence_type ne SEQUENCE_TYPE_BOTH) { + return "${name}-${sequence_type}"; + } + return $name; +} + +sub as_rules_target_command { + my ($this) = shift; + my $rules_name = $this->rules_target_name(@_); + return "debian/rules ${rules_name}"; +} + +sub flatten_sequence { + my ($this, $sequence_type) = @_; + die("Invalid sequence type $sequence_type") if $sequence_type eq SEQUENCE_NO_SUBSEQUENCES; + my @cmds; + for my $cmd_desc (@{$this->{'_cmds'}}) { + my $seq_limitation = $cmd_desc->{'sequence-limitation'}; + if ($seq_limitation eq $sequence_type or $sequence_type eq SEQUENCE_TYPE_BOTH or $seq_limitation eq SEQUENCE_TYPE_BOTH) { + my $cmd = $cmd_desc->{'command'}; + my @cmd_options = $cmd_desc->{'command-options'}; + push(@cmds, [$cmd, @cmd_options]); + next; + } + } + return @cmds; +} + +1; diff --git a/lib/Debian/Debhelper/SequencerUtil.pm b/lib/Debian/Debhelper/SequencerUtil.pm index 9a9ce2bf..a943357d 100644 --- a/lib/Debian/Debhelper/SequencerUtil.pm +++ b/lib/Debian/Debhelper/SequencerUtil.pm @@ -5,17 +5,30 @@ package Debian::Debhelper::SequencerUtil; use strict; use warnings; -use constant DUMMY_TARGET => 'debhelper-fail-me'; +use constant { + 'DUMMY_TARGET' => 'debhelper-fail-me', + 'SEQUENCE_NO_SUBSEQUENCES' => 'none', + 'SEQUENCE_ARCH_INDEP_SUBSEQUENCES' => 'both', + 'SEQUENCE_TYPE_ARCH_ONLY' => 'arch', + 'SEQUENCE_TYPE_INDEP_ONLY' => 'indep', + 'SEQUENCE_TYPE_BOTH' => 'both', +}; use Exporter qw(import); our @EXPORT = qw( extract_rules_target_name to_rules_target + sequence_type unpack_sequence rules_explicit_target extract_skipinfo DUMMY_TARGET + SEQUENCE_NO_SUBSEQUENCES + SEQUENCE_ARCH_INDEP_SUBSEQUENCES + SEQUENCE_TYPE_ARCH_ONLY + SEQUENCE_TYPE_INDEP_ONLY + SEQUENCE_TYPE_BOTH ); our (%EXPLICIT_TARGETS, $RULES_PARSED); @@ -32,39 +45,135 @@ sub to_rules_target { return 'debian/rules '.join(' ', @_); } +sub sequence_type { + my ($sequence_name) = @_; + if ($sequence_name =~ m/-indep$/) { + return 'indep'; + } elsif ($sequence_name =~ m/-arch/) { + return 'arch'; + } + return 'both'; +} + +sub _agg_subseq { + my ($current_subseq, $outer_subseq) = @_; + if ($current_subseq eq $outer_subseq) { + return $current_subseq; + } + if ($current_subseq eq 'both') { + return $outer_subseq; + } + return $current_subseq; +} + sub unpack_sequence { my ($sequences, $sequence_name, $always_inline, $completed_sequences) = @_; my (@sequence, @targets, %seen, %non_inlineable_targets, @stack); + my $sequence_type = sequence_type($sequence_name); # Walk through the sequence effectively doing a DFS of the rules targets # (when we are allowed to inline them). - push(@stack, [@{$sequences->{$sequence_name}}]); + my $seq = $sequences->{$sequence_name}; + push(@stack, [$seq->flatten_sequence($sequence_type)]); while (@stack) { my $current_sequence = pop(@stack); COMMAND: while (@{$current_sequence}) { my $command = shift(@{$current_sequence}); + if (ref($command) eq 'ARRAY') { + $command = $command->[0]; + } my $rules_target=extract_rules_target_name($command); next if (defined($rules_target) and exists($completed_sequences->{$rules_target})); - if (defined($rules_target) && ($always_inline || - ! exists($non_inlineable_targets{$rules_target}) && - ! defined(rules_explicit_target($rules_target)))) { - - # inline the sequence for this implicit target. + if (defined($rules_target) and $always_inline) { + my $subsequence = $sequences->{$rules_target}; + my $subseq_type = _agg_subseq(sequence_type($rules_target), $sequence_type); push(@stack, $current_sequence); - $current_sequence = [@{$sequences->{$rules_target}}]; + $current_sequence = [$subsequence->flatten_sequence($subseq_type)]; + } elsif (defined($rules_target)) { + my $subsequence = $sequences->{$rules_target}; + my $subseq_type = _agg_subseq(sequence_type($rules_target), $sequence_type); + my @subseq_types = ($subseq_type); + my %subtarget_status; + my ($transparent_subseq, $opaque_subseq, $subtarget_decided_both); + if ($subseq_type eq SEQUENCE_TYPE_BOTH) { + push(@subseq_types, SEQUENCE_TYPE_ARCH_ONLY, SEQUENCE_TYPE_INDEP_ONLY); + } + for my $ss_type (@subseq_types) { + my $full_rule_target = ($ss_type eq SEQUENCE_TYPE_BOTH) ? $rules_target : "${rules_target}-${ss_type}"; + if (exists($completed_sequences->{$full_rule_target})) { + $subtarget_status{$ss_type} = 'complete'; + last if $ss_type eq $subseq_type; + } + elsif (defined(rules_explicit_target($full_rule_target))) { + $subtarget_status{$ss_type} = 'opaque'; + last if $ss_type eq $subseq_type; + } + else { + $subtarget_status{$ss_type} = 'transparent'; + } + } + # At this point, %subtarget_status has 1 or 3 kv-pairs. + # - If it has 1, then just check that and be done + # - If it has 3, then "both" must be "transparent". + + if (scalar(keys(%subtarget_status)) == 3) { + if ($subtarget_status{${\SEQUENCE_TYPE_ARCH_ONLY}} eq $subtarget_status{${\SEQUENCE_TYPE_INDEP_ONLY}}) { + # The "both" target is transparent and the subtargets agree. This is the common case + # of "everything is transparent" (or both subtargets are opaque) and we reduce that by + # reducing it to only have one key. + %subtarget_status = ( $subseq_type => $subtarget_status{${\SEQUENCE_TYPE_ARCH_ONLY}} ); + # There is one special-case for this flow if both targets are opaque. + $subtarget_decided_both = 1; + } else { + # The subtargets have different status but we know that the "both" key must be irrelevant + # then. Remove it to simplify matters below. + delete($subtarget_status{${\SEQUENCE_TYPE_BOTH}}); + } + } + + if (scalar(keys(%subtarget_status)) == 1) { + # "Simple" case where we only have to check exactly one result + if ($subtarget_status{$subseq_type} eq 'opaque') { + $opaque_subseq = $subseq_type; + } + elsif ($subtarget_status{$subseq_type} eq 'transparent') { + $transparent_subseq = $subseq_type; + } + } else { + # Either can be transparent, opaque or complete at this point. + if ($subtarget_status{${\SEQUENCE_TYPE_ARCH_ONLY}} eq 'transparent') { + $transparent_subseq = SEQUENCE_TYPE_ARCH_ONLY + } elsif ($subtarget_status{${\SEQUENCE_TYPE_INDEP_ONLY}} eq 'transparent') { + $transparent_subseq = SEQUENCE_TYPE_INDEP_ONLY + } + if ($subtarget_status{${\SEQUENCE_TYPE_ARCH_ONLY}} eq 'opaque') { + $opaque_subseq = SEQUENCE_TYPE_ARCH_ONLY + } elsif ($subtarget_status{${\SEQUENCE_TYPE_INDEP_ONLY}} eq 'opaque') { + $opaque_subseq = SEQUENCE_TYPE_INDEP_ONLY + } + } + if ($opaque_subseq) { + if ($subtarget_decided_both) { + # Final special-case - we are here because the rules file define X-arch AND X-indep but + # not X. In this case, we want two d/rules X-{arch,indep} calls rather than a single + # d/rules X call. + for my $ss_type ((SEQUENCE_TYPE_ARCH_ONLY, SEQUENCE_TYPE_INDEP_ONLY)) { + my $rules_target_cmd = $subsequence->as_rules_target_command($ss_type); + push(@targets, $rules_target_cmd) if not $seen{$rules_target_cmd}++; + } + } else { + my $rules_target_cmd = $subsequence->as_rules_target_command($opaque_subseq); + push(@targets, $rules_target_cmd) if not $seen{$rules_target_cmd}++; + } + } + if ($transparent_subseq) { + push(@stack, $current_sequence); + $current_sequence = [$subsequence->flatten_sequence($transparent_subseq)]; + } next COMMAND; } else { if (defined($rules_target) and not $always_inline) { next COMMAND if exists($non_inlineable_targets{$rules_target}); - my @opaque_targets = ($rules_target); - while (my $opaque_target = pop(@opaque_targets)) { - for my $c (@{$sequences->{$opaque_target}}) { - my $subtarget = extract_rules_target_name($c); - next if not defined($subtarget); - next if exists($non_inlineable_targets{$subtarget}); - $non_inlineable_targets{$subtarget} = $rules_target; - } - } push(@targets, $command) if not $seen{$command}++; } elsif (! $seen{$command}) { $seen{$command} = 1; diff --git a/t/dh-sequencer.t b/t/dh-sequencer.t index dd5c264a..6c38eae2 100755 --- a/t/dh-sequencer.t +++ b/t/dh-sequencer.t @@ -4,6 +4,7 @@ use strict; use warnings; use Test::More; +use Debian::Debhelper::Sequence; use Debian::Debhelper::SequencerUtil; # Shorten variants of the sequences. @@ -21,46 +22,85 @@ my @i = (qw{ dh_install dh_missing }); -my @ba=qw{ - dh_strip - dh_makeshlibs - dh_shlibdeps -}; +my @ba = ( + { + 'command' => 'dh_strip', + 'command-options' => [], + 'sequence-limitation' => SEQUENCE_TYPE_ARCH_ONLY, + }, + { + 'command' => 'dh_makeshlibs', + 'command-options' => [], + 'sequence-limitation' => SEQUENCE_TYPE_ARCH_ONLY, + }, + { + 'command' => 'dh_shlibdeps', + 'command-options' => [], + 'sequence-limitation' => SEQUENCE_TYPE_ARCH_ONLY, + } +); my @b=qw{ dh_installdeb dh_gencontrol dh_builddeb }; +my @c=qw{ + dh_testdir + dh_auto_clean + dh_clean +}; -my %sequences = ( - 'build-indep' => [@bd], - 'build-arch' => [@bd], - 'build' => [to_rules_target("build-arch"), to_rules_target("build-indep")], - - 'install-indep' => [to_rules_target("build-indep"), @i], - 'install-arch' => [to_rules_target("build-arch"), @i], - 'install' => [to_rules_target("build"), to_rules_target("install-arch"), to_rules_target("install-indep")], +my %sequences; + +sub _add_sequence { + my @args = @_; + my $seq = Debian::Debhelper::Sequence->new(@args); + my $name = $seq->name; + $sequences{$name} = $seq; + if ($seq->allowed_subsequences eq SEQUENCE_ARCH_INDEP_SUBSEQUENCES) { + for my $subseq ((SEQUENCE_TYPE_ARCH_ONLY, SEQUENCE_TYPE_INDEP_ONLY)) { + my $subname = "${name}-${subseq}"; + $sequences{$subname} = $seq; + } + } + return; +} - 'binary-indep' => [to_rules_target("install-indep"), @b], - 'binary-arch' => [to_rules_target("install-arch"), @ba, @b], - 'binary' => [to_rules_target("install"), to_rules_target("binary-arch"), to_rules_target("binary-indep")], -); +_add_sequence('build', SEQUENCE_ARCH_INDEP_SUBSEQUENCES, @bd); +_add_sequence('install', SEQUENCE_ARCH_INDEP_SUBSEQUENCES, to_rules_target("build"), @i); +_add_sequence('binary', SEQUENCE_ARCH_INDEP_SUBSEQUENCES, to_rules_target("install"), @ba, @b); +_add_sequence('clean', SEQUENCE_NO_SUBSEQUENCES, @c); + +sub _cmd_names { + my (@input) = @_; + my @cmds; + for my $cmd (@input) { + if (ref($cmd) eq 'HASH') { + push(@cmds, $cmd->{'command'}); + } else { + push(@cmds, $cmd); + } + } + return \@cmds; +} my %sequences_unpacked = ( - 'build-indep' => [@bd], - 'build-arch' => [@bd], - 'build' => [@bd], + 'build-indep' => _cmd_names(@bd), + 'build-arch' => _cmd_names(@bd), + 'build' => _cmd_names(@bd), + + 'install-indep' => _cmd_names(@bd, @i), + 'install-arch' => _cmd_names(@bd, @i), + 'install' => _cmd_names(@bd, @i), - 'install-indep' => [@bd, @i], - 'install-arch' => [@bd, @i], - 'install' => [@bd, @i], + 'binary-indep' => _cmd_names(@bd, @i, @b), + 'binary-arch' => _cmd_names(@bd, @i, @ba, @b), + 'binary' => _cmd_names(@bd, @i, @ba, @b), - 'binary-indep' => [@bd, @i, @b], - 'binary-arch' => [@bd, @i, @ba, @b], - 'binary' => [@bd, @i, @ba, @b], + 'clean' => _cmd_names(@c), ); -plan tests => 11 + 3 * scalar(keys(%sequences)); +plan tests => 18 + 3 * scalar(keys(%sequences)); # 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. @@ -95,20 +135,73 @@ is_deeply( is_deeply( [unpack_sequence(\%sequences, 'binary', 0, { 'build' => 1, 'build-arch' => 1, 'build-indep' => 1})], - [[], [@i, @ba, @b]], + [[], _cmd_names(@i, @ba, @b)], + 'Inlined binary sequence with build-* done has @i, @ba and @b'); + + +is_deeply( + [unpack_sequence(\%sequences, 'binary', 0, { 'build-arch' => 1, 'build-indep' => 1})], + [[], _cmd_names(@i, @ba, @b)], 'Inlined binary sequence with build-* done has @i, @ba and @b'); +{ + local $Debian::Debhelper::SequencerUtil::EXPLICIT_TARGETS{'build-arch'} = 1; + local $Debian::Debhelper::SequencerUtil::EXPLICIT_TARGETS{'build-indep'} = 1; + + is_deeply( + [unpack_sequence(\%sequences, 'binary', 0, { 'build-arch' => 1, 'build-indep' => 1})], + [[], _cmd_names(@i, @ba, @b)], + 'Inlined binary sequence with build-* done has @i, @ba and @b'); + my $actual = [unpack_sequence(\%sequences, 'binary')]; + # @i should be "-i"-only, @ba + @b should be both. + # Unfortunately, unpack_sequence cannot show that. + my $expected = [[to_rules_target('build-arch'), to_rules_target('build-indep')], _cmd_names(@i, @ba, @b)]; + # Permit some fuzz on the order between build-arch and build-arch + if ($actual->[0][0] eq to_rules_target('build-indep')) { + $expected->[0][0] = to_rules_target('build-indep'); + $expected->[0][1] = to_rules_target('build-arch'); + } + is_deeply( + $actual, + $expected, + 'Inlined binary sequence with explicit build-* has explicit d/rules build-{arch,indep} + @i, @ba, @b'); + + is_deeply( + [unpack_sequence(\%sequences, 'binary', 0, { 'build' => 1})], + [[], _cmd_names(@i, @ba, @b)], + 'Inlined binary sequence with explicit build-* but done build has only @i, @ba and @b'); +} + +{ + local $Debian::Debhelper::SequencerUtil::EXPLICIT_TARGETS{'build-indep'} = 1; + is_deeply( + [ unpack_sequence(\%sequences, 'binary', 0, { 'build-arch' => 1 }) ], + [ [to_rules_target('build-indep')], _cmd_names(@i, @ba, @b) ], + 'Inlined binary sequence with build-arch done and build-indep explicit has d/rules build-indep + @i, @ba and @b'); + + is_deeply( + [ unpack_sequence(\%sequences, 'binary-arch', 0, { 'build-arch' => 1 }) ], + [ [], _cmd_names(@i, @ba, @b) ], + 'Inlined binary-arch sequence with build-arch done and build-indep explicit has @i, @ba and @b'); + + + is_deeply( + [ unpack_sequence(\%sequences, 'binary-indep', 0, { 'build-arch' => 1 }) ], + [ [to_rules_target('build-indep')], _cmd_names(@i, @b) ], + 'Inlined binary-indep sequence with build-arch done and build-indep explicit has d/rules build-indep + @i and @b'); +} + { local $Debian::Debhelper::SequencerUtil::EXPLICIT_TARGETS{'build'} = 1; is_deeply( [unpack_sequence(\%sequences, 'binary')], - [[to_rules_target('build')], [@i, @ba, @b]], + [[to_rules_target('build')], _cmd_names(@i, @ba, @b)], 'Inlined binary sequence has all the commands but build target is opaque'); is_deeply( [unpack_sequence(\%sequences, 'binary', 0, { 'build' => 1, 'build-arch' => 1, 'build-indep' => 1})], - [[], [@i, @ba, @b]], + [[], _cmd_names(@i, @ba, @b)], 'Inlined binary sequence has all the commands with build-* done and not build-target'); is_deeply( @@ -133,7 +226,7 @@ is_deeply( [unpack_sequence(\%sequences, 'binary')], # @bd_minimal, @bd and @i should be "-i"-only, @ba + @b should be both. # Unfortunately, unpack_sequence cannot show that. - [[to_rules_target('install-arch')], [@bd, @i, @ba, @b]], + [[to_rules_target('install-arch')], _cmd_names(@bd, @i, @ba, @b)], 'Inlined binary sequence has all the commands'); # Compat <= 8 ignores explicit targets! @@ -152,7 +245,7 @@ is_deeply( my $actual = [unpack_sequence(\%sequences, 'binary')]; # @i should be "-i"-only, @ba + @b should be both. # Unfortunately, unpack_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')], _cmd_names(@i, @ba, @b)]; # Permit some fuzz on the order between build and install-arch if ($actual->[0][0] eq to_rules_target('install-arch')) { $expected->[0][0] = to_rules_target('install-arch'); -- cgit v1.2.3 From aa3bf822ff200d6cf47e60ad348f6b745828f7a2 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Fri, 16 Aug 2019 20:52:04 +0000 Subject: Fix logic for arch-only commands to make it testable again Signed-off-by: Niels Thykier --- dh | 11 +++++++++-- lib/Debian/Debhelper/Sequence.pm | 12 +++++------- lib/Debian/Debhelper/SequencerUtil.pm | 26 ++++++++++++++++---------- t/dh-sequencer.t | 18 +++++++++++++++++- 4 files changed, 47 insertions(+), 20 deletions(-) (limited to 't') diff --git a/dh b/dh index 6c41d24e..a7423297 100755 --- a/dh +++ b/dh @@ -749,7 +749,7 @@ my %completed_sequences; # Get the options to pass to commands in the sequence. # Filter out options intended only for this program. my (@options, %seen_options); -my ($unoptimizable_user_option, $unoptimizable_option_bundle) = (0, 0); +my ($unoptimizable_user_option, $unoptimizable_option_bundle, $sequence_unpack_flags) = (0, 0, 0); if ($sequence eq 'build-arch' || $sequence eq 'install-arch' || @@ -767,6 +767,12 @@ elsif ($sequence eq 'build-indep' || @packages = @{$sequence2packages{$sequence}}; } +if (not @arch_packages) { + $sequence_unpack_flags = FLAG_OPT_SOURCE_BUILDS_NO_ARCH_PACKAGES; +} elsif (not @indep_packages) { + $sequence_unpack_flags = FLAG_OPT_SOURCE_BUILDS_NO_INDEP_PACKAGES; +} + @addons = _compute_addons($sequence, @addon_requests); # Load addons, which can modify sequences. @@ -886,7 +892,8 @@ if ( -f $build_stamp_file and not compat(9)) { my ($rules_targets, $full_sequence) = unpack_sequence(\%sequences, $sequence, (!compat(8) ? 0 : 1), - \%completed_sequences + \%completed_sequences, + $sequence_unpack_flags, ); my $stoppoint = $#{$full_sequence}; diff --git a/lib/Debian/Debhelper/Sequence.pm b/lib/Debian/Debhelper/Sequence.pm index 4fd599b8..6911c826 100644 --- a/lib/Debian/Debhelper/Sequence.pm +++ b/lib/Debian/Debhelper/Sequence.pm @@ -8,9 +8,9 @@ use warnings; use Exporter qw(import); -use Debian::Debhelper::Dh_Lib qw(getpackages); use Debian::Debhelper::SequencerUtil qw(extract_rules_target_name sequence_type SEQUENCE_NO_SUBSEQUENCES - SEQUENCE_ARCH_INDEP_SUBSEQUENCES SEQUENCE_TYPE_ARCH_ONLY SEQUENCE_TYPE_INDEP_ONLY SEQUENCE_TYPE_BOTH); + SEQUENCE_ARCH_INDEP_SUBSEQUENCES SEQUENCE_TYPE_ARCH_ONLY SEQUENCE_TYPE_INDEP_ONLY SEQUENCE_TYPE_BOTH + FLAG_OPT_SOURCE_BUILDS_NO_ARCH_PACKAGES FLAG_OPT_SOURCE_BUILDS_NO_INDEP_PACKAGES); sub _as_command { @@ -110,15 +110,13 @@ sub as_rules_target_command { } sub flatten_sequence { - my ($this, $sequence_type) = @_; + my ($this, $sequence_type, $flags) = @_; die("Invalid sequence type $sequence_type") if $sequence_type eq SEQUENCE_NO_SUBSEQUENCES; my @cmds; - my $has_arch_pkgs = getpackages("arch") ? 1 : 0; - my $has_indep_pkgs = getpackages("indep") ? 1 : 0; for my $cmd_desc (@{$this->{'_cmds'}}) { my $seq_limitation = $cmd_desc->{'sequence-limitation'}; - next if ($seq_limitation eq SEQUENCE_TYPE_ARCH_ONLY and not $has_arch_pkgs); - next if ($seq_limitation eq SEQUENCE_TYPE_INDEP_ONLY and not $has_indep_pkgs); + next if ($seq_limitation eq SEQUENCE_TYPE_ARCH_ONLY and ($flags & FLAG_OPT_SOURCE_BUILDS_NO_ARCH_PACKAGES)); + next if ($seq_limitation eq SEQUENCE_TYPE_INDEP_ONLY and ($flags & FLAG_OPT_SOURCE_BUILDS_NO_INDEP_PACKAGES)); if ($seq_limitation eq $sequence_type or $sequence_type eq SEQUENCE_TYPE_BOTH or $seq_limitation eq SEQUENCE_TYPE_BOTH) { my $cmd = $cmd_desc->{'command'}; my @cmd_options = $cmd_desc->{'command-options'}; diff --git a/lib/Debian/Debhelper/SequencerUtil.pm b/lib/Debian/Debhelper/SequencerUtil.pm index a943357d..6dd3eb5d 100644 --- a/lib/Debian/Debhelper/SequencerUtil.pm +++ b/lib/Debian/Debhelper/SequencerUtil.pm @@ -6,12 +6,14 @@ package Debian::Debhelper::SequencerUtil; use strict; use warnings; use constant { - 'DUMMY_TARGET' => 'debhelper-fail-me', - 'SEQUENCE_NO_SUBSEQUENCES' => 'none', - 'SEQUENCE_ARCH_INDEP_SUBSEQUENCES' => 'both', - 'SEQUENCE_TYPE_ARCH_ONLY' => 'arch', - 'SEQUENCE_TYPE_INDEP_ONLY' => 'indep', - 'SEQUENCE_TYPE_BOTH' => 'both', + 'DUMMY_TARGET' => 'debhelper-fail-me', + 'SEQUENCE_NO_SUBSEQUENCES' => 'none', + 'SEQUENCE_ARCH_INDEP_SUBSEQUENCES' => 'both', + 'SEQUENCE_TYPE_ARCH_ONLY' => 'arch', + 'SEQUENCE_TYPE_INDEP_ONLY' => 'indep', + 'SEQUENCE_TYPE_BOTH' => 'both', + 'FLAG_OPT_SOURCE_BUILDS_NO_ARCH_PACKAGES' => 0x1, + 'FLAG_OPT_SOURCE_BUILDS_NO_INDEP_PACKAGES' => 0x2, }; use Exporter qw(import); @@ -29,6 +31,8 @@ our @EXPORT = qw( SEQUENCE_TYPE_ARCH_ONLY SEQUENCE_TYPE_INDEP_ONLY SEQUENCE_TYPE_BOTH + FLAG_OPT_SOURCE_BUILDS_NO_ARCH_PACKAGES + FLAG_OPT_SOURCE_BUILDS_NO_INDEP_PACKAGES ); our (%EXPLICIT_TARGETS, $RULES_PARSED); @@ -67,13 +71,15 @@ sub _agg_subseq { } sub unpack_sequence { - my ($sequences, $sequence_name, $always_inline, $completed_sequences) = @_; + my ($sequences, $sequence_name, $always_inline, $completed_sequences, $flags) = @_; my (@sequence, @targets, %seen, %non_inlineable_targets, @stack); my $sequence_type = sequence_type($sequence_name); # Walk through the sequence effectively doing a DFS of the rules targets # (when we are allowed to inline them). my $seq = $sequences->{$sequence_name}; - push(@stack, [$seq->flatten_sequence($sequence_type)]); + $flags //= 0; + + push(@stack, [$seq->flatten_sequence($sequence_type, $flags)]); while (@stack) { my $current_sequence = pop(@stack); COMMAND: @@ -88,7 +94,7 @@ sub unpack_sequence { my $subsequence = $sequences->{$rules_target}; my $subseq_type = _agg_subseq(sequence_type($rules_target), $sequence_type); push(@stack, $current_sequence); - $current_sequence = [$subsequence->flatten_sequence($subseq_type)]; + $current_sequence = [$subsequence->flatten_sequence($subseq_type, $flags)]; } elsif (defined($rules_target)) { my $subsequence = $sequences->{$rules_target}; my $subseq_type = _agg_subseq(sequence_type($rules_target), $sequence_type); @@ -168,7 +174,7 @@ sub unpack_sequence { } if ($transparent_subseq) { push(@stack, $current_sequence); - $current_sequence = [$subsequence->flatten_sequence($transparent_subseq)]; + $current_sequence = [$subsequence->flatten_sequence($transparent_subseq, $flags)]; } next COMMAND; } else { diff --git a/t/dh-sequencer.t b/t/dh-sequencer.t index 6c38eae2..f8643866 100755 --- a/t/dh-sequencer.t +++ b/t/dh-sequencer.t @@ -100,7 +100,7 @@ my %sequences_unpacked = ( 'clean' => _cmd_names(@c), ); -plan tests => 18 + 3 * scalar(keys(%sequences)); +plan tests => 21 + 3 * scalar(keys(%sequences)); # 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. @@ -144,6 +144,22 @@ is_deeply( [[], _cmd_names(@i, @ba, @b)], 'Inlined binary sequence with build-* done has @i, @ba and @b'); +is_deeply( + [unpack_sequence(\%sequences, 'binary', 0, {}, 0)], + [[], _cmd_names(@bd, @i, @ba, @b)], + 'Inlined binary sequence and arch:all + arch:any is reduced to @bd, @i, @ba and @b'); + +is_deeply( + [unpack_sequence(\%sequences, 'binary', 0, {}, FLAG_OPT_SOURCE_BUILDS_NO_ARCH_PACKAGES)], + [[], _cmd_names(@bd, @i, @b)], + 'Inlined binary sequence and not arch:any is reduced to @bd, @i and @b'); + +is_deeply( + [unpack_sequence(\%sequences, 'binary', 0, {}, FLAG_OPT_SOURCE_BUILDS_NO_INDEP_PACKAGES)], + [[], _cmd_names(@bd, @i, @ba, @b)], + 'Inlined binary sequence and not arch:all is reduced to @bd, @i, @ba and @b'); + + { local $Debian::Debhelper::SequencerUtil::EXPLICIT_TARGETS{'build-arch'} = 1; local $Debian::Debhelper::SequencerUtil::EXPLICIT_TARGETS{'build-indep'} = 1; -- cgit v1.2.3 From 038efd099e142aea043d128c6cd3bc6a93e68fc1 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 20 Aug 2019 10:43:56 +0000 Subject: Add tests for dh_installgsettings Extracted from the patch proposed in https://bugs.debian.org/934893. Signed-off-by: Niels Thykier Signed-off-by: Simon McVittie --- t/dh_installgsettings/debian/changelog | 5 +++ t/dh_installgsettings/debian/control | 17 ++++++++++ t/dh_installgsettings/dh_installgsettings.t | 49 +++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 t/dh_installgsettings/debian/changelog create mode 100644 t/dh_installgsettings/debian/control create mode 100755 t/dh_installgsettings/dh_installgsettings.t (limited to 't') diff --git a/t/dh_installgsettings/debian/changelog b/t/dh_installgsettings/debian/changelog new file mode 100644 index 00000000..5850f0e2 --- /dev/null +++ b/t/dh_installgsettings/debian/changelog @@ -0,0 +1,5 @@ +foo (1.0-1) unstable; urgency=low + + * Initial release. (Closes: #XXXXXX) + + -- Test Mon, 11 Jul 2016 18:10:59 +0200 diff --git a/t/dh_installgsettings/debian/control b/t/dh_installgsettings/debian/control new file mode 100644 index 00000000..c3cb827c --- /dev/null +++ b/t/dh_installgsettings/debian/control @@ -0,0 +1,17 @@ +Source: foo +Section: misc +Priority: optional +Maintainer: Test +Standards-Version: 3.9.8 + +Package: has-settings +Architecture: all +Depends: ${misc:Depends} +Description: package has-settings + This package has a GSettings schema. + +Package: no-settings +Architecture: all +Depends: ${misc:Depends} +Description: package no-settings + This package doesn't have a GSettings schema. diff --git a/t/dh_installgsettings/dh_installgsettings.t b/t/dh_installgsettings/dh_installgsettings.t new file mode 100755 index 00000000..fe76e8dc --- /dev/null +++ b/t/dh_installgsettings/dh_installgsettings.t @@ -0,0 +1,49 @@ +#!/usr/bin/perl +use strict; +use Test::More tests => 1; + +use autodie; +use File::Basename qw(dirname); +use lib dirname(dirname(__FILE__)); +use Test::DH; +use File::Path qw(remove_tree make_path); +use Debian::Debhelper::Dh_Lib qw(!dirname); + +our @TEST_DH_EXTRA_TEMPLATE_FILES = (qw( + debian/changelog + debian/control +)); + +my $SCHEMAS = 'usr/share/glib-2.0/schemas'; + +sub touch { + my $path = shift; + open(my $fh, '>>', $path); + close $fh; +} + +sub slurp { + my $path = shift; + local $/ = undef; + open(my $fh, '<', $path); + my $contents = <$fh>; + close $fh; + return $contents; +} + +each_compat_subtest { + make_path("debian/has-settings/$SCHEMAS"); + touch("debian/has-settings/$SCHEMAS/com.example.HasSettings.xml"); + make_path("debian/has-unimportant-settings/$SCHEMAS"); + touch("debian/no-settings.substvars"); + ok(run_dh_tool('dh_installgsettings', '-phas-settings'), 'run for has-settings'); + ok(run_dh_tool('dh_installgsettings', '-pno-settings'), 'run for no-settings'); + remove_tree(qw(debian/has-settings debian/has-unimportant-settings)); + like(slurp('debian/has-settings.substvars'), + qr{^misc:Depends=dconf-gsettings-backend \| gsettings-backend$}m, + 'has-settings should depend on a backend'); + unlike(slurp('debian/no-settings.substvars'), + qr{^misc:Depends=dconf-gsettings-backend \| gsettings-backend$}m, + 'no-settings should not depend on a backend'); +}; + -- cgit v1.2.3 From 0ad076642f049193bc57cd571c40c59ca07074db Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Fri, 25 Oct 2019 06:01:14 +0000 Subject: Add test to catch regressions of #943376 Signed-off-by: Niels Thykier --- t/dh_installman/01-basics.t | 36 ++++++++++++++++++++++++++++++++ t/dh_installman/manpage-compressed.pod | 17 +++++++++++++++ t/dh_installman/manpage-uncompressed.pod | 17 +++++++++++++++ 3 files changed, 70 insertions(+) create mode 100755 t/dh_installman/01-basics.t create mode 100644 t/dh_installman/manpage-compressed.pod create mode 100644 t/dh_installman/manpage-uncompressed.pod (limited to 't') diff --git a/t/dh_installman/01-basics.t b/t/dh_installman/01-basics.t new file mode 100755 index 00000000..84857190 --- /dev/null +++ b/t/dh_installman/01-basics.t @@ -0,0 +1,36 @@ +#!/usr/bin/perl +use strict; +use warnings; +use Test::More; + +use File::Basename qw(dirname); +use lib dirname(dirname(__FILE__)); +use Test::DH; +use File::Path qw(remove_tree make_path); +use Debian::Debhelper::Dh_Lib qw(!dirname); + +plan(tests => 1); + +our @TEST_DH_EXTRA_TEMPLATE_FILES = (qw( + manpage-uncompressed.pod + manpage-compressed.pod +)); + +each_compat_subtest { + my ($compat) = @_; + if (! -d 'generated-manpages') { + # Static data that can be reused. Generate only in the first test + make_path('generated-manpages'); + for my $basename (qw(manpage-uncompressed manpage-compressed)) { + doit('pod2man', '--utf8', '-c', 'Debhelper', '-r', '1.0', "${basename}.pod", + "generated-manpages/${basename}.1"); + } + doit('gzip', '-9n', 'generated-manpages/manpage-compressed.1'); + } + ok(run_dh_tool('dh_installman', 'generated-manpages/manpage-uncompressed.1', + 'generated-manpages/manpage-compressed.1.gz')); + ok(-e 'debian/debhelper/usr/share/man/man1/manpage-uncompressed.1'); + ok(-e 'debian/debhelper/usr/share/man/man1/manpage-compressed.1'); + remove_tree('debian/debhelper', 'debian/tmp', 'debian/.debhelper'); +}; + diff --git a/t/dh_installman/manpage-compressed.pod b/t/dh_installman/manpage-compressed.pod new file mode 100644 index 00000000..d2c05968 --- /dev/null +++ b/t/dh_installman/manpage-compressed.pod @@ -0,0 +1,17 @@ +=head1 NAME + +manpage - Something very exciting + +=head1 SYNOPSIS + +This tool does not exist but would be awesome. + +=head1 SEE ALSO + +L + +=head1 AUTHORS + +Niels Thykier + +=cut diff --git a/t/dh_installman/manpage-uncompressed.pod b/t/dh_installman/manpage-uncompressed.pod new file mode 100644 index 00000000..c95037cf --- /dev/null +++ b/t/dh_installman/manpage-uncompressed.pod @@ -0,0 +1,17 @@ +=head1 NAME + +manpage-uncompressed - Something very exciting + +=head1 SYNOPSIS + +This tool does not exist but would be awesome. + +=head1 SEE ALSO + +L + +=head1 AUTHORS + +Niels Thykier + +=cut -- cgit v1.2.3 From ceb7bae3d2d118379deb01fd9beed9f930cfea9a Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Fri, 25 Oct 2019 06:14:43 +0000 Subject: t: Expand dh_installman test to cover already installed manpages Signed-off-by: Niels Thykier --- t/dh_installman/01-basics.t | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 't') diff --git a/t/dh_installman/01-basics.t b/t/dh_installman/01-basics.t index 84857190..de7293d7 100755 --- a/t/dh_installman/01-basics.t +++ b/t/dh_installman/01-basics.t @@ -9,7 +9,7 @@ use Test::DH; use File::Path qw(remove_tree make_path); use Debian::Debhelper::Dh_Lib qw(!dirname); -plan(tests => 1); +plan(tests => 2); our @TEST_DH_EXTRA_TEMPLATE_FILES = (qw( manpage-uncompressed.pod @@ -34,3 +34,23 @@ each_compat_subtest { remove_tree('debian/debhelper', 'debian/tmp', 'debian/.debhelper'); }; +each_compat_subtest { + my ($compat) = @_; + if (! -d 'generated-manpages') { + # Static data that can be reused. Generate only in the first test + make_path('generated-manpages'); + for my $basename (qw(manpage-uncompressed manpage-compressed)) { + doit('pod2man', '--utf8', '-c', 'Debhelper', '-r', '1.0', "${basename}.pod", + "generated-manpages/${basename}.1"); + } + doit('gzip', '-9n', 'generated-manpages/manpage-compressed.1'); + } + install_dir('debian/debhelper/usr/share/man/man1'); + install_file('generated-manpages/manpage-uncompressed.1', 'debian/debhelper/usr/share/man/man1/manpage-uncompressed.1'); + install_file('generated-manpages/manpage-compressed.1.gz', 'debian/debhelper/usr/share/man/man1/manpage-compressed.1.gz'); + ok(run_dh_tool('dh_installman')); + ok(-e 'debian/debhelper/usr/share/man/man1/manpage-uncompressed.1'); + ok(-e 'debian/debhelper/usr/share/man/man1/manpage-compressed.1'); + remove_tree('debian/debhelper', 'debian/tmp', 'debian/.debhelper'); +}; + -- cgit v1.2.3 From 1f7b871e13ba716b32f956fdf3b8deb8389a0565 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 26 Oct 2019 06:15:11 +0000 Subject: Skip dh_installman test by default as it requires extra dependencies Signed-off-by: Niels Thykier --- .gitlab-ci.yml | 8 ++++++++ debian/control | 2 ++ t/dh_installman/01-basics.t | 17 ++++++++++++++++- 3 files changed, 26 insertions(+), 1 deletion(-) (limited to 't') diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cfecfe10..713ae1ab 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -14,6 +14,14 @@ tests-testing: - apt-get build-dep -y . - dpkg-buildpackage -us -uc -tc +tests-unstable-all: + stage: test + image: debian:unstable + script: + - apt-get update + - apt-get build-dep -y -Ppkg.debhelper.ci . + - dpkg-buildpackage -us -uc -tc + tests-unstable: stage: test image: debian:unstable diff --git a/debian/control b/debian/control index 656456df..e2574f68 100644 --- a/debian/control +++ b/debian/control @@ -6,6 +6,8 @@ Uploaders: Niels Thykier , Build-Depends: dpkg-dev (>= 1.18.0~), perl:any, po4a, + man-db , + libtest-pod-perl , Rules-Requires-Root: no Standards-Version: 4.4.0 Testsuite: autopkgtest-pkg-perl diff --git a/t/dh_installman/01-basics.t b/t/dh_installman/01-basics.t index de7293d7..6e90cc94 100755 --- a/t/dh_installman/01-basics.t +++ b/t/dh_installman/01-basics.t @@ -9,7 +9,22 @@ use Test::DH; use File::Path qw(remove_tree make_path); use Debian::Debhelper::Dh_Lib qw(!dirname); -plan(tests => 2); +sub has_man_db_tool { + my ($tool) = @_; + open(my $old_stderr, '>&', *STDERR) or error("dup(STDERR, tmp_fd): $!"); + open(*STDERR, '>', '/dev/null') or error("re-open stderr as /dev/null: $!"); + + my $res = defined(`$tool --version`); + open(*STDERR, '>&', $old_stderr) or error("dup(tmp_fd, STDERR): $!"); + close($old_stderr); + return $res; +} + +if (has_man_db_tool('man') || has_man_db_tool('man-recode')) { + plan(tests => 2); +} else { + plan(skip_all => 'Test requires man or man-recode'); +} our @TEST_DH_EXTRA_TEMPLATE_FILES = (qw( manpage-uncompressed.pod -- cgit v1.2.3