git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: "Kyle Lippincott" <spectral@google.com>,
	"Rubén Justo" <rjusto@gmail.com>
Subject: [PATCH v3] CodingGuidelines: document a shell that "fails" "VAR=VAL shell_func"
Date: Mon, 22 Jul 2024 17:04:54 -0700	[thread overview]
Message-ID: <xmqqwmld55y1.fsf@gitster.g> (raw)
In-Reply-To: <xmqqjzhd81la.fsf@gitster.g> (Junio C. Hamano's message of "Mon, 22 Jul 2024 16:10:41 -0700")

Over the years, we accumulated the community wisdom to avoid the
common "one-short export" construct for shell functions, but seem to
have lost on which exact platform it is known to fail.  Now during
an investigation on a breakage for a recent topic, we found one
example of failing shell.  Let's document that.

This does *not* mean that we can freely start using the construct
once Ubuntu 20.04 is retired.  But it does mean that we cannot use
the construct until Ubuntu 20.04 is fully retired from the machines
that matter.  Moreover, posix explicitly says that the behaviour for
the construct is unspecified.

Helped-by: Kyle Lippincott <spectral@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 1d92b2da03..ad71c26152 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -204,6 +204,33 @@ For shell scripts specifically (not exhaustive):
 	local variable="$value"
 	local variable="$(command args)"
 
+ - The common construct
+
+	VAR=VAL command args
+
+   to temporarily set and export environment variable VAR only while
+   "command args" is running is handy, but this triggers an
+   unspecified behaviour accoreding to POSIX when used for a command
+   that is not an external command (like shell functions).  Indeed,
+   some versions of dash (like 0.5.10.2-6 found on Ubuntu 20.04) and
+   AT&T ksh do make a temporary assignment without exporting the
+   variable, in such a case.  Do not use it for shell functions.  A
+   common workaround is to do an explicit export in a subshell, like
+   so:
+
+	(incorrect)
+	VAR=VAL func args
+
+	(correct)
+	(
+		VAR=VAL &&
+		export VAR &&
+		func args
+	)
+
+   but be careful that the effect "func" makes to the variables in the
+   current shell will be lost across the subshell boundary.
+
  - Use octal escape sequences (e.g. "\302\242"), not hexadecimal (e.g.
    "\xc2\xa2") in printf format strings, since hexadecimal escape
    sequences are not portable.

Range-diff:
1:  75d07c05c7 ! 1:  8462cbb740 CodingGuidelines: document a shell that "fails" "VAR=VAL shell_func"
    @@ Commit message
         This does *not* mean that we can freely start using the construct
         once Ubuntu 20.04 is retired.  But it does mean that we cannot use
         the construct until Ubuntu 20.04 is fully retired from the machines
    -    that matter.
    +    that matter.  Moreover, posix explicitly says that the behaviour for
    +    the construct is unspecified.
     
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
    @@ Documentation/CodingGuidelines: For shell scripts specifically (not exhaustive):
     +	VAR=VAL command args
     +
     +   to temporarily set and export environment variable VAR only while
    -+   "command args" is running is handy, but some versions of dash (like
    -+   0.5.10.2-6 found on Ubuntu 20.04) makes a temporary assignment
    -+   without exporting the variable, when command is *not* an external
    -+   command.  Do not use it for shell functions.  A common workaround
    -+   is to do an explicit export in a subshell, like so:
    ++   "command args" is running is handy, but this triggers an
    ++   unspecified behaviour accoreding to POSIX when used for a command
    ++   that is not an external command (like shell functions).  Indeed,
    ++   some versions of dash (like 0.5.10.2-6 found on Ubuntu 20.04) and
    ++   AT&T ksh do make a temporary assignment without exporting the
    ++   variable, in such a case.  Do not use it for shell functions.  A
    ++   common workaround is to do an explicit export in a subshell, like
    ++   so:
     +
     +	(incorrect)
     +	VAR=VAL func args
-- 
2.46.0-rc1-52-g816ffef0a1


  reply	other threads:[~2024-07-23  0:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-22 23:10 [PATCH v2] CodingGuidelines: document a shell that "fails" "VAR=VAL shell_func" Junio C Hamano
2024-07-23  0:04 ` Junio C Hamano [this message]
2024-07-23  0:10   ` [PATCH v3] " Eric Sunshine
2024-07-23  0:23     ` Junio C Hamano
2024-07-23  3:34 ` [PATCH v2] " Jeff King
2024-07-23 15:28   ` Junio C Hamano
2024-07-23 21:55   ` Rubén Justo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqwmld55y1.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=rjusto@gmail.com \
    --cc=spectral@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).