Re: [PATCH v3 2/2] meson: prefer shell at "/bin/sh"

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> Meson detects the path of the target shell via `find_program("sh")`,
> which essentially does a lookup via `PATH`. This may easily lead to a
> subtly-broken Git distribution when the build host has its shell in a
> location that the target host doesn't know about.
>
> Fix the issue by appending "/bin" to the custom program path, which
> causes us to prefer "/bin/sh" over a `PATH`-based lookup. While
> "/bin/sh" isn't standardized, this path tends to work alright on Linux
> and BSD distributions. Furthermore, "/bin/sh" is also the path we pick
> in our Makefile by default, which further demonstrates that this shell
> fulfills our needs.
>
> Note that we intentionally append, not prepend, to the custom program
> path. This is because the program path can be configured by the user via
> the `-Dsane_tool_path=` build option, which should take precedence over
> any defaults we pick for the user.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  meson.build | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Looking good.

> diff --git a/meson.build b/meson.build
> index a180c66ee69..6a90310a2ca 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -236,7 +236,11 @@ sed = find_program('sed', dirs: program_path, native: true)
>  shell = find_program('sh', dirs: program_path, native: true)
>  tar = find_program('tar', dirs: program_path, native: true)
>  
> -target_shell = find_program('sh', dirs: program_path, native: false)
> +# Detect the target shell that is used by Git at runtime. Note that we prefer
> +# "/bin/sh" over a PATH-based lookup, which provides a working shell on most
> +# supported systems. This path is also the default shell path used by our
> +# Makefile. This lookup can be overridden via `program_path`.
> +target_shell = find_program('sh', dirs: program_path + [ '/bin' ], native: false)

I wonder if we should be a bit more friendly to beginners (either
'meson' beginner or a newcomer to the project who are not yet
familiar with how our meson.build files are written), than saying
"via 'program_path'" by referring to "-Dsane_tool_path=", possibly
even with an example.

Now I am showing my ignorance, but does this support folks whose
shell are not spelled "sh" (like "/usr/local/bin/dash"), and more
importantly, if it does not, shouldn't we be using a mechanism that
does?  I think -Dsane_tool_path=/usr/local/bin would help with the
leading directory path, but I suspect that find_program() does not
help specifying "dash" to be used as our target_shell (or host
shell), or "perl5" as our perl.

Of course, this "my sh is called dash" can be left totally outside
of the topic of these two patches.

Thanks.