git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup
@ 2025-04-24 13:38 Patrick Steinhardt
  2025-04-24 13:38 ` [PATCH 1/2] meson: report detected runtime executable paths Patrick Steinhardt
                   ` (4 more replies)
  0 siblings, 5 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-24 13:38 UTC (permalink / raw)
  To: git; +Cc: Peter Seiderer

Hi,

at GitLab, we recently got a couple of bug reports about Git not being
able to find its shell anymore. The root cause is that with Meson we
have started to look up the shell via PATH, which may exist on the build
host, but not on the target host. We have worked around this issue with
a cross file:

    $ cat >cross.ini <<-EOF
    [binaries]
    sh = '/bin/sh'
    EOF
    $ meson setup build --cross-file=./cross.ini

But this made me remember the report from Peter [1] that Debian also
faced this issue. So I decided to address the issue in Meson directly by
preferring `/bin/sh` over a PATH-based lookup.

Thanks!

Patrick

[1]: <20250209133027.64a865aa@gmx.net>

---
Patrick Steinhardt (2):
      meson: report detected runtime executable paths
      meson: prefer POSIX-specified shell path

 meson.build | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


---
base-commit: a2955b34f48265d240ab8c7deb0a929ec2d65fd0
change-id: 20250424-pks-meson-posix-shell-4969161025c5


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 1/2] meson: report detected runtime executable paths
  2025-04-24 13:38 [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Patrick Steinhardt
@ 2025-04-24 13:38 ` Patrick Steinhardt
  2025-04-25  0:45   ` Eli Schwartz
  2025-04-24 13:38 ` [PATCH 2/2] meson: prefer POSIX-specified shell path Patrick Steinhardt
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-24 13:38 UTC (permalink / raw)
  To: git; +Cc: Peter Seiderer

Git needs to know about a couple of executable paths to pick at runtime.
This includes the system shell, but may also optionally include the Perl
and Python interpreters. Meson detects the location of these paths
automatically via `find_program()`, which does a lookup via the `PATH`
environment variable. As such, it may not be immediately obvious to the
developer which paths have been autodetected.

Improve this by exposing runtime executable paths at setup time.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meson.build b/meson.build
index c47cb79af08..8f04534c7ff 100644
--- a/meson.build
+++ b/meson.build
@@ -2080,3 +2080,9 @@ summary({
   'sha256': sha256_backend,
   'zlib': zlib_backend,
 }, section: 'Backends')
+
+summary({
+  'perl': target_perl.found() ? target_perl.full_path() : 'none',
+  'python': target_python.found() ? target_python.full_path() : 'none',
+  'shell': target_shell.full_path(),
+}, section: 'Runtime executable paths')

-- 
2.49.0.901.g37484f566f.dirty


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 2/2] meson: prefer POSIX-specified shell path
  2025-04-24 13:38 [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Patrick Steinhardt
  2025-04-24 13:38 ` [PATCH 1/2] meson: report detected runtime executable paths Patrick Steinhardt
@ 2025-04-24 13:38 ` Patrick Steinhardt
  2025-04-24 20:18   ` Justin Tobler
  2025-04-24 18:28 ` [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Junio C Hamano
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-24 13:38 UTC (permalink / raw)
  To: git; +Cc: Peter Seiderer

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
non-standard 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` lookup. As this location is
specified by POSIX this should make us pick a better default shell path
on all POSIX-compliant systems.

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 8f04534c7ff..1db768380bd 100644
--- a/meson.build
+++ b/meson.build
@@ -236,7 +236,7 @@ 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)
+target_shell = find_program('sh', dirs: program_path + [ '/bin' ], native: false)
 
 # Sanity-check that programs required for the build exist.
 foreach tool : ['cat', 'cut', 'grep', 'sort', 'tr', 'uname']

