git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: "Patrick Steinhardt" <ps@pks.im>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"René Scharfe" <l.s.r@web.de>
Subject: [PATCH] CodingGuidelines: quote assigned value with "local" and "export"
Date: Fri, 05 Apr 2024 09:12:34 -0700	[thread overview]
Message-ID: <xmqqbk6nyej1.fsf_-_@gitster.g> (raw)
In-Reply-To: <xmqqmsq7yezc.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 05 Apr 2024 09:02:47 -0700")

Dash bug https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
lets the shell erroneously perform field splitting on the expansion
of a command substitution during declaration of a local or an extern
variable.

The explanation was stolen from ebee5580 (parallel-checkout: avoid
dash local bug in tests, 2021-06-06).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * The coding guidelines now forbids use of local/export var=$val
   without having $val quoted inside a pair of double quotes to
   avoid portability bugs.

> Isn't this the same as what ebee5580 (parallel-checkout: avoid dash
> local bug in tests, 2021-06-06) fixed?

 Documentation/CodingGuidelines | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git i/Documentation/CodingGuidelines w/Documentation/CodingGuidelines
index 32e69f798e..8aa76094ca 100644
--- i/Documentation/CodingGuidelines
+++ w/Documentation/CodingGuidelines
@@ -188,6 +188,20 @@ For shell scripts specifically (not exhaustive):
    hopefully nobody starts using "local" before they are reimplemented
    in C ;-)
 
+ - Some versions of dash has broken variable assignment when prefixed
+   with "local", "export", and "readonly", in that the value to be
+   assigned goes through field splitting at $IFS unless quoted.  
+
+   DO NOT write:
+
+     local variable=$value           ;# wrong
+     export variable=$(command args) ;# wrong
+
+   and instead write:
+
+     local variable="$value"
+     export variable="$(command args)"
+
  - 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.


  reply	other threads:[~2024-04-05 16:12 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-04 13:25 [PATCH 00/12] t: exercise Git/JGit reftable compatibility Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 01/12] ci: rename "runs_on_pool" to "distro" Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 02/12] ci: expose distro name in dockerized GitHub jobs Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 03/12] ci: allow skipping sudo on dockerized jobs Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 04/12] ci: drop duplicate package installation for "linux-gcc-default" Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 05/12] ci: convert "install-dependencies.sh" to use "/bin/sh" Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 06/12] ci: merge custom PATH directories Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 07/12] ci: merge scripts which install dependencies Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 08/12] ci: make Perforce binaries executable for all users Patrick Steinhardt
