git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Collection of build fixes
@ 2025-03-28  8:38 Patrick Steinhardt
  2025-03-28  8:38 ` [PATCH 1/4] meson: fix handling of '-Dcurl=auto' Patrick Steinhardt
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-28  8:38 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Johannes Schindelin

Hi,

this small patch series collects various different smallish fixes for
issues with the build systems. The intent here is to bundle all of them
into a single series to make it a bit easier for Junio to keep track of
them.

More specifically, this series:

  - Fixes an issue with handling "-Dcurl=auto" that I spotted recently.

  - Replaces Sam's "sj/meson-test-environ-fix" [1] with an alternative
    solution. The branch is currently in "seen".

  - Picks up Eli's patch from [2] to fix building docs when all optional
    Perl features have been disabled. The fix has not yet been picked up
    by Junio.

  - Picks up and massages Thorsten's patch from [3] to fix generation of
    "gitweb.js". The fix has not yet been picked up by Junio.

Please let me know if any of you are unhappy with the way I have given
credit. I'm totally happy to change authorship or adjust trailers.

Thanks!

Patrick

[1]: <310a34bace801d288e369c6a01a8d04ffc4c3c06.1741975367.git.sam@gentoo.org>
[2]: <20250316060605.166364-1-eschwartz@gentoo.org>
[3]: <070641d0-730c-7d92-af4a-9157dc1edd3d@debian.org>

---
Eli Schwartz (1):
      meson: require Perl when building docs

Patrick Steinhardt (3):
      meson: fix handling of '-Dcurl=auto'
      gitweb: fix generation of "gitweb.js"
      meson: respect 'tests' build option in contrib

 contrib/credential/netrc/meson.build | 22 ++++++++++++----------
 contrib/subtree/meson.build          | 20 +++++++++++---------
 gitweb/Makefile                      |  2 +-
 meson.build                          |  4 ++--
 4 files changed, 26 insertions(+), 22 deletions(-)


---
base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
change-id: 20250328-b4-pks-collect-build-fixes-b5a6ce086b72


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

* [PATCH 1/4] meson: fix handling of '-Dcurl=auto'
  2025-03-28  8:38 [PATCH 0/4] Collection of build fixes Patrick Steinhardt
@ 2025-03-28  8:38 ` Patrick Steinhardt
  2025-03-28  8:38 ` [PATCH 2/4] gitweb: fix generation of "gitweb.js" Patrick Steinhardt
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-28  8:38 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Johannes Schindelin

The "curl" option controls whether or not a couple of features that
depend on curl shall be included. Most importantly, these features
include the HTTP remote helpers, which are rather quintessential for a
well-functioning Git installation. So while the dependency can in theory
be dropped, most users wouldn't consider the resulting installation to
be fully functional.

The "curl" option is defined as a feature, which means that it can be
"enabled", "disabled" or "auto", which has the effect that the feature
will be enabled if the dependency itself has been found. While most of
the other features have "auto" as default value, the "curl" option is
set to "enabled" by default due to it being so important. Consequently,
autoconfiguration of Git will fail by default if the library cannot be
found.

There is a bug though with how we handle the option in case the user
overrides the feature with `meson setup -Dcurl=auto`: while we will try
to find the library in that case, we won't ever use it because we later
on check for `get_option('curl').enabled()` when deciding whether or not
we want to build dependent sources. But `enabled()` only returns true if
the option has the value "enabled", for "auto" it will return false.

Fix the issue by instead checking for `curl.found()`, which is only true
if the library has been found. And as we only try to find the library
when `get_option('curl')` returns "true" or "auto" this is exactly what
we want.

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 efe2871c9db..a8d1e63ccc6 100644
--- a/meson.build
+++ b/meson.build
@@ -1686,7 +1686,7 @@ bin_wrappers += executable('scalar',
   install_dir: get_option('libexecdir') / 'git-core',
 )
 
-if get_option('curl').enabled()
+if curl.found()
   libgit_curl = declare_dependency(
     sources: [
       'http.c',

-- 
2.49.0.472.ge94155a9ec.dirty


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

* [PATCH 2/4] gitweb: fix generation of "gitweb.js"
  2025-03-28  8:38 [PATCH 0/4] Collection of build fixes Patrick Steinhardt
  2025-03-28  8:38 ` [PATCH 1/4] meson: fix handling of '-Dcurl=auto' Patrick Steinhardt
@ 2025-03-28  8:38 ` Patrick Steinhardt
  2025-03-28  8:38 ` [PATCH 3/4] meson: require Perl when building docs Patrick Steinhardt
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-28  8:38 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Johannes Schindelin

In 19d8fe7da65 (Makefile: extract script to generate gitweb.js,
2024-12-06) we have extracted the logic to build "gitweb.js" into a
separate script. As part of that the rules that builds the script
has gained a new dependency on that script.

This refactoring is broken though because we use "$^" to determine
the set of JavaScript files that need to be concatenated, and this
implicit variable now also contains the build script itself. As a
result, the build script ends up ni the generated "gitweb.js" file,
which is wrong.

Fix the issue by filtering out non-JavaScript files.

Based-on-patch-by: Thorsten Glaser <tg@debian.org>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 gitweb/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/Makefile b/gitweb/Makefile
index d5748e93594..26a683d4421 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -118,7 +118,7 @@ $(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl
 $(MAK_DIR_GITWEB)static/gitweb.js: $(MAK_DIR_GITWEB)generate-gitweb-js.sh
 $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES))
 	$(QUIET_GEN)$(RM) $@ $@+ && \
