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