On Thu, 13 Oct 2016 09:36:33 +0200 Mathieu Parent <math.par...@gmail.com> wrote:
> 2016-10-12 23:01 GMT+02:00 Antonio Ospite <a...@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 <a...@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.par...@nantesmetropole.fr> Signed-off-by: Antonio Ospite <a...@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 <a...@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 <a...@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