-	$(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $^ && \
+	$(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $(filter %.js,$^) && \
 	mv $@+ $@
 
 ### Installation rules

-- 
2.49.0.472.ge94155a9ec.dirty


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

* [PATCH 3/4] meson: require Perl when building docs
  2025-03-28  8:38 [PATCH 0/4] Collection of build fixes Patrick Steinhardt
  2025-03-28  8:38 ` [PATCH 1/4] meson: fix handling of '-Dcurl=auto' Patrick Steinhardt
  2025-03-28  8:38 ` [PATCH 2/4] gitweb: fix generation of "gitweb.js" Patrick Steinhardt
@ 2025-03-28  8:38 ` Patrick Steinhardt
  2025-03-29 17:56   ` Junio C Hamano
  2025-03-28  8:38 ` [PATCH 4/4] meson: respect 'tests' build option in contrib Patrick Steinhardt
  2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
  4 siblings, 1 reply; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-28  8:38 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Johannes Schindelin

From: Eli Schwartz <eschwartz@gentoo.org>

When building our documentation we require Perl to generate the list of
commands via "cmd-list.perl". Having a Perl interpreter available is
thus mandatory when building documentation, but Meson does not enforce
this prerequisite. Thus, when all optional features that depend on Perl
are disabled, we won't look up the Perl interpreter, which will in the
end lead to an error at setup time:

```
$ meson setup builddir/ -Ddocs=man -Dperl=disabled -Dtests=false
[...]
Documentation/meson.build:308:22: ERROR: Tried to use not-found external program in "command"
```

There is already a list of other cases where we do need the Perl
interpreter. Building documentation should be one of those cases, but
is missing from the list. Add it to fix the issue.

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
Commit-message-edited-by: Patrick Steinhardt <ps@pks.im>
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 a8d1e63ccc6..51013c70de9 100644
--- a/meson.build
+++ b/meson.build
@@ -772,7 +772,7 @@ endif
 # features. It is optional if you want to neither execute tests nor use any of
 # these optional features.
 perl_required = get_option('perl')
-if get_option('tests') or get_option('gitweb').enabled() or 'netrc' in get_option('credential_helpers')
+if get_option('tests') or get_option('gitweb').enabled() or 'netrc' in get_option('credential_helpers') or get_options('docs') != []
   perl_required = true
 endif
 

-- 
2.49.0.472.ge94155a9ec.dirty


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

* [PATCH 4/4] meson: respect 'tests' build option in contrib
  2025-03-28  8:38 [PATCH 0/4] Collection of build fixes Patrick Steinhardt
                   ` (2 preceding siblings ...)
  2025-03-28  8:38 ` [PATCH 3/4] meson: require Perl when building docs Patrick Steinhardt
@ 2025-03-28  8:38 ` Patrick Steinhardt
  2025-03-28 18:25   ` Sam James
  2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
  4 siblings, 1 reply; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-28  8:38 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Johannes Schindelin

Both the "netrc" credential helper and git-subtree(1) from "contrib/"
carry a couple of tests with them. These tests get wired up in Meson
unconditionally even in the case where `-Dtests=false`. As those tests
depend on the `test_enviroment` variable, which only gets defined in
case `-Dtests=true`, the result is an error:

```
$ meson setup -Dtests=false -Dcontrib=subtree build
[...]

contrib/subtree/meson.build:15:27: ERROR: Unknown variable "test_environment".
```

Fix the issue by not defining these tests at all in case the "tests"
option is set to `false`.

Reported-by: Sam James <sam@gentoo.org>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 contrib/credential/netrc/meson.build | 22 ++++++++++++----------
 contrib/subtree/meson.build          | 20 +++++++++++---------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/contrib/credential/netrc/meson.build b/contrib/credential/netrc/meson.build
index a990dbb86da..3d74547c8ae 100644
--- a/contrib/credential/netrc/meson.build
+++ b/contrib/credential/netrc/meson.build
@@ -7,14 +7,16 @@ credential_netrc = custom_target(
   install_dir: get_option('libexecdir') / 'git-core',
 )
 
-credential_netrc_testenv = test_environment
-credential_netrc_testenv.set('CREDENTIAL_NETRC_PATH', credential_netrc.full_path())
+if get_option('tests')
+  credential_netrc_testenv = test_environment
+  credential_netrc_testenv.set('CREDENTIAL_NETRC_PATH', credential_netrc.full_path())
 
-test('t-git-credential-netrc',
-  shell,
-  args: [ meson.current_source_dir() / 't-git-credential-netrc.sh' ],
-  workdir: meson.current_source_dir(),
-  env: credential_netrc_testenv,
-  depends: test_dependencies + bin_wrappers + [credential_netrc],
-  timeout: 0,
-)
+  test('t-git-credential-netrc',
+    shell,
+    args: [ meson.current_source_dir() / 't-git-credential-netrc.sh' ],
+    workdir: meson.current_source_dir(),
+    env: credential_netrc_testenv,
+    depends: test_dependencies + bin_wrappers + [credential_netrc],
+    timeout: 0,
+  )
+endif
diff --git a/contrib/subtree/meson.build b/contrib/subtree/meson.build
index 9c72b236259..63714166a61 100644
--- a/contrib/subtree/meson.build
+++ b/contrib/subtree/meson.build
@@ -12,16 +12,18 @@ git_subtree = custom_target(
   install_dir: get_option('libexecdir') / 'git-core',
 )
 
-subtree_test_environment = test_environment
-subtree_test_environment.prepend('PATH', meson.current_build_dir())
+if get_option('tests')
+  subtree_test_environment = test_environment
+  subtree_test_environment.prepend('PATH', meson.current_build_dir())
 
-test('t7900-subtree', shell,
-  args: [ 't7900-subtree.sh' ],
-  env: subtree_test_environment,
-  workdir: meson.current_source_dir() / 't',
-  depends: test_dependencies + bin_wrappers + [ git_subtree ],
-  timeout: 0,
-)
+  test('t7900-subtree', shell,
+    args: [ 't7900-subtree.sh' ],
+    env: subtree_test_environment,
+    workdir: meson.current_source_dir() / 't',
+    depends: test_dependencies + bin_wrappers + [ git_subtree ],
+    timeout: 0,
+  )
+endif
 
 if get_option('docs').contains('man')
   subtree_xml = custom_target(

-- 
2.49.0.472.ge94155a9ec.dirty


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

* Re: [PATCH 4/4] meson: respect 'tests' build option in contrib
  2025-03-28  8:38 ` [PATCH 4/4] meson: respect 'tests' build option in contrib Patrick Steinhardt
@ 2025-03-28 18:25   ` Sam James
  0 siblings, 0 replies; 22+ messages in thread
From: Sam James @ 2025-03-28 18:25 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: git, Junio C Hamano, Eli Schwartz, Thorsten Glaser,
	Johannes Schindelin

Patrick Steinhardt <ps@pks.im> writes:

> Both the "netrc" credential helper and git-subtree(1) from "contrib/"
> carry a couple of tests with them. These tests get wired up in Meson
> unconditionally even in the case where `-Dtests=false`. As those tests
> depend on the `test_enviroment` variable, which only gets defined in
> case `-Dtests=true`, the result is an error:
>
> ```
> $ meson setup -Dtests=false -Dcontrib=subtree build
> [...]
>
> contrib/subtree/meson.build:15:27: ERROR: Unknown variable "test_environment".
> ```
>
> Fix the issue by not defining these tests at all in case the "tests"
> option is set to `false`.

Thank you! Sorry for the delay in getting back to you -- I had some
thought that I was trying to resolve about the nicer style and then
dropped the ball. LGTM.

>
> Reported-by: Sam James <sam@gentoo.org>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  contrib/credential/netrc/meson.build | 22 ++++++++++++----------
>  contrib/subtree/meson.build          | 20 +++++++++++---------
>  2 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/contrib/credential/netrc/meson.build b/contrib/credential/netrc/meson.build
> index a990dbb86da..3d74547c8ae 100644
> --- a/contrib/credential/netrc/meson.build
> +++ b/contrib/credential/netrc/meson.build
> @@ -7,14 +7,16 @@ credential_netrc = custom_target(
>    install_dir: get_option('libexecdir') / 'git-core',
>  )
>  
> -credential_netrc_testenv = test_environment
> -credential_netrc_testenv.set('CREDENTIAL_NETRC_PATH', credential_netrc.full_path())
> +if get_option('tests')
> +  credential_netrc_testenv = test_environment
> +  credential_netrc_testenv.set('CREDENTIAL_NETRC_PATH', credential_netrc.full_path())
>  
> -test('t-git-credential-netrc',
> -  shell,
> -  args: [ meson.current_source_dir() / 't-git-credential-netrc.sh' ],
> -  workdir: meson.current_source_dir(),
> -  env: credential_netrc_testenv,
> -  depends: test_dependencies + bin_wrappers + [credential_netrc],
> -  timeout: 0,
> -)
> +  test('t-git-credential-netrc',
> +    shell,
> +    args: [ meson.current_source_dir() / 't-git-credential-netrc.sh' ],
> +    workdir: meson.current_source_dir(),
> +    env: credential_netrc_testenv,
> +    depends: test_dependencies + bin_wrappers + [credential_netrc],
> +    timeout: 0,
> +  )
> +endif
> diff --git a/contrib/subtree/meson.build b/contrib/subtree/meson.build
> index 9c72b236259..63714166a61 100644
> --- a/contrib/subtree/meson.build
> +++ b/contrib/subtree/meson.build
> @@ -12,16 +12,18 @@ git_subtree = custom_target(
>    install_dir: get_option('libexecdir') / 'git-core',
>  )
>  
> -subtree_test_environment = test_environment
> -subtree_test_environment.prepend('PATH', meson.current_build_dir())
> +if get_option('tests')
> +  subtree_test_environment = test_environment
> +  subtree_test_environment.prepend('PATH', meson.current_build_dir())
>  
> -test('t7900-subtree', shell,
> -  args: [ 't7900-subtree.sh' ],
> -  env: subtree_test_environment,
> -  workdir: meson.current_source_dir() / 't',
> -  depends: test_dependencies + bin_wrappers + [ git_subtree ],
> -  timeout: 0,
> -)
> +  test('t7900-subtree', shell,
> +    args: [ 't7900-subtree.sh' ],
> +    env: subtree_test_environment,
> +    workdir: meson.current_source_dir() / 't',
> +    depends: test_dependencies + bin_wrappers + [ git_subtree ],
> +    timeout: 0,
> +  )
> +endif
>  
>  if get_option('docs').contains('man')
>    subtree_xml = custom_target(

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

* Re: [PATCH 3/4] meson: require Perl when building docs
  2025-03-28  8:38 ` [PATCH 3/4] meson: require Perl when building docs Patrick Steinhardt
@ 2025-03-29 17:56   ` Junio C Hamano
  2025-03-31  5:59     ` Patrick Steinhardt
  0 siblings, 1 reply; 22+ messages in thread
From: Junio C Hamano @ 2025-03-29 17:56 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: git, Sam James, Eli Schwartz, Thorsten Glaser,
	Johannes Schindelin

Patrick Steinhardt <ps@pks.im> writes:

> From: Eli Schwartz <eschwartz@gentoo.org>
>
> When building our documentation we require Perl to generate the list of
> commands via "cmd-list.perl". Having a Perl interpreter available is
> thus mandatory when building documentation, but Meson does not enforce
> this prerequisite. Thus, when all optional features that depend on Perl
> are disabled, we won't look up the Perl interpreter, which will in the
> end lead to an error at setup time:
>
> ```
> $ meson setup builddir/ -Ddocs=man -Dperl=disabled -Dtests=false
> [...]
> Documentation/meson.build:308:22: ERROR: Tried to use not-found external program in "command"
> ```
>
> There is already a list of other cases where we do need the Perl
> interpreter. Building documentation should be one of those cases, but
> is missing from the list. Add it to fix the issue.
>
> Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
> Commit-message-edited-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Two puzzling things.

 * How is this different from 7c8cd9c1 (meson: fix perl detection
   when docs are enabled, but perl bindings aren't, 2025-03-16)?

 * This uses get_options('docs'); shouldn't it be
   get_option('docs')?  With that changed, the patch becomes
   identical to the patch from May 16th, but the proposed log
   message seems to be vastly different.

I'll drop this step from the series for now, as the other one
already has been in 'next'.

Thanks.

> diff --git a/meson.build b/meson.build
> index a8d1e63ccc6..51013c70de9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -772,7 +772,7 @@ endif
>  # features. It is optional if you want to neither execute tests nor use any of
>  # these optional features.
>  perl_required = get_option('perl')
> -if get_option('tests') or get_option('gitweb').enabled() or 'netrc' in get_option('credential_helpers')
> +if get_option('tests') or get_option('gitweb').enabled() or 'netrc' in get_option('credential_helpers') or get_options('docs') != []
>    perl_required = true
>  endif

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

* Re: [PATCH 3/4] meson: require Perl when building docs
  2025-03-29 17:56   ` Junio C Hamano
@ 2025-03-31  5:59     ` Patrick Steinhardt
  0 siblings, 0 replies; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-31  5:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Sam James, Eli Schwartz, Thorsten Glaser,
	Johannes Schindelin

On Sat, Mar 29, 2025 at 10:56:45AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > From: Eli Schwartz <eschwartz@gentoo.org>
> >
> > When building our documentation we require Perl to generate the list of
> > commands via "cmd-list.perl". Having a Perl interpreter available is
> > thus mandatory when building documentation, but Meson does not enforce
> > this prerequisite. Thus, when all optional features that depend on Perl
> > are disabled, we won't look up the Perl interpreter, which will in the
> > end lead to an error at setup time:
> >
> > ```
> > $ meson setup builddir/ -Ddocs=man -Dperl=disabled -Dtests=false
> > [...]
> > Documentation/meson.build:308:22: ERROR: Tried to use not-found external program in "command"
> > ```
> >
> > There is already a list of other cases where we do need the Perl
> > interpreter. Building documentation should be one of those cases, but
> > is missing from the list. Add it to fix the issue.
> >
> > Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
> > Commit-message-edited-by: Patrick Steinhardt <ps@pks.im>
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  meson.build | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Two puzzling things.
> 
>  * How is this different from 7c8cd9c1 (meson: fix perl detection
>    when docs are enabled, but perl bindings aren't, 2025-03-16)?
> 
>  * This uses get_options('docs'); shouldn't it be
>    get_option('docs')?  With that changed, the patch becomes
>    identical to the patch from May 16th, but the proposed log
>    message seems to be vastly different.

Oh, yeah. I did try to double-check that the topic didn't yet end up in
'seen' or 'next', but I obviously failed.

> I'll drop this step from the series for now, as the other one
> already has been in 'next'.

Yup, makes sense, thanks!

Patrick

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

* [PATCH v2 0/5] Collection of build fixes
  2025-03-28  8:38 [PATCH 0/4] Collection of build fixes Patrick Steinhardt
                   ` (3 preceding siblings ...)
  2025-03-28  8:38 ` [PATCH 4/4] meson: respect 'tests' build option in contrib Patrick Steinhardt
@ 2025-03-31  8:33 ` Patrick Steinhardt
  2025-03-31  8:33   ` [PATCH v2 1/5] meson: fix handling of '-Dcurl=auto' Patrick Steinhardt
                     ` (6 more replies)
  4 siblings, 7 replies; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-31  8:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

Hi,

this small patch series collects various different smallish fixes for
issues with the build systems. The intent here is to bundle all of them
into a single series to make it a bit easier for Junio to keep track of
them.

More specifically, this series:

  - Fixes an issue with handling "-Dcurl=auto" that I spotted recently.

  - Replaces Sam's "sj/meson-test-environ-fix" [1] with an alternative
    solution. The branch is currently in "seen".

  - Picks up and massages Thorsten's patch from [2] to fix generation of
    "gitweb.js". The fix has not yet been picked up by Junio.

  - Picks up a cross-compilation fix for Meson [3]. There has been a bit
    of discussion with Peter whether this is the proper fix, but based
    on Eli's feedback it should be okay. I'm still open for alternative
    implementations in case anybody has suggestions for how to do them.

Please let me know if any of you are unhappy with the way I have given
credit. I'm totally happy to change authorship or adjust trailers.

Changes in v2:
  - Drop the fix for Perl-less documentation builds.
  - Pick up the fix to use correct environment in our CI builds.
    Johannes mentioned that he wants to eventually get rid of those
    builds completely, but meanwhile this is a trivial change to make
    the jobs do what they should.
  - Pick up the improvement for cross-compiling Git.
  - Link to v1: https://lore.kernel.org/r/20250328-b4-pks-collect-build-fixes-v1-0-ead9deda3fbc@pks.im

Thanks!

Patrick

[1]: <310a34bace801d288e369c6a01a8d04ffc4c3c06.1741975367.git.sam@gentoo.org>
[2]: <070641d0-730c-7d92-af4a-9157dc1edd3d@debian.org>
[3]: <20250303-pks-meson-cross-compiling-v1-1-73002ef6432e@pks.im>

---
Patrick Steinhardt (5):
      meson: fix handling of '-Dcurl=auto'
      gitweb: fix generation of "gitweb.js"
      meson: respect 'tests' build option in contrib
      meson: distinguish build and target host binaries
      ci: use Visual Studio for win+meson job on GitHub Workflows

 .github/workflows/main.yml           |  2 +-
 .gitlab-ci.yml                       |  2 +-
 Documentation/meson.build            | 12 +++----
 contrib/credential/netrc/meson.build | 22 ++++++------
 contrib/subtree/meson.build          | 20 ++++++-----
 gitweb/Makefile                      |  2 +-
 gitweb/meson.build                   |  2 +-
 meson.build                          | 68 +++++++++++++++++++++++++++---------
 templates/meson.build                |  4 +--
 9 files changed, 87 insertions(+), 47 deletions(-)

Range-diff versus v1:

1:  4bc8060a975 = 1:  3e9137c2d18 meson: fix handling of '-Dcurl=auto'
2:  4365cfc4a4e = 2:  7ba983d446e gitweb: fix generation of "gitweb.js"
3:  02d6ae13dd2 < -:  ----------- meson: require Perl when building docs
4:  fcf2478bd82 = 3:  33cd3e490eb meson: respect 'tests' build option in contrib
-:  ----------- > 4:  1cb210c91a1 meson: distinguish build and target host binaries
-:  ----------- > 5:  3172db10a10 ci: use Visual Studio for win+meson job on GitHub Workflows

---
base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
change-id: 20250328-b4-pks-collect-build-fixes-b5a6ce086b72


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

* [PATCH v2 1/5] meson: fix handling of '-Dcurl=auto'
  2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
@ 2025-03-31  8:33   ` Patrick Steinhardt
  2025-04-03  8:24     ` Karthik Nayak
  2025-03-31  8:33   ` [PATCH v2 2/5] gitweb: fix generation of "gitweb.js" Patrick Steinhardt
                     ` (5 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-31  8:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

The "curl" option controls whether or not a couple of features that
depend on curl shall be included. Most importantly, these features
include the HTTP remote helpers, which are rather quintessential for a
well-functioning Git installation. So while the dependency can in theory
be dropped, most users wouldn't consider the resulting installation to
be fully functional.

The "curl" option is defined as a feature, which means that it can be
"enabled", "disabled" or "auto", which has the effect that the feature
will be enabled if the dependency itself has been found. While most of
the other features have "auto" as default value, the "curl" option is
set to "enabled" by default due to it being so important. Consequently,
autoconfiguration of Git will fail by default if the library cannot be
found.

There is a bug though with how we handle the option in case the user
overrides the feature with `meson setup -Dcurl=auto`: while we will try
to find the library in that case, we won't ever use it because we later
on check for `get_option('curl').enabled()` when deciding whether or not
we want to build dependent sources. But `enabled()` only returns true if
the option has the value "enabled", for "auto" it will return false.

Fix the issue by instead checking for `curl.found()`, which is only true
if the library has been found. And as we only try to find the library
when `get_option('curl')` returns "true" or "auto" this is exactly what
we want.

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 efe2871c9db..a8d1e63ccc6 100644
--- a/meson.build
+++ b/meson.build
@@ -1686,7 +1686,7 @@ bin_wrappers += executable('scalar',
   install_dir: get_option('libexecdir') / 'git-core',
 )
 
-if get_option('curl').enabled()
+if curl.found()
   libgit_curl = declare_dependency(
     sources: [
       'http.c',

-- 
2.49.0.604.gff1f9ca942.dirty


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

* [PATCH v2 2/5] gitweb: fix generation of "gitweb.js"
  2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
  2025-03-31  8:33   ` [PATCH v2 1/5] meson: fix handling of '-Dcurl=auto' Patrick Steinhardt
@ 2025-03-31  8:33   ` Patrick Steinhardt
  2025-04-01 16:30     ` Johannes Schindelin
  2025-04-01 16:30     ` Toon Claes
  2025-03-31  8:33   ` [PATCH v2 3/5] meson: respect 'tests' build option in contrib Patrick Steinhardt
                     ` (4 subsequent siblings)
  6 siblings, 2 replies; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-31  8:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

In 19d8fe7da65 (Makefile: extract script to generate gitweb.js,
2024-12-06) we have extracted the logic to build "gitweb.js" into a
separate script. As part of that the rules that builds the script
has gained a new dependency on that script.

This refactoring is broken though because we use "$^" to determine
the set of JavaScript files that need to be concatenated, and this
implicit variable now also contains the build script itself. As a
result, the build script ends up ni the generated "gitweb.js" file,
which is wrong.

Fix the issue by filtering out non-JavaScript files.

Based-on-patch-by: Thorsten Glaser <tg@debian.org>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 gitweb/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/Makefile b/gitweb/Makefile
index d5748e93594..26a683d4421 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -118,7 +118,7 @@ $(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl
 $(MAK_DIR_GITWEB)static/gitweb.js: $(MAK_DIR_GITWEB)generate-gitweb-js.sh
 $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES))
 	$(QUIET_GEN)$(RM) $@ $@+ && \
-	$(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $^ && \
+	$(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $(filter %.js,$^) && \
 	mv $@+ $@
 
 ### Installation rules

-- 
2.49.0.604.gff1f9ca942.dirty


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

* [PATCH v2 3/5] meson: respect 'tests' build option in contrib
  2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
  2025-03-31  8:33   ` [PATCH v2 1/5] meson: fix handling of '-Dcurl=auto' Patrick Steinhardt
  2025-03-31  8:33   ` [PATCH v2 2/5] gitweb: fix generation of "gitweb.js" Patrick Steinhardt
@ 2025-03-31  8:33   ` Patrick Steinhardt
  2025-04-01 16:31     ` Johannes Schindelin
  2025-03-31  8:33   ` [PATCH v2 4/5] meson: distinguish build and target host binaries Patrick Steinhardt
                     ` (3 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-31  8:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

Both the "netrc" credential helper and git-subtree(1) from "contrib/"
carry a couple of tests with them. These tests get wired up in Meson
unconditionally even in the case where `-Dtests=false`. As those tests
depend on the `test_enviroment` variable, which only gets defined in
case `-Dtests=true`, the result is an error:

```
$ meson setup -Dtests=false -Dcontrib=subtree build
[...]

contrib/subtree/meson.build:15:27: ERROR: Unknown variable "test_environment".
```

Fix the issue by not defining these tests at all in case the "tests"
option is set to `false`.

Reported-by: Sam James <sam@gentoo.org>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 contrib/credential/netrc/meson.build | 22 ++++++++++++----------
 contrib/subtree/meson.build          | 20 +++++++++++---------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/contrib/credential/netrc/meson.build b/contrib/credential/netrc/meson.build
index a990dbb86da..3d74547c8ae 100644
--- a/contrib/credential/netrc/meson.build
+++ b/contrib/credential/netrc/meson.build
@@ -7,14 +7,16 @@ credential_netrc = custom_target(
   install_dir: get_option('libexecdir') / 'git-core',
 )
 
-credential_netrc_testenv = test_environment
-credential_netrc_testenv.set('CREDENTIAL_NETRC_PATH', credential_netrc.full_path())
+if get_option('tests')
+  credential_netrc_testenv = test_environment
+  credential_netrc_testenv.set('CREDENTIAL_NETRC_PATH', credential_netrc.full_path())
 
-test('t-git-credential-netrc',
-  shell,
-  args: [ meson.current_source_dir() / 't-git-credential-netrc.sh' ],
-  workdir: meson.current_source_dir(),
-  env: credential_netrc_testenv,
-  depends: test_dependencies + bin_wrappers + [credential_netrc],
-  timeout: 0,
-)
+  test('t-git-credential-netrc',
+    shell,
+    args: [ meson.current_source_dir() / 't-git-credential-netrc.sh' ],
+    workdir: meson.current_source_dir(),
+    env: credential_netrc_testenv,
+    depends: test_dependencies + bin_wrappers + [credential_netrc],
+    timeout: 0,
+  )
+endif
diff --git a/contrib/subtree/meson.build b/contrib/subtree/meson.build
index 9c72b236259..63714166a61 100644
--- a/contrib/subtree/meson.build
+++ b/contrib/subtree/meson.build
@@ -12,16 +12,18 @@ git_subtree = custom_target(
   install_dir: get_option('libexecdir') / 'git-core',
 )
 
-subtree_test_environment = test_environment
-subtree_test_environment.prepend('PATH', meson.current_build_dir())
+if get_option('tests')
+  subtree_test_environment = test_environment
+  subtree_test_environment.prepend('PATH', meson.current_build_dir())
 
-test('t7900-subtree', shell,
-  args: [ 't7900-subtree.sh' ],
-  env: subtree_test_environment,
-  workdir: meson.current_source_dir() / 't',
-  depends: test_dependencies + bin_wrappers + [ git_subtree ],
-  timeout: 0,
-)
+  test('t7900-subtree', shell,
+    args: [ 't7900-subtree.sh' ],
+    env: subtree_test_environment,
+    workdir: meson.current_source_dir() / 't',
+    depends: test_dependencies + bin_wrappers + [ git_subtree ],
+    timeout: 0,
+  )
+endif
 
 if get_option('docs').contains('man')
   subtree_xml = custom_target(

-- 
2.49.0.604.gff1f9ca942.dirty


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

* [PATCH v2 4/5] meson: distinguish build and target host binaries
  2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
                     ` (2 preceding siblings ...)
  2025-03-31  8:33   ` [PATCH v2 3/5] meson: respect 'tests' build option in contrib Patrick Steinhardt
@ 2025-03-31  8:33   ` Patrick Steinhardt
  2025-04-03  8:38     ` Karthik Nayak
  2025-03-31  8:33   ` [PATCH v2 5/5] ci: use Visual Studio for win+meson job on GitHub Workflows Patrick Steinhardt
                     ` (2 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-31  8:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

Almost all of the tools we discover during the build process need to be
native programs. There are only a handful of exceptions, which typically
are programs whose paths we need to embed into the resulting executable
so that they can be found on the target system when Git executes. While
this distinction typically doesn't matter, it does start to matter when
considering cross-compilation where the build and target machines are
different.

Meson supports cross-compilation via so-called machine files. These
machine files allow the user to override parameters for the build
machine, but also for the target machine when cross-compiling. Part of
the machine file is a section that allows the user to override the
location where binaries are to be found in the target system. The
following machine file would for example override the path of the POSIX
shell:

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

It can be handed over to Meson via `meson setup --cross-file`.

We do not handle this correctly right now though because we don't know
to distinguish binaries for the build and target hosts at all. Address
this by explicitly passing the `native:` parameter to `find_program()`:

  - When set to `true`, we get binaries discovered on the build host.

  - When set to `false`, we get either the path specified in the
    machine file. Or, if no machine file exists or it doesn't specify
    the binary path, then we fall back to the binary discovered on the
    build host.

As mentioned, only a handful of binaries are not native: only the system
shell, Python and Perl need to be treated specially here.

Reported-by: Peter Seiderer <ps.report@gmx.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/meson.build | 12 ++++-----
 gitweb/meson.build        |  2 +-
 meson.build               | 66 ++++++++++++++++++++++++++++++++++++-----------
 templates/meson.build     |  4 +--
 4 files changed, 60 insertions(+), 24 deletions(-)

diff --git a/Documentation/meson.build b/Documentation/meson.build
index 594546d68b1..32f0c5de12a 100644
--- a/Documentation/meson.build
+++ b/Documentation/meson.build
@@ -207,9 +207,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.')
@@ -217,7 +217,7 @@ if docs_backend == 'auto'
 endif
 
 if docs_backend == 'asciidoc'
-  asciidoc = find_program('asciidoc', dirs: program_path)
+  asciidoc = find_program('asciidoc', dirs: program_path, native: true)
   asciidoc_html = 'xhtml11'
   asciidoc_docbook = 'docbook'
   xmlto_extra = [ ]
@@ -246,7 +246,7 @@ if docs_backend == 'asciidoc'
     asciidoc_conf,
   ]
 elif docs_backend == 'asciidoctor'
-  asciidoctor = find_program('asciidoctor', dirs: program_path)
+  asciidoctor = find_program('asciidoctor', dirs: program_path, native: true)
   asciidoc_html = 'xhtml5'
   asciidoc_docbook = 'docbook5'
   xmlto_extra = [
@@ -288,7 +288,7 @@ if get_option('breaking_changes')
    asciidoc_common_options += ['--attribute', 'with-breaking-changes']
 endif
 
-xmlto = find_program('xmlto', dirs: program_path)
+xmlto = find_program('xmlto', dirs: program_path, native: true)
 
 cmd_lists = [
   'cmds-ancillaryinterrogators.adoc',
@@ -409,7 +409,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 89b403dc9de..88a54b4dc99 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 a8d1e63ccc6..79a50599ba8 100644
--- a/meson.build
+++ b/meson.build
@@ -155,6 +155,37 @@
 # These machine files can be passed to `meson setup` via the `--native-file`
 # option.
 #
+# Cross compilation
+# =================
+#
+# Machine files can also be used in the context of cross-compilation to
+# describe the target machine as well as the cross-compiler toolchain that
+# shall be used. An example machine file could look like the following:
+#
+#   [binaries]
+#   c = 'x86_64-w64-mingw32-gcc'
+#   cpp = 'x86_64-w64-mingw32-g++'
+#   ar = 'x86_64-w64-mingw32-ar'
+#   windres = 'x86_64-w64-mingw32-windres'
+#   strip = 'x86_64-w64-mingw32-strip'
+#   exe_wrapper = 'wine64'
+#   sh = 'C:/Program Files/Git for Windows/usr/bin/sh.exe'
+#
+#   [host_machine]
+#   system = 'windows'
+#   cpu_family = 'x86_64'
+#   cpu = 'x86_64'
+#   endian = 'little'
+#
+# These machine files can be passed to `meson setup` via the `--cross-file`
+# option.
+#
+# Note that next to the cross-compiler toolchain, the `[binaries]` section is
+# also used to locate a couple of binaries that will be built into Git. This
+# includes `sh`, `python` and `perl`, so when cross-compiling Git you likely
+# want to set these binary paths in addition to the cross-compiler toolchain
+# binaries.
+#
 # Subproject wrappers
 # ===================
 #
@@ -173,7 +204,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 +229,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()
@@ -706,7 +739,7 @@ libgit_c_args = [
   '-DGIT_LOCALE_PATH="' + get_option('localedir') + '"',
   '-DGIT_MAN_PATH="' + get_option('mandir') + '"',
   '-DPAGER_ENV="' + get_option('pager_environment') + '"',
-  '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
+  '-DSHELL_PATH="' + fs.as_posix(target_shell.full_path()) + '"',
 ]
 libgit_include_directories = [ '.' ]
 libgit_dependencies = [ ]
@@ -761,6 +794,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
@@ -790,9 +824,11 @@ endif
 # which we can do starting with Meson 1.5.0 and newer, or we have to
 # match against the minor version.
 if meson.version().version_compare('>=1.5.0')
-  perl = find_program('perl', dirs: program_path, required: perl_required, version: '>=5.26.0', version_argument: '-V:version')
+  perl = find_program('perl', dirs: program_path, native: true, required: perl_required, version: '>=5.26.0', version_argument: '-V:version')
+  target_perl = find_program('perl', dirs: program_path, native: false, required: perl.found(), version: '>=5.26.0', version_argument: '-V:version')
 else
-  perl = find_program('perl', dirs: program_path, required: perl_required, version: '>=26')
+  perl = find_program('perl', dirs: program_path, native: true, required: perl_required, version: '>=26')
+  target_perl = find_program('perl', dirs: program_path, native: false, required: perl.found(), version: '>=26')
 endif
 perl_features_enabled = perl.found() and get_option('perl').allowed()
 if perl_features_enabled
@@ -843,7 +879,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')
@@ -1974,9 +2010,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 1faf9a44cea..02e6eebe80b 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', 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(

-- 
2.49.0.604.gff1f9ca942.dirty


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

* [PATCH v2 5/5] ci: use Visual Studio for win+meson job on GitHub Workflows
  2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
                     ` (3 preceding siblings ...)
  2025-03-31  8:33   ` [PATCH v2 4/5] meson: distinguish build and target host binaries Patrick Steinhardt
@ 2025-03-31  8:33   ` Patrick Steinhardt
  2025-04-01 16:41   ` [PATCH v2 0/5] Collection of build fixes Johannes Schindelin
  2025-04-03  8:39   ` Karthik Nayak
  6 siblings, 0 replies; 22+ messages in thread
From: Patrick Steinhardt @ 2025-03-31  8:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

In 7304bd2bc39 (ci: wire up Visual Studio build with Meson, 2025-01-22)
we have wired up a new CI job that builds and tests Git with Meson on a
Windows machine. The expectation here was that this build uses the
Visual Studio toolchain to do so, and that is true on GitLab CI. But on
GitHub Workflows it is not the case because we've got GCC in our PATH,
and thus Meson favors that compiler toolchain over Visual Studio's.

Fix this by explicitly asking Meson to use the Visual Studio toolchain.
While this is only really required for GitHub Workflows, let's also pass
the flag in GitLab CI so that we don't implicitly assume the toolchain
that Meson is going to pick.

Reported-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 2 +-
 .gitlab-ci.yml             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 9959b61ece2..6a002485aeb 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -265,7 +265,7 @@ jobs:
       run: pip install meson ninja
     - name: Setup
       shell: pwsh
-      run: meson setup build -Dperl=disabled -Dcredential_helpers=wincred
+      run: meson setup build --vsenv -Dperl=disabled -Dcredential_helpers=wincred
     - name: Compile
       shell: pwsh
       run: meson compile -C build
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 2805cdeecb6..4798b283745 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -164,7 +164,7 @@ build:msvc-meson:
   extends: .msvc-meson
   stage: build
   script:
-    - meson setup build -Dperl=disabled -Dbackend_max_links=1 -Dcredential_helpers=wincred
+    - meson setup build --vsenv -Dperl=disabled -Dbackend_max_links=1 -Dcredential_helpers=wincred
     - meson compile -C build
   artifacts:
     paths:

-- 
2.49.0.604.gff1f9ca942.dirty


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

* Re: [PATCH v2 2/5] gitweb: fix generation of "gitweb.js"
  2025-03-31  8:33   ` [PATCH v2 2/5] gitweb: fix generation of "gitweb.js" Patrick Steinhardt
@ 2025-04-01 16:30     ` Johannes Schindelin
  2025-04-02  6:40       ` Patrick Steinhardt
  2025-04-01 16:30     ` Toon Claes
  1 sibling, 1 reply; 22+ messages in thread
From: Johannes Schindelin @ 2025-04-01 16:30 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: git, Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer

Hi Patrick,

On Mon, 31 Mar 2025, Patrick Steinhardt wrote:

> diff --git a/gitweb/Makefile b/gitweb/Makefile
> index d5748e93594..26a683d4421 100644
> --- a/gitweb/Makefile
> +++ b/gitweb/Makefile
> @@ -118,7 +118,7 @@ $(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl
>  $(MAK_DIR_GITWEB)static/gitweb.js: $(MAK_DIR_GITWEB)generate-gitweb-js.sh
>  $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES))
>  	$(QUIET_GEN)$(RM) $@ $@+ && \
> -	$(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $^ && \
> +	$(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $(filter %.js,$^) && \
>  	mv $@+ $@

A safer way might be to use `$(filter-out %.sh,$^)` just in case the
Javascript libraries might at some stage be renamed (I could imagine, for
example, that someone aims for ideological purity and renames them to
`*.cjs`).

Ciao,
Johannes

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

* Re: [PATCH v2 2/5] gitweb: fix generation of "gitweb.js"
  2025-03-31  8:33   ` [PATCH v2 2/5] gitweb: fix generation of "gitweb.js" Patrick Steinhardt
  2025-04-01 16:30     ` Johannes Schindelin
@ 2025-04-01 16:30     ` Toon Claes
  1 sibling, 0 replies; 22+ messages in thread
From: Toon Claes @ 2025-04-01 16:30 UTC (permalink / raw)
  To: Patrick Steinhardt, git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

Patrick Steinhardt <ps@pks.im> writes:

> In 19d8fe7da65 (Makefile: extract script to generate gitweb.js,
> 2024-12-06) we have extracted the logic to build "gitweb.js" into a
> separate script. As part of that the rules that builds the script
> has gained a new dependency on that script.
>
> This refactoring is broken though because we use "$^" to determine
> the set of JavaScript files that need to be concatenated, and this
> implicit variable now also contains the build script itself. As a
> result, the build script ends up ni the generated "gitweb.js" file,

Tiniest typo: ni -> in

But that's all I've got about this patch series. :+1:

-- 
Toon

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

* Re: [PATCH v2 3/5] meson: respect 'tests' build option in contrib
  2025-03-31  8:33   ` [PATCH v2 3/5] meson: respect 'tests' build option in contrib Patrick Steinhardt
@ 2025-04-01 16:31     ` Johannes Schindelin
  0 siblings, 0 replies; 22+ messages in thread
From: Johannes Schindelin @ 2025-04-01 16:31 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: git, Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer

Hi Patrick,

On Mon, 31 Mar 2025, Patrick Steinhardt wrote:

> Both the "netrc" credential helper and git-subtree(1) from "contrib/"
> carry a couple of tests with them. These tests get wired up in Meson
> unconditionally even in the case where `-Dtests=false`. As those tests
> depend on the `test_enviroment` variable, which only gets defined in
> case `-Dtests=true`, the result is an error:
>
> ```
> $ meson setup -Dtests=false -Dcontrib=subtree build
> [...]
>
> contrib/subtree/meson.build:15:27: ERROR: Unknown variable "test_environment".
> ```
>
> Fix the issue by not defining these tests at all in case the "tests"
> option is set to `false`.

Sounds good, and the patch looks good to me (it would look even better if
I had proper code reviewing tools here where I could use the `-w` option,
but mailing lists do not offer such tools).

Ciao,
Johannes

>
> Reported-by: Sam James <sam@gentoo.org>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  contrib/credential/netrc/meson.build | 22 ++++++++++++----------
>  contrib/subtree/meson.build          | 20 +++++++++++---------
>  2 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/contrib/credential/netrc/meson.build b/contrib/credential/netrc/meson.build
> index a990dbb86da..3d74547c8ae 100644
> --- a/contrib/credential/netrc/meson.build
> +++ b/contrib/credential/netrc/meson.build
> @@ -7,14 +7,16 @@ credential_netrc = custom_target(
>    install_dir: get_option('libexecdir') / 'git-core',
>  )
>
> -credential_netrc_testenv = test_environment
> -credential_netrc_testenv.set('CREDENTIAL_NETRC_PATH', credential_netrc.full_path())
> +if get_option('tests')
> +  credential_netrc_testenv = test_environment
> +  credential_netrc_testenv.set('CREDENTIAL_NETRC_PATH', credential_netrc.full_path())
>
> -test('t-git-credential-netrc',
> -  shell,
> -  args: [ meson.current_source_dir() / 't-git-credential-netrc.sh' ],
> -  workdir: meson.current_source_dir(),
> -  env: credential_netrc_testenv,
> -  depends: test_dependencies + bin_wrappers + [credential_netrc],
> -  timeout: 0,
> -)
> +  test('t-git-credential-netrc',
> +    shell,
> +    args: [ meson.current_source_dir() / 't-git-credential-netrc.sh' ],
> +    workdir: meson.current_source_dir(),
> +    env: credential_netrc_testenv,
> +    depends: test_dependencies + bin_wrappers + [credential_netrc],
> +    timeout: 0,
> +  )
> +endif
> diff --git a/contrib/subtree/meson.build b/contrib/subtree/meson.build
> index 9c72b236259..63714166a61 100644
> --- a/contrib/subtree/meson.build
> +++ b/contrib/subtree/meson.build
> @@ -12,16 +12,18 @@ git_subtree = custom_target(
>    install_dir: get_option('libexecdir') / 'git-core',
>  )
>
> -subtree_test_environment = test_environment
> -subtree_test_environment.prepend('PATH', meson.current_build_dir())
> +if get_option('tests')
> +  subtree_test_environment = test_environment
> +  subtree_test_environment.prepend('PATH', meson.current_build_dir())
>
> -test('t7900-subtree', shell,
> -  args: [ 't7900-subtree.sh' ],
> -  env: subtree_test_environment,
> -  workdir: meson.current_source_dir() / 't',
> -  depends: test_dependencies + bin_wrappers + [ git_subtree ],
> -  timeout: 0,
> -)
> +  test('t7900-subtree', shell,
> +    args: [ 't7900-subtree.sh' ],
> +    env: subtree_test_environment,
> +    workdir: meson.current_source_dir() / 't',
> +    depends: test_dependencies + bin_wrappers + [ git_subtree ],
> +    timeout: 0,
> +  )
> +endif
>
>  if get_option('docs').contains('man')
>    subtree_xml = custom_target(
>
> --
> 2.49.0.604.gff1f9ca942.dirty
>
>
>

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

* Re: [PATCH v2 0/5] Collection of build fixes
  2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
                     ` (4 preceding siblings ...)
  2025-03-31  8:33   ` [PATCH v2 5/5] ci: use Visual Studio for win+meson job on GitHub Workflows Patrick Steinhardt
@ 2025-04-01 16:41   ` Johannes Schindelin
  2025-04-03  8:39   ` Karthik Nayak
  6 siblings, 0 replies; 22+ messages in thread
From: Johannes Schindelin @ 2025-04-01 16:41 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: git, Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer

Hi Patrick,

On Mon, 31 Mar 2025, Patrick Steinhardt wrote:

> this small patch series collects various different smallish fixes for
> issues with the build systems. The intent here is to bundle all of them
> into a single series to make it a bit easier for Junio to keep track of
> them.
>
> More specifically, this series:
>
>   - Fixes an issue with handling "-Dcurl=auto" that I spotted recently.
>
>   - Replaces Sam's "sj/meson-test-environ-fix" [1] with an alternative
>     solution. The branch is currently in "seen".
>
>   - Picks up and massages Thorsten's patch from [2] to fix generation of
>     "gitweb.js". The fix has not yet been picked up by Junio.
>
>   - Picks up a cross-compilation fix for Meson [3]. There has been a bit
>     of discussion with Peter whether this is the proper fix, but based
>     on Eli's feedback it should be okay. I'm still open for alternative
>     implementations in case anybody has suggestions for how to do them.

These all look good to me (with the exception of the cross-compilation
fix, but only because I am too unfamiliar with Meson to speak about the
correctness of this patch).

>
> Please let me know if any of you are unhappy with the way I have given
> credit. I'm totally happy to change authorship or adjust trailers.
>
> Changes in v2:
>   - Drop the fix for Perl-less documentation builds.
>   - Pick up the fix to use correct environment in our CI builds.
>     Johannes mentioned that he wants to eventually get rid of those
>     builds completely, but meanwhile this is a trivial change to make
>     the jobs do what they should.

More precisely, since we are now spending around 9.5 hours of total CPU
time for every single CI build (completely running over the concurrency
limit of 20 parallel jobs on GitHub even for a single CI run, with the
obvious congestion when there are parallel CI runs), I want to drop all
pretense that Git supports CMake builds on Windows (or Visual Studio
builds, for that matter). It's just too much of an uphill battle and I no
longer have the will to deal with it.

Thank you for reminding me that I wanted to work on that patch.

Ciao,
Johannes

>   - Pick up the improvement for cross-compiling Git.
>   - Link to v1: https://lore.kernel.org/r/20250328-b4-pks-collect-build-fixes-v1-0-ead9deda3fbc@pks.im
>
> Thanks!
>
> Patrick
>
> [1]: <310a34bace801d288e369c6a01a8d04ffc4c3c06.1741975367.git.sam@gentoo.org>
> [2]: <070641d0-730c-7d92-af4a-9157dc1edd3d@debian.org>
> [3]: <20250303-pks-meson-cross-compiling-v1-1-73002ef6432e@pks.im>
>
> ---
> Patrick Steinhardt (5):
>       meson: fix handling of '-Dcurl=auto'
>       gitweb: fix generation of "gitweb.js"
>       meson: respect 'tests' build option in contrib
>       meson: distinguish build and target host binaries
>       ci: use Visual Studio for win+meson job on GitHub Workflows
>
>  .github/workflows/main.yml           |  2 +-
>  .gitlab-ci.yml                       |  2 +-
>  Documentation/meson.build            | 12 +++----
>  contrib/credential/netrc/meson.build | 22 ++++++------
>  contrib/subtree/meson.build          | 20 ++++++-----
>  gitweb/Makefile                      |  2 +-
>  gitweb/meson.build                   |  2 +-
>  meson.build                          | 68 +++++++++++++++++++++++++++---------
>  templates/meson.build                |  4 +--
>  9 files changed, 87 insertions(+), 47 deletions(-)
>
> Range-diff versus v1:
>
> 1:  4bc8060a975 = 1:  3e9137c2d18 meson: fix handling of '-Dcurl=auto'
> 2:  4365cfc4a4e = 2:  7ba983d446e gitweb: fix generation of "gitweb.js"
> 3:  02d6ae13dd2 < -:  ----------- meson: require Perl when building docs
> 4:  fcf2478bd82 = 3:  33cd3e490eb meson: respect 'tests' build option in contrib
> -:  ----------- > 4:  1cb210c91a1 meson: distinguish build and target host binaries
> -:  ----------- > 5:  3172db10a10 ci: use Visual Studio for win+meson job on GitHub Workflows
>
> ---
> base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
> change-id: 20250328-b4-pks-collect-build-fixes-b5a6ce086b72
>
>
>

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

* Re: [PATCH v2 2/5] gitweb: fix generation of "gitweb.js"
  2025-04-01 16:30     ` Johannes Schindelin
@ 2025-04-02  6:40       ` Patrick Steinhardt
  0 siblings, 0 replies; 22+ messages in thread
From: Patrick Steinhardt @ 2025-04-02  6:40 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer

On Tue, Apr 01, 2025 at 06:30:01PM +0200, Johannes Schindelin wrote:
> Hi Patrick,
> 
> On Mon, 31 Mar 2025, Patrick Steinhardt wrote:
> 
> > diff --git a/gitweb/Makefile b/gitweb/Makefile
> > index d5748e93594..26a683d4421 100644
> > --- a/gitweb/Makefile
> > +++ b/gitweb/Makefile
> > @@ -118,7 +118,7 @@ $(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl
> >  $(MAK_DIR_GITWEB)static/gitweb.js: $(MAK_DIR_GITWEB)generate-gitweb-js.sh
> >  $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES))
> >  	$(QUIET_GEN)$(RM) $@ $@+ && \
> > -	$(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $^ && \
> > +	$(MAK_DIR_GITWEB)generate-gitweb-js.sh $@+ $(filter %.js,$^) && \
> >  	mv $@+ $@
> 
> A safer way might be to use `$(filter-out %.sh,$^)` just in case the
> Javascript libraries might at some stage be renamed (I could imagine, for
> example, that someone aims for ideological purity and renames them to
> `*.cjs`).

I could see arguments both ways:

  - If we use "filter-out" the developer now has to remember to also
    filter out files whenever a new dependency is added.

  - If we use "filter" the developer has to remember to update the
    pattern if any of the files are renamed.

I think the developer is going to be more on the guard in the second
case -- after all, renaming files always requires you to also update the
build instructions. On the other hand it's quite easy to miss that you
have to adapt the "filter-out" logic when adding a new dependency. In
the end neither of these solutions is perfect, but the worst part is
that we don't have any tests at all that would detect a broken build.

So I lean towards keeping the current mechanism, but don't feel strongly
about it. Let me know in case you still prefer "filter-out" and I'll
adapt accordingly.

Patrick

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

* Re: [PATCH v2 1/5] meson: fix handling of '-Dcurl=auto'
  2025-03-31  8:33   ` [PATCH v2 1/5] meson: fix handling of '-Dcurl=auto' Patrick Steinhardt
@ 2025-04-03  8:24     ` Karthik Nayak
  0 siblings, 0 replies; 22+ messages in thread
From: Karthik Nayak @ 2025-04-03  8:24 UTC (permalink / raw)
  To: Patrick Steinhardt, git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

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

Patrick Steinhardt <ps@pks.im> writes:

> The "curl" option controls whether or not a couple of features that
> depend on curl shall be included. Most importantly, these features
> include the HTTP remote helpers, which are rather quintessential for a
> well-functioning Git installation. So while the dependency can in theory
> be dropped, most users wouldn't consider the resulting installation to
> be fully functional.
>
> The "curl" option is defined as a feature, which means that it can be
> "enabled", "disabled" or "auto", which has the effect that the feature
> will be enabled if the dependency itself has been found. While most of
> the other features have "auto" as default value, the "curl" option is
> set to "enabled" by default due to it being so important. Consequently,
> autoconfiguration of Git will fail by default if the library cannot be
> found.
>
> There is a bug though with how we handle the option in case the user
> overrides the feature with `meson setup -Dcurl=auto`: while we will try
> to find the library in that case, we won't ever use it because we later
> on check for `get_option('curl').enabled()` when deciding whether or not
> we want to build dependent sources. But `enabled()` only returns true if
> the option has the value "enabled", for "auto" it will return false.
>
> Fix the issue by instead checking for `curl.found()`, which is only true
> if the library has been found. And as we only try to find the library
> when `get_option('curl')` returns "true" or "auto" this is exactly what
> we want.
>
> 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 efe2871c9db..a8d1e63ccc6 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1686,7 +1686,7 @@ bin_wrappers += executable('scalar',
>    install_dir: get_option('libexecdir') / 'git-core',
>  )
>
> -if get_option('curl').enabled()
> +if curl.found()
>    libgit_curl = declare_dependency(
>      sources: [
>        'http.c',
>

So here, curl is defined as a dependency on 'libcurl'. Ok makes sense.

> --
> 2.49.0.604.gff1f9ca942.dirty

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

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

* Re: [PATCH v2 4/5] meson: distinguish build and target host binaries
  2025-03-31  8:33   ` [PATCH v2 4/5] meson: distinguish build and target host binaries Patrick Steinhardt
@ 2025-04-03  8:38     ` Karthik Nayak
  0 siblings, 0 replies; 22+ messages in thread
From: Karthik Nayak @ 2025-04-03  8:38 UTC (permalink / raw)
  To: Patrick Steinhardt, git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

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

Patrick Steinhardt <ps@pks.im> writes:

> Almost all of the tools we discover during the build process need to be
> native programs. There are only a handful of exceptions, which typically
> are programs whose paths we need to embed into the resulting executable
> so that they can be found on the target system when Git executes. While
> this distinction typically doesn't matter, it does start to matter when
> considering cross-compilation where the build and target machines are
> different.
>
> Meson supports cross-compilation via so-called machine files. These
> machine files allow the user to override parameters for the build
> machine, but also for the target machine when cross-compiling. Part of
> the machine file is a section that allows the user to override the
> location where binaries are to be found in the target system. The
> following machine file would for example override the path of the POSIX
> shell:
>
>     [binaries]
>     sh = '/usr/xpg4/bin/sh'
>
> It can be handed over to Meson via `meson setup --cross-file`.
>
> We do not handle this correctly right now though because we don't know
> to distinguish binaries for the build and target hosts at all. Address
> this by explicitly passing the `native:` parameter to `find_program()`:
>
>   - When set to `true`, we get binaries discovered on the build host.
>
>   - When set to `false`, we get either the path specified in the
>     machine file. Or, if no machine file exists or it doesn't specify
>     the binary path, then we fall back to the binary discovered on the
>     build host.
>
> As mentioned, only a handful of binaries are not native: only the system
> shell, Python and Perl need to be treated specially here.
>

These are not native because they'll be run onthe target machine, while
the rest are run on the host machine. Makes sense.

> Reported-by: Peter Seiderer <ps.report@gmx.net>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Documentation/meson.build | 12 ++++-----
>  gitweb/meson.build        |  2 +-
>  meson.build               | 66 ++++++++++++++++++++++++++++++++++++-----------
>  templates/meson.build     |  4 +--
>  4 files changed, 60 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index 594546d68b1..32f0c5de12a 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -207,9 +207,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.')
> @@ -217,7 +217,7 @@ if docs_backend == 'auto'
>  endif
>
>  if docs_backend == 'asciidoc'
> -  asciidoc = find_program('asciidoc', dirs: program_path)
> +  asciidoc = find_program('asciidoc', dirs: program_path, native: true)
>    asciidoc_html = 'xhtml11'
>    asciidoc_docbook = 'docbook'
>    xmlto_extra = [ ]
> @@ -246,7 +246,7 @@ if docs_backend == 'asciidoc'
>      asciidoc_conf,
>    ]
>  elif docs_backend == 'asciidoctor'
> -  asciidoctor = find_program('asciidoctor', dirs: program_path)
> +  asciidoctor = find_program('asciidoctor', dirs: program_path, native: true)
>    asciidoc_html = 'xhtml5'
>    asciidoc_docbook = 'docbook5'
>    xmlto_extra = [
> @@ -288,7 +288,7 @@ if get_option('breaking_changes')
>     asciidoc_common_options += ['--attribute', 'with-breaking-changes']
>  endif
>
> -xmlto = find_program('xmlto', dirs: program_path)
> +xmlto = find_program('xmlto', dirs: program_path, native: true)
>
>  cmd_lists = [
>    'cmds-ancillaryinterrogators.adoc',
> @@ -409,7 +409,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 89b403dc9de..88a54b4dc99 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 a8d1e63ccc6..79a50599ba8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -155,6 +155,37 @@
>  # These machine files can be passed to `meson setup` via the `--native-file`
>  # option.
>  #
> +# Cross compilation
> +# =================
> +#
> +# Machine files can also be used in the context of cross-compilation to
> +# describe the target machine as well as the cross-compiler toolchain that
> +# shall be used. An example machine file could look like the following:
> +#
> +#   [binaries]
> +#   c = 'x86_64-w64-mingw32-gcc'
> +#   cpp = 'x86_64-w64-mingw32-g++'
> +#   ar = 'x86_64-w64-mingw32-ar'
> +#   windres = 'x86_64-w64-mingw32-windres'
> +#   strip = 'x86_64-w64-mingw32-strip'
> +#   exe_wrapper = 'wine64'
> +#   sh = 'C:/Program Files/Git for Windows/usr/bin/sh.exe'
> +#
> +#   [host_machine]
> +#   system = 'windows'
> +#   cpu_family = 'x86_64'
> +#   cpu = 'x86_64'
> +#   endian = 'little'
> +#
> +# These machine files can be passed to `meson setup` via the `--cross-file`
> +# option.
> +#
> +# Note that next to the cross-compiler toolchain, the `[binaries]` section is
> +# also used to locate a couple of binaries that will be built into Git. This
> +# includes `sh`, `python` and `perl`, so when cross-compiling Git you likely
> +# want to set these binary paths in addition to the cross-compiler toolchain
> +# binaries.
> +#
>  # Subproject wrappers
>  # ===================
>  #
> @@ -173,7 +204,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(

While we generally want a target shell, this one is simply to get the
verson during build time. Makes sense.

>      'GIT-VERSION-GEN', meson.current_source_dir(), '--format=@GIT_VERSION@',
>      capture: true,
>      check: true,
> @@ -198,16 +229,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()
> @@ -706,7 +739,7 @@ libgit_c_args = [
>    '-DGIT_LOCALE_PATH="' + get_option('localedir') + '"',
>    '-DGIT_MAN_PATH="' + get_option('mandir') + '"',
>    '-DPAGER_ENV="' + get_option('pager_environment') + '"',
> -  '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
> +  '-DSHELL_PATH="' + fs.as_posix(target_shell.full_path()) + '"',
>  ]
>  libgit_include_directories = [ '.' ]
>  libgit_dependencies = [ ]
> @@ -761,6 +794,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
> @@ -790,9 +824,11 @@ endif
>  # which we can do starting with Meson 1.5.0 and newer, or we have to
>  # match against the minor version.
>  if meson.version().version_compare('>=1.5.0')
> -  perl = find_program('perl', dirs: program_path, required: perl_required, version: '>=5.26.0', version_argument: '-V:version')
> +  perl = find_program('perl', dirs: program_path, native: true, required: perl_required, version: '>=5.26.0', version_argument: '-V:version')
> +  target_perl = find_program('perl', dirs: program_path, native: false, required: perl.found(), version: '>=5.26.0', version_argument: '-V:version')
>  else
> -  perl = find_program('perl', dirs: program_path, required: perl_required, version: '>=26')
> +  perl = find_program('perl', dirs: program_path, native: true, required: perl_required, version: '>=26')
> +  target_perl = find_program('perl', dirs: program_path, native: false, required: perl.found(), version: '>=26')
>  endif
>  perl_features_enabled = perl.found() and get_option('perl').allowed()
>  if perl_features_enabled
> @@ -843,7 +879,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')
> @@ -1974,9 +2010,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 1faf9a44cea..02e6eebe80b 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', 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(
>
> --
> 2.49.0.604.gff1f9ca942.dirty

Looks good!

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

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

* Re: [PATCH v2 0/5] Collection of build fixes
  2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
                     ` (5 preceding siblings ...)
  2025-04-01 16:41   ` [PATCH v2 0/5] Collection of build fixes Johannes Schindelin
@ 2025-04-03  8:39   ` Karthik Nayak
  6 siblings, 0 replies; 22+ messages in thread
From: Karthik Nayak @ 2025-04-03  8:39 UTC (permalink / raw)
  To: Patrick Steinhardt, git
  Cc: Junio C Hamano, Sam James, Eli Schwartz, Thorsten Glaser,
	Peter Seiderer, Johannes Schindelin

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

Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> this small patch series collects various different smallish fixes for
> issues with the build systems. The intent here is to bundle all of them
> into a single series to make it a bit easier for Junio to keep track of
> them.
>
> More specifically, this series:
>
>   - Fixes an issue with handling "-Dcurl=auto" that I spotted recently.
>
>   - Replaces Sam's "sj/meson-test-environ-fix" [1] with an alternative
>     solution. The branch is currently in "seen".
>
>   - Picks up and massages Thorsten's patch from [2] to fix generation of
>     "gitweb.js". The fix has not yet been picked up by Junio.
>
>   - Picks up a cross-compilation fix for Meson [3]. There has been a bit
>     of discussion with Peter whether this is the proper fix, but based
>     on Eli's feedback it should be okay. I'm still open for alternative
>     implementations in case anybody has suggestions for how to do them.
>
> Please let me know if any of you are unhappy with the way I have given
> credit. I'm totally happy to change authorship or adjust trailers.
>

These patches look good to me, as someone who is getting upto date with
the meson build system, I really appreciate the commit messages.

> Changes in v2:
>   - Drop the fix for Perl-less documentation builds.
>   - Pick up the fix to use correct environment in our CI builds.
>     Johannes mentioned that he wants to eventually get rid of those
>     builds completely, but meanwhile this is a trivial change to make
>     the jobs do what they should.
>   - Pick up the improvement for cross-compiling Git.
>   - Link to v1: https://lore.kernel.org/r/20250328-b4-pks-collect-build-fixes-v1-0-ead9deda3fbc@pks.im
>
> Thanks!
>
> Patrick
>
> [1]: <310a34bace801d288e369c6a01a8d04ffc4c3c06.1741975367.git.sam@gentoo.org>
> [2]: <070641d0-730c-7d92-af4a-9157dc1edd3d@debian.org>
> [3]: <20250303-pks-meson-cross-compiling-v1-1-73002ef6432e@pks.im>
>
> ---
> Patrick Steinhardt (5):
>       meson: fix handling of '-Dcurl=auto'
>       gitweb: fix generation of "gitweb.js"
>       meson: respect 'tests' build option in contrib
>       meson: distinguish build and target host binaries
>       ci: use Visual Studio for win+meson job on GitHub Workflows
>
>  .github/workflows/main.yml           |  2 +-
>  .gitlab-ci.yml                       |  2 +-
>  Documentation/meson.build            | 12 +++----
>  contrib/credential/netrc/meson.build | 22 ++++++------
>  contrib/subtree/meson.build          | 20 ++++++-----
>  gitweb/Makefile                      |  2 +-
>  gitweb/meson.build                   |  2 +-
>  meson.build                          | 68 +++++++++++++++++++++++++++---------
>  templates/meson.build                |  4 +--
>  9 files changed, 87 insertions(+), 47 deletions(-)
>
> Range-diff versus v1:
>
> 1:  4bc8060a975 = 1:  3e9137c2d18 meson: fix handling of '-Dcurl=auto'
> 2:  4365cfc4a4e = 2:  7ba983d446e gitweb: fix generation of "gitweb.js"
> 3:  02d6ae13dd2 < -:  ----------- meson: require Perl when building docs
> 4:  fcf2478bd82 = 3:  33cd3e490eb meson: respect 'tests' build option in contrib
> -:  ----------- > 4:  1cb210c91a1 meson: distinguish build and target host binaries
> -:  ----------- > 5:  3172db10a10 ci: use Visual Studio for win+meson job on GitHub Workflows
>
> ---
> base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
> change-id: 20250328-b4-pks-collect-build-fixes-b5a6ce086b72

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

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

end of thread, other threads:[~2025-04-03  8:39 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-28  8:38 [PATCH 0/4] Collection of build fixes Patrick Steinhardt
2025-03-28  8:38 ` [PATCH 1/4] meson: fix handling of '-Dcurl=auto' Patrick Steinhardt
2025-03-28  8:38 ` [PATCH 2/4] gitweb: fix generation of "gitweb.js" Patrick Steinhardt
2025-03-28  8:38 ` [PATCH 3/4] meson: require Perl when building docs Patrick Steinhardt
2025-03-29 17:56   ` Junio C Hamano
2025-03-31  5:59     ` Patrick Steinhardt
2025-03-28  8:38 ` [PATCH 4/4] meson: respect 'tests' build option in contrib Patrick Steinhardt
2025-03-28 18:25   ` Sam James
2025-03-31  8:33 ` [PATCH v2 0/5] Collection of build fixes Patrick Steinhardt
2025-03-31  8:33   ` [PATCH v2 1/5] meson: fix handling of '-Dcurl=auto' Patrick Steinhardt
2025-04-03  8:24     ` Karthik Nayak
2025-03-31  8:33   ` [PATCH v2 2/5] gitweb: fix generation of "gitweb.js" Patrick Steinhardt
2025-04-01 16:30     ` Johannes Schindelin
2025-04-02  6:40       ` Patrick Steinhardt
2025-04-01 16:30     ` Toon Claes
2025-03-31  8:33   ` [PATCH v2 3/5] meson: respect 'tests' build option in contrib Patrick Steinhardt
2025-04-01 16:31     ` Johannes Schindelin
2025-03-31  8:33   ` [PATCH v2 4/5] meson: distinguish build and target host binaries Patrick Steinhardt
2025-04-03  8:38     ` Karthik Nayak
2025-03-31  8:33   ` [PATCH v2 5/5] ci: use Visual Studio for win+meson job on GitHub Workflows Patrick Steinhardt
2025-04-01 16:41   ` [PATCH v2 0/5] Collection of build fixes Johannes Schindelin
2025-04-03  8:39   ` Karthik Nayak

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