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 "u", $$_);' $(file).uuencoded > $(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