-- 
2.49.0.901.g37484f566f.dirty


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup
  2025-04-24 13:38 [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Patrick Steinhardt
  2025-04-24 13:38 ` [PATCH 1/2] meson: report detected runtime executable paths Patrick Steinhardt
  2025-04-24 13:38 ` [PATCH 2/2] meson: prefer POSIX-specified shell path Patrick Steinhardt
@ 2025-04-24 18:28 ` Junio C Hamano
  2025-04-25  5:21   ` Patrick Steinhardt
  2025-04-25  5:47 ` [PATCH v2 " Patrick Steinhardt
  2025-04-25 14:11 ` [PATCH v3 " Patrick Steinhardt
  4 siblings, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2025-04-24 18:28 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Peter Seiderer

Patrick Steinhardt <ps@pks.im> writes:

> at GitLab, we recently got a couple of bug reports about Git not being
> able to find its shell anymore. The root cause is that with Meson we
> have started to look up the shell via PATH, which may exist on the build
> host, but not on the target host. We have worked around this issue with
> a cross file:
>
>     $ cat >cross.ini <<-EOF
>     [binaries]
>     sh = '/bin/sh'
>     EOF
>     $ meson setup build --cross-file=./cross.ini
>
> But this made me remember the report from Peter [1] that Debian also
> faced this issue. So I decided to address the issue in Meson directly by
> preferring `/bin/sh` over a PATH-based lookup.

Perhaps use the same SHELL_PATH environment Makefile based build
has used for ages?  That way, those who are dipping their toes and
possibly migrating to Meson based build eventually would know what
they want to twaek, no?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/2] meson: prefer POSIX-specified shell path
  2025-04-24 13:38 ` [PATCH 2/2] meson: prefer POSIX-specified shell path Patrick Steinhardt
@ 2025-04-24 20:18   ` Justin Tobler
  2025-04-25  5:21     ` Patrick Steinhardt
  0 siblings, 1 reply; 31+ messages in thread
From: Justin Tobler @ 2025-04-24 20:18 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Peter Seiderer

On 25/04/24 03:38PM, Patrick Steinhardt wrote:
> 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
> non-standard location that the target host doesn't know about.

Ok, so we run into this issue if the shell path picked up from the build
host's $PATH doesn't exist on the target host. Makes sense.

> Fix the issue by appending "/bin" to the custom program path, which
> causes us to prefer "/bin/sh" over a `PATH` lookup. As this location is
> specified by POSIX this should make us pick a better default shell path
> on all POSIX-compliant systems.

So if the build host has "/bin/sh", but the target host doesn't we would
still have an issue, but that is still probably a better default. I
guess now $PATH would only be used as the fallback if the build host is
even most non-standard.

> 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.

IIUC, then the order precedence is "program_path", "/bin", and finally
$PATH. That makes sense.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index 8f04534c7ff..1db768380bd 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -236,7 +236,7 @@ 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)
> +target_shell = find_program('sh', dirs: program_path + [ '/bin' ], native: false)

It might be nice to leave a comment explaining the ordering intent.

>  # Sanity-check that programs required for the build exist.
>  foreach tool : ['cat', 'cut', 'grep', 'sort', 'tr', 'uname']

-Justin

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/2] meson: report detected runtime executable paths
  2025-04-24 13:38 ` [PATCH 1/2] meson: report detected runtime executable paths Patrick Steinhardt
@ 2025-04-25  0:45   ` Eli Schwartz
  2025-04-25  5:21     ` Patrick Steinhardt
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Schwartz @ 2025-04-25  0:45 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Peter Seiderer


[-- Attachment #1.1: Type: text/plain, Size: 1529 bytes --]

On 4/24/25 9:38 AM, Patrick Steinhardt wrote:
> Git needs to know about a couple of executable paths to pick at runtime.
> This includes the system shell, but may also optionally include the Perl
> and Python interpreters. Meson detects the location of these paths
> automatically via `find_program()`, which does a lookup via the `PATH`
> environment variable. As such, it may not be immediately obvious to the
> developer which paths have been autodetected.
> 
> Improve this by exposing runtime executable paths at setup time.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  meson.build | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index c47cb79af08..8f04534c7ff 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2080,3 +2080,9 @@ summary({
>    'sha256': sha256_backend,
>    'zlib': zlib_backend,
>  }, section: 'Backends')
> +
> +summary({
> +  'perl': target_perl.found() ? target_perl.full_path() : 'none',
> +  'python': target_python.found() ? target_python.full_path() : 'none',
> +  'shell': target_shell.full_path(),
> +}, section: 'Runtime executable paths')

summary({
  'perl': target_perl,
  'python': target_python,
  'shell': target_shell,
}, section: 'Runtime executable paths')


No need to check if they are found. Meson will print the full_path()
already, if it is found, and if it is not found, it will print "NO" in
its standard color code (red) for things-that-are-missing.




-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/2] meson: report detected runtime executable paths
  2025-04-25  0:45   ` Eli Schwartz
@ 2025-04-25  5:21     ` Patrick Steinhardt
  2025-04-25 16:34       ` Junio C Hamano
  0 siblings, 1 reply; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25  5:21 UTC (permalink / raw)
  To: Eli Schwartz; +Cc: git, Peter Seiderer

On Thu, Apr 24, 2025 at 08:45:44PM -0400, Eli Schwartz wrote:
> On 4/24/25 9:38 AM, Patrick Steinhardt wrote:
> > diff --git a/meson.build b/meson.build
> > index c47cb79af08..8f04534c7ff 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -2080,3 +2080,9 @@ summary({
> >    'sha256': sha256_backend,
> >    'zlib': zlib_backend,
> >  }, section: 'Backends')
> > +
> > +summary({
> > +  'perl': target_perl.found() ? target_perl.full_path() : 'none',
> > +  'python': target_python.found() ? target_python.full_path() : 'none',
> > +  'shell': target_shell.full_path(),
> > +}, section: 'Runtime executable paths')
> 
> summary({
>   'perl': target_perl,
>   'python': target_python,
>   'shell': target_shell,
> }, section: 'Runtime executable paths')
> 
> 
> No need to check if they are found. Meson will print the full_path()
> already, if it is found, and if it is not found, it will print "NO" in
> its standard color code (red) for things-that-are-missing.

Oh, that's much nicer indeed. Thanks!

Patrick

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup
  2025-04-24 18:28 ` [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Junio C Hamano
@ 2025-04-25  5:21   ` Patrick Steinhardt
  0 siblings, 0 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25  5:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Peter Seiderer

On Thu, Apr 24, 2025 at 11:28:30AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > at GitLab, we recently got a couple of bug reports about Git not being
> > able to find its shell anymore. The root cause is that with Meson we
> > have started to look up the shell via PATH, which may exist on the build
> > host, but not on the target host. We have worked around this issue with
> > a cross file:
> >
> >     $ cat >cross.ini <<-EOF
> >     [binaries]
> >     sh = '/bin/sh'
> >     EOF
> >     $ meson setup build --cross-file=./cross.ini
> >
> > But this made me remember the report from Peter [1] that Debian also
> > faced this issue. So I decided to address the issue in Meson directly by
> > preferring `/bin/sh` over a PATH-based lookup.
> 
> Perhaps use the same SHELL_PATH environment Makefile based build
> has used for ages?  That way, those who are dipping their toes and
> possibly migrating to Meson based build eventually would know what
> they want to twaek, no?

Yeah, that's basically what we do with this patch series now. How
exactly this is wired up is different compared to our Makefile so that
users can use Meson features to override this, e.g native files. But the
end result is the same on all POSIX-compliant systems that have
'/bin/sh'.

Patrick

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/2] meson: prefer POSIX-specified shell path
  2025-04-24 20:18   ` Justin Tobler
@ 2025-04-25  5:21     ` Patrick Steinhardt
  0 siblings, 0 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25  5:21 UTC (permalink / raw)
  To: Justin Tobler; +Cc: git, Peter Seiderer

On Thu, Apr 24, 2025 at 03:18:29PM -0500, Justin Tobler wrote:
> On 25/04/24 03:38PM, Patrick Steinhardt wrote:
> > diff --git a/meson.build b/meson.build
> > index 8f04534c7ff..1db768380bd 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -236,7 +236,7 @@ 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)
> > +target_shell = find_program('sh', dirs: program_path + [ '/bin' ], native: false)
> 
> It might be nice to leave a comment explaining the ordering intent.

Will do.

Patrick

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 0/2] meson: prefer '/bin/sh' over PATH lookup
  2025-04-24 13:38 [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Patrick Steinhardt
                   ` (2 preceding siblings ...)
  2025-04-24 18:28 ` [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Junio C Hamano
@ 2025-04-25  5:47 ` Patrick Steinhardt
  2025-04-25  5:47   ` [PATCH v2 1/2] meson: report detected runtime executable paths Patrick Steinhardt
                     ` (2 more replies)
  2025-04-25 14:11 ` [PATCH v3 " Patrick Steinhardt
  4 siblings, 3 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25  5:47 UTC (permalink / raw)
  To: git; +Cc: Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

Hi,

at GitLab, we recently got a couple of bug reports about Git not being
able to find its shell anymore. The root cause is that with Meson we
have started to look up the shell via PATH, which may exist on the build
host, but not on the target host. We have worked around this issue with
a cross file:

    $ cat >cross.ini <<-EOF
    [binaries]
    sh = '/bin/sh'
    EOF
    $ meson setup build --cross-file=./cross.ini

But this made me remember the report from Peter [1] that Debian also
faced this issue. So I decided to address the issue in Meson directly by
preferring `/bin/sh` over a PATH-based lookup.

Changes in v2:
  - Simplify how we generate the summary.
  - Add a comment to explain ordering of the program path.
  - Link to v1: https://lore.kernel.org/r/20250424-pks-meson-posix-shell-v1-0-45e06ee4b6ad@pks.im

Thanks!

Patrick

[1]: <20250209133027.64a865aa@gmx.net>

---
Patrick Steinhardt (2):
      meson: report detected runtime executable paths
      meson: prefer POSIX-specified shell path

 meson.build | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Range-diff versus v1:

1:  cdb4db30677 ! 1:  b606f3ffe2e meson: report detected runtime executable paths
    @@ meson.build: summary({
      }, section: 'Backends')
     +
     +summary({
    -+  'perl': target_perl.found() ? target_perl.full_path() : 'none',
    -+  'python': target_python.found() ? target_python.full_path() : 'none',
    -+  'shell': target_shell.full_path(),
    ++  'perl': target_perl,
    ++  'python': target_python,
    ++  'shell': target_shell,
     +}, section: 'Runtime executable paths')
2:  d439c859fbb ! 2:  3804c32b879 meson: prefer POSIX-specified shell path
    @@ meson.build: sed = find_program('sed', 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 given that '/bin/sh' is the location
    ++# specified by POSIX. This lookup can be overridden via `program_path`.
     +target_shell = find_program('sh', dirs: program_path + [ '/bin' ], native: false)
      
      # Sanity-check that programs required for the build exist.

---
base-commit: a2955b34f48265d240ab8c7deb0a929ec2d65fd0
change-id: 20250424-pks-meson-posix-shell-4969161025c5


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 1/2] meson: report detected runtime executable paths
  2025-04-25  5:47 ` [PATCH v2 " Patrick Steinhardt
@ 2025-04-25  5:47   ` Patrick Steinhardt
  2025-04-25  8:27     ` Toon Claes
  2025-04-25  5:47   ` [PATCH v2 2/2] meson: prefer POSIX-specified shell path Patrick Steinhardt
  2025-04-25 11:24   ` [PATCH v2 0/2] meson: prefer '/bin/sh' over PATH lookup Toon Claes
  2 siblings, 1 reply; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25  5:47 UTC (permalink / raw)
  To: git; +Cc: Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

Git needs to know about a couple of executable paths to pick at runtime.
This includes the system shell, but may also optionally include the Perl
and Python interpreters. Meson detects the location of these paths
automatically via `find_program()`, which does a lookup via the `PATH`
environment variable. As such, it may not be immediately obvious to the
developer which paths have been autodetected.

Improve this by exposing runtime executable paths at setup time.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meson.build b/meson.build
index c47cb79af08..a180c66ee69 100644
--- a/meson.build
+++ b/meson.build
@@ -2080,3 +2080,9 @@ summary({
   'sha256': sha256_backend,
   'zlib': zlib_backend,
 }, section: 'Backends')
+
+summary({
+  'perl': target_perl,
+  'python': target_python,
+  'shell': target_shell,
+}, section: 'Runtime executable paths')

-- 
2.49.0.967.g6a0df3ecc3.dirty


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 2/2] meson: prefer POSIX-specified shell path
  2025-04-25  5:47 ` [PATCH v2 " Patrick Steinhardt
  2025-04-25  5:47   ` [PATCH v2 1/2] meson: report detected runtime executable paths Patrick Steinhardt
@ 2025-04-25  5:47   ` Patrick Steinhardt
  2025-04-25  8:35     ` Toon Claes
  2025-04-25 10:49     ` brian m. carlson
  2025-04-25 11:24   ` [PATCH v2 0/2] meson: prefer '/bin/sh' over PATH lookup Toon Claes
  2 siblings, 2 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25  5:47 UTC (permalink / raw)
  To: git; +Cc: Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

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
non-standard 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` lookup. As this location is
specified by POSIX this should make us pick a better default shell path
on all POSIX-compliant systems.

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 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index a180c66ee69..c0d0982b00f 100644
--- a/meson.build
+++ b/meson.build
@@ -236,7 +236,10 @@ 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 given that '/bin/sh' is the location
+# specified by POSIX. This lookup can be overridden via `program_path`.
+target_shell = find_program('sh', dirs: program_path + [ '/bin' ], native: false)
 
 # Sanity-check that programs required for the build exist.
 foreach tool : ['cat', 'cut', 'grep', 'sort', 'tr', 'uname']

-- 
2.49.0.967.g6a0df3ecc3.dirty


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 1/2] meson: report detected runtime executable paths
  2025-04-25  5:47   ` [PATCH v2 1/2] meson: report detected runtime executable paths Patrick Steinhardt
@ 2025-04-25  8:27     ` Toon Claes
  0 siblings, 0 replies; 31+ messages in thread
From: Toon Claes @ 2025-04-25  8:27 UTC (permalink / raw)
  To: Patrick Steinhardt, git
  Cc: Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

Patrick Steinhardt <ps@pks.im> writes:

> Git needs to know about a couple of executable paths to pick at runtime.
> This includes the system shell, but may also optionally include the Perl
> and Python interpreters. Meson detects the location of these paths
> automatically via `find_program()`, which does a lookup via the `PATH`
> environment variable. As such, it may not be immediately obvious to the
> developer which paths have been autodetected.
>
> Improve this by exposing runtime executable paths at setup time.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  meson.build | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index c47cb79af08..a180c66ee69 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2080,3 +2080,9 @@ summary({
>    'sha256': sha256_backend,
>    'zlib': zlib_backend,
>  }, section: 'Backends')
> +
> +summary({
> +  'perl': target_perl,
> +  'python': target_python,
> +  'shell': target_shell,
> +}, section: 'Runtime executable paths')

I appreciate this change. Without [PATCH 2/2] applied I'm getting:

  Runtime executable paths
    perl         : /usr/bin/perl
    python       : /usr/bin/python3
    shell        : /usr/bin/sh

And with [PATHCH 2/2] I'm getting:

  Runtime executable paths
    perl         : /usr/bin/perl
    python       : /usr/bin/python3
    shell        : /bin/sh

As expected. :+1:

--
Toon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 2/2] meson: prefer POSIX-specified shell path
  2025-04-25  5:47   ` [PATCH v2 2/2] meson: prefer POSIX-specified shell path Patrick Steinhardt
@ 2025-04-25  8:35     ` Toon Claes
  2025-04-25 10:49     ` brian m. carlson
  1 sibling, 0 replies; 31+ messages in thread
From: Toon Claes @ 2025-04-25  8:35 UTC (permalink / raw)
  To: Patrick Steinhardt, git
  Cc: Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

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
> non-standard 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` lookup. As this location is
> specified by POSIX this should make us pick a better default shell path
> on all POSIX-compliant systems.
>
> 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 | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index a180c66ee69..c0d0982b00f 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -236,7 +236,10 @@ 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 given that '/bin/sh' is the location
> +# specified by POSIX. This lookup can be overridden via `program_path`.
> +target_shell = find_program('sh', dirs: program_path + [ '/bin' ], native: false)

It was not instantly obvious to me, but this is what the docs[1] say
about the use of `dirs` in `find_program()`:

   extra list of absolute paths where to look for program names

So the function *first* looks in `dirs` *before* it looks in $PATH. I
wasn't fully aware what `program_path` contained, but I assumed it had
to contain the dirs in $PATH. But it seems $PATH is searched if the
program is not found in `dirs`.

So I agree with these changes.

[1]: https://mesonbuild.com/Reference-manual_functions.html#find_program

--
Toon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 2/2] meson: prefer POSIX-specified shell path
  2025-04-25  5:47   ` [PATCH v2 2/2] meson: prefer POSIX-specified shell path Patrick Steinhardt
  2025-04-25  8:35     ` Toon Claes
@ 2025-04-25 10:49     ` brian m. carlson
  2025-04-25 11:52       ` Patrick Steinhardt
  1 sibling, 1 reply; 31+ messages in thread
From: brian m. carlson @ 2025-04-25 10:49 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: git, Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]

On 2025-04-25 at 05:47:45, Patrick Steinhardt wrote:
> 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
> non-standard 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` lookup. As this location is
> specified by POSIX this should make us pick a better default shell path
> on all POSIX-compliant systems.

Can you provide a citation for that?  I don't see that in the POSIX
1003.1-2024 directory structure document[0].  More specifically, I think
there are some proprietary Unix systems where `/bin/sh` is the original
Bourne shell and is not POSIX compliant and some other path is the
POSIX-compliant `sh`.

I'll also point out that we require more than POSIX compliance in that
we require `local`, so even if `/bin/sh` is POSIX compliant, that
doesn't mean that it's suitable for Git.  `/bin/sh` meets our needs on
all the Linux distros I'm aware of, plus the BSDs, but if it were AT&T
ksh, that would not meet our needs since it doesn't support `local`,
even though it's POSIX compliant.

[0] https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap10.html
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/2] meson: prefer '/bin/sh' over PATH lookup
  2025-04-25  5:47 ` [PATCH v2 " Patrick Steinhardt
  2025-04-25  5:47   ` [PATCH v2 1/2] meson: report detected runtime executable paths Patrick Steinhardt
  2025-04-25  5:47   ` [PATCH v2 2/2] meson: prefer POSIX-specified shell path Patrick Steinhardt
@ 2025-04-25 11:24   ` Toon Claes
  2 siblings, 0 replies; 31+ messages in thread
From: Toon Claes @ 2025-04-25 11:24 UTC (permalink / raw)
  To: Patrick Steinhardt, git
  Cc: Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> at GitLab, we recently got a couple of bug reports about Git not being
> able to find its shell anymore. The root cause is that with Meson we
> have started to look up the shell via PATH, which may exist on the build
> host, but not on the target host. We have worked around this issue with
> a cross file:
>
>     $ cat >cross.ini <<-EOF
>     [binaries]
>     sh = '/bin/sh'
>     EOF
>     $ meson setup build --cross-file=./cross.ini
>
> But this made me remember the report from Peter [1] that Debian also
> faced this issue. So I decided to address the issue in Meson directly by
> preferring `/bin/sh` over a PATH-based lookup.
>
> Changes in v2:
>   - Simplify how we generate the summary.
>   - Add a comment to explain ordering of the program path.
>   - Link to v1: https://lore.kernel.org/r/20250424-pks-meson-posix-shell-v1-0-45e06ee4b6ad@pks.im
>
> Thanks!
>
> Patrick

Reviewed and looks good to me. It behaves as explained, although I don't
know how to easily test the original error we've been seeing.

-- 
Toon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 2/2] meson: prefer POSIX-specified shell path
  2025-04-25 10:49     ` brian m. carlson
@ 2025-04-25 11:52       ` Patrick Steinhardt
  0 siblings, 0 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25 11:52 UTC (permalink / raw)
  To: brian m. carlson, git, Peter Seiderer, Junio C Hamano,
	Eli Schwartz, Justin Tobler

On Fri, Apr 25, 2025 at 10:49:10AM +0000, brian m. carlson wrote:
> On 2025-04-25 at 05:47:45, Patrick Steinhardt wrote:
> > 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
> > non-standard 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` lookup. As this location is
> > specified by POSIX this should make us pick a better default shell path
> > on all POSIX-compliant systems.
> 
> Can you provide a citation for that?  I don't see that in the POSIX
> 1003.1-2024 directory structure document[0].  More specifically, I think
> there are some proprietary Unix systems where `/bin/sh` is the original
> Bourne shell and is not POSIX compliant and some other path is the
> POSIX-compliant `sh`.

Hrmpf, you're right. I feel like I relearn this piece of trivia every
couple years. POSIX is quite specific here:

    Applications should note that the standard PATH to the shell cannot
    be assumed to be either /bin/sh or /usr/bin/sh, and should be
    determined by interrogation of the PATH returned by getconf PATH ,
    ensuring that the returned pathname is an absolute pathname and not
    a shell built-in.

Anyway, given the following...

> I'll also point out that we require more than POSIX compliance in that
> we require `local`, so even if `/bin/sh` is POSIX compliant, that
> doesn't mean that it's suitable for Git.  `/bin/sh` meets our needs on
> all the Linux distros I'm aware of, plus the BSDs, but if it were AT&T
> ksh, that would not meet our needs since it doesn't support `local`,
> even though it's POSIX compliant.

... prefering "/bin/sh" is still the right thing to do as it tends to
work on most systems supported by us, even though it's non-POSIX. But in
any case, the commit message needs to be adjusted.

Thanks!

Patrick

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 0/2] meson: prefer '/bin/sh' over PATH lookup
  2025-04-24 13:38 [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Patrick Steinhardt
                   ` (3 preceding siblings ...)
  2025-04-25  5:47 ` [PATCH v2 " Patrick Steinhardt
@ 2025-04-25 14:11 ` Patrick Steinhardt
  2025-04-25 14:11   ` [PATCH v3 1/2] meson: report detected runtime executable paths Patrick Steinhardt
                     ` (2 more replies)
  4 siblings, 3 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25 14:11 UTC (permalink / raw)
  To: git; +Cc: Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

