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

Bug#818962: fix the php-script-but-no-phpX-cli-dep error



On Tue, 1 Nov 2016 10:51:41 +0100
Antonio Ospite <ao2@ao2.it> wrote:

> On Tue, 1 Nov 2016 00:10:15 +0100
> Antonio Ospite <ao2@ao2.it> wrote:
[...]
> [...]
> >
> > This also demote the case of when the interpreter uses a version number
> > in the interpreter to a "unusual-interpreter" warning.
> > 
> 
> After thinking a little more about it, this still isn't right, we
> should get the php-script-but-no-php-cli-dep error also when the
> shebang line has an "unusual" interpreter, like /usr/bin/php7.0.
> 

OK, the updated patches are attached.

The changes since v1 are:
 - the dependency checks are now triggered also when an unusual
   interpreter is found

Ondřej do you have any comment?

Thanks,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
>From 4a5a744c15efc15bdbd6f1baab510fd3811c80f0 Mon Sep 17 00:00:00 2001
From: Antonio Ospite <ao2@ao2.it>
Date: Thu, 3 Nov 2016 22:51:08 +0100
Subject: [PATCHv2 1/2] Give error for packages shipping php scripts but not
 depending on php-cli

The PHP maintainers recommend to have PHP dependencies without
specifying the PHP version explicitly, e.g.:

  Depends: php-cli, php-curl, php-xsl

instead of:

  Depends: php7.0-cli, php7.0-curl, php7.0-xsl

This is a safe thing to do considering that each Debian stable version
is going to ship with only one major version of PHP.

However in the case of packages shipping php scripts, lintian now
wrongly suggests to depend on a versioned phpX-cli package.

Fix this by triggering the lintian error when the package ships a script
and but does not depend on the unversioned php-cli dependency.

This also covers the deprecated case of using a versioned phpX-cli
dependency. To confirm that, a dependency to php7.0-cli has been added
to t/tests/legacy-scripts/debian/debian/control and it has been verified
that the package still gives the error.

Moving php from the "versioned interpreters" to the unversioned
interpreters also results in an "unusual-interpreter" warning when, for
example, an interpreter with a version number in the filename is used.

In a future commit the "unusual-interpreter" warning will be turned into
a more specific warning suggesting to use the unversioned php
interpreter in the shebang line.

The changes pass these tests:
  debian/rules runtests onlyrun=scripts-missing-dep
  debian/rules runtests onlyrun=legacy-scripts

Closes: #818962
Thanks: Mathieu Parent <Mathieu.PARENT@nantesmetropole.fr>

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 checks/scripts.desc                          | 14 +++-----------
 checks/scripts.pm                            | 18 +++++++++++-------
 data/scripts/interpreters                    |  1 +
 data/scripts/versioned-interpreters          |  1 -
 t/tests/legacy-scripts/debian/debian/control |  2 +-
 t/tests/legacy-scripts/debian/debian/rules   |  4 ++--
 t/tests/legacy-scripts/tags                  |  5 +++--
 t/tests/scripts-missing-dep/desc             |  2 +-
 t/tests/scripts-missing-dep/tags             |  2 +-
 9 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/checks/scripts.desc b/checks/scripts.desc
index 12a642b..d2c14b8 100644
--- a/checks/scripts.desc
+++ b/checks/scripts.desc
@@ -217,24 +217,16 @@ Info: Packages that use mawk scripts must depend on the mawk package.
  In some cases a weaker relationship, such as Suggests or Recommends, will
  be more appropriate.
 
-Tag: php-script-but-no-phpX-cli-dep
+Tag: php-script-but-no-php-cli-dep
 Severity: important
 Certainty: certain
-Info: Packages with PHP scripts must depend on a phpX-cli package such as
- php5-cli.  Note that a dependency on a php-cgi package (such as php5-cgi)
+Info: Packages with PHP scripts must depend on the php-cli package.
+ Note that a dependency on a php-cgi package (such as php-cgi or php7.0-cgi)
  is needlessly strict and forces the user to install a package that isn't
  needed.
  .
  In some cases a weaker relationship, such as Suggests or Recommends, will
  be more appropriate.
