On 19 July 2016 at 13:28, Niels Thykier <ni...@thykier.net> wrote: > Hi Peter, > > Thanks for the report - at first glance it sounds like #830208 (Cc'ed > accordingly). > > Peter's message quoted in full (for those not subscribed to > debhelper-devel): > >> Hi, >> >> Now that dh_systemd_start and dh_systemd_enable are part of debhelper >> proper, there's a bit of duplication of work between dh_installinit and >> dh_systemd_start. Now don't get me wrong: I do agree that installing >> and configuring systemd service files correctly is a Good Thing(tm); >> it's just that I'm afraid that there are some more rough edges to >> polish. >> >> So far I've only seen the problem in a package that provides both >> a systemd service file and a SysV init script. If the service file is >> named debian/package.service, dh_systemd_start and dh_systemd_enable >> will pick it up and process it just fine... but then dh_installinit will >> *also* pick it up. It's invoked because debian/package.init exists, but >> it tries to process systemd and upstart files, too, if it finds them, >> so it tries to do once again what dh_systemd_start just did, and, well, >> it even gets it subtly wrong :) >> >> So, hm, I'm not sure what the proper resolution would be. Would it be >> possible for dh_installinit to figure out that the systemd sequence is >> enabled? If so, then this might be the best solution - in compat 10 >> with the systemd sequence enabled, ignore any systemd service files. >> >> Again, don't get me wrong, I *am* very happy with the way debhelper >> development is progressing; thanks a lot for that! >> >> G'luck, >> Peter >> >> [...] > > Michael/Martin: What is your take - should we just disable the service > handling in dh_installinit for compat 10 or newer? AFAICT, it should > "just work(tm)" and now would be the time to do it if we want it in > compat 10.
I think that installinit should stop looking at .service and .tmpfiles. Making dh_installinit ignore .service and .tmpfiles is a compat break so I think this should only be enabled for compat >= 11. I think something like the attached patch should do. -- Saludos, Felipe Sateler
From 47f5c88087f204431c7db6b873c10d69accf1572 Mon Sep 17 00:00:00 2001 From: Felipe Sateler <fsate...@debian.org> Date: Wed, 14 Dec 2016 22:31:27 -0300 Subject: [PATCH] installinit: do not process systemd files from compat 11 onwards --- debhelper.pod | 6 ++++ dh_installinit | 12 ++++---- t/dh_installinit/debian/changelog | 5 ++++ t/dh_installinit/debian/compat | 1 + t/dh_installinit/debian/control | 20 +++++++++++++ t/dh_installinit/debian/foo.service | 5 ++++ t/dh_installinit/dh_installinit.t | 57 +++++++++++++++++++++++++++++++++++++ 7 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 t/dh_installinit/debian/changelog create mode 100644 t/dh_installinit/debian/compat create mode 100644 t/dh_installinit/debian/control create mode 100644 t/dh_installinit/debian/foo.service create mode 100755 t/dh_installinit/dh_installinit.t diff --git a/debhelper.pod b/debhelper.pod index e7697ead..c9a3739e 100644 --- a/debhelper.pod +++ b/debhelper.pod @@ -586,6 +586,12 @@ F<menu-method> files are still installed. =item - +B<dh_installinit> no longer installs F<service> or F<tmpfile> files, nor +generates maintainer scripts for those files. Use B<dh_systemd_enable> and +B<dh_systemd_start> instead. + +=item - + The B<-s> (B<--same-arch>) option is removed. =item - diff --git a/dh_installinit b/dh_installinit index 087a3bd8..3f80d127 100755 --- a/dh_installinit +++ b/dh_installinit @@ -47,13 +47,13 @@ build directory. =item debian/I<package>.service If this exists, it is installed into lib/systemd/system/I<package>.service in -the package build directory. +the package build directory. Only compat levels 10 and below. =item debian/I<package>.tmpfile If this exists, it is installed into usr/lib/tmpfiles.d/I<package>.conf in the package build directory. (The tmpfiles.d mechanism is currently only used -by systemd.) +by systemd.) Only compa levels 10 and below. =back @@ -216,14 +216,16 @@ foreach my $package (@{$dh{DOPACKAGES}}) { } } - my $service=pkgfile($package,"service"); + my $service=''; + $service=pkgfile($package,"service") if compat(10); if ($service ne '' && ! $dh{ONLYSCRIPTS}) { my $path="$tmp/lib/systemd/system"; install_dir($path); install_file($service, "$path/$script.service"); } - my $tmpfile=pkgfile($package,"tmpfile"); + my $tmpfile=''; + $tmpfile=pkgfile($package,"tmpfile") if compat(10); if ($tmpfile ne '' && ! $dh{ONLYSCRIPTS}) { my $path="$tmp/usr/lib/tmpfiles.d"; install_dir($path); @@ -254,7 +256,7 @@ foreach my $package (@{$dh{DOPACKAGES}}) { error("Can't use --init-script with an upstart job"); } - if (!$dh{NOSCRIPTS}) { + if (compat(10) && !$dh{NOSCRIPTS}) { # Include postinst-init-tmpfiles if the package ships any files # in /usr/lib/tmpfiles.d or /etc/tmpfiles.d my @tmpfiles; diff --git a/t/dh_installinit/debian/changelog b/t/dh_installinit/debian/changelog new file mode 100644 index 00000000..5850f0e2 --- /dev/null +++ b/t/dh_installinit/debian/changelog @@ -0,0 +1,5 @@ +foo (1.0-1) unstable; urgency=low + + * Initial release. (Closes: #XXXXXX) + + -- Test <testing@nowhere> Mon, 11 Jul 2016 18:10:59 +0200 diff --git a/t/dh_installinit/debian/compat b/t/dh_installinit/debian/compat new file mode 100644 index 00000000..f599e28b --- /dev/null +++ b/t/dh_installinit/debian/compat @@ -0,0 +1 @@ +10 diff --git a/t/dh_installinit/debian/control b/t/dh_installinit/debian/control new file mode 100644 index 00000000..48d4de2f --- /dev/null +++ b/t/dh_installinit/debian/control @@ -0,0 +1,20 @@ +Source: foo +Section: misc +Priority: optional +Maintainer: Test <testing@nowhere> +Standards-Version: 3.9.8 + +Package: foo +Architecture: all +Description: package foo + Package foo + +Package: bar +Architecture: all +Description: package bar + Package bar + +Package: baz +Architecture: all +Description: package baz + Package baz diff --git a/t/dh_installinit/debian/foo.service b/t/dh_installinit/debian/foo.service new file mode 100644 index 00000000..aa216362 --- /dev/null +++ b/t/dh_installinit/debian/foo.service @@ -0,0 +1,5 @@ +[Unit] +Description=A unit + +[Service] +ExecStart=/bin/true diff --git a/t/dh_installinit/dh_installinit.t b/t/dh_installinit/dh_installinit.t new file mode 100755 index 00000000..d05c2074 --- /dev/null +++ b/t/dh_installinit/dh_installinit.t @@ -0,0 +1,57 @@ +#!/usr/bin/perl +use strict; +use Test::More; +use File::Basename (); + +# Let the tests be run from anywhere, but current directory +# is expected to be the one where this test lives in. +chdir File::Basename::dirname($0) or die "Unable to chdir to ".File::Basename::dirname($0); + +my $TOPDIR = "../.."; +my $rootcmd; + +if ($< == 0) { + $rootcmd = ''; +} +else { + system("fakeroot true 2>/dev/null"); + $rootcmd = $? ? undef : 'fakeroot'; +} + +if (not defined($rootcmd)) { + plan skip_all => 'fakeroot required'; +} +else { + plan(tests => 5); +} + +system("$TOPDIR/dh_clean"); + +my $service = "debian/foo.service"; + +system("mkdir -p debian/foo debian/bar debian/baz"); +system("$rootcmd $TOPDIR/dh_installinit"); +ok(-e "debian/foo/lib/systemd/system/foo.service"); +ok(-e "debian/foo.postinst.debhelper"); +system("$TOPDIR/dh_clean"); + +system("mkdir -p debian/foo debian/bar debian/baz"); +system("DH_COMPAT=11 $rootcmd $TOPDIR/dh_installinit"); +ok(! -e "debian/foo/lib/systemd/system/foo.service"); +ok(! -e "debian/foo.postinst.debhelper"); +system("$TOPDIR/dh_clean"); + +system("mkdir -p debian/foo debian/bar debian/baz"); +system("mkdir -p debian/foo/lib/systemd/system/"); +system("cp debian/foo.service debian/foo/lib/systemd/system/"); +system("DH_COMPAT=11 $rootcmd $TOPDIR/dh_installinit"); +ok(! -e "debian/foo.postinst.debhelper"); +system("$TOPDIR/dh_clean"); + +system("$TOPDIR/dh_clean"); + +# Local Variables: +# indent-tabs-mode: t +# tab-width: 4 +# cperl-indent-level: 4 +# End: -- 2.11.0