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

Bug#883742: marked as done (developers-reference: §6.4: Please don't recommend to use "if [ -x /usr/bin/<command> ]; …" in maintainer scripts)

Your message dated Fri, 13 Jul 2018 10:05:01 +0000
with message-id <E1fduwf-000HI0-SO@fasolo.debian.org>
and subject line Bug#883742: fixed in developers-reference 3.4.20
has caused the Debian Bug report #883742,
regarding developers-reference: §6.4: Please don't recommend to use "if [ -x /usr/bin/<command> ]; …" in maintainer scripts
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
883742: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=883742
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: developers-reference
Version: 3.4.19
Severity: normal
Tags: patch

Hi,

§6.4 recommends:
> If you need to check for the existence of a command, you should use
> something like
>
> if [ -x /usr/sbin/install-docs ]; then ...

This has been proven to introduce technical debt (see
https://lists.debian.org/debian-devel/2014/11/msg00044.html and
https://bugs.debian.org/769845) if any of those paths is ever changed
like e.g. moved from /usr/sbin/ to /usr/bin/.

Since #769845 has been fixed, Lintian even emits a warning if such a
case is found.

Bascially there are two cases of "if [ -x /usr/bin/<command> ]; …" in
maintainer scripts:

A) The maintainer controls the checked path (and doesn't forget to
   update the maintainer script in case he ever changes the program's
   path).

B) The maintainer doesn't control the checked path. (See #769845 and
   https://lists.debian.org/debian-devel/2014/11/msg00044.html for real
   examples.)

Overriding this tag in case (A) is ok-ish if the maintainer indeed
doesn't forget to also update the maintainer scripts if he changes the
path.

But case (B) is a real issue and should be fixed and not overridden,
because the maintainer script definitely behaves worse if the wrong path
(e.g. a path only valid in earlier Debian releases) is used in the
check.

So in general, using "if [ -x /usr/bin/<command> ]; …" in maintainer
scripts should be avoided and definitely not recommended.

Leaves the question what should be recommended instead.

According to the discussion in https://bugs.debian.org/807695 the
current recommendation of a 12 lines shell script is unacceptable for
those developers who took part in the discussion and a simple "if which
$command > /dev/null; …" should be used instead.

(Note: Using "type" instead of "which" seems disputed or at least
unclear due to differences between bash and dash. See
https://bugs.debian.org/747320.)

So my suggestion is the following patch:

diff --git a/best-pkging-practices.dbk b/best-pkging-practices.dbk
index 27b5eca..51617ec 100644
--- a/best-pkging-practices.dbk
+++ b/best-pkging-practices.dbk
@@ -659,12 +659,7 @@ maintainer script.
 If you need to check for the existence of a command, you should use something
 like
 </para>
-<programlisting>if [ -x /usr/sbin/install-docs ]; then ...</programlisting>
-<para>
-If you don't wish to hard-code the path of a command in your maintainer script,
-the following POSIX-compliant shell function may help:
-</para>
-&example-pathfind;
+<programlisting>if which install-docs > /dev/null; then ...</programlisting>
 <para>
 You can use this function to search <varname>$PATH</varname> for a command
 name, passed as an argument.  It returns true (zero) if the command was found,
diff --git a/common.ent b/common.ent
index 1bbed6a..66e0e58 100644
--- a/common.ent
+++ b/common.ent
@@ -256,16 +256,3 @@ pool/non-free/f/firmware-nonfree/
 
 uudecode-file:
     perl -ne 'print(unpack &quot;u&quot;, $$_);' $(file).uuencoded &gt; $(file)</programlisting>">
-
-<!ENTITY example-pathfind '<programlisting>pathfind() {
-    OLDIFS="$IFS"
-    IFS=:
-    for p in $PATH; do
-        if [ -x "$p/$*" ]; then
-            IFS="$OLDIFS"
-            return 0
-        fi
-    done
-    IFS="$OLDIFS"
-    return 1
-}</programlisting>'>

I've pushed that patch into a new branch named "devref-vs-#769845" in
the git repository:

https://anonscm.debian.org/cgit/collab-maint/developers-reference.git/log/?h=devref-vs-%23769845

Feel free to continue to work on the fix for this bug in that branch
until there is a consent on merging the branch.

P.S.: I've taken the same severity as used in #769845 since I consider
this bug report equivalent to #769845, just against a different package.

-- System Information:
Debian Release: buster/sid
  APT prefers unstable
  APT policy: (990, 'unstable'), (600, 'testing'), (500, 'unstable-debug'), (500, 'buildd-unstable'), (110, 'experimental'), (1, 'experimental-debug'), (1, 'buildd-experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 4.13.0-rc7-amd64 (SMP w/8 CPU cores)
Locale: LANG=C.UTF-8, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE=C.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)

developers-reference depends on no packages.

Versions of packages developers-reference recommends:
ii  debian-policy  4.1.2.0

Versions of packages developers-reference suggests:
ii  doc-base  0.10.7

-- no debconf information

--- End Message ---
--- Begin Message --- 
Source: developers-reference
Source-Version: 3.4.20

