[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 Thu, 13 Oct 2016 09:36:33 +0200
Mathieu Parent <math.parent@gmail.com> wrote:

> 2016-10-12 23:01 GMT+02:00 Antonio Ospite <ao2@ao2.it>:
[...]
> > I think a variant of this should really go in before the next stable
> > debian release.
> 
> OK. Feel free to improve it and submit a new patch.
> 
> I won't work more on this. My time is sparse.
> 

Hi,

I am attaching a revised patchset to address this bug.

Patch 0001 is based on Mathieu's work, but I think it follows more
strictly what Ondřej had in mind: the error tells the user to depend on
php-cli when a package contains a php script. The error triggers also
when a phpX-cli dep is used.

Patch 0002 adds a warning about using an unusual php interpreter in
scripts shebang lines (e.g. a versioned interpreter like
/usr/bin/php7.0). IIRC Ondřej suggested that too in a previous
conversation, it may have been on pkg-php-maint, I don't have the text
handy but anyhow I think the warning makes sense.

Unrelated: while looking at the code I noticed that the "tag" subroutine
in checks/scripts.pm is called sometimes with parentheses and sometimes
not, it's not a big deal so I am not sending a patch to fix the mixed
style but I wanted at lest to mention it n case lintian devs wanted to
fix that themselves.

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 97e3a3a3eea534c25842ed61e8ab3950028b9138 Mon Sep 17 00:00:00 2001
From: Antonio Ospite <ao2@ao2.it>
Date: Thu, 3 Mar 2016 10:50:36 +0100
Subject: [PATCHv2 1/2] Give error for packages shipping php scripts but not
 depending on php-cli
X-Face: z*RaLf`X<@C75u6Ig9}{oW$H;1_\2t5)({*|jhM<pyWR#k60!#=#>/Vb;]yA5<GWI5`6u&+
 ;6b'@y|8w"wB;4/e!7wYYrcqdJFY,~%Gk_4]cq$Ei/7<j&N3ah(m`ku?pX.&+~:_/wC~dwn^)MizBG
 !pE^+iDQQ1yC6^,)YDKkxDd!T>\I~93>J<_`<4)A{':UrE

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

  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.

Triggering the lintian error when  the php-cli dependency is missing,
also covers the case when a versioned phpX-cli dependency is used; 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.

So, remove php from the interpreters following versioned dependencies.

This also demote the case of when the interpreter uses a version number
in the interpreter to a "unusual-interpreter" warning.

The latter can be made later into a more specific warning suggesting the
users to also use unversioned interpreters in the shebang lines.

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                            | 10 +++++-----
 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                  |  4 ++--
 t/tests/scripts-missing-dep/desc             |  2 +-
 t/tests/scripts-missing-dep/tags             |  2 +-
 9 files changed, 16 insertions(+), 24 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..bc3e51f 100644
--- a/checks/scripts.pm
+++ b/checks/scripts.pm
@@ -432,7 +432,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 +461,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",
@@ -474,7 +474,7 @@ sub run {
             $depends =~ s/\$1/$version/g;
             unless ($all_parsed->implies($depends)) {
                 if ($base =~ /^php/) {
-                    tag 'php-script-but-no-phpX-cli-dep', $filename;
+                    tag 'php-script-but-no-php-cli-dep', $filename;
                 } elsif ($base =~ /^(python|ruby)/) {
                     tag "$1-script-but-no-$1-dep", $filename;
                 } else {
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..3856438 100644
--- a/t/tests/legacy-scripts/tags
+++ b/t/tests/legacy-scripts/tags
@@ -14,8 +14,7 @@ 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/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 +91,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 ad0585ae1c8deb2ad69e7a0484cfca610173e65f Mon Sep 17 00:00:00 2001
From: Antonio Ospite <ao2@ao2.it>
Date: Mon, 31 Oct 2016 23:10:05 +0100
Subject: [PATCHv2 2/2] Add a new php-script-with-unusual-interpreter check
X-Face: z*RaLf`X<@C75u6Ig9}{oW$H;1_\2t5)({*|jhM<pyWR#k60!#=#>/Vb;]yA5<GWI5`6u&+
 ;6b'@y|8w"wB;4/e!7wYYrcqdJFY,~%Gk_4]cq$Ei/7<j&N3ah(m`ku?pX.&+~:_/wC~dwn^)MizBG
 !pE^+iDQQ1yC6^,)YDKkxDd!T>\I~93>J<_`<4)A{':UrE

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                          | 2 ++
 t/tests/legacy-scripts/debian/debian/rules | 4 ++++
 t/tests/legacy-scripts/tags                | 4 +++-
 t/tests/legacy-scripts/upstream/phpenvfoo  | 7 +++++++
 5 files changed, 23 insertions(+), 1 deletion(-)
 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 bc3e51f..25f56cf 100644
--- a/checks/scripts.pm
+++ b/checks/scripts.pm
@@ -373,6 +373,8 @@ 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");
         } else {
             my $pinter = 0;
             if ($interpreter =~ m,^/,) {
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 3856438..1f814ac 100644
--- a/t/tests/legacy-scripts/tags
+++ b/t/tests/legacy-scripts/tags
@@ -14,6 +14,7 @@ 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/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
@@ -86,9 +87,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: