Package: lintian Version: 2.5.32 Severity: wishlist Tags: patch Hi,
Please find attached a patch that does $subject. I have taken the liberty to refactor the code a bit in order to stop tagging multiple times for the same error. Patches 1-3 are the refactoring, patch 4 is the new check. There is a test for the new check. I'm wondering if tag systemd-no-service-for-init-script should be lowered in severity but added inconditionally... but that is a separate issue. If/when this patch is merged, I can provide a patch for changing that so we can discuss that. -- System Information: Debian Release: stretch/sid APT prefers unstable APT policy: (500, 'unstable'), (1, 'experimental') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 4.0.0-2-amd64 (SMP w/4 CPU cores) Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system) Versions of packages lintian depends on: ii binutils 2.25-8 ii bzip2 1.0.6-8 ii diffstat 1.58-1 ii file 1:5.22+15-2 ii gettext 0.19.4-1 ii hardening-includes 2.7 ii intltool-debian 0.35.0+20060710.2 ii libapt-pkg-perl 0.1.29+b2 ii libarchive-zip-perl 1.48-1 ii libclass-accessor-perl 0.34-1 ii libclone-perl 0.38-1 ii libdpkg-perl 1.18.1 ii libemail-valid-perl 1.195-1 ii libfile-basedir-perl 0.07-1 ii libipc-run-perl 0.94-1 ii liblist-moreutils-perl 0.410-1 ii libparse-debianchangelog-perl 1.2.0-4 ii libtext-levenshtein-perl 0.12-1 ii libtimedate-perl 2.3000-2 ii liburi-perl 1.64-1 ii man-db 2.7.0.2-5 ii patchutils 0.3.4-1 ii perl [libdigest-sha-perl] 5.20.2-6 ii t1utils 1.38-4 ii xz-utils 5.1.1alpha+20120614-2+b3 Versions of packages lintian recommends: ii dpkg 1.18.1 ii libautodie-perl 2.27-2 ii libperlio-gzip-perl 0.18-3+b1 ii perl 5.20.2-6 ii perl-modules [libautodie-perl] 5.20.2-6 Versions of packages lintian suggests: pn binutils-multiarch <none> ii dpkg-dev 1.18.1 ii libhtml-parser-perl 3.71-2 ii libtext-template-perl 1.46-1 ii libyaml-perl 1.13-1 -- no debconf information
>From 1c4ad47fead2a6d32b5fdc6888ba7b5333804bcb Mon Sep 17 00:00:00 2001 From: Felipe Sateler <fsate...@debian.org> Date: Sat, 27 Jun 2015 16:32:42 -0300 Subject: [PATCH 1/4] Reorder systemd checks This reorder groups most checks inside the corresponding check_* --- checks/systemd.pm | 140 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 79 insertions(+), 61 deletions(-) diff --git a/checks/systemd.pm b/checks/systemd.pm index d36cf65..4a45b49 100644 --- a/checks/systemd.pm +++ b/checks/systemd.pm @@ -37,81 +37,67 @@ use Lintian::Util qw(fail lstrip rstrip); sub run { my (undef, undef, $info) = @_; - # Figure out whether the maintainer of this package did any effort to - # make the package work with systemd. If not, we will not warn in case - # of an init script that has no systemd equivalent, for example. - my $ships_systemd_file = any { m,/systemd/, } $info->sorted_index; - - # An array of names which are provided by the service files. - # This includes Alias= directives, so after parsing - # NetworkManager.service, it will contain NetworkManager and - # network-manager. - my @systemd_targets; - + # non-service checks for my $file ($info->sorted_index) { if ($file =~ m,^etc/tmpfiles\.d/.*\.conf$,) { tag 'systemd-tmpfiles.d-outside-usr-lib', $file; } - if ($file =~ m,^etc/systemd/system/.*\.service$,) { - tag 'systemd-service-file-outside-lib', $file; - } - if ($file =~ m,^usr/lib/systemd/system/.*\.service$,) { - tag 'systemd-service-file-outside-lib', $file; - } - if ($file =~ m,/systemd/system/.*\.service$,) { - check_systemd_service_file($info, $file); - for my $name (extract_service_file_names($info, $file)) { - push @systemd_targets, $name; - } - } } - my @init_scripts = grep { m,^etc/init\.d/.+, } $info->sorted_index; - - # Verify that each init script includes /lib/lsb/init-functions, - # because that is where the systemd diversion happens. - for my $init_script (@init_scripts) { - check_init_script($info, $init_script); + my @init_scripts = get_init_scripts($info); + my @service_files = get_systemd_service_files($info); + + # A hash of names which are provided by the service files. + # This includes Alias= directives, so after parsing + # NetworkManager.service, it will contain NetworkManager and + # network-manager. + my %services = get_systemd_service_names($info); + + for my $script (@init_scripts) { + check_init_script($info, $script, %services); } - @init_scripts = map { basename($_) } @init_scripts; - - if ($ships_systemd_file) { - for my $init_script (@init_scripts) { - tag 'systemd-no-service-for-init-script', $init_script - unless any { m/\Q$init_script\E\.service/ } @systemd_targets; - } + for my $service (@service_files) { + check_systemd_service_file($info, $service); } check_maintainer_scripts($info); return; } +sub get_init_scripts { + my $info = shift @_; + my @ignore = ( + 'README', + 'skeleton', + 'rc', + 'rcS', + ); + my @scripts; + if (my $initd_path = $info->index_resolved_path('etc/init.d/')) { + for my $init_script ($initd_path->children) { + next if any { $_ eq $init_script->basename } @ignore; + next if $init_script->is_symlink && $init_script->link eq '/lib/init/upstart-job'; + + push(@scripts, $init_script); + } + } + return @scripts; +} + +# Verify that each init script includes /lib/lsb/init-functions, +# because that is where the systemd diversion happens. sub check_init_script { - my ($info, $file) = @_; + my ($info, $file, %services) = @_; my $basename = $file->basename; my $lsb_source_seen; - # Couple of special cases we don't care about... - return - if $basename eq 'README' - or $basename eq 'skeleton' - or $basename eq 'rc' - or $basename eq 'rcS'; - - if ($file->is_symlink) { - # We cannot test upstart-jobs - return if $file->link eq '/lib/init/upstart-job'; - } - if (!$file->is_regular_file) { unless ($file->is_open_ok) { tag 'init-script-is-not-a-file', $file; return; } - } - my $fh = $file->open; while (<$fh>) { lstrip; @@ -127,15 +113,54 @@ sub check_init_script { } close($fh); - if (!$lsb_source_seen) { - tag 'init.d-script-does-not-source-init-functions', $file; - } + tag 'init.d-script-does-not-source-init-functions', $file + unless $lsb_source_seen; + # Only tag if the maintainer of this package did any effort to + # make the package work with systemd. + tag 'systemd-no-service-for-init-script', $basename + if (%services and !$services{$basename}); return; } +sub get_systemd_service_files { + my $info = shift @_; + + return grep { m,/systemd/system/.*\.service$, } $info->sorted_index; +} + +sub get_systemd_service_names { + my ($info) = @_; + my %services; + + my $safe_add_service = sub { + my ($name, $file) = @_; + if (exists $services{$name}) { + # should add a tag here + return; + } + $services{$name} = 1; + }; + + for my $file (get_systemd_service_files($info)) { + my $name = $file->basename; + $name =~ s/\.service$//; + $safe_add_service->($name, $file); + + my @aliases = extract_service_file_values($info, $file, 'Install', 'Alias'); + + for my $alias (@aliases) { + $safe_add_service->($alias, $file); + } + } + return %services; +} + sub check_systemd_service_file { my ($info, $file) = @_; + tag 'systemd-service-file-outside-lib', $file if ($file =~ m,^etc/systemd/system/,); + tag 'systemd-service-file-outside-lib', $file if ($file =~ m,^usr/lib/systemd/system/,); + my @values = extract_service_file_values($info, $file, 'Unit', 'After'); my @obsolete = grep { /^(?:syslog|dbus)\.target$/ } @values; tag 'systemd-service-file-refers-to-obsolete-target', $file, $_ @@ -236,13 +261,6 @@ sub extract_service_file_values { return @values; } -sub extract_service_file_names { - my ($info, $file) = @_; - - my @aliases= extract_service_file_values($info, $file, 'Install', 'Alias'); - return (basename($file), @aliases); -} - sub check_maintainer_scripts { my ($info) = @_; -- 2.1.4
>From f712f9246412799088f2893cb5323b8b9f295de3 Mon Sep 17 00:00:00 2001 From: Felipe Sateler <fsate...@debian.org> Date: Sat, 27 Jun 2015 21:56:11 -0300 Subject: [PATCH 2/4] Check files as we detect them, and discard invalid files prevents duplicate service-file-is-not-a-file --- checks/systemd.pm | 32 +++++++++++++++++--------------- t/tests/systemd-general/tags | 1 - 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/checks/systemd.pm b/checks/systemd.pm index 4a45b49..2fd2c82 100644 --- a/checks/systemd.pm +++ b/checks/systemd.pm @@ -51,16 +51,12 @@ sub run { # This includes Alias= directives, so after parsing # NetworkManager.service, it will contain NetworkManager and # network-manager. - my %services = get_systemd_service_names($info); + my %services = get_systemd_service_names($info, @service_files); for my $script (@init_scripts) { check_init_script($info, $script, %services); } - for my $service (@service_files) { - check_systemd_service_file($info, $service); - } - check_maintainer_scripts($info); return; } @@ -124,12 +120,18 @@ sub check_init_script { sub get_systemd_service_files { my $info = shift @_; + my @res; + my @potential = grep { m,/systemd/system/.*\.service$, } $info->sorted_index; - return grep { m,/systemd/system/.*\.service$, } $info->sorted_index; + for my $file (@potential) { + push(@res, $file) if check_systemd_service_file($info, $file); + } + return @res; } sub get_systemd_service_names { - my ($info) = @_; + my $info = shift @_; + my @files = @_; my %services; my $safe_add_service = sub { @@ -141,7 +143,7 @@ sub get_systemd_service_names { $services{$name} = 1; }; - for my $file (get_systemd_service_files($info)) { + for my $file (@files) { my $name = $file->basename; $name =~ s/\.service$//; $safe_add_service->($name, $file); @@ -160,12 +162,17 @@ sub check_systemd_service_file { tag 'systemd-service-file-outside-lib', $file if ($file =~ m,^etc/systemd/system/,); tag 'systemd-service-file-outside-lib', $file if ($file =~ m,^usr/lib/systemd/system/,); - + + unless ($file->is_open_ok + || ($file->is_symlink && $file->link eq '/dev/null')) { + tag 'service-file-is-not-a-file', $file; + return 0; + } my @values = extract_service_file_values($info, $file, 'Unit', 'After'); my @obsolete = grep { /^(?:syslog|dbus)\.target$/ } @values; tag 'systemd-service-file-refers-to-obsolete-target', $file, $_ for @obsolete; - return; + return 1; } sub service_file_lines { @@ -207,11 +214,6 @@ sub extract_service_file_values { my (@values, $section); - unless ($file->is_open_ok - || ($file->is_symlink && $file->link eq '/dev/null')) { - tag 'service-file-is-not-a-file', $file; - return; - } my @lines = service_file_lines($file); my $key_ws = first_index { /^[[:alnum:]]+(\s*=\s|\s+=)/ } @lines; if ($key_ws > -1) { diff --git a/t/tests/systemd-general/tags b/t/tests/systemd-general/tags index 47fe757..3223f6a 100644 --- a/t/tests/systemd-general/tags +++ b/t/tests/systemd-general/tags @@ -1,6 +1,5 @@ E: systemd-general: init-script-is-not-a-file etc/init.d/fifo-pipe-as-init E: systemd-general: service-file-is-not-a-file etc/systemd/system/fifo-pipe-as-init.service -E: systemd-general: service-file-is-not-a-file etc/systemd/system/fifo-pipe-as-init.service E: systemd-general: service-key-has-whitespace etc/systemd/system/test.service at line 3 E: systemd-general: service-key-has-whitespace etc/systemd/system/test.service at line 3 E: systemd-general: service-key-has-whitespace usr/lib/systemd/system/test.service at line 3 -- 2.1.4
>From f98b16ffd7c8adb603fa6de4afc9dfc06c142764 Mon Sep 17 00:00:00 2001 From: Felipe Sateler <fsate...@debian.org> Date: Sat, 27 Jun 2015 22:01:19 -0300 Subject: [PATCH 3/4] Add parameter to prevent tagging when parsing values Enables us to prevent multiple service-key-has-whitespace --- checks/systemd.pm | 6 +++--- t/tests/systemd-complex-service-file/tags | 1 - t/tests/systemd-general/tags | 2 -- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/checks/systemd.pm b/checks/systemd.pm index 2fd2c82..dda5a09 100644 --- a/checks/systemd.pm +++ b/checks/systemd.pm @@ -148,7 +148,7 @@ sub get_systemd_service_names { $name =~ s/\.service$//; $safe_add_service->($name, $file); - my @aliases = extract_service_file_values($info, $file, 'Install', 'Alias'); + my @aliases = extract_service_file_values($info, $file, 'Install', 'Alias', 1); for my $alias (@aliases) { $safe_add_service->($alias, $file); @@ -210,14 +210,14 @@ sub service_file_lines { # Extracts the values of a specific Key from a .service file sub extract_service_file_values { - my ($info, $file, $extract_section, $extract_key) = @_; + my ($info, $file, $extract_section, $extract_key, $skip_tag) = @_; my (@values, $section); my @lines = service_file_lines($file); my $key_ws = first_index { /^[[:alnum:]]+(\s*=\s|\s+=)/ } @lines; if ($key_ws > -1) { - tag 'service-key-has-whitespace', $file, 'at line', $key_ws; + tag 'service-key-has-whitespace', $file, 'at line', $key_ws unless $skip_tag; } if (any { /^\.include / } @lines) { my $parent_dir = $file->parent_dir; diff --git a/t/tests/systemd-complex-service-file/tags b/t/tests/systemd-complex-service-file/tags index 1ffee42..61a9669 100644 --- a/t/tests/systemd-complex-service-file/tags +++ b/t/tests/systemd-complex-service-file/tags @@ -1,4 +1,3 @@ E: systemd-complex-service-file: service-key-has-whitespace lib/systemd/system/test3.service at line 3 -E: systemd-complex-service-file: service-key-has-whitespace lib/systemd/system/test3.service at line 3 W: systemd-complex-service-file: systemd-service-file-refers-to-obsolete-target lib/systemd/system/test.service dbus.target W: systemd-complex-service-file: systemd-service-file-refers-to-obsolete-target lib/systemd/system/test2.service syslog.target diff --git a/t/tests/systemd-general/tags b/t/tests/systemd-general/tags index 3223f6a..6f693e3 100644 --- a/t/tests/systemd-general/tags +++ b/t/tests/systemd-general/tags @@ -1,8 +1,6 @@ E: systemd-general: init-script-is-not-a-file etc/init.d/fifo-pipe-as-init E: systemd-general: service-file-is-not-a-file etc/systemd/system/fifo-pipe-as-init.service E: systemd-general: service-key-has-whitespace etc/systemd/system/test.service at line 3 -E: systemd-general: service-key-has-whitespace etc/systemd/system/test.service at line 3 -E: systemd-general: service-key-has-whitespace usr/lib/systemd/system/test.service at line 3 E: systemd-general: service-key-has-whitespace usr/lib/systemd/system/test.service at line 3 E: systemd-general: special-file etc/init.d/fifo-pipe-as-init 0644 E: systemd-general: special-file etc/systemd/system/fifo-pipe-as-init.service 0644 -- 2.1.4
>From 0827f5304dc2912226e99672116c19a1ee0e9d7b Mon Sep 17 00:00:00 2001 From: Felipe Sateler <fsate...@debian.org> Date: Sat, 27 Jun 2015 12:20:19 -0300 Subject: [PATCH 4/4] systemd.{desc,pm}: add check for rcS.d init scripts without native systemd unit --- checks/systemd.desc | 14 ++++++++++++++ checks/systemd.pm | 12 ++++++++++-- t/tests/systemd-general/debian/debian/init | 2 +- t/tests/systemd-general/desc | 1 + t/tests/systemd-general/tags | 1 + 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/checks/systemd.desc b/checks/systemd.desc index f84835f..6554f58 100644 --- a/checks/systemd.desc +++ b/checks/systemd.desc @@ -47,6 +47,20 @@ Info: The listed init.d script has no systemd equivalent. Your package ships a service file, but for the listed init.d script, there is no corresponding systemd service file. +Tag: systemd-no-service-for-init-rcS-script +Severity: serious +Certainty: certain +Ref: https://wiki.debian.org/Teams/pkg-systemd/rcSMigration +Info: The rcS init.d script has no systemd equivalent. + . + Systemd has a SysV init.d script compatibility mode. It provides access to + each SysV init.d script as long as there is no native service file with the + same name (e.g. <tt>/lib/systemd/system/rsyslog.service</tt> corresponds to + <tt>/etc/init.d/rsyslog</tt>). + . + Services in rcS.d are particularly problematic, because they often cause + dependency loops, as they are ordered very early in the boot sequence. + Tag: init.d-script-does-not-source-init-functions Severity: normal Certainty: certain diff --git a/checks/systemd.pm b/checks/systemd.pm index dda5a09..f7864ae 100644 --- a/checks/systemd.pm +++ b/checks/systemd.pm @@ -87,6 +87,7 @@ sub check_init_script { my ($info, $file, %services) = @_; my $basename = $file->basename; my $lsb_source_seen; + my $is_rcs_script = 0; if (!$file->is_regular_file) { unless ($file->is_open_ok) { @@ -99,12 +100,14 @@ sub check_init_script { lstrip; if ($. == 1 and m{\A [#]! \s*/lib/init/init-d-script}xsm) { $lsb_source_seen = 1; - last; } + if (m,#.*Default-Start:.*S,) { + $is_rcs_script = 1; + } + next if /^#/; if (m,(?:\.|source)\s+/lib/(?:lsb/init-functions|init/init-d-script),){ $lsb_source_seen = 1; - last; } } close($fh); @@ -115,6 +118,11 @@ sub check_init_script { # make the package work with systemd. tag 'systemd-no-service-for-init-script', $basename if (%services and !$services{$basename}); + + # rcS scripts are particularly bad, warn even if there is + # no systemd integration + tag 'systemd-no-service-for-init-rcS-script', $basename + if (!$services{$basename} and $is_rcs_script); return; } diff --git a/t/tests/systemd-general/debian/debian/init b/t/tests/systemd-general/debian/debian/init index afffa18..42cb175 100644 --- a/t/tests/systemd-general/debian/debian/init +++ b/t/tests/systemd-general/debian/debian/init @@ -3,7 +3,7 @@ # Provides: systemd-general # Required-Start: $remote_fs $syslog # Required-Stop: $remote_fs $syslog -# Default-Start: 2 3 4 5 +# Default-Start: S 2 3 4 5 # Default-Stop: 0 1 6 # Short-Description: Example initscript # Description: This file should be used to construct scripts to be diff --git a/t/tests/systemd-general/desc b/t/tests/systemd-general/desc index 0bffbeb..f865251 100644 --- a/t/tests/systemd-general/desc +++ b/t/tests/systemd-general/desc @@ -12,3 +12,4 @@ Test-For: systemd-tmpfiles.d-outside-usr-lib systemd-service-file-refers-to-obsolete-target systemd-no-service-for-init-script + systemd-no-service-for-init-rcS-script diff --git a/t/tests/systemd-general/tags b/t/tests/systemd-general/tags index 6f693e3..3dc3b91 100644 --- a/t/tests/systemd-general/tags +++ b/t/tests/systemd-general/tags @@ -4,6 +4,7 @@ E: systemd-general: service-key-has-whitespace etc/systemd/system/test.service a E: systemd-general: service-key-has-whitespace usr/lib/systemd/system/test.service at line 3 E: systemd-general: special-file etc/init.d/fifo-pipe-as-init 0644 E: systemd-general: special-file etc/systemd/system/fifo-pipe-as-init.service 0644 +E: systemd-general: systemd-no-service-for-init-rcS-script systemd-general E: systemd-general: systemd-no-service-for-init-script systemd-general E: systemd-general: systemd-service-file-outside-lib etc/systemd/system/fifo-pipe-as-init.service E: systemd-general: systemd-service-file-outside-lib etc/systemd/system/test.service -- 2.1.4