# [PATCH 0/5] ps/build follow-ups 8 messages from 2024-12-17 to 2024-12-18. Participants: Johannes Schindelin via GitGitGadget, Junio C Hamano, Patrick Steinhardt. Thread: https://gitlist.dev/t/62657 ## Johannes Schindelin via GitGitGadget, 2024-12-17 17:31 Subject: [PATCH 0/5] ps/build follow-ups Message-ID: URL: https://gitlist.dev/e/pull.1840.git.1734456721.gitgitgadget%40gmail.com ``` 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 -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2024-12-17 17:31 Subject: [PATCH 1/5] cmake: better support for out-of-tree builds follow-up Message-ID: <187888ca9cd723d78b7bb96ba4d10a64450f553d.1734456721.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/187888ca9cd723d78b7bb96ba4d10a64450f553d.1734456721.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin 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@ Signed-off-by: Johannes Schindelin --- contrib/buildsystems/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 49904ca8a93..926f0d7b86c 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -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}") -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2024-12-17 17:31 Subject: [PATCH 2/5] cmake(mergetools): better support for out-of-tree builds Message-ID: <1170b6d01f1a9aadd403a833650b16f9f3378e53.1734456721.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/1170b6d01f1a9aadd403a833650b16f9f3378e53.1734456721.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin 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 Signed-off-by: Johannes Schindelin --- contrib/buildsystems/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 926f0d7b86c..3dd6b3a130e 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -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}") -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2024-12-17 17:31 Subject: [PATCH 3/5] cmake: use the correct file name for the Perl header Message-ID: URL: https://gitlist.dev/e/e687da666eed9a25a717ef6d09cd0f471610b835.1734456721.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin 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. Signed-off-by: Johannes Schindelin --- contrib/buildsystems/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 3dd6b3a130e..6f35cd66f39 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -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" -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2024-12-17 17:32 Subject: [PATCH 4/5] cmake: put the Perl modules into the correct location again Message-ID: <0f1808e37cc75ac6d2bd5f28deb7c91bfeedcba9.1734456721.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/0f1808e37cc75ac6d2bd5f28deb7c91bfeedcba9.1734456721.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin 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/`. Signed-off-by: Johannes Schindelin --- contrib/buildsystems/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 6f35cd66f39..36f18ab2dd1 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -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}" -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2024-12-17 17:32 Subject: [PATCH 5/5] cmake/vcxproj: stop special-casing `remote-ext` Message-ID: URL: https://gitlist.dev/e/c539b5200a72a512c800012660ca997b35232083.1734456721.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin 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. Signed-off-by: Johannes Schindelin --- config.mak.uname | 4 ---- contrib/buildsystems/CMakeLists.txt | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index d5112168a4c..b12d4e168ae 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -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-remote-ext - git add -f bin-wrappers/git-remote-ext # Add templates $(MAKE) -C templates diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 36f18ab2dd1..802445c1eb0 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -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) -- gitgitgadget ``` ## Junio C Hamano, 2024-12-17 20:49 Subject: Re: [PATCH 0/5] ps/build follow-ups Message-ID: URL: https://gitlist.dev/e/xmqqzfkuxcf7.fsf%40gitster.g In-Reply-To: ``` "Johannes Schindelin via GitGitGadget" 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. > > 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 ``` ## Patrick Steinhardt, 2024-12-18 05:54 Subject: Re: [PATCH 0/5] ps/build follow-ups Message-ID: URL: https://gitlist.dev/e/Z2JjgJX28n9jRSFW%40pks.im In-Reply-To: ``` On Tue, Dec 17, 2024 at 12:49:32PM -0800, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" > 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 ```