Hi,

at GitLab, we recently got a couple of bug reports about Git not being
able to find its shell anymore. The root cause is that with Meson we
have started to look up the shell via PATH, which may exist on the build
host, but not on the target host. We have worked around this issue with
a cross file:

    $ cat >cross.ini <<-EOF
    [binaries]
    sh = '/bin/sh'
    EOF
    $ meson setup build --cross-file=./cross.ini

But this made me remember the report from Peter [1] that Debian also
faced this issue. So I decided to address the issue in Meson directly by
preferring `/bin/sh` over a PATH-based lookup.

Changes in v2:
  - Simplify how we generate the summary.
  - Add a comment to explain ordering of the program path.
  - Link to v1: https://lore.kernel.org/r/20250424-pks-meson-posix-shell-v1-0-45e06ee4b6ad@pks.im

Changes in v3:
  - Stop claiming that "/bin/sh" is a POSIX-compliant path.
  - Link to v2: https://lore.kernel.org/r/20250425-pks-meson-posix-shell-v2-0-fddc6123511b@pks.im

Thanks!

Patrick

[1]: <20250209133027.64a865aa@gmx.net>

---
Patrick Steinhardt (2):
      meson: report detected runtime executable paths
      meson: prefer shell at "/bin/sh"

 meson.build | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Range-diff versus v2:

1:  e749055ac00 = 1:  750aa492d76 meson: report detected runtime executable paths
2:  159a05d3533 ! 2:  d6417ba5ff6 meson: prefer POSIX-specified shell path
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    meson: prefer POSIX-specified shell path
    +    meson: prefer shell at "/bin/sh"
     
         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
    -    non-standard location that the target host doesn't know about.
    +    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` lookup. As this location is
    -    specified by POSIX this should make us pick a better default shell path
    -    on all POSIX-compliant systems.
    +    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
    @@ meson.build: sed = find_program('sed', 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 given that '/bin/sh' is the location
    -+# specified by POSIX. This lookup can be overridden via `program_path`.
    ++# "/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)
      
      # Sanity-check that programs required for the build exist.

---
base-commit: a2955b34f48265d240ab8c7deb0a929ec2d65fd0
change-id: 20250424-pks-meson-posix-shell-4969161025c5


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 1/2] meson: report detected runtime executable paths
  2025-04-25 14:11 ` [PATCH v3 " Patrick Steinhardt
@ 2025-04-25 14:11   ` Patrick Steinhardt
  2025-04-25 14:11   ` [PATCH v3 2/2] meson: prefer shell at "/bin/sh" Patrick Steinhardt
  2025-05-02 21:16   ` [PATCH v3 0/2] meson: prefer '/bin/sh' over PATH lookup Junio C Hamano
  2 siblings, 0 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25 14:11 UTC (permalink / raw)
  To: git; +Cc: Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

Git needs to know about a couple of executable paths to pick at runtime.
This includes the system shell, but may also optionally include the Perl
and Python interpreters. Meson detects the location of these paths
automatically via `find_program()`, which does a lookup via the `PATH`
environment variable. As such, it may not be immediately obvious to the
developer which paths have been autodetected.

Improve this by exposing runtime executable paths at setup time.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meson.build b/meson.build
index c47cb79af08..a180c66ee69 100644
--- a/meson.build
+++ b/meson.build
@@ -2080,3 +2080,9 @@ summary({
   'sha256': sha256_backend,
   'zlib': zlib_backend,
 }, section: 'Backends')
+
+summary({
+  'perl': target_perl,
+  'python': target_python,
+  'shell': target_shell,
+}, section: 'Runtime executable paths')

-- 
2.49.0.901.g37484f566f.dirty


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 2/2] meson: prefer shell at "/bin/sh"
  2025-04-25 14:11 ` [PATCH v3 " Patrick Steinhardt
  2025-04-25 14:11   ` [PATCH v3 1/2] meson: report detected runtime executable paths Patrick Steinhardt
@ 2025-04-25 14:11   ` Patrick Steinhardt
  2025-04-25 17:04     ` Junio C Hamano
  2025-04-25 20:13     ` brian m. carlson
  2025-05-02 21:16   ` [PATCH v3 0/2] meson: prefer '/bin/sh' over PATH lookup Junio C Hamano
  2 siblings, 2 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-04-25 14:11 UTC (permalink / raw)
  To: git; +Cc: Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

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(-)

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)
 
 # Sanity-check that programs required for the build exist.
 foreach tool : ['cat', 'cut', 'grep', 'sort', 'tr', 'uname']

-- 
2.49.0.901.g37484f566f.dirty


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/2] meson: report detected runtime executable paths
  2025-04-25  5:21     ` Patrick Steinhardt
@ 2025-04-25 16:34       ` Junio C Hamano
  0 siblings, 0 replies; 31+ messages in thread
From: Junio C Hamano @ 2025-04-25 16:34 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Eli Schwartz, git, Peter Seiderer

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Apr 24, 2025 at 08:45:44PM -0400, Eli Schwartz wrote:
>> On 4/24/25 9:38 AM, Patrick Steinhardt wrote:
>> > diff --git a/meson.build b/meson.build
>> > index c47cb79af08..8f04534c7ff 100644
>> > --- a/meson.build
>> > +++ b/meson.build
>> > @@ -2080,3 +2080,9 @@ summary({
>> >    'sha256': sha256_backend,
>> >    'zlib': zlib_backend,
>> >  }, section: 'Backends')
>> > +
>> > +summary({
>> > +  'perl': target_perl.found() ? target_perl.full_path() : 'none',
>> > +  'python': target_python.found() ? target_python.full_path() : 'none',
>> > +  'shell': target_shell.full_path(),
>> > +}, section: 'Runtime executable paths')
>> 
>> summary({
>>   'perl': target_perl,
>>   'python': target_python,
>>   'shell': target_shell,
>> }, section: 'Runtime executable paths')
>> 
>> 
>> No need to check if they are found. Meson will print the full_path()
>> already, if it is found, and if it is not found, it will print "NO" in
>> its standard color code (red) for things-that-are-missing.
>
> Oh, that's much nicer indeed. Thanks!

That is a lot more pleasant to the eyes.

Thanks for working well together.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 2/2] meson: prefer shell at "/bin/sh"
  2025-04-25 14:11   ` [PATCH v3 2/2] meson: prefer shell at "/bin/sh" Patrick Steinhardt
@ 2025-04-25 17:04     ` Junio C Hamano
  2025-04-25 18:07       ` Eli Schwartz
  2025-04-25 20:13     ` brian m. carlson
  1 sibling, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2025-04-25 17:04 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Peter Seiderer, Eli Schwartz, Justin Tobler

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.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 2/2] meson: prefer shell at "/bin/sh"
  2025-04-25 17:04     ` Junio C Hamano
@ 2025-04-25 18:07       ` Eli Schwartz
  2025-04-25 18:51         ` Junio C Hamano
  2025-04-25 20:10         ` brian m. carlson
  0 siblings, 2 replies; 31+ messages in thread
From: Eli Schwartz @ 2025-04-25 18:07 UTC (permalink / raw)
  To: Junio C Hamano, Patrick Steinhardt; +Cc: git, Peter Seiderer, Justin Tobler


[-- Attachment #1.1: Type: text/plain, Size: 2549 bytes --]

On 4/25/25 1:04 PM, Junio C Hamano wrote:
> 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.


POSIX does not require a specific absolute file path for "sh", but it
does mandate that you have a shell and its name is "sh", whichever
directory it may be found in.

There is (most of the time) not actually a program called "sh". Various
different programs may provide a symlink "sh", pointing to their own shell:

- GNU Bash (bash)
- Korn Shell (ksh93)
- Policy-compliant Ordinary Shell (Debian `posh`)
- Almquist Shell (ash)
- Debian Almquist Shell (dash)
- busybox
- MirBSD Korn Shell (mksh)

(Commercial Unixes will tend to have a unique "sh" program without an
actual name, just called "$UNIX sh".)

You can call them by either name, but the general rule is that when
running an interactive command prompt you probably have your specific
favorite whereas when running a script you just need something that
complies with the POSIX spec. It's common to install dash as the /bin/sh
because it is faster than GNU Bash due to supporting much less.

There cannot be anyone who has a sh that is not spelled "sh", there are
only people who have multiple options, one of which has been assigned to
the name "sh".

If it is desirable for Git to allow people to experiment with different
shells for the git internal scripts, that is one thing (though I don't
think it's particularly useful). But there's no need to worry about
people that don't have an "sh". They have to.

Even on Solaris where /bin/sh is a non-POSIX shell that cannot run our
scripts, that is a backwards compatibility requirement and they expect
you to add /usr/xpg4/bin to the front of $PATH for applications that use
POSIX, while leaving the "broken forever" version in /bin to be used by
legacy pre-1990s applications that may still exist and haven't been
updated in well over 30 years and counting. The "sh" in $PATH at
/usr/xpg4/bin/sh is a POSIX-compliant shell. It even has the `local`
vendor extension.


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 2/2] meson: prefer shell at "/bin/sh"
  2025-04-25 18:07       ` Eli Schwartz
@ 2025-04-25 18:51         ` Junio C Hamano
  2025-04-25 22:21           ` Eli Schwartz
  2025-04-25 20:10         ` brian m. carlson
  1 sibling, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2025-04-25 18:51 UTC (permalink / raw)
  To: Eli Schwartz; +Cc: Patrick Steinhardt, git, Peter Seiderer, Justin Tobler

Eli Schwartz <eschwartz@gentoo.org> writes:

> On 4/25/25 1:04 PM, Junio C Hamano wrote:
>> 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.
>
>
> POSIX does not require a specific absolute file path for "sh", but it
> does mandate that you have a shell and its name is "sh", whichever
> directory it may be found in.
> ...
> There is (most of the time) not actually a program called "sh". Various
> different programs may provide a symlink "sh", pointing to their own shell:

Exactly.  And with many systems being personal these days, /bin/sh
may point at a shell that is better for interactive use (like
"bash"), while the user may prefer another (like "dash") scripted
use that is not pointed by that single /bin/sh symbolic link.

In any case, we live in real world where things are not strictly
POSIX.  Our Makefile does support with SHELL_PATH "sh", "dash", and
"bash" just fine.  Why shouldn't I wish for feature parity in a new
build framework that aims to at least compete and become an
alternative?


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 2/2] meson: prefer shell at "/bin/sh"
  2025-04-25 18:07       ` Eli Schwartz
  2025-04-25 18:51         ` Junio C Hamano
@ 2025-04-25 20:10         ` brian m. carlson
  2025-04-25 22:25           ` Eli Schwartz
  1 sibling, 1 reply; 31+ messages in thread
From: brian m. carlson @ 2025-04-25 20:10 UTC (permalink / raw)
  To: Eli Schwartz
  Cc: Junio C Hamano, Patrick Steinhardt, git, Peter Seiderer,
	Justin Tobler

[-- Attachment #1: Type: text/plain, Size: 2455 bytes --]

On 2025-04-25 at 18:07:18, Eli Schwartz wrote:
> On 4/25/25 1:04 PM, Junio C Hamano wrote:
> > 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.
> 
> 
> POSIX does not require a specific absolute file path for "sh", but it
> does mandate that you have a shell and its name is "sh", whichever
> directory it may be found in.
> 
> There is (most of the time) not actually a program called "sh". Various
> different programs may provide a symlink "sh", pointing to their own shell:
> 
> - GNU Bash (bash)
> - Korn Shell (ksh93)
> - Policy-compliant Ordinary Shell (Debian `posh`)
> - Almquist Shell (ash)
> - Debian Almquist Shell (dash)
> - busybox
> - MirBSD Korn Shell (mksh)

All of what you said here is true, but I will point out that AT&T ksh
(ksh93 and also ksh88) doesn't support `local`.  All of the others do,
as do other pdksh derivatives (like OpenBSD's sh and ksh[0]).

I believe on NonStop that `sh` is AT&T ksh, so there is no program or
symlink named `sh` on the system which meets our needs.  The customary
option there is to use bash instead.

Additionally, Debian allows zsh as `/bin/sh`, since it meets their
requirements, but older versions do not run all elements of a pipeline
in a subshell, which, while allowed by POSIX as an extension,
practically breaks our code (and lots of other code as well).  (New
versions contain a patch I sent that fixes this behaviour when in `sh`
mode.)  As a result, a user compiling their own Git might need to
specify something that is not `sh` on such a system.

And Junio points out correctly that some systems have Perl as `perl5`,
not `perl`.  (Mostly in environments that once had or still have Perl
4.)

So all that to say that we do need to be able to specify an arbitrary
path to a binary in order for things to work on some systems.

[0] Which are the same thing.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 2/2] meson: prefer shell at "/bin/sh"
  2025-04-25 14:11   ` [PATCH v3 2/2] meson: prefer shell at "/bin/sh" Patrick Steinhardt
  2025-04-25 17:04     ` Junio C Hamano
@ 2025-04-25 20:13     ` brian m. carlson
  1 sibling, 0 replies; 31+ messages in thread
From: brian m. carlson @ 2025-04-25 20:13 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: git, Peter Seiderer, Junio C Hamano, Eli Schwartz, Justin Tobler

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

On 2025-04-25 at 14:11:29, Patrick Steinhardt wrote:
> 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.

I think this description is much better, thanks.  I agree that choosing
`/bin/sh` is the right thing to do on Linux and the BSDs, even on
usr-merged systems (where `/bin` is always a symlink to `/usr/bin`).
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 2/2] meson: prefer shell at "/bin/sh"
  2025-04-25 18:51         ` Junio C Hamano
@ 2025-04-25 22:21           ` Eli Schwartz
  0 siblings, 0 replies; 31+ messages in thread
From: Eli Schwartz @ 2025-04-25 22:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Patrick Steinhardt, git, Peter Seiderer, Justin Tobler


[-- Attachment #1.1: Type: text/plain, Size: 1949 bytes --]

On 4/25/25 2:51 PM, Junio C Hamano wrote:
> Eli Schwartz <eschwartz@gentoo.org> writes:
>> POSIX does not require a specific absolute file path for "sh", but it
>> does mandate that you have a shell and its name is "sh", whichever
>> directory it may be found in.
>> ...
>> There is (most of the time) not actually a program called "sh". Various
>> different programs may provide a symlink "sh", pointing to their own shell:
> 
> Exactly.  And with many systems being personal these days, /bin/sh
> may point at a shell that is better for interactive use (like
> "bash"), while the user may prefer another (like "dash") scripted
> use that is not pointed by that single /bin/sh symbolic link.


I would argue 100% of interactive bash users, are running "bash" to get
it. They are not running "sh" and then rewriting all scripts in /usr/bin
from

#!/bin/sh

to

#!/usr/bin/dash

for the speedup.

The point is that if users set the "non-interactive scripts" command
/bin/sh to GNU bash, they are fine with that being used everywhere (and
it will in fact run all Git's scripts well).


> In any case, we live in real world where things are not strictly
> POSIX.  Our Makefile does support with SHELL_PATH "sh", "dash", and
> "bash" just fine.  Why shouldn't I wish for feature parity in a new
> build framework that aims to at least compete and become an
> alternative?


But it's a very fair point that it's possible to need more than just
POSIX. Meson can override program lookup:

$ cat paths.ini

[binaries]
sh = '/usr/bin/bash'

$ meson setup builddirbash/ --native-file=paths.ini
[...]
  Runtime executable paths
    perl        : /usr/bin/perl
    python      : /usr/bin/python3
    shell       : /usr/bin/bash


A dedicated build option might be more discoverable, but it's certainly
possible to override this today. Autodetecting the right one can be
tricky...


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 2/2] meson: prefer shell at "/bin/sh"
  2025-04-25 20:10         ` brian m. carlson
@ 2025-04-25 22:25           ` Eli Schwartz
  0 siblings, 0 replies; 31+ messages in thread
From: Eli Schwartz @ 2025-04-25 22:25 UTC (permalink / raw)
  To: brian m. carlson, Junio C Hamano, Patrick Steinhardt, git,
	Peter Seiderer, Justin Tobler


[-- Attachment #1.1: Type: text/plain, Size: 1803 bytes --]

On 4/25/25 4:10 PM, brian m. carlson wrote:
> All of what you said here is true, but I will point out that AT&T ksh
> (ksh93 and also ksh88) doesn't support `local`.  All of the others do,
> as do other pdksh derivatives (like OpenBSD's sh and ksh[0]).


Right, though I seem to recall e.g. all the BSDs use a pdksh variant at
least.


> I believe on NonStop that `sh` is AT&T ksh, so there is no program or
> symlink named `sh` on the system which meets our needs.  The customary
> option there is to use bash instead.


Aha. :(


> Additionally, Debian allows zsh as `/bin/sh`, since it meets their
> requirements, but older versions do not run all elements of a pipeline
> in a subshell, which, while allowed by POSIX as an extension,
> practically breaks our code (and lots of other code as well).  (New
> versions contain a patch I sent that fixes this behaviour when in `sh`
> mode.)  As a result, a user compiling their own Git might need to
> specify something that is not `sh` on such a system.


Nobody should be using zsh as /bin/sh at all, since it is not a POSIX
shell to begin with. e.g. Gentoo does not permit it. I think Debian
should treat this as a conformance bug and fix it by removing zsh
support for /bin/sh until upstream zsh makes a serious effort to conform
to POSIX (i.e. never)...


> And Junio points out correctly that some systems have Perl as `perl5`,
> not `perl`.  (Mostly in environments that once had or still have Perl
> 4.)


Yup. Same applies for overriding via a machine specification file as I
replied regarding "sh".



> So all that to say that we do need to be able to specify an arbitrary
> path to a binary in order for things to work on some systems.
> 
> [0] Which are the same thing.


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/2] meson: prefer '/bin/sh' over PATH lookup
  2025-04-25 14:11 ` [PATCH v3 " Patrick Steinhardt
  2025-04-25 14:11   ` [PATCH v3 1/2] meson: report detected runtime executable paths Patrick Steinhardt
  2025-04-25 14:11   ` [PATCH v3 2/2] meson: prefer shell at "/bin/sh" Patrick Steinhardt
@ 2025-05-02 21:16   ` Junio C Hamano
  2025-05-02 22:37     ` Eli Schwartz
  2 siblings, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2025-05-02 21:16 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Peter Seiderer, Eli Schwartz, Justin Tobler

Patrick Steinhardt <ps@pks.im> writes:

> But this made me remember the report from Peter [1] that Debian also
> faced this issue. So I decided to address the issue in Meson directly by
> preferring `/bin/sh` over a PATH-based lookup.
>
> Changes in v2:
>   - Simplify how we generate the summary.
>   - Add a comment to explain ordering of the program path.
>   - Link to v1: https://lore.kernel.org/r/20250424-pks-meson-posix-shell-v1-0-45e06ee4b6ad@pks.im
>
> Changes in v3:
>   - Stop claiming that "/bin/sh" is a POSIX-compliant path.
>   - Link to v2: https://lore.kernel.org/r/20250425-pks-meson-posix-shell-v2-0-fddc6123511b@pks.im

So the discussion seems to have died out.  Have we decided that
unlike Makefile-based approach, it is too cumbersome to teach the
Meson based approach to allow user-specified commands that have
different basename to stand in for the command we expect in the
build based on Meson [*], and what the v3 iteration of this series
does is a good place to stop?




[Footnote]

 * It is trivial to say "make SHELL_PATH=/bin/dash", but we do not
   add support for anything like 'meson -dSHELL_PATH=/bin/dash', and
   we only allow the search path for fixed-name commands to be
   configured and tell our developers that they have to write an
   extra file paths.ini just to be able to do so.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/2] meson: prefer '/bin/sh' over PATH lookup
  2025-05-02 21:16   ` [PATCH v3 0/2] meson: prefer '/bin/sh' over PATH lookup Junio C Hamano
@ 2025-05-02 22:37     ` Eli Schwartz
  2025-05-05  6:08       ` Patrick Steinhardt
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Schwartz @ 2025-05-02 22:37 UTC (permalink / raw)
  To: Junio C Hamano, Patrick Steinhardt; +Cc: git, Peter Seiderer, Justin Tobler


[-- Attachment #1.1: Type: text/plain, Size: 1479 bytes --]

On 5/2/25 5:16 PM, Junio C Hamano wrote:
> So the discussion seems to have died out.  Have we decided that
> unlike Makefile-based approach, it is too cumbersome to teach the
> Meson based approach to allow user-specified commands that have
> different basename to stand in for the command we expect in the
> build based on Meson [*], and what the v3 iteration of this series
> does is a good place to stop?


I don't have any objections to teaching git's own meson.build to do
this, and I don't think it would be particularly cumbersome. But as I'm
not the person who would use it, really, I was hoping others would state
their preferences.

...

One possibility would be if meson itself was adapted to support setting
simple "machine description" settings via the command line. I seem to
recall someone had proposed at one point on the meson ticket tracker, to
support e.g.

```
meson setup -Dbinaries.cc=gcc -Dbinaries.sh=/bin/dash
```
but I cannot recall what came of the discussion. I'll try to find the
relevant ticket after the weekend (going offline right around now).



> [Footnote]
> 
>  * It is trivial to say "make SHELL_PATH=/bin/dash", but we do not
>    add support for anything like 'meson -dSHELL_PATH=/bin/dash', and
>    we only allow the search path for fixed-name commands to be
>    configured and tell our developers that they have to write an
>    extra file paths.ini just to be able to do so.


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/2] meson: prefer '/bin/sh' over PATH lookup
  2025-05-02 22:37     ` Eli Schwartz
@ 2025-05-05  6:08       ` Patrick Steinhardt
  0 siblings, 0 replies; 31+ messages in thread
From: Patrick Steinhardt @ 2025-05-05  6:08 UTC (permalink / raw)
  To: Eli Schwartz; +Cc: Junio C Hamano, git, Peter Seiderer, Justin Tobler

On Fri, May 02, 2025 at 06:37:45PM -0400, Eli Schwartz wrote:
> On 5/2/25 5:16 PM, Junio C Hamano wrote:
> > So the discussion seems to have died out.  Have we decided that
> > unlike Makefile-based approach, it is too cumbersome to teach the
> > Meson based approach to allow user-specified commands that have
> > different basename to stand in for the command we expect in the
> > build based on Meson [*], and what the v3 iteration of this series
> > does is a good place to stop?
> 
> 
> I don't have any objections to teaching git's own meson.build to do
> this, and I don't think it would be particularly cumbersome. But as I'm
> not the person who would use it, really, I was hoping others would state
> their preferences.

It wouldn't be hard to do indeed. But I think that the proposed patch
should be good enough for now as it does what we want in basically every
usecase I can think of, and it does allow the user to tweak as required.

So I'd propose to go with the proposed pragmatic approach and then
iterate in the future if we ever see that it continues to be a problem.

> One possibility would be if meson itself was adapted to support setting
> simple "machine description" settings via the command line. I seem to
> recall someone had proposed at one point on the meson ticket tracker, to
> support e.g.
> 
> ```
> meson setup -Dbinaries.cc=gcc -Dbinaries.sh=/bin/dash
> ```
> but I cannot recall what came of the discussion. I'll try to find the
> relevant ticket after the weekend (going offline right around now).

That would be nice to have indeed.

Thanks!

Patrick

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2025-05-05  6:08 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-24 13:38 [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Patrick Steinhardt
2025-04-24 13:38 ` [PATCH 1/2] meson: report detected runtime executable paths Patrick Steinhardt
2025-04-25  0:45   ` Eli Schwartz
2025-04-25  5:21     ` Patrick Steinhardt
2025-04-25 16:34       ` Junio C Hamano
2025-04-24 13:38 ` [PATCH 2/2] meson: prefer POSIX-specified shell path Patrick Steinhardt
2025-04-24 20:18   ` Justin Tobler
2025-04-25  5:21     ` Patrick Steinhardt
2025-04-24 18:28 ` [PATCH 0/2] meson: prefer '/bin/sh' over PATH lookup Junio C Hamano
2025-04-25  5:21   ` Patrick Steinhardt
2025-04-25  5:47 ` [PATCH v2 " Patrick Steinhardt
2025-04-25  5:47   ` [PATCH v2 1/2] meson: report detected runtime executable paths Patrick Steinhardt
2025-04-25  8:27     ` Toon Claes
2025-04-25  5:47   ` [PATCH v2 2/2] meson: prefer POSIX-specified shell path Patrick Steinhardt
2025-04-25  8:35     ` Toon Claes
2025-04-25 10:49     ` brian m. carlson
2025-04-25 11:52       ` Patrick Steinhardt
2025-04-25 11:24   ` [PATCH v2 0/2] meson: prefer '/bin/sh' over PATH lookup Toon Claes
2025-04-25 14:11 ` [PATCH v3 " Patrick Steinhardt
2025-04-25 14:11   ` [PATCH v3 1/2] meson: report detected runtime executable paths Patrick Steinhardt
2025-04-25 14:11   ` [PATCH v3 2/2] meson: prefer shell at "/bin/sh" Patrick Steinhardt
2025-04-25 17:04     ` Junio C Hamano
2025-04-25 18:07       ` Eli Schwartz
2025-04-25 18:51         ` Junio C Hamano
2025-04-25 22:21           ` Eli Schwartz
2025-04-25 20:10         ` brian m. carlson
2025-04-25 22:25           ` Eli Schwartz
2025-04-25 20:13     ` brian m. carlson
2025-05-02 21:16   ` [PATCH v3 0/2] meson: prefer '/bin/sh' over PATH lookup Junio C Hamano
2025-05-02 22:37     ` Eli Schwartz
2025-05-05  6:08       ` Patrick Steinhardt

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).