We believe that the bug you reported is fixed in the latest version of
developers-reference, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 883742@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Raphaël Hertzog <hertzog@debian.org> (supplier of updated developers-reference package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Fri, 13 Jul 2018 11:23:40 +0200
Source: developers-reference
Binary: developers-reference developers-reference-de developers-reference-fr developers-reference-ja developers-reference-ru developers-reference-it
Architecture: source
Version: 3.4.20
Distribution: unstable
Urgency: medium
Maintainer: Developers Reference Maintainers <debian-policy@lists.debian.org>
Changed-By: Raphaël Hertzog <hertzog@debian.org>
Description:
 developers-reference - guidelines and information for Debian developers
 developers-reference-de - guidelines and information for Debian developers, in German
 developers-reference-fr - guidelines and information for Debian developers, in French
 developers-reference-it - guidelines and information for Debian developers, in Italian
 developers-reference-ja - guidelines and information for Debian developers, in Japanese
 developers-reference-ru - guidelines and information for Debian developers, in Russian
Closes: 540852 544135 604193 773544 774837 798106 834070 874291 878033 883742 888978 896191
Changes:
 developers-reference (3.4.20) unstable; urgency=medium
 .
   [ Raphaël Hertzog ]
   * Team upload.
   * Reduce the large package tracker section to a few small paragraphs
     pointing to the real documentation hosted on
     https://qa.pages.debian.net/distro-tracker/
   * Apply typo/rewording patch from Paul Hardy. Closes: #878033
   * Update IRC channel list with better examples. Closes: #773544
   * Be more specific in the recommendation to use gender-neutral
     constructions. Thanks to Johannes Schauer <j.schauer@email.de>
     for the initial patch. Closes: #774837
   * Bump the minimal size of the GPG key to 2018 bits and put link
     to keyring.debian.org. Closes: #798106
   * Recommend using "which" to test availability of a command.
     Thanks to Axel Beckert for the patch. Closes: #883742
   * Point out that a non-working email is a policy violation. Improve
     MIA explanations. Closes: #604193, #544135
     Thanks to Caitlin Matos for the patch.
   * Replace some backticks with normal quotes in japanese translation
     to fix a build failure with LaTeX 2018. Closes: #896191
     Thanks to Hilmar Preuße for the patch.
   * Simplify generation of PUBDATE by relying on SOURCE_DATE_EPOCH.
   * Drop Marc 'HE' Brockschmidt <he@debian.org> from Uploaders.
 .
   [ Ansgar Burchardt ]
   * security uploads should go to *.security.upload.d.o (closes: #874291)
 .
   [ Julien Cristau ]
   * Update Vcs-* control fields.
   * Fix upload host name (ssh.upload.debian.org, not ssh.debian.org);
     closes: #888978.  Thanks, Thorsten Alteholz!
 .
   [ Sean Whitton ]
   * Include link to Riseup's "OpenPGP Best Practices" (closes: #834070)
   * Add section "Selecting the upload urgency" to best-pkging-practices
     (Closes: #540852).
 .
   [ Aurélien COUDERC ]
   * Fix CSS text color to avoid the HTML version being unreadable when
     using a light on dark default browser stylesheet.
   * d/control: Bump Standards-Version to 4.1.5, no change needed.
Checksums-Sha1:
 ac0259aeb579d2b642fcadef0564f6b45b85a56c 2091 developers-reference_3.4.20.dsc
 92f88160a7ea832cf57a14d6d289739cca721c27 657112 developers-reference_3.4.20.tar.xz
 36282caa7e00761e861c6544737dcae079c58a4a 6037 developers-reference_3.4.20_source.buildinfo
Checksums-Sha256:
 da7a75f91f9e8a2769483676da6d3a3dcee2423d0dbe550b47076db74993a37b 2091 developers-reference_3.4.20.dsc
 50518eabe0627b301624129da698e8b8688c0300f8cf708a817ddd1a3be7582d 657112 developers-reference_3.4.20.tar.xz
 59921cf0ebfc005821010fb130be36c52578bb46cc9440d4189e724fead4b5ec 6037 developers-reference_3.4.20_source.buildinfo
Files:
 ff251472824f335d82bbbe9b87523edf 2091 doc optional developers-reference_3.4.20.dsc
 aaaa86b0147307f960caeffd5d824a7c 657112 doc optional developers-reference_3.4.20.tar.xz
 ec817eec4c2f50ae1f01f41464db9418 6037 doc optional developers-reference_3.4.20_source.buildinfo

-----BEGIN PGP SIGNATURE-----
Comment: Signed by Raphael Hertzog

iQEzBAEBCgAdFiEE1823g1EQnhJ1LsbSA4gdq+vCmrkFAltIc8YACgkQA4gdq+vC
mrmSMggAov/gFn6QOu1Kl/ASQMKYCRvdmSV0e+3ISLh7K5jrWNL/LpW9BfE7m8Vg
/goPD8VgscZyKylvGGieq9JSJQlZXzs8WeD5yHBpF3IuKVmX7lY9Dia1r2Y11Hhp
0cXrfL1hP985CWcfjHIfi4sosLGMsSj9qbs+Krinq+4Vu7T5ZRt0hoHSPP6oiHfn
m84AJq7HyEUG4WhY9WQfTs4nl2W02izTg+HV4NGMgrw+P7x+h6DBg7xfs/8jAvte
PmEEadqxlxbHrIydLTd4c8ygOELhtFRFX7P5u2MpOajxapO9E61hKc0JeGMW9ZUu
YdN959zjqEL5/FuxLJMYNpjm0HEOVQ==
=w4G9
-----END PGP SIGNATURE-----

--- End Message ---
Reply to: