ps/build follow-ups
cover ps/build follow-ups
These patches were required in the course of getting Git for Windows rebased onto Git v2.48.0-rc0. They are based on ps/build.
Johannes Schindelin (5):
cmake: better support for out-of-tree builds follow-up cmake(mergetools): better support for out-of-tree builds
cmake: use the correct file name for the Perl header
cmake: put the Perl modules into the correct location again cmake/vcxproj: stop special-casing remote-ext
config.mak.uname | 4 ---- contrib/buildsystems/CMakeLists.txt | 15 +++++++++++---- 2 files changed, 11 insertions(+), 8 deletions(-)
base-commit: 904339edbd80ec5676616af6e072b41804c1c8eb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1840%2Fdscho%2Fps-build-followups-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1840/dscho/ps-build-followups-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1840
Signature · 1 line
gitgitgadget
PATCH 1/5 cmake: better support for out-of-tree builds follow-up
From: Johannes Schindelin <[email protected]>
In 7e0730c8baa (t: better support for out-of-tree builds, 2024-12-06), the bin-wrappers/ strategy was changed so that it no longer hard-codes the template directory to be @BUILD_DIR@/templates/blt, but instead interpolates the @TEMPLATE_DIR@ placeholder during the build.
However, this commit only adjusted the Makefile-based build.
Let's adjust the CMake-based build as well. This fixes t0000.15 which would otherwise fail with:
++ echo ''\''t1234-verbose/err'\'' is not empty, it contains:' 't1234-verbose/err' is not empty, it contains:
++ cat t1234-verbose/err
warning: templates not found in @TEMPLATE_DIR@
--- contrib/buildsystems/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+)
contrib/buildsystems/CMakeLists.txt +3 -0
@@ -1087,6 +1087,7 @@ set(wrapper_test_scripts foreach(script ${wrapper_scripts}) file(STRINGS ${CMAKE_SOURCE_DIR}/bin-wrappers/wrap-for-bin.sh content NEWLINE_CONSUME) string(REPLACE "@BUILD_DIR@" "${CMAKE_BINARY_DIR}" content "${content}") + string(REPLACE "@TEMPLATE_DIR@" "'${CMAKE_BINARY_DIR}/templates/blt'" content "${content}") string(REPLACE "@PROG@" "${CMAKE_BINARY_DIR}/${script}${EXE_EXTENSION}" content "${content}") file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/${script} ${content}) endforeach()
@@ -1094,12 +1095,14 @@ endforeach() foreach(script ${wrapper_test_scripts}) file(STRINGS ${CMAKE_SOURCE_DIR}/bin-wrappers/wrap-for-bin.sh content NEWLINE_CONSUME) string(REPLACE "@BUILD_DIR@" "${CMAKE_BINARY_DIR}" content "${content}") + string(REPLACE "@TEMPLATE_DIR@" "'${CMAKE_BINARY_DIR}/templates/blt'" content "${content}") string(REPLACE "@PROG@" "${CMAKE_BINARY_DIR}/t/helper/${script}${EXE_EXTENSION}" content "${content}") file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/${script} ${content}) endforeach() file(STRINGS ${CMAKE_SOURCE_DIR}/bin-wrappers/wrap-for-bin.sh content NEWLINE_CONSUME) string(REPLACE "@BUILD_DIR@" "${CMAKE_BINARY_DIR}" content "${content}") +string(REPLACE "@TEMPLATE_DIR@" "'${CMAKE_BINARY_DIR}/templates/blt'" content "${content}") string(REPLACE "@GIT_TEXTDOMAINDIR@" "${CMAKE_BINARY_DIR}/po/build/locale" content "${content}") string(REPLACE "@GITPERLLIB@" "${CMAKE_BINARY_DIR}/perl/build/lib" content "${content}") string(REPLACE "@MERGE_TOOLS_DIR@" "${CMAKE_SOURCE_DIR}/mergetools" content "${content}")
Signature · 1 line
gitgitgadget
PATCH 2/5 cmake(mergetools): better support for out-of-tree builds
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
In 7e0730c8baa (t: better support for out-of-tree builds, 2024-12-06) the strategy was changed from letting t7609-mergetool--lib.sh hard-code the directory where it expects to find the merge tools to hard-coding that value in the placeholder @GIT_TEST_MERGE_TOOLS_DIR@ that is replaced during the build.
However, likely due to a copy/paste mistake (and reviewers missed this, too), the CMake-based build was adjusted incorrectly, replacing that placeholder not with the path to the merge tools, but with a Boolean indicating whether to use a runtime-generated path prefix or not.
Let's fix that, addressing this CMake-build's symptom:
Initialized empty Git repository in D:/a/git/git/t/trash directory.t7609-mergetool--lib/.git/ ++ . true/vimdiff
./test-lib.sh: line 1021: true/vimdiff: No such file or directory
--- contrib/buildsystems/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
contrib/buildsystems/CMakeLists.txt +1 -1
@@ -1195,7 +1195,7 @@ string(REPLACE "@GIT_TEST_TEXTDOMAINDIR@" "'${CMAKE_BINARY_DIR}/po/build/locale' string(REPLACE "@GIT_TEST_POPATH@" "'${CMAKE_BINARY_DIR}/po'" git_build_options "${git_build_options}") string(REPLACE "@GIT_TEST_TEMPLATE_DIR@" "'${CMAKE_BINARY_DIR}/templates/blt'" git_build_options "${git_build_options}") string(REPLACE "@GIT_TEST_GITPERLLIB@" "'${CMAKE_BINARY_DIR}/perl/build/lib'" git_build_options "${git_build_options}") -string(REPLACE "@GIT_TEST_MERGE_TOOLS_DIR@" "'${RUNTIME_PREFIX}'" git_build_options "${git_build_options}") +string(REPLACE "@GIT_TEST_MERGE_TOOLS_DIR@" "'${CMAKE_BINARY_DIR}/mergetools'" git_build_options "${git_build_options}") string(REPLACE "@RUNTIME_PREFIX@" "'${RUNTIME_PREFIX}'" git_build_options "${git_build_options}") string(REPLACE "@GITWEBDIR@" "'${GITWEBDIR}'" git_build_options "${git_build_options}") string(REPLACE "@USE_GETTEXT_SCHEME@" "" git_build_options "${git_build_options}")
Signature · 1 line
gitgitgadget
PATCH 3/5 cmake: use the correct file name for the Perl header
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
In e4b488049a5 (Makefile: extract script to massage Perl scripts, 2024-12-06), the code was refactored that is used to transform the Perl scripts/modules to their final form.
Even the CMake-based build was adjusted, but the change used the file name PERL-HEADER instead of the file name used by the Makefile-based build (same name but with the GIT- prefix). Let's adjust the former to the latter.
--- contrib/buildsystems/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
contrib/buildsystems/CMakeLists.txt +2 -2
@@ -867,7 +867,7 @@ list(TRANSFORM perl_modules REPLACE "${CMAKE_SOURCE_DIR}/" "") file(STRINGS ${CMAKE_SOURCE_DIR}/perl/header_templates/fixed_prefix.template.pl perl_header ) string(REPLACE "@PATHSEP@" ":" perl_header "${perl_header}") string(REPLACE "@INSTLIBDIR@" "${INSTLIBDIR}" perl_header "${perl_header}") -file(WRITE ${CMAKE_BINARY_DIR}/PERL-HEADER ${perl_header}) +file(WRITE ${CMAKE_BINARY_DIR}/GIT-PERL-HEADER ${perl_header}) add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/GIT-VERSION-FILE" COMMAND "${SH_EXE}" "${CMAKE_SOURCE_DIR}/GIT-VERSION-GEN"
@@ -888,7 +888,7 @@ foreach(script ${git_perl_scripts} ${perl_modules}) COMMAND "${SH_EXE}" "${CMAKE_SOURCE_DIR}/generate-perl.sh" "${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS" "${CMAKE_BINARY_DIR}/GIT-VERSION-FILE" - "${CMAKE_BINARY_DIR}/PERL-HEADER" + "${CMAKE_BINARY_DIR}/GIT-PERL-HEADER" "${CMAKE_SOURCE_DIR}/${script}" "${CMAKE_BINARY_DIR}/${perl_gen_path}" DEPENDS "${CMAKE_SOURCE_DIR}/generate-perl.sh"
Signature · 1 line
gitgitgadget
PATCH 4/5 cmake: put the Perl modules into the correct location again
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
In ccfba9e0c45 (Makefile: use "generate-perl.sh" to massage Perl library, 2024-12-06), the previous strategy (which avoided spawning a shell script to transform the files) was replaced by the same generate-perl.sh invocation as for the Makefile-based build.
The only difference is that now the transformation tries to handle the Perl modules in-place (which ends up in empty files because the same file is used as input and output via stdin/stdout redirection), and the Perl script cannot find them anymore because they are not in the expected place.
Let's put them into the expected place again, i.e. into perl/build/lib/ instead of perl/.
--- contrib/buildsystems/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+)
contrib/buildsystems/CMakeLists.txt +4 -0
@@ -882,6 +882,10 @@ foreach(script ${git_perl_scripts} ${perl_modules}) string(REPLACE ".perl" "" perl_gen_path "${script}") get_filename_component(perl_gen_dir "${perl_gen_path}" DIRECTORY) + if(script MATCHES "\.pm$") + string(REGEX REPLACE "^perl" "perl/build/lib" perl_gen_dir "${perl_gen_dir}") + string(REGEX REPLACE "^perl" "perl/build/lib" perl_gen_path "${perl_gen_path}") + endif() file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/${perl_gen_dir}") add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/${perl_gen_path}"
Signature · 1 line
gitgitgadget
PATCH 5/5 cmake/vcxproj: stop special-casing `remote-ext`
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When the vcxproj target was introduced in config.mak.uname to allow building Git with the Visual C toolchain, the git remote-ext command was always executed in its dashed form. Therefore, it was impossible to pass the test suite unless that command existed in its dashed form, and we had to special-case this.
Later, when the vcxproj target got out of fashion because Visual Studio gained native support for CMake builds, this special-casing was copied without questioning it.
But as of 675df192c5f (transport-helper: do not run git-remote-ext etc. in dashed form, 2020-08-26), the reason for this special-casing no longer exists. So let's just drop it.
--- config.mak.uname | 4 ---- contrib/buildsystems/CMakeLists.txt | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-)
config.mak.uname +0 -4
@@ -819,10 +819,6 @@ vcxproj: sed -i 's|\(git\)-\([-a-z]*\)\.exe"|\1.exe" \2|g' \ bin-wrappers/git-{receive-pack,upload-archive} git add -f $(test_bindir_programs) - # remote-ext is a builtin, but invoked as if it were external - sed 's|receive-pack|remote-ext|g' \ - <bin-wrappers/git-receive-pack >bin-wrappers/git-remote-ext - git add -f bin-wrappers/git-remote-ext # Add templates $(MAKE) -C templates
contrib/buildsystems/CMakeLists.txt +1 -1
@@ -1082,7 +1082,7 @@ endif() #wrapper scripts set(wrapper_scripts - git git-upload-pack git-receive-pack git-upload-archive git-shell git-remote-ext scalar) + git git-upload-pack git-receive-pack git-upload-archive git-shell scalar) set(wrapper_test_scripts test-fake-ssh test-tool)
Signature · 1 line
gitgitgadget
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 2 lines · writes:
These patches were required in the course of getting Git for Windows rebased onto Git v2.48.0-rc0. They are based on ps/build.
Thanks. Let me mark it to be fast-tracked to 'master'. Queued.
Quoted 17 lines
Johannes Schindelin (5):
cmake: better support for out-of-tree builds follow-up cmake(mergetools): better support for out-of-tree builds
cmake: use the correct file name for the Perl header
cmake: put the Perl modules into the correct location again cmake/vcxproj: stop special-casingremote-extconfig.mak.uname | 4 ---- contrib/buildsystems/CMakeLists.txt | 15 +++++++++++---- 2 files changed, 11 insertions(+), 8 deletions(-)base-commit:
904339edbd80ec5676616af6e072b41804c1c8eb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1840%2Fdscho%2Fps-build-followups-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1840/dscho/ps-build-followups-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1840
Quoted 8 lines · Junio C Hamano
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 2 lines · writes:
These patches were required in the course of getting Git for Windows rebased onto Git v2.48.0-rc0. They are based on ps/build.
Thanks. Let me mark it to be fast-tracked to 'master'. Queued.
Thanks for these fixups, Dscho. All of them look good to me.
Patrick