- .
- Lintian can only recognize phpX-cli dependencies for values of X that it
- knows are available in the archive.  You will get this warning if you
- allow, as alternatives, versions of PHP that are so old they're not
- available in stable.  The correct fix in those cases is probably to drop
- the old alternative.  If this package depends on a newer php-cli package
- that Lintian doesn't know about, please file a bug against Lintian so
- that it can be updated.
 
 Tag: python-script-but-no-python-dep
 Severity: important
diff --git a/checks/scripts.pm b/checks/scripts.pm
index f40fddd..8dd0e3c 100644
--- a/checks/scripts.pm
+++ b/checks/scripts.pm
@@ -390,6 +390,12 @@ sub run {
             }
             script_tag('unusual-interpreter', $filename, "#!$interpreter")
               unless $pinter;
+
+            # This allows to still perform the dependencies checks below even
+            # when an unusual interpreter has been found.
+            if ($base =~ /^php/) {
+                $data = $INTERPRETERS->value('php');
+            }
         }
 
         # Check for obsolete perl libraries
@@ -432,7 +438,9 @@ sub run {
                 $depends = $base;
             }
             if ($depends && !$all_parsed->implies($depends)) {
-                if ($base =~ /^(python|ruby|[mg]awk)$/) {
+                if ($base =~ /^php/) {
+                    tag 'php-script-but-no-php-cli-dep', $filename;
+                } elsif ($base =~ /^(python|ruby|[mg]awk)$/) {
                     tag("$base-script-but-no-$base-dep", $filename);
                 } elsif ($base eq 'csh' && $filename =~ m,^etc/csh/login\.d/,){
                     # Initialization files for csh.
@@ -459,9 +467,7 @@ sub run {
             unshift(@depends, $data->[1]) if length $data->[1];
             my $depends = join(' | ',  @depends);
             unless ($all_parsed->implies($depends)) {
-                if ($base eq 'php') {
-                    tag 'php-script-but-no-phpX-cli-dep', $filename;
-                } elsif ($base =~ /^(wish|tclsh)/) {
+                if ($base =~ /^(wish|tclsh)/) {
                     tag "$1-script-but-no-$1-dep", $filename;
                 } else {
                     tag 'missing-dep-for-interpreter', "$base => $depends",
@@ -473,9 +479,7 @@ sub run {
             my $depends = $data->[3];
             $depends =~ s/\$1/$version/g;
             unless ($all_parsed->implies($depends)) {
-                if ($base =~ /^php/) {
-                    tag 'php-script-but-no-phpX-cli-dep', $filename;
-                } elsif ($base =~ /^(python|ruby)/) {
+                if ($base =~ /^(python|ruby)/) {
                     tag "$1-script-but-no-$1-dep", $filename;
                 } else {
                     tag 'missing-dep-for-interpreter', "$base => $depends",
diff --git a/data/scripts/interpreters b/data/scripts/interpreters
index 9b02c15..81100d2 100644
--- a/data/scripts/interpreters
+++ b/data/scripts/interpreters
@@ -68,6 +68,7 @@ parrot         => /usr/bin
 perl           => /usr/bin, @NODEPS@
 perl6          => /usr/bin, rakudo
 perl6-m        => /usr/bin, rakudo
+php            => /usr/bin, php-cli
 plackup        => /usr/bin, libplack-perl
 procmail       => /usr/bin
 pypy           => /usr/bin
diff --git a/data/scripts/versioned-interpreters b/data/scripts/versioned-interpreters
index fff44c2..7e14059 100644
--- a/data/scripts/versioned-interpreters
+++ b/data/scripts/versioned-interpreters
@@ -73,7 +73,6 @@ guile   => /usr/bin, guile-([\d.]+), guile-$1, 1.6 1.8,
 jruby   => /usr/bin, jruby([\d.]+), jruby$1, 1.0 1.1 1.2
 lua     => /usr/bin, lua([\d.]+), lua$1, 40 50 5.1 5.2
 octave  => /usr/bin, octave([\d.]+), octave$1, 3.0 3.2
-php     => /usr/bin, php(\d+), php$1-cli, 5, @NO_DEFAULT_DEPS@
 pike    => /usr/bin, pike([\d.]+), pike$1 | pike$1-core, 7.6 7.8, @NO_DEFAULT_DEPS@
 python  => /usr/bin, python([\d.]+), python$1:any | python$1-minimal:any, 2.7, @SKIP_UNVERSIONED@
 ruby    => /usr/bin, ruby([\d.]+), ruby$1, 1.8 1.9, @SKIP_UNVERSIONED@
diff --git a/t/tests/legacy-scripts/debian/debian/control b/t/tests/legacy-scripts/debian/debian/control
index 07e2ea2..0b44570 100644
--- a/t/tests/legacy-scripts/debian/debian/control
+++ b/t/tests/legacy-scripts/debian/debian/control
@@ -8,7 +8,7 @@ Standards-Version: 3.2.1
 
 Package: scripts
 Architecture: all
-Depends: test, ruby1.8, build-essential, libssl0.9.7
+Depends: test, ruby1.8, build-essential, libssl0.9.7, php7.0-cli
 Recommends: tk8.4 | wish
 Description: test lintian's script file checks
  This is a test package designed to exercise some feature or tag of
diff --git a/t/tests/legacy-scripts/debian/debian/rules b/t/tests/legacy-scripts/debian/debian/rules
index 25b6f9e..a615bd6 100755
--- a/t/tests/legacy-scripts/debian/debian/rules
+++ b/t/tests/legacy-scripts/debian/debian/rules
@@ -63,8 +63,8 @@ binary-indep:
 	install -m 755 init-lsb-other $(tmp)/etc/init.d/lsb-other
 
 	install -m 755 phpfoo $(tmp)/usr/share/scripts/
-	sed 's/php$$/php5/' phpfoo > $(tmp)/usr/share/scripts/php5foo
-	chmod 755 $(tmp)/usr/share/scripts/php5foo
+	sed 's/php$$/php7.0/' phpfoo > $(tmp)/usr/share/scripts/php7.0foo
+	chmod 755 $(tmp)/usr/share/scripts/php7.0foo
 
 	echo "#!/usr/bin/perl" >> $(tmp)/usr/share/scripts/foobar.in
 	chmod 644 $(tmp)/usr/share/scripts/foobar.in
diff --git a/t/tests/legacy-scripts/tags b/t/tests/legacy-scripts/tags
index 0436902..365ab09 100644
--- a/t/tests/legacy-scripts/tags
+++ b/t/tests/legacy-scripts/tags
@@ -14,8 +14,8 @@ E: scripts: init.d-script-needs-depends-on-lsb-base etc/init.d/skeleton (line 40
 E: scripts: missing-dep-for-interpreter jruby => jruby | jruby1.0 | jruby1.1 | jruby1.2 (usr/bin/jruby-broken)
 E: scripts: missing-dep-for-interpreter lefty => graphviz (usr/bin/lefty-foo)
 E: scripts: package-installs-python-bytecode usr/lib/python2.3/site-packages/test.pyc
-E: scripts: php-script-but-no-phpX-cli-dep usr/share/scripts/php5foo
-E: scripts: php-script-but-no-phpX-cli-dep usr/share/scripts/phpfoo
+E: scripts: php-script-but-no-php-cli-dep usr/share/scripts/php7.0foo
+E: scripts: php-script-but-no-php-cli-dep usr/share/scripts/phpfoo
 E: scripts: python-script-but-no-python-dep usr/bin/py2foo
 E: scripts: python-script-but-no-python-dep usr/bin/pyfoo
 E: scripts: shell-script-fails-syntax-check usr/bin/sh-broken
@@ -92,3 +92,4 @@ W: scripts: script-with-language-extension usr/bin/test.sh
 W: scripts: setuid-binary usr/bin/suidperlfoo 4555 root/root
 W: scripts: setuid-binary usr/bin/suidperlfoo2 4751 root/root
 W: scripts: unusual-interpreter usr/bin/suidperlfoo #!/usr/bin/suidperl
+W: scripts: unusual-interpreter usr/share/scripts/php7.0foo #!/usr/bin/php7.0
diff --git a/t/tests/scripts-missing-dep/desc b/t/tests/scripts-missing-dep/desc
index c5cad4e..be0c491 100644
--- a/t/tests/scripts-missing-dep/desc
+++ b/t/tests/scripts-missing-dep/desc
@@ -7,6 +7,6 @@ Test-For: wish-script-but-no-wish-dep
  maintainer-script-needs-depends-on-adduser
  maintainer-script-needs-depends-on-update-inetd
  mawk-script-but-no-mawk-dep
- php-script-but-no-phpX-cli-dep
+ php-script-but-no-php-cli-dep
  python-script-but-no-python-dep
  tclsh-script-but-no-tclsh-dep
diff --git a/t/tests/scripts-missing-dep/tags b/t/tests/scripts-missing-dep/tags
index 4585ed7..6b9773a 100644
--- a/t/tests/scripts-missing-dep/tags
+++ b/t/tests/scripts-missing-dep/tags
@@ -1,6 +1,6 @@
 E: scripts-missing-dep: gawk-script-but-no-gawk-dep usr/bin/gawk-script
 E: scripts-missing-dep: mawk-script-but-no-mawk-dep usr/bin/mawk-script
-E: scripts-missing-dep: php-script-but-no-phpX-cli-dep usr/bin/php-script
+E: scripts-missing-dep: php-script-but-no-php-cli-dep usr/bin/php-script
 E: scripts-missing-dep: python-script-but-no-python-dep usr/bin/python-script
 E: scripts-missing-dep: ruby-script-but-no-ruby-dep usr/bin/ruby-script
 E: scripts-missing-dep: tclsh-script-but-no-tclsh-dep usr/bin/tclsh-script
-- 
2.10.2

>From 93b4ba4e797c740b897aa491f0e23e7577783bbe Mon Sep 17 00:00:00 2001
From: Antonio Ospite <ao2@ao2.it>
Date: Thu, 3 Nov 2016 22:51:23 +0100
Subject: [PATCHv2 2/2] Add a new php-script-with-unusual-interpreter check

The PHP maintainers suggest to use unversioned interpreters in the
shebang line of scripts, add a check for that which allows either
"#!/usr/bin/php" or "#!/usr/bin/env php" as shebang lines.

The changes pass these tests:
  debian/rules runtests onlyrun=scripts-missing-dep
  debian/rules runtests onlyrun=legacy-scripts

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 checks/scripts.desc                        |  7 +++++++
 checks/scripts.pm                          | 12 ++++++------
 t/tests/legacy-scripts/debian/debian/rules |  4 ++++
 t/tests/legacy-scripts/tags                |  5 ++++-
 t/tests/legacy-scripts/upstream/phpenvfoo  |  7 +++++++
 5 files changed, 28 insertions(+), 7 deletions(-)
 create mode 100644 t/tests/legacy-scripts/upstream/phpenvfoo

diff --git a/checks/scripts.desc b/checks/scripts.desc
index d2c14b8..db34239 100644
--- a/checks/scripts.desc
+++ b/checks/scripts.desc
@@ -71,6 +71,13 @@ Info: This package contains an example script for an interpreter that
  If not, please file a wishlist bug against lintian, so it can be
  added to the list of known interpreters.
 
+Tag: php-script-with-unusual-interpreter
+Severity: normal
+Certainty: possible
+Info: This package contains a php script using an unusual interpreter in the
+ shebang line.  The recommended shebang line is either <tt>#!/usr/bin/php</tt>
+ or <tt>#!/usr/bin/env php</tt> without any version number.
+
 Tag: script-uses-bin-env
 Severity: normal
 Certainty: certain
diff --git a/checks/scripts.pm b/checks/scripts.pm
index 8dd0e3c..dc9bb58 100644
--- a/checks/scripts.pm
+++ b/checks/scripts.pm
@@ -373,6 +373,12 @@ sub run {
             script_tag('interpreter-in-usr-local', $filename,"#!$interpreter");
         } elsif ($interpreter eq '/bin/env') {
             script_tag('script-uses-bin-env', $filename);
+        } elsif ($base =~ /^php/) {
+            script_tag('php-script-with-unusual-interpreter', $filename, "$interpreter");
+
+            # This allows to still perform the dependencies checks below even
+            # when an unusual interpreter has been found.
+            $data = $INTERPRETERS->value('php');
         } else {
             my $pinter = 0;
             if ($interpreter =~ m,^/,) {
@@ -390,12 +396,6 @@ sub run {
             }
             script_tag('unusual-interpreter', $filename, "#!$interpreter")
               unless $pinter;
-
-            # This allows to still perform the dependencies checks below even
-            # when an unusual interpreter has been found.
-            if ($base =~ /^php/) {
-                $data = $INTERPRETERS->value('php');
-            }
         }
 
         # Check for obsolete perl libraries
diff --git a/t/tests/legacy-scripts/debian/debian/rules b/t/tests/legacy-scripts/debian/debian/rules
index a615bd6..3283726 100755
--- a/t/tests/legacy-scripts/debian/debian/rules
+++ b/t/tests/legacy-scripts/debian/debian/rules
@@ -66,6 +66,10 @@ binary-indep:
 	sed 's/php$$/php7.0/' phpfoo > $(tmp)/usr/share/scripts/php7.0foo
 	chmod 755 $(tmp)/usr/share/scripts/php7.0foo
 
+	install -m 755 phpenvfoo $(tmp)/usr/share/scripts/
+	sed 's/php$$/php7.0/' phpenvfoo > $(tmp)/usr/share/scripts/php7.0envfoo
+	chmod 755 $(tmp)/usr/share/scripts/php7.0envfoo
+
 	echo "#!/usr/bin/perl" >> $(tmp)/usr/share/scripts/foobar.in
 	chmod 644 $(tmp)/usr/share/scripts/foobar.in
 
diff --git a/t/tests/legacy-scripts/tags b/t/tests/legacy-scripts/tags
index 365ab09..ae8a896 100644
--- a/t/tests/legacy-scripts/tags
+++ b/t/tests/legacy-scripts/tags
@@ -14,7 +14,9 @@ E: scripts: init.d-script-needs-depends-on-lsb-base etc/init.d/skeleton (line 40
 E: scripts: missing-dep-for-interpreter jruby => jruby | jruby1.0 | jruby1.1 | jruby1.2 (usr/bin/jruby-broken)
 E: scripts: missing-dep-for-interpreter lefty => graphviz (usr/bin/lefty-foo)
 E: scripts: package-installs-python-bytecode usr/lib/python2.3/site-packages/test.pyc
+E: scripts: php-script-but-no-php-cli-dep usr/share/scripts/php7.0envfoo
 E: scripts: php-script-but-no-php-cli-dep usr/share/scripts/php7.0foo
+E: scripts: php-script-but-no-php-cli-dep usr/share/scripts/phpenvfoo
 E: scripts: php-script-but-no-php-cli-dep usr/share/scripts/phpfoo
 E: scripts: python-script-but-no-python-dep usr/bin/py2foo
 E: scripts: python-script-but-no-python-dep usr/bin/pyfoo
@@ -87,9 +89,10 @@ W: scripts: maintainer-script-has-unexpanded-debhelper-token preinst
 W: scripts: maintainer-script-ignores-errors postinst
 W: scripts: non-standard-executable-perm usr/bin/perl-bizarre-3 0754 != 0755
 W: scripts: non-standard-setuid-executable-perm usr/bin/suidperlfoo 4555
+W: scripts: php-script-with-unusual-interpreter usr/share/scripts/php7.0envfoo php7.0
+W: scripts: php-script-with-unusual-interpreter usr/share/scripts/php7.0foo /usr/bin/php7.0
 W: scripts: script-uses-bin-env usr/bin/envfoo
 W: scripts: script-with-language-extension usr/bin/test.sh
 W: scripts: setuid-binary usr/bin/suidperlfoo 4555 root/root
 W: scripts: setuid-binary usr/bin/suidperlfoo2 4751 root/root
 W: scripts: unusual-interpreter usr/bin/suidperlfoo #!/usr/bin/suidperl
-W: scripts: unusual-interpreter usr/share/scripts/php7.0foo #!/usr/bin/php7.0
diff --git a/t/tests/legacy-scripts/upstream/phpenvfoo b/t/tests/legacy-scripts/upstream/phpenvfoo
new file mode 100644
index 0000000..cbbfb2e
--- /dev/null
+++ b/t/tests/legacy-scripts/upstream/phpenvfoo
@@ -0,0 +1,7 @@
+#!/usr/bin/env php
+<html>
+<head>
+<title>Dumb PHP script</title>
+</head>
+<body><? print(Date("l F d, Y")); ?></body>
+</html>
-- 
2.10.2


Reply to: