dh_installsystemd and Also= directive

Daniele Nicolodi daniele at grinta.net
Sun Jun 10 22:55:29 BST 2018


Hello,

please keep me in Cc, I'm not subscribed to the list.

I'm implementing user units support in dh_installsystemd. For doing so
the code needs a bit of refactoring and thus I'm trying to understand
how the tool is supposed to operate and adding some tests to check that
nothing will break in the process.

dh_installsystemd uses the following code to find all the units
installed by a package:

> 	my $oldcwd = getcwd();
> 	find({
> 		wanted => sub {
> 			my $name = $File::Find::name;
> 			return unless -f $name;
> 			return unless $name =~ m,^\Q${tmpdir}\E/lib/systemd/system/[^/]+$,;
> 			if (-l) {
> 				my $target = abs_path(readlink());
> 				$target =~ s,^\Q${oldcwd}\E/,,g;
> 				$aliases{$target} = [ $_ ];
> 			} else {
> 				push @installed_units, $name;
> 			}
> 		},
> 		no_chdir => 1,
> 	}, "${tmpdir}/lib/systemd/system") if -d "${tmpdir}/lib/systemd/system";


My Perl is very rusty, but I think that the above code does not to what
at some point it was intended to do. What it was intended was to iterate
the contents of the "${tmpdir}/lib/systemd/system" directory pushing all
file paths into the @installed_units array and reading all symbolic
links into the %aliases hash.  However, in the case of symlinks
execution never goes past the 'return unless -f $name' test and %aliases
is never populated.

Unfortunately there aren't existing tests exercising this code.

Should the above be fixed? Given that it does not work and no one
complains, can the code be simplified? Something like the untested code
below

> opendir(LIB_SYSTEMD_SYSTEM, "$tmpdir/lib/sytemd/system") or die $!;
> push @installed_units, $_ for grep { -f } readdir(LIB_SYSTEMD_SYSTEM);
> closedir(LIB_SYSTEMD_SYSTEM);

should be equivalent.

Thanks. Cheers,
Dan




More information about the Pkg-systemd-maintainers mailing list