Bug#790323: lintian: warn when init.d script for rcS does not have a native systemd unit
On 2015-06-30 05:40, Felipe Sateler wrote: [...] I forgot this part. Attached a new patchset applying perltidy on each commit. Thanks, I have applied all four patches with only 3 minor modifications: * 0001 + 0004: Fix a use of low + high precedence boolean operators - Example: x and !y = x and not y (or x !y) - Found by perlcritic - Test suite: t/runtests -k t debian/test-out 01-critic/checks * 0004: Summary / Synopsis message shorten as it was rather long. - Here I just dropped the checks/systemd prefix Once again, thanks for the patches, :) ~Niels -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#790323: lintian: warn when init.d script for rcS does not have a native systemd unit
On 30 June 2015 at 17:40, Niels Thykier ni...@thykier.net wrote: On 2015-06-30 05:40, Felipe Sateler wrote: [...] I forgot this part. Attached a new patchset applying perltidy on each commit. Thanks, I have applied all four patches with only 3 minor modifications: * 0001 + 0004: Fix a use of low + high precedence boolean operators - Example: x and !y = x and not y (or x !y) - Found by perlcritic - Test suite: t/runtests -k t debian/test-out 01-critic/checks * 0004: Summary / Synopsis message shorten as it was rather long. - Here I just dropped the checks/systemd prefix Once again, thanks for the patches, :) Thanks to you for maintaining lintian, and guiding me through the process. -- Saludos, Felipe Sateler -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#790323: lintian: warn when init.d script for rcS does not have a native systemd unit
Hi Niels,
On 28 June 2015 at 08:50, Niels Thykier ni...@thykier.net wrote:
On 2015-06-28 03:09, Felipe Sateler wrote:
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.
Hi,
Thanks for working on this. I have a couple of remarks to some of the
patches (interleaved into the patches below).
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.
Ok. :)
[...]
0001-Reorder-systemd-checks.patch
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
[...]
+sub get_systemd_service_files {
+my $info = shift @_;
Please use the my ($info) = @_; notation instead.
The use of shift is generally discouraged throughout the lintian code
base (I think we
... the suspense is killing me ;). Removed all references to shift.
+
+return grep { m,/systemd/system/.*\.service$, } $info-sorted_index;
+}
+
+sub get_systemd_service_names {
+my ($info) = @_;
+my %services;
+
+[...]
+return %services;
+}
Please consider returning this as a ref. Since it is passed around, we
end up copying it several times. Admittedly, I suspect it is a small
hash, I am mostly in it for being consistent with similar usage for
larger hashes.
Quick cheat-sheet
return \%services;
$services{foo} = $services-{foo}
if (%foo) = if (%{$foo})
Thanks, I got quite dizzy trying to understand references. Fixed this
+
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/,);
Note this will match non-files (including the etc/systemd/system
directory). If the systemd package ships that as an empty dir (or
containing a README).
Though presumably something already filters this out before we get that far?
Yes, this sub is only called for files returned by get_systemd_service_files.
+
An empty whitespace line - please apply perltidy -it=4 -b
checks/systemd.pm (it will possibly reformat other things as well).
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 {
[...]
0002-Check-files-as-we-detect-them-and-discard-invalid-fi.patch
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
\o/
---
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
[...]
@@ -124,12 +120,18 @@ sub check_init_script {
[...]
sub get_systemd_service_names {
-my ($info) = @_;
+my $info = shift @_;
+my @files = @_;
Again, please prefer:
my ($info, @files) = @_;
Alternatively, consider making @files a ref. Again, not a high priority
since I doubt any package will ever ship a significant amount of
service files.
If you do, the cheat sheet is:
get_systemd_service_names($info, @files) =
get_systemd_service_names($info, \@files)
my ($info, $files_ref) = @_;
@files = @{$files_ref}
Done as a ref.
my %services;
my $safe_add_service = sub {
@@ -141,7 +143,7 @@ sub get_systemd_service_names {
[...]
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
[...]
\o/
I hope the new attached version is
Bug#790323: lintian: warn when init.d script for rcS does not have a native systemd unit
On 29 June 2015 at 15:39, Felipe Sateler fsate...@debian.org wrote:
Hi Niels,
On 28 June 2015 at 08:50, Niels Thykier ni...@thykier.net wrote:
On 2015-06-28 03:09, Felipe Sateler wrote:
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.
Hi,
Thanks for working on this. I have a couple of remarks to some of the
patches (interleaved into the patches below).
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.
Ok. :)
[...]
0001-Reorder-systemd-checks.patch
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
[...]
+sub get_systemd_service_files {
+my $info = shift @_;
Please use the my ($info) = @_; notation instead.
The use of shift is generally discouraged throughout the lintian code
base (I think we
... the suspense is killing me ;). Removed all references to shift.
But not all in the first commit :/. New version fixes that.
+
+return grep { m,/systemd/system/.*\.service$, } $info-sorted_index;
+}
+
+sub get_systemd_service_names {
+my ($info) = @_;
+my %services;
+
+[...]
+return %services;
+}
Please consider returning this as a ref. Since it is passed around, we
end up copying it several times. Admittedly, I suspect it is a small
hash, I am mostly in it for being consistent with similar usage for
larger hashes.
Quick cheat-sheet
return \%services;
$services{foo} = $services-{foo}
if (%foo) = if (%{$foo})
Thanks, I got quite dizzy trying to understand references. Fixed this
+
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/,);
Note this will match non-files (including the etc/systemd/system
directory). If the systemd package ships that as an empty dir (or
containing a README).
Though presumably something already filters this out before we get that far?
Yes, this sub is only called for files returned by get_systemd_service_files.
+
An empty whitespace line - please apply perltidy -it=4 -b
checks/systemd.pm (it will possibly reformat other things as well).
I forgot this part. Attached a new patchset applying perltidy on each commit.
--
Saludos,
Felipe Sateler
From bcea42d3db13d7f5f24405a2fd37e5d0d2579fd9 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 | 138 ++
1 file changed, 78 insertions(+), 60 deletions(-)
diff --git a/checks/systemd.pm b/checks/systemd.pm
index d36cf65..5815827 100644
--- a/checks/systemd.pm
+++ b/checks/systemd.pm
@@ -37,81 +37,64 @@ 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$,) {
-
Bug#790323: lintian: warn when init.d script for rcS does not have a native systemd unit
On 2015-06-28 03:09, Felipe Sateler wrote:
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.
Hi,
Thanks for working on this. I have a couple of remarks to some of the
patches (interleaved into the patches below).
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.
Ok. :)
[...]
0001-Reorder-systemd-checks.patch
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
[...]
+sub get_systemd_service_files {
+my $info = shift @_;
Please use the my ($info) = @_; notation instead.
The use of shift is generally discouraged throughout the lintian code
base (I think we
+
+return grep { m,/systemd/system/.*\.service$, } $info-sorted_index;
+}
+
+sub get_systemd_service_names {
+my ($info) = @_;
+my %services;
+
+[...]
+return %services;
+}
Please consider returning this as a ref. Since it is passed around, we
end up copying it several times. Admittedly, I suspect it is a small
hash, I am mostly in it for being consistent with similar usage for
larger hashes.
Quick cheat-sheet
return \%services;
$services{foo} = $services-{foo}
if (%foo) = if (%{$foo})
+
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/,);
Note this will match non-files (including the etc/systemd/system
directory). If the systemd package ships that as an empty dir (or
containing a README).
Though presumably something already filters this out before we get that far?
+
An empty whitespace line - please apply perltidy -it=4 -b
checks/systemd.pm (it will possibly reformat other things as well).
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 {
[...]
0002-Check-files-as-we-detect-them-and-discard-invalid-fi.patch
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
\o/
---
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
[...]
@@ -124,12 +120,18 @@ sub check_init_script {
[...]
sub get_systemd_service_names {
-my ($info) = @_;
+my $info = shift @_;
+my @files = @_;
Again, please prefer:
my ($info, @files) = @_;
Alternatively, consider making @files a ref. Again, not a high priority
since I doubt any package will ever ship a significant amount of
service files.
If you do, the cheat sheet is:
get_systemd_service_names($info, @files) =
get_systemd_service_names($info, \@files)
my ($info, $files_ref) = @_;
@files = @{$files_ref}
my %services;
my $safe_add_service = sub {
@@ -141,7 +143,7 @@ sub get_systemd_service_names {
[...]
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
[...]
\o/
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
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 gettext0.19.4-1
ii hardening-includes 2.7
ii intltool-debian0.35.0+20060710.2
ii libapt-pkg-perl0.1.29+b2
ii libarchive-zip-perl1.48-1
ii libclass-accessor-perl 0.34-1
ii libclone-perl 0.38-1
ii libdpkg-perl 1.18.1
ii libemail-valid-perl1.195-1
ii libfile-basedir-perl 0.07-1
ii libipc-run-perl0.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-perl1.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 t1utils1.38-4
ii xz-utils 5.1.1alpha+20120614-2+b3
Versions of packages lintian recommends:
ii dpkg1.18.1
ii libautodie-perl 2.27-2
ii libperlio-gzip-perl 0.18-3+b1
ii perl5.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-perl3.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
