Re: Re* [PATCH v3 4/4] add-patch: render hunks through the pager

["Rubén Justo" <rjusto@gmail.com>]

On Mon, Jul 22, 2024 at 02:06:37PM -0700, Junio C Hamano wrote:

> So we have an answer:
> 
>   https://github.com/git/git/actions/runs/10047627546/job/27769808515
> 
> tells us that the problematic shell is used in the job.
> 
> It is
> 
> ii  dash           0.5.10.2-6     amd64        POSIX-compliant shell
> 
> running on Ubuntu 20.04 that is "too POSIXly correct"[*] and behaves
> differently from what the tests expect.
> 
> Somebody should write this combination down somewhere in the
> documentation so that we can answer (better yet, we do not have to
> answer) when somebody wonders if we know of a version of shell that
> refuses to do an one-shot export for shell functions as we naïvely
> expect.
> 
> 
> [Reference]
> 
>  * https://lore.kernel.org/git/4B5027B8.2090507@viscovery.net/
> 
> 
> ----- >8 --------- >8 --------- >8 --------- >8 ----
> CodingGuidelines: give an example shell that "fails" "VAR=VAL shell_func"
> 
> 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, let's document
> one example of failing shell.
> 
> 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.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/CodingGuidelines | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git c/Documentation/CodingGuidelines w/Documentation/CodingGuidelines
> index 1d92b2da03..a3ecb4ac5a 100644
> --- c/Documentation/CodingGuidelines
> +++ w/Documentation/CodingGuidelines
> @@ -204,6 +204,29 @@ 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 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.  We often have to resort to subshell with explicit export,
> +   i.e.
> +
> +	(incorrect)
> +	VAR=VAL func args
> +
> +	(correct)
> +	(
> +		VAR=VAL && export VAR &&
> +		func args

Just a small comment; maybe it's worth adding an extra line to make the
example clearer:  

		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.

Thank you for digging into the test to find an explanation and for
adding the comment to the documentation.