[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#790323: lintian: warn when init.d script for rcS does not have a native systemd unit



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 <fsateler@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 <fsateler@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 <fsateler@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 <fsateler@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


Reply to: