threads / patch / 29762

v2, 2 partsCodingGuidelines: Add note forbidding use of 'which' in shell scripts

Subject: [PATCH v2 2/2] CodingGuidelines: Add note forbidding use of 'which' in shell scripts

## tl;dr

3 messages between Feb 24, 2012 and Feb 27, 2012. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Tim Henigan· Feb 24, 2012, 23:12 UTC · lore

[PATCH v2 1/2] CodingGuidelines: Add a note about spaces after redirection

During code review of some patches, it was noted that redirection operators should have space before, but no space after them.

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---

Updated to include double-quotes around redirection target and also document why they are needed.

 Documentation/CodingGuidelines |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
Show changes to Documentation/CodingGuidelines +10 −0
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 4830086..5a190b9 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -35,6 +35,16 @@ For shell scripts specifically (not exhaustive):
 
  - Case arms are indented at the same depth as case and esac lines.
 
+ - Redirection operators should be written with space before, but
+   no space after them.  For example:
+      'echo test >"$file"'  is preferred over
+      'echo test > "$file"'
+
+   Note that even though it is not required by POSIX to double-
+   quote the redirection target in a variable (as shown above),
+   our code does so because some versions of bash issue a warning
+   without them.
+
  - We prefer $( ... ) for command substitution; unlike ``, it
    properly nests.  It should have been the way Bourne spelled
    it from day one, but unfortunately isn't.

-- 
1.7.9.1
Tim Henigan· Feb 24, 2012, 23:12 UTC · re: Tim Henigan · lore

During the code review of a recent patch, it was noted that shell scripts must not use 'which'. The output of the command is not machine parseable and its exit code is not reliable across platforms.

It is better to use 'type' to accomplish this task.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---

Updated to the documentation pattern recommended by Junio Hamano: "If you want to do Z, use X not Y, because Y is broken ..."

I grepped through the code and found the 'type <command' is indeed used in place of 'which' to test for the presence of commands on $PATH.

 Documentation/CodingGuidelines |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Show changes to Documentation/CodingGuidelines +5 −0
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 5a190b9..816c5ad 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -49,6 +49,11 @@ For shell scripts specifically (not exhaustive):
    properly nests.  It should have been the way Bourne spelled
    it from day one, but unfortunately isn't.
 
+ - If you want to find out if a command is available on the user's
+   $PATH, you should use 'type <command>', instead of 'which'.
+   The output of 'which' is not machine parseable and its exit code
+   is not reliable across platforms.
+
  - We use POSIX compliant parameter substitutions and avoid bashisms;
    namely:
 

-- 
1.7.9.1

← back to recent threads