Meson build leaks host 'sh' path to target build when cross-compiled

Meson build leaks host 'sh' path to target build when cross-compiled

[Peter Seiderer]

What did you do before the bug happened? (Steps to reproduce your issue)

  Cross compile git from source (git-2.48.1) using the meson build system
  (Buildroot package experimental converted from autoconf to meson).

What did you expect to happen? (Expected behavior)

  Execute command 'git fetch' on the target without failure.

What happened instead? (Actual behavior)

  Command 'git fetch' on the target failed.

What's different between what you expected and what actually happened?

  Debug with strace showed the following difference:

  - o.k (autoconf):

    679   execve("/bin/sh", ["/bin/sh", "-c", "git-upload-pack '/home/git-repo"..., "git-upload-pack '/home/git-repo"...], 0x55c342ce4420 /* 17 vars */ <unfinished ...>

  - failure (meson build):

    6861  execve("/usr/bin/sh", ["/usr/bin/sh", "-c", "git-upload-pack '/home/git-repo"..., "git-upload-pack '/home/git-repo"...], 0x5639ab382210 /* 17 vars */) = -1 ENOENT (No such file or directory)

  The meson build tries to execute the non-existent '/usr/bin/sh' (instead of
  '/bin/sh' as the autoconf build), 'which sh' on the host returns
  '/usr/bin/sh'...

  From meson.build

   [...]
   186 shell = find_program('sh', dirs: program_path)
   [...]
   685   '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',

  Do not use the result of 'find_program('sh',...)' for '-DSHELL_PATH='
  (at least not for cross-compile), use fix '/bin/sh' instead or make it
  configurable via a meson option?

Regards,
Peter


[System Info]
git version:
git version 2.48.1
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
libcurl: 8.11.1
OpenSSL: OpenSSL 3.2.3 3 Sep 2024
zlib: 1.3.1
uname: Linux 6.13.1-1-default #1 SMP PREEMPT_DYNAMIC Mon Feb  3 05:33:25 UTC 2025 (1918d13) x86_64
compiler info: gnuc: 14.2
libc info: glibc: 2.40
$SHELL (typically, interactive shell): /bin/bash


[Enabled Hooks]
not run from a git repository - no hooks to show

Re: Meson build leaks host 'sh' path to target build when cross-compiled

[Patrick Steinhardt]

On Sun, Feb 09, 2025 at 01:30:27PM +0100, Peter Seiderer wrote:
[snip]
>   The meson build tries to execute the non-existent '/usr/bin/sh' (instead of
>   '/bin/sh' as the autoconf build), 'which sh' on the host returns
>   '/usr/bin/sh'...
> 
>   From meson.build
> 
>    [...]
>    186 shell = find_program('sh', dirs: program_path)
>    [...]
>    685   '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
> 
>   Do not use the result of 'find_program('sh',...)' for '-DSHELL_PATH='
>   (at least not for cross-compile), use fix '/bin/sh' instead or make it
>   configurable via a meson option?

Hm, very true. We're mixing up concerns here by treating the build
environment and the target environment the same.

I guess the proper fix is to wire up the "native:" parameter when we
call `find_program()`, which allows us to tell Meson whether it should
find an executable for the build or the target host. And then, for those
binaries where we actually need to know about both the build and target
host's locations, we'd end up calling `find_program()` twice.

For executables that are supposed to be used on the target host Meson
would then know to first consult the cross file, which could look like
this:

    [binaries]
    sh = '/target/path/to/sh'
    perl = '/target/path/to/perl'

Meson would then pick up that file via `meson setup --cross-file
<CROSSFILE_PATH> <BUILDDIR>`.

The patch should look somewhat like the attached patch, but it conflicts
with my in-flight patch series at [1]. I'll wait for that series to be
merged to `next` before sending out the fix.

Thanks for your report!

Patrick

[1]: <20250129-b4-pks-meson-improvements-v1-0-ab709f0be12c@pks.im>

-- >8 --

diff --git a/Documentation/meson.build b/Documentation/meson.build
index c6117366ff..b033f4a93a 100644
--- a/Documentation/meson.build
+++ b/Documentation/meson.build
@@ -206,9 +206,9 @@ manpages = {
 
 docs_backend = get_option('docs_backend')
 if docs_backend == 'auto'
-  if find_program('asciidoc', dirs: program_path, required: false).found()
+  if find_program('asciidoc', dirs: program_path, native: true, required: false).found()
     docs_backend = 'asciidoc'
-  elif find_program('asciidoctor', dirs: program_path, required: false).found()
+  elif find_program('asciidoctor', dirs: program_path, native: true, required: false).found()
     docs_backend = 'asciidoctor'
   else
     error('Neither asciidoc nor asciidoctor were found.')
@@ -216,7 +216,7 @@ if docs_backend == 'auto'
 endif
 
 if docs_backend == 'asciidoc'
-  asciidoc = find_program('asciidoc', dirs: program_path)
+  asciidoc = find_program('asciidoc', native: true, dirs: program_path)
   asciidoc_html = 'xhtml11'
   asciidoc_docbook = 'docbook'
   xmlto_extra = [ ]
@@ -245,7 +245,7 @@ if docs_backend == 'asciidoc'
     asciidoc_conf,
   ]
 elif docs_backend == 'asciidoctor'
-  asciidoctor = find_program('asciidoctor', dirs: program_path)
+  asciidoctor = find_program('asciidoctor', native: true, dirs: program_path)
   asciidoc_html = 'xhtml5'
   asciidoc_docbook = 'docbook5'
   xmlto_extra = [
@@ -283,7 +283,7 @@ elif docs_backend == 'asciidoctor'
   ]
 endif
 
-xmlto = find_program('xmlto', dirs: program_path)
+xmlto = find_program('xmlto', dirs: program_path, native: true)
 
 cmd_lists = [
   'cmds-ancillaryinterrogators.txt',
@@ -404,7 +404,7 @@ if get_option('docs').contains('html')
     pointing_to: 'git.html',
   )
 
-  xsltproc = find_program('xsltproc', dirs: program_path)
+  xsltproc = find_program('xsltproc', dirs: program_path, native: true)
 
   user_manual_xml = custom_target(
     command: asciidoc_common_options + [
diff --git a/gitweb/meson.build b/gitweb/meson.build
index 89b403dc9d..88a54b4dc9 100644
--- a/gitweb/meson.build
+++ b/gitweb/meson.build
@@ -1,5 +1,5 @@
 gitweb_config = configuration_data()
-gitweb_config.set_quoted('PERL_PATH', perl.full_path())
+gitweb_config.set_quoted('PERL_PATH', target_perl.full_path())
 gitweb_config.set_quoted('CSSMIN', '')
 gitweb_config.set_quoted('JSMIN', '')
 gitweb_config.set_quoted('GIT_BINDIR', get_option('prefix') / get_option('bindir'))
diff --git a/meson.build b/meson.build
index e153a43918..5a5662bc02 100644
--- a/meson.build
+++ b/meson.build
@@ -173,7 +173,7 @@ project('git', 'c',
   # The version is only of cosmetic nature, so if we cannot find a shell yet we
   # simply don't set up a version at all. This may be the case for example on
   # Windows systems, where we first have to bootstrap the host environment.
-  version: find_program('sh', required: false).found() ? run_command(
+  version: find_program('sh', native: true, required: false).found() ? run_command(
     'GIT-VERSION-GEN', meson.current_source_dir(), '--format=@GIT_VERSION@',
     capture: true,
     check: true,
@@ -198,16 +198,18 @@ elif host_machine.system() == 'windows'
   program_path = [ 'C:/Program Files/Git/bin', 'C:/Program Files/Git/usr/bin' ]
 endif
 
-cygpath = find_program('cygpath', dirs: program_path, required: false)
-diff = find_program('diff', dirs: program_path)
-git = find_program('git', dirs: program_path, required: false)
-sed = find_program('sed', dirs: program_path)
-shell = find_program('sh', dirs: program_path)
-tar = find_program('tar', dirs: program_path)
+cygpath = find_program('cygpath', dirs: program_path, native: true, required: false)
+diff = find_program('diff', dirs: program_path, native: true)
+git = find_program('git', dirs: program_path, native: true, required: false)
+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)
 
 # Sanity-check that programs required for the build exist.
 foreach tool : ['cat', 'cut', 'grep', 'sort', 'tr', 'uname']
-  find_program(tool, dirs: program_path)
+  find_program(tool, dirs: program_path, native: true)
 endforeach
 
 script_environment = environment()
@@ -758,6 +760,7 @@ endif
 build_options_config.set_quoted('X', executable_suffix)
 
 python = import('python').find_installation('python3', required: get_option('python'))
+target_python = find_program('python3', native: false, required: python.found())
 if python.found()
   build_options_config.set('NO_PYTHON', '')
 else
@@ -775,7 +778,8 @@ endif
 
 # Note that we only set NO_PERL if the Perl features were disabled by the user.
 # It may not be set when we have found Perl, but only use it to run tests.
-perl = find_program('perl', version: '>=5.8.1', dirs: program_path, required: perl_required)
+perl = find_program('perl', version: '>=5.8.1', dirs: program_path, native: true, required: perl_required)
+target_perl = find_program('perl', version: '>=5.8.1', native: false, required: perl.found())
 perl_features_enabled = perl.found() and get_option('perl').allowed()
 if perl_features_enabled
   build_options_config.set('NO_PERL', '')
@@ -825,7 +829,7 @@ else
   build_options_config.set('NO_PTHREADS', '1')
 endif
 
-msgfmt = find_program('msgfmt', dirs: program_path, required: false)
+msgfmt = find_program('msgfmt', dirs: program_path, native: true, required: false)
 gettext_option = get_option('gettext').disable_auto_if(not msgfmt.found())
 if not msgfmt.found() and gettext_option.enabled()
   error('Internationalization via libintl requires msgfmt')
@@ -1954,9 +1958,9 @@ foreach key, value : {
   'GIT_TEST_TEMPLATE_DIR': meson.project_build_root() / 'templates',
   'GIT_TEST_TEXTDOMAINDIR': meson.project_build_root() / 'po',
   'PAGER_ENV': get_option('pager_environment'),
-  'PERL_PATH': perl.found() ? perl.full_path() : '',
-  'PYTHON_PATH': python.found () ? python.full_path() : '',
-  'SHELL_PATH': shell.full_path(),
+  'PERL_PATH': target_perl.found() ? target_perl.full_path() : '',
+  'PYTHON_PATH': target_python.found () ? target_python.full_path() : '',
+  'SHELL_PATH': target_shell.full_path(),
   'TAR': tar.full_path(),
   'TEST_OUTPUT_DIRECTORY': test_output_directory,
   'TEST_SHELL_PATH': shell.full_path(),
diff --git a/templates/meson.build b/templates/meson.build
index 1faf9a44ce..986c2e03be 100644
--- a/templates/meson.build
+++ b/templates/meson.build
@@ -1,6 +1,6 @@
 template_config = configuration_data()
-template_config.set('PERL_PATH', perl.found() ? fs.as_posix(perl.full_path()) : '')
-template_config.set('SHELL_PATH', fs.as_posix(shell.full_path()))
+template_config.set('PERL_PATH', perl.found() ? fs.as_posix(target_perl.full_path()) : '')
+template_config.set('SHELL_PATH', fs.as_posix(target_shell.full_path()))
 template_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
 
 configure_file(

Re: Meson build leaks host 'sh' path to target build when cross-compiled

[Peter Seiderer]

Hello Patrick,

On Mon, 10 Feb 2025 08:41:18 +0100, Patrick Steinhardt <ps@pks.im> wrote:

> On Sun, Feb 09, 2025 at 01:30:27PM +0100, Peter Seiderer wrote:
> [snip]
> >   The meson build tries to execute the non-existent '/usr/bin/sh' (instead of
> >   '/bin/sh' as the autoconf build), 'which sh' on the host returns
> >   '/usr/bin/sh'...
> >
> >   From meson.build
> >
> >    [...]
> >    186 shell = find_program('sh', dirs: program_path)
> >    [...]
> >    685   '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
> >
> >   Do not use the result of 'find_program('sh',...)' for '-DSHELL_PATH='
> >   (at least not for cross-compile), use fix '/bin/sh' instead or make it
> >   configurable via a meson option?
>
> Hm, very true. We're mixing up concerns here by treating the build
> environment and the target environment the same.
>
> I guess the proper fix is to wire up the "native:" parameter when we
> call `find_program()`, which allows us to tell Meson whether it should
> find an executable for the build or the target host. And then, for those
> binaries where we actually need to know about both the build and target
> host's locations, we'd end up calling `find_program()` twice.
>
> For executables that are supposed to be used on the target host Meson
> would then know to first consult the cross file, which could look like
> this:
>
>     [binaries]
>     sh = '/target/path/to/sh'
>     perl = '/target/path/to/perl'
>
> Meson would then pick up that file via `meson setup --cross-file
> <CROSSFILE_PATH> <BUILDDIR>`.

Sorry, I believe this will not work..., the description of the native
parameter in find_program ([2]) on the first sight sounds like doing the
right thing, but as far as I read the 'Cross compilation' page ([3], [4]) the
tools under the '[binaries]' section are the tools used while cross-compiling
(running on the build machine) and not the paths/tools on the target
(or as meson nomenclature host/target)...

One tiny finding below...

>
> The patch should look somewhat like the attached patch, but it conflicts
> with my in-flight patch series at [1]. I'll wait for that series to be
> merged to `next` before sending out the fix.
>
> Thanks for your report!
>
> Patrick
>
> [1]: <20250129-b4-pks-meson-improvements-v1-0-ab709f0be12c@pks.im>
>
> -- >8 --
>
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index c6117366ff..b033f4a93a 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -206,9 +206,9 @@ manpages = {
>
>  docs_backend = get_option('docs_backend')
>  if docs_backend == 'auto'
> -  if find_program('asciidoc', dirs: program_path, required: false).found()
> +  if find_program('asciidoc', dirs: program_path, native: true, required: false).found()
>      docs_backend = 'asciidoc'
> -  elif find_program('asciidoctor', dirs: program_path, required: false).found()
> +  elif find_program('asciidoctor', dirs: program_path, native: true, required: false).found()
>      docs_backend = 'asciidoctor'
>    else
>      error('Neither asciidoc nor asciidoctor were found.')
> @@ -216,7 +216,7 @@ if docs_backend == 'auto'
>  endif
>
>  if docs_backend == 'asciidoc'
> -  asciidoc = find_program('asciidoc', dirs: program_path)
> +  asciidoc = find_program('asciidoc', native: true, dirs: program_path)
>    asciidoc_html = 'xhtml11'
>    asciidoc_docbook = 'docbook'
>    xmlto_extra = [ ]
> @@ -245,7 +245,7 @@ if docs_backend == 'asciidoc'
>      asciidoc_conf,
>    ]
>  elif docs_backend == 'asciidoctor'
> -  asciidoctor = find_program('asciidoctor', dirs: program_path)
> +  asciidoctor = find_program('asciidoctor', native: true, dirs: program_path)
>    asciidoc_html = 'xhtml5'
>    asciidoc_docbook = 'docbook5'
>    xmlto_extra = [
> @@ -283,7 +283,7 @@ elif docs_backend == 'asciidoctor'
>    ]
>  endif
>
> -xmlto = find_program('xmlto', dirs: program_path)
> +xmlto = find_program('xmlto', dirs: program_path, native: true)
>
>  cmd_lists = [
>    'cmds-ancillaryinterrogators.txt',
> @@ -404,7 +404,7 @@ if get_option('docs').contains('html')
>      pointing_to: 'git.html',
>    )
>
> -  xsltproc = find_program('xsltproc', dirs: program_path)
> +  xsltproc = find_program('xsltproc', dirs: program_path, native: true)
>
>    user_manual_xml = custom_target(
>      command: asciidoc_common_options + [
> diff --git a/gitweb/meson.build b/gitweb/meson.build
> index 89b403dc9d..88a54b4dc9 100644
> --- a/gitweb/meson.build
> +++ b/gitweb/meson.build
> @@ -1,5 +1,5 @@
>  gitweb_config = configuration_data()
> -gitweb_config.set_quoted('PERL_PATH', perl.full_path())
> +gitweb_config.set_quoted('PERL_PATH', target_perl.full_path())
>  gitweb_config.set_quoted('CSSMIN', '')
>  gitweb_config.set_quoted('JSMIN', '')
>  gitweb_config.set_quoted('GIT_BINDIR', get_option('prefix') / get_option('bindir'))
> diff --git a/meson.build b/meson.build
> index e153a43918..5a5662bc02 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -173,7 +173,7 @@ project('git', 'c',
>    # The version is only of cosmetic nature, so if we cannot find a shell yet we
>    # simply don't set up a version at all. This may be the case for example on
>    # Windows systems, where we first have to bootstrap the host environment.
> -  version: find_program('sh', required: false).found() ? run_command(
> +  version: find_program('sh', native: true, required: false).found() ? run_command(
>      'GIT-VERSION-GEN', meson.current_source_dir(), '--format=@GIT_VERSION@',
>      capture: true,
>      check: true,
> @@ -198,16 +198,18 @@ elif host_machine.system() == 'windows'
>    program_path = [ 'C:/Program Files/Git/bin', 'C:/Program Files/Git/usr/bin' ]
>  endif
>
> -cygpath = find_program('cygpath', dirs: program_path, required: false)
> -diff = find_program('diff', dirs: program_path)
> -git = find_program('git', dirs: program_path, required: false)
> -sed = find_program('sed', dirs: program_path)
> -shell = find_program('sh', dirs: program_path)
> -tar = find_program('tar', dirs: program_path)
> +cygpath = find_program('cygpath', dirs: program_path, native: true, required: false)
> +diff = find_program('diff', dirs: program_path, native: true)
> +git = find_program('git', dirs: program_path, native: true, required: false)
> +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)
>
>  # Sanity-check that programs required for the build exist.
>  foreach tool : ['cat', 'cut', 'grep', 'sort', 'tr', 'uname']
> -  find_program(tool, dirs: program_path)
> +  find_program(tool, dirs: program_path, native: true)
>  endforeach
>
>  script_environment = environment()
> @@ -758,6 +760,7 @@ endif
>  build_options_config.set_quoted('X', executable_suffix)
>
>  python = import('python').find_installation('python3', required: get_option('python'))
> +target_python = find_program('python3', native: false, required: python.found())
>  if python.found()
>    build_options_config.set('NO_PYTHON', '')
>  else
> @@ -775,7 +778,8 @@ endif
>
>  # Note that we only set NO_PERL if the Perl features were disabled by the user.
>  # It may not be set when we have found Perl, but only use it to run tests.
> -perl = find_program('perl', version: '>=5.8.1', dirs: program_path, required: perl_required)
> +perl = find_program('perl', version: '>=5.8.1', dirs: program_path, native: true, required: perl_required)
> +target_perl = find_program('perl', version: '>=5.8.1', native: false, required: perl.found())
>  perl_features_enabled = perl.found() and get_option('perl').allowed()
>  if perl_features_enabled
>    build_options_config.set('NO_PERL', '')
> @@ -825,7 +829,7 @@ else
>    build_options_config.set('NO_PTHREADS', '1')
>  endif
>
> -msgfmt = find_program('msgfmt', dirs: program_path, required: false)
> +msgfmt = find_program('msgfmt', dirs: program_path, native: true, required: false)
>  gettext_option = get_option('gettext').disable_auto_if(not msgfmt.found())
>  if not msgfmt.found() and gettext_option.enabled()
>    error('Internationalization via libintl requires msgfmt')
> @@ -1954,9 +1958,9 @@ foreach key, value : {
>    'GIT_TEST_TEMPLATE_DIR': meson.project_build_root() / 'templates',
>    'GIT_TEST_TEXTDOMAINDIR': meson.project_build_root() / 'po',
>    'PAGER_ENV': get_option('pager_environment'),
> -  'PERL_PATH': perl.found() ? perl.full_path() : '',
> -  'PYTHON_PATH': python.found () ? python.full_path() : '',
> -  'SHELL_PATH': shell.full_path(),
> +  'PERL_PATH': target_perl.found() ? target_perl.full_path() : '',
> +  'PYTHON_PATH': target_python.found () ? target_python.full_path() : '',
> +  'SHELL_PATH': target_shell.full_path(),
>    'TAR': tar.full_path(),
>    'TEST_OUTPUT_DIRECTORY': test_output_directory,
>    'TEST_SHELL_PATH': shell.full_path(),
> diff --git a/templates/meson.build b/templates/meson.build
> index 1faf9a44ce..986c2e03be 100644
> --- a/templates/meson.build
> +++ b/templates/meson.build
> @@ -1,6 +1,6 @@
>  template_config = configuration_data()
> -template_config.set('PERL_PATH', perl.found() ? fs.as_posix(perl.full_path()) : '')
> -template_config.set('SHELL_PATH', fs.as_posix(shell.full_path()))
> +template_config.set('PERL_PATH', perl.found() ? fs.as_posix(target_perl.full_path()) : '')

Above should read (perl.found() vs. target_perl.found()):

   +template_config.set('PERL_PATH', target_perl.found() ? fs.as_posix(target_perl.full_path()) : '')

> +template_config.set('SHELL_PATH', fs.as_posix(target_shell.full_path()))
>  template_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
>
>  configure_file(

Regards,
Peter

[2] https://mesonbuild.com/Reference-manual_functions.html#find_program
[3] https://mesonbuild.com/Cross-compilation.html
[4] https://mesonbuild.com/Machine-files.html

Re: Meson build leaks host 'sh' path to target build when cross-compiled

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Sun, Feb 09, 2025 at 01:30:27PM +0100, Peter Seiderer wrote:
> [snip]
>>   The meson build tries to execute the non-existent '/usr/bin/sh' (instead of
>>   '/bin/sh' as the autoconf build), 'which sh' on the host returns
>>   '/usr/bin/sh'...
>> 
>>   From meson.build
>> 
>>    [...]
>>    186 shell = find_program('sh', dirs: program_path)
>>    [...]
>>    685   '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
>> 
>>   Do not use the result of 'find_program('sh',...)' for '-DSHELL_PATH='
>>   (at least not for cross-compile), use fix '/bin/sh' instead or make it
>>   configurable via a meson option?
>
> Hm, very true. We're mixing up concerns here by treating the build
> environment and the target environment the same.
> ...
> The patch should look somewhat like the attached patch, but it conflicts
> with my in-flight patch series at [1]. I'll wait for that series to be
> merged to `next` before sending out the fix.

Interesting.  When we did our make-based build, we never seriously
considered cross building into a platform where the path to the
basic tools differed between the host and target hosts.  At least in
our build procedure in olden times, I think we used to assume that
what we just built can be run inside the build procedure on the host
platform even outside the tests, which would make cross building
impossible.

Now, since we are "fixing" this aspect of the build for meson-based
build, should we also make the same fix for make-based build as well?

I'd have to say that I prefer to see it done out of pure principle
(i.e. we earlier declared that meson is not yet replacing make, so
adding new shinies only to meson-world is like making the make-world
bitrot as if we do not care).

But on the other hand, nobody complained that they cannot cross
build with make-based build seriously enough to cause us consider
doing something about it for the past 20 years, so the pragmast in
me tells me that it is not worth it doing it in make-based build.

> Thanks for your report!

Yup, thanks.

Re: Meson build leaks host 'sh' path to target build when cross-compiled

[Peter Seiderer]

Hello Junio,

On Mon, 10 Feb 2025 08:18:44 -0800, Junio C Hamano <gitster@pobox.com> wrote:

> Patrick Steinhardt <ps@pks.im> writes:
>
> > On Sun, Feb 09, 2025 at 01:30:27PM +0100, Peter Seiderer wrote:
> > [snip]
> >>   The meson build tries to execute the non-existent '/usr/bin/sh' (instead of
> >>   '/bin/sh' as the autoconf build), 'which sh' on the host returns
> >>   '/usr/bin/sh'...
> >>
> >>   From meson.build
> >>
> >>    [...]
> >>    186 shell = find_program('sh', dirs: program_path)
> >>    [...]
> >>    685   '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
> >>
> >>   Do not use the result of 'find_program('sh',...)' for '-DSHELL_PATH='
> >>   (at least not for cross-compile), use fix '/bin/sh' instead or make it
> >>   configurable via a meson option?
> >
> > Hm, very true. We're mixing up concerns here by treating the build
> > environment and the target environment the same.
> > ...
> > The patch should look somewhat like the attached patch, but it conflicts
> > with my in-flight patch series at [1]. I'll wait for that series to be
> > merged to `next` before sending out the fix.
>
> Interesting.  When we did our make-based build, we never seriously
> considered cross building into a platform where the path to the
> basic tools differed between the host and target hosts.  At least in
> our build procedure in olden times, I think we used to assume that
> what we just built can be run inside the build procedure on the host
> platform even outside the tests, which would make cross building
> impossible.
>
> Now, since we are "fixing" this aspect of the build for meson-based
> build, should we also make the same fix for make-based build as well?
>
> I'd have to say that I prefer to see it done out of pure principle
> (i.e. we earlier declared that meson is not yet replacing make, so
> adding new shinies only to meson-world is like making the make-world
> bitrot as if we do not care).
>
> But on the other hand, nobody complained that they cannot cross
> build with make-based build seriously enough to cause us consider
> doing something about it for the past 20 years, so the pragmast in
> me tells me that it is not worth it doing it in make-based build.

Maybe all doing (autoconf) cross builds where happy with the defaults
from the Makefile (SHELL_PATH = /bin/sh, PERL_PATH = /usr/bin/perl)
on host and target (as the buildroot autoconf package since 2013) and
only users doing native builds fiddled around with non-default values?

Regards,
Peter

>
> > Thanks for your report!
>
> Yup, thanks.

Re: Meson build leaks host 'sh' path to target build when cross-compiled

[Junio C Hamano]

Peter Seiderer <ps.report@gmx.net> writes:

> Maybe all doing (autoconf) cross builds where happy with the defaults
> from the Makefile (SHELL_PATH = /bin/sh, PERL_PATH = /usr/bin/perl)
> on host and target (as the buildroot autoconf package since 2013) and
> only users doing native builds fiddled around with non-default values?

I somehow doubt it.

The problem I see is that there is no distinction between "this is
the path for the shell on the target system" vs "this is the shell
we run on the host while building the package" in the Makefile.  Use
of autoconf would not magically change it; the Makefile that
includes the config.mak.autogen needs to be aware of the
distinction.

Re: Meson build leaks host 'sh' path to target build when cross-compiled

[Peter Seiderer]

Hello Junio,

On Mon, 10 Feb 2025 11:54:25 -0800, Junio C Hamano <gitster@pobox.com> wrote:

> Peter Seiderer <ps.report@gmx.net> writes:
>
> > Maybe all doing (autoconf) cross builds where happy with the defaults
> > from the Makefile (SHELL_PATH = /bin/sh, PERL_PATH = /usr/bin/perl)
> > on host and target (as the buildroot autoconf package since 2013) and
> > only users doing native builds fiddled around with non-default values?
>
> I somehow doubt it.
>
> The problem I see is that there is no distinction between "this is
> the path for the shell on the target system" vs "this is the shell
> we run on the host while building the package" in the Makefile.  Use
> of autoconf would not magically change it; the Makefile that
> includes the config.mak.autogen needs to be aware of the
> distinction.

Found no problem/special-handling in buildroot ([1]) or yocto ([2]) for
SHELL_PATH, but you are right regarding the PERL_PATH in yocto using

  38 EXTRA_OECONF = "--with-perl=${STAGING_BINDIR_NATIVE}/perl-native/perl \

and

  49 EXTRA_OEMAKE += "'PERL_PATH=/usr/bin/env perl'"

and a fixup for the resulting files

  76 perl_native_fixup () {
  77 	sed -i -e 's#${STAGING_BINDIR_NATIVE}/perl-native/#${bindir}/#' \
  78 	       -e 's#${libdir}/perl-native/#${libdir}/#' \
  79 	    ${@d.getVar("PERLTOOLS").replace(' /',d.getVar('D') + '/')}

Yes, doing a distinct handling for host/target path for sh/perl for
meson/autoconf is for sure the right thing ;-)

Regards,
Peter

[1] https://gitlab.com/buildroot.org/buildroot/-/blob/master/package/git/git.mk
[2] https://git.yoctoproject.org/poky/tree/meta/recipes-devtools/git/git_2.48.1.bb

Re: Meson build leaks host 'sh' path to target build when cross-compiled

[Patrick Steinhardt]

On Mon, Feb 10, 2025 at 12:26:03PM +0100, Peter Seiderer wrote:
> On Mon, 10 Feb 2025 08:41:18 +0100, Patrick Steinhardt <ps@pks.im> wrote:
> > On Sun, Feb 09, 2025 at 01:30:27PM +0100, Peter Seiderer wrote:
> > [snip]
> > >   The meson build tries to execute the non-existent '/usr/bin/sh' (instead of
> > >   '/bin/sh' as the autoconf build), 'which sh' on the host returns
> > >   '/usr/bin/sh'...
> > >
> > >   From meson.build
> > >
> > >    [...]
> > >    186 shell = find_program('sh', dirs: program_path)
> > >    [...]
> > >    685   '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
> > >
> > >   Do not use the result of 'find_program('sh',...)' for '-DSHELL_PATH='
> > >   (at least not for cross-compile), use fix '/bin/sh' instead or make it
> > >   configurable via a meson option?
> >
> > Hm, very true. We're mixing up concerns here by treating the build
> > environment and the target environment the same.
> >
> > I guess the proper fix is to wire up the "native:" parameter when we
> > call `find_program()`, which allows us to tell Meson whether it should
> > find an executable for the build or the target host. And then, for those
> > binaries where we actually need to know about both the build and target
> > host's locations, we'd end up calling `find_program()` twice.
> >
> > For executables that are supposed to be used on the target host Meson
> > would then know to first consult the cross file, which could look like
> > this:
> >
> >     [binaries]
> >     sh = '/target/path/to/sh'
> >     perl = '/target/path/to/perl'
> >
> > Meson would then pick up that file via `meson setup --cross-file
> > <CROSSFILE_PATH> <BUILDDIR>`.
> 
> Sorry, I believe this will not work..., the description of the native
> parameter in find_program ([2]) on the first sight sounds like doing the
> right thing, but as far as I read the 'Cross compilation' page ([3], [4]) the
> tools under the '[binaries]' section are the tools used while cross-compiling
> (running on the build machine) and not the paths/tools on the target
> (or as meson nomenclature host/target)...

Quoting the documentation of `find_program()`'s `native` parameter [1]:

    Defines how this executable should be searched. By default it is set to
    false, which causes Meson to first look for the executable in the cross
    file (when cross building) and if it is not defined there, then from the
    system. If set to true, the cross file is ignored and the program is
    only searched from the system.

So I think this should work as expected when passing the file via
`--cross-file`, shouldn't it? If we are cross-compiling we'd find the
target binaries via the cross file when `native: false`, which is
exactly what we want.

But I see what you're saying. The _intent_ is to specify the environment
of the cross-compiling environment, and not to describe the target
environment.

I can think of two alternatives:

  - We can introduce build options for this. If unset, we continue to
    use the result of `find_program()`. Otherwise, we use the value
    specified by the user.

  - We can introduce properties into the cross file that allow the user
    to specify those parameters. We can then retrieve them by calling
    `meson.get_external_property()`, but only when cross-compiling.

Let me also Cc Eli, he might have an opinion on how to do this.

> > diff --git a/templates/meson.build b/templates/meson.build
> > index 1faf9a44ce..986c2e03be 100644
> > --- a/templates/meson.build
> > +++ b/templates/meson.build
> > @@ -1,6 +1,6 @@
> >  template_config = configuration_data()
> > -template_config.set('PERL_PATH', perl.found() ? fs.as_posix(perl.full_path()) : '')
> > -template_config.set('SHELL_PATH', fs.as_posix(shell.full_path()))
> > +template_config.set('PERL_PATH', perl.found() ? fs.as_posix(target_perl.full_path()) : '')
> 
> Above should read (perl.found() vs. target_perl.found()):
> 
>    +template_config.set('PERL_PATH', target_perl.found() ? fs.as_posix(target_perl.full_path()) : '')

Ah, yes, indeed.

Patrick

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

Re: Meson build leaks host 'sh' path to target build when cross-compiled

[Eli Schwartz]

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

On 2/18/25 6:47 AM, Patrick Steinhardt wrote:
> On Mon, Feb 10, 2025 at 12:26:03PM +0100, Peter Seiderer wrote:
>> On Mon, 10 Feb 2025 08:41:18 +0100, Patrick Steinhardt <ps@pks.im> wrote:
>>> On Sun, Feb 09, 2025 at 01:30:27PM +0100, Peter Seiderer wrote:
>>> [snip]
>>>>   The meson build tries to execute the non-existent '/usr/bin/sh' (instead of
>>>>   '/bin/sh' as the autoconf build), 'which sh' on the host returns
>>>>   '/usr/bin/sh'...
>>>>
>>>>   From meson.build
>>>>
>>>>    [...]
>>>>    186 shell = find_program('sh', dirs: program_path)
>>>>    [...]
>>>>    685   '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
>>>>
>>>>   Do not use the result of 'find_program('sh',...)' for '-DSHELL_PATH='
>>>>   (at least not for cross-compile), use fix '/bin/sh' instead or make it
>>>>   configurable via a meson option?
>>>
>>> Hm, very true. We're mixing up concerns here by treating the build
>>> environment and the target environment the same.
>>>
>>> I guess the proper fix is to wire up the "native:" parameter when we
>>> call `find_program()`, which allows us to tell Meson whether it should
>>> find an executable for the build or the target host. And then, for those
>>> binaries where we actually need to know about both the build and target
>>> host's locations, we'd end up calling `find_program()` twice.
>>>
>>> For executables that are supposed to be used on the target host Meson
>>> would then know to first consult the cross file, which could look like
>>> this:
>>>
>>>     [binaries]
>>>     sh = '/target/path/to/sh'
>>>     perl = '/target/path/to/perl'
>>>
>>> Meson would then pick up that file via `meson setup --cross-file
>>> <CROSSFILE_PATH> <BUILDDIR>`.
>>
>> Sorry, I believe this will not work..., the description of the native
>> parameter in find_program ([2]) on the first sight sounds like doing the
>> right thing, but as far as I read the 'Cross compilation' page ([3], [4]) the
>> tools under the '[binaries]' section are the tools used while cross-compiling
>> (running on the build machine) and not the paths/tools on the target
>> (or as meson nomenclature host/target)...
> 
> Quoting the documentation of `find_program()`'s `native` parameter [1]:
> 
>     Defines how this executable should be searched. By default it is set to
>     false, which causes Meson to first look for the executable in the cross
>     file (when cross building) and if it is not defined there, then from the
>     system. If set to true, the cross file is ignored and the program is
>     only searched from the system.
> 
> So I think this should work as expected when passing the file via
> `--cross-file`, shouldn't it? If we are cross-compiling we'd find the
> target binaries via the cross file when `native: false`, which is
> exactly what we want.
> 
> But I see what you're saying. The _intent_ is to specify the environment
> of the cross-compiling environment, and not to describe the target
> environment.
> 
> I can think of two alternatives:
> 
>   - We can introduce build options for this. If unset, we continue to
>     use the result of `find_program()`. Otherwise, we use the value
>     specified by the user.
> 
>   - We can introduce properties into the cross file that allow the user
>     to specify those parameters. We can then retrieve them by calling
>     `meson.get_external_property()`, but only when cross-compiling.
> 
> Let me also Cc Eli, he might have an opinion on how to do this.


For the specific case of detecting sh, the portable "API filename" is
exactly /bin/sh and nothing else. It should always exist on pretty much
any unix system ever... except for the ones where it exists but is a
pre-POSIX shell and the actual POSIX one isn't always on PATH at all. Hi
there, Solaris /usr/xpg4/bin/sh ! :)

Overriding it via the cross file would be fine -- if your goal is to
only ever find_program(..., native: false) in order to detect a path and
embed it, then it doesn't matter whether cross files are for running
cross tools on the build machine or for looking up cross tools to detect
a path and embed it, since the two goals would never *come into
conflict*. And that's what actually matters -- if you are concerned that
cross files will be wrong as they specify the cross-compile environment
not the install environment, then you shouldn't be using find_program()
either, you should be exclusively using build options.



But again -- that's the general case, and for the specific case you
should be defaulting to /bin/sh

This does have a highly practical application to it. Gentoo supports
split-usr systems, and binary package support needs to consistently use
paths that are present on both merged- and split-usr systems. There is
no /usr/bin/sh on a split-usr system, but on a merged-usr system they
are the same directory, so it will exist "anyway".

Note that this is NOT a cross compile environment. I'm compiling for the
current x86-64 environment, but also tarring it up for installation on
other x86-64 environments that happen to use a different filesystem
partition scheme.


Cc Sam as we will need to fix this in our Gentoo packaging one way or
another:

$ ebuild git-2.48.1.ebuild clean install
[...]
$ ag usr/bin /var/tmp/portage/dev-vcs/git-2.48.1/image

It is finding and embedding various hits for /usr/bin/sh because I
happened to build on a merged-usr profile.



-- 
Eli Schwartz

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