2024-04-05 20:01   ` Josh Steadmon
2024-04-08  5:48     ` Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 09/12] ci: install JGit dependency Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 10/12] t06xx: always execute backend-specific tests Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 11/12] t0610: fix non-portable variable assignment Patrick Steinhardt
2024-04-05  9:14   ` Eric Sunshine
2024-04-05  9:40     ` Patrick Steinhardt
2024-04-05 16:02   ` Junio C Hamano
2024-04-05 16:12     ` Junio C Hamano [this message]
2024-04-05 16:21       ` [PATCH] CodingGuidelines: quote assigned value with "local" and "export" Junio C Hamano
2024-04-05 17:48       ` Jeff King
2024-04-05 19:36         ` Junio C Hamano
2024-04-05 23:15           ` Junio C Hamano
2024-04-07  1:23             ` Jeff King
2024-04-05 16:44     ` [PATCH 11/12] t0610: fix non-portable variable assignment Patrick Steinhardt
2024-04-04 13:25 ` [PATCH 12/12] t0612: add tests to exercise Git/JGit reftable compatibility Patrick Steinhardt
2024-04-04 21:40   ` Han-Wen Nienhuys
2024-04-05  9:41     ` Patrick Steinhardt
2024-04-06 13:03       ` Han-Wen Nienhuys
2024-04-06 15:53         ` Patrick Steinhardt
2024-04-08  5:24     ` Patrick Steinhardt
2024-04-04 13:37 ` [PATCH 00/12] t: " Patrick Steinhardt
2024-04-08  6:45 ` [PATCH v2 " Patrick Steinhardt
2024-04-08  6:46   ` [PATCH v2 02/12] ci: expose distro name in dockerized GitHub jobs Patrick Steinhardt
2024-04-08  6:46   ` [PATCH v2 03/12] ci: allow skipping sudo on dockerized jobs Patrick Steinhardt
2024-04-10 16:53     ` Toon claes
2024-04-10 17:21       ` Junio C Hamano
2024-04-11  8:55         ` Patrick Steinhardt
2024-04-08  6:46   ` [PATCH v2 04/12] ci: drop duplicate package installation for "linux-gcc-default" Patrick Steinhardt
2024-04-08  6:46   ` [PATCH v2 05/12] ci: convert "install-dependencies.sh" to use "/bin/sh" Patrick Steinhardt
2024-04-10 20:46     ` Justin Tobler
2024-04-11  8:55       ` Patrick Steinhardt
2024-04-08  6:46   ` [PATCH v2 06/12] ci: merge custom PATH directories Patrick Steinhardt
2024-04-08  6:46   ` [PATCH v2 07/12] ci: merge scripts which install dependencies Patrick Steinhardt
2024-04-08  6:46   ` [PATCH v2 08/12] ci: make Perforce binaries executable for all users Patrick Steinhardt
2024-04-08  6:46   ` [PATCH v2 09/12] ci: install JGit dependency Patrick Steinhardt
2024-04-08  6:46   ` [PATCH v2 10/12] t06xx: always execute backend-specific tests Patrick Steinhardt
2024-04-08  6:47   ` [PATCH v2 11/12] t0610: fix non-portable variable assignment Patrick Steinhardt
2024-04-08  6:57     ` Eric Sunshine
2024-04-08  8:56       ` Patrick Steinhardt
2024-04-08  6:47   ` [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility Patrick Steinhardt
2024-04-08  7:10     ` Eric Sunshine
2024-04-08 16:07       ` Junio C Hamano
2024-04-08 16:19         ` Eric Sunshine
2024-04-08 16:29           ` Eric Sunshine
2024-04-08 16:22         ` Patrick Steinhardt
2024-04-08 17:26           ` Jeff King
2024-04-08 21:52             ` Junio C Hamano
2024-04-08 21:51           ` Junio C Hamano
2024-04-10 20:43     ` Justin Tobler
2024-04-11  8:55       ` Patrick Steinhardt
2024-04-08  6:47   ` [PATCH v2 01/12] ci: rename "runs_on_pool" to "distro" Patrick Steinhardt
2024-04-09  2:17   ` [PATCH v2 00/12] t: exercise Git/JGit reftable compatibility Junio C Hamano
2024-04-09  3:43     ` Patrick Steinhardt
2024-04-09  5:34       ` Junio C Hamano
2024-04-09  6:07         ` Patrick Steinhardt
2024-04-09  9:57     ` [PATCH 0/2] t0610: fix umask tests Patrick Steinhardt
2024-04-09  9:57       ` [PATCH 1/2] t0610: make `--shared=` tests reusable Patrick Steinhardt
2024-04-09  9:57       ` [PATCH 2/2] t0610: execute git-pack-refs(1) with specified umask Patrick Steinhardt
2024-04-10 21:49       ` [PATCH 0/2] t0610: fix umask tests Justin Tobler
2024-04-11  9:09 ` [PATCH v3 00/13] t: exercise Git/JGit reftable compatibility Patrick Steinhardt
2024-04-11  9:09   ` [PATCH v3 01/13] ci: rename "runs_on_pool" to "distro" Patrick Steinhardt
2024-04-11  9:10   ` [PATCH v3 02/13] ci: expose distro name in dockerized GitHub jobs Patrick Steinhardt
2024-04-11  9:10   ` [PATCH v3 03/13] ci: skip sudo when we are already root Patrick Steinhardt
2024-04-11  9:10   ` [PATCH v3 04/13] ci: drop duplicate package installation for "linux-gcc-default" Patrick Steinhardt
2024-04-11  9:10   ` [PATCH v3 05/13] ci: convert "install-dependencies.sh" to use "/bin/sh" Patrick Steinhardt
2024-04-11  9:10   ` [PATCH v3 06/13] ci: merge custom PATH directories Patrick Steinhardt
2024-04-11  9:10   ` [PATCH v3 07/13] ci: fix setup of custom path for GitLab CI Patrick Steinhardt
2024-04-11 10:06     ` Eric Sunshine
2024-04-11 10:26       ` Patrick Steinhardt
2024-04-11  9:10   ` [PATCH v3 08/13] ci: merge scripts which install dependencies Patrick Steinhardt
2024-04-11  9:10   ` [PATCH v3 09/13] ci: make Perforce binaries executable for all users Patrick Steinhardt
2024-04-11  9:10   ` [PATCH v3 10/13] ci: install JGit dependency Patrick Steinhardt
2024-04-11  9:11   ` [PATCH v3 11/13] t06xx: always execute backend-specific tests Patrick Steinhardt
2024-04-11  9:11   ` [PATCH v3 12/13] t0610: fix non-portable variable assignment Patrick Steinhardt
2024-04-11  9:11   ` [PATCH v3 13/13] t0612: add tests to exercise Git/JGit reftable compatibility Patrick Steinhardt
2024-04-12  4:43 ` [PATCH v4 00/13] t: " Patrick Steinhardt
2024-04-12  4:43   ` [PATCH v4 01/13] ci: rename "runs_on_pool" to "distro" Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 02/13] ci: expose distro name in dockerized GitHub jobs Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 03/13] ci: skip sudo when we are already root Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 04/13] ci: drop duplicate package installation for "linux-gcc-default" Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 05/13] ci: convert "install-dependencies.sh" to use "/bin/sh" Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 06/13] ci: merge custom PATH directories Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 07/13] ci: fix setup of custom path for GitLab CI Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 08/13] ci: merge scripts which install dependencies Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 09/13] ci: make Perforce binaries executable for all users Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 10/13] ci: install JGit dependency Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 11/13] t06xx: always execute backend-specific tests Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 12/13] t0610: fix non-portable variable assignment Patrick Steinhardt
2024-04-12  4:44   ` [PATCH v4 13/13] t0612: add tests to exercise Git/JGit reftable compatibility Patrick Steinhardt
2024-05-03 18:42   ` [PATCH v4 00/13] t: " Justin Tobler
2024-05-03 18:48     ` Patrick Steinhardt
2024-05-03 18:57       ` Patrick Steinhardt
2024-05-03 19:35         ` Justin Tobler
2024-05-03 19:49           ` Patrick Steinhardt
2024-05-04 17:32             ` Justin Tobler
2024-05-06  5:53               ` Patrick Steinhardt

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqbk6nyej1.fsf_-_@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=ps@pks.im \
    --cc=sunshine@sunshineco.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).