{"thread":{"id":"62657","subject":"[PATCH 0/5] ps/build follow-ups","startedAt":"2024-12-17T17:32:04Z","lastAt":"2024-12-18T05:54:28Z","messageCount":8,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"509228","messageId":"pull.1840.git.1734456721.gitgitgadget@gmail.com","threadId":"62657","inReplyTo":null,"subject":"[PATCH 0/5] ps/build follow-ups","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-17T17:31:56Z","receivedAt":"2024-12-17T17:32:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"These patches were required in the course of getting Git for Windows rebased\nonto Git v2.48.0-rc0. They are based on ps/build.\n\nJohannes Schindelin (5):\n  cmake: better support for out-of-tree builds follow-up\n  cmake(mergetools): better support for out-of-tree builds\n  cmake: use the correct file name for the Perl header\n  cmake: put the Perl modules into the correct location again\n  cmake/vcxproj: stop special-casing `remote-ext`\n\n config.mak.uname                    |  4 ----\n contrib/buildsystems/CMakeLists.txt | 15 +++++++++++----\n 2 files changed, 11 insertions(+), 8 deletions(-)\n\n\nbase-commit: 904339edbd80ec5676616af6e072b41804c1c8eb\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1840%2Fdscho%2Fps-build-followups-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1840/dscho/ps-build-followups-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1840\n-- \ngitgitgadget\n"},{"id":"509229","messageId":"187888ca9cd723d78b7bb96ba4d10a64450f553d.1734456721.git.gitgitgadget@gmail.com","threadId":"62657","inReplyTo":"pull.1840.git.1734456721.gitgitgadget@gmail.com","subject":"[PATCH 1/5] cmake: better support for out-of-tree builds follow-up","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-17T17:31:57Z","receivedAt":"2024-12-17T17:32:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn 7e0730c8baa (t: better support for out-of-tree builds, 2024-12-06),\nthe `bin-wrappers/` strategy was changed so that it no longer hard-codes\nthe template directory to be `@BUILD_DIR@/templates/blt`, but instead\ninterpolates the `@TEMPLATE_DIR@` placeholder during the build.\n\nHowever, this commit only adjusted the `Makefile`-based build.\n\nLet's adjust the CMake-based build as well. This fixes t0000.15 which\nwould otherwise fail with:\n\n  ++ echo ''\\''t1234-verbose/err'\\'' is not empty, it contains:'\n  't1234-verbose/err' is not empty, it contains:\n  ++ cat t1234-verbose/err\n  warning: templates not found in @TEMPLATE_DIR@\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 49904ca8a93..926f0d7b86c 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1087,6 +1087,7 @@ set(wrapper_test_scripts\n foreach(script ${wrapper_scripts})\n \tfile(STRINGS ${CMAKE_SOURCE_DIR}/bin-wrappers/wrap-for-bin.sh content NEWLINE_CONSUME)\n \tstring(REPLACE \"@BUILD_DIR@\" \"${CMAKE_BINARY_DIR}\" content \"${content}\")\n+\tstring(REPLACE \"@TEMPLATE_DIR@\" \"'${CMAKE_BINARY_DIR}/templates/blt'\" content \"${content}\")\n \tstring(REPLACE \"@PROG@\" \"${CMAKE_BINARY_DIR}/${script}${EXE_EXTENSION}\" content \"${content}\")\n \tfile(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/${script} ${content})\n endforeach()\n@@ -1094,12 +1095,14 @@ endforeach()\n foreach(script ${wrapper_test_scripts})\n \tfile(STRINGS ${CMAKE_SOURCE_DIR}/bin-wrappers/wrap-for-bin.sh content NEWLINE_CONSUME)\n \tstring(REPLACE \"@BUILD_DIR@\" \"${CMAKE_BINARY_DIR}\" content \"${content}\")\n+\tstring(REPLACE \"@TEMPLATE_DIR@\" \"'${CMAKE_BINARY_DIR}/templates/blt'\" content \"${content}\")\n \tstring(REPLACE \"@PROG@\" \"${CMAKE_BINARY_DIR}/t/helper/${script}${EXE_EXTENSION}\" content \"${content}\")\n \tfile(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/${script} ${content})\n endforeach()\n \n file(STRINGS ${CMAKE_SOURCE_DIR}/bin-wrappers/wrap-for-bin.sh content NEWLINE_CONSUME)\n string(REPLACE \"@BUILD_DIR@\" \"${CMAKE_BINARY_DIR}\" content \"${content}\")\n+string(REPLACE \"@TEMPLATE_DIR@\" \"'${CMAKE_BINARY_DIR}/templates/blt'\" content \"${content}\")\n string(REPLACE \"@GIT_TEXTDOMAINDIR@\" \"${CMAKE_BINARY_DIR}/po/build/locale\" content \"${content}\")\n string(REPLACE \"@GITPERLLIB@\" \"${CMAKE_BINARY_DIR}/perl/build/lib\" content \"${content}\")\n string(REPLACE \"@MERGE_TOOLS_DIR@\" \"${CMAKE_SOURCE_DIR}/mergetools\" content \"${content}\")\n-- \ngitgitgadget\n\n"},{"id":"509230","messageId":"1170b6d01f1a9aadd403a833650b16f9f3378e53.1734456721.git.gitgitgadget@gmail.com","threadId":"62657","inReplyTo":"pull.1840.git.1734456721.gitgitgadget@gmail.com","subject":"[PATCH 2/5] cmake(mergetools): better support for out-of-tree builds","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-17T17:31:58Z","receivedAt":"2024-12-17T17:32:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn 7e0730c8baa (t: better support for out-of-tree builds, 2024-12-06)\nthe strategy was changed from letting `t7609-mergetool--lib.sh`\nhard-code the directory where it expects to find the merge tools to\nhard-coding that value in the placeholder `@GIT_TEST_MERGE_TOOLS_DIR@`\nthat is replaced during the build.\n\nHowever, likely due to a copy/paste mistake (and reviewers missed this,\ntoo), the CMake-based build was adjusted incorrectly, replacing that\nplaceholder not with the path to the merge tools, but with a Boolean\nindicating whether to use a runtime-generated path prefix or not.\n\nLet's fix that, addressing this CMake-build's symptom:\n\n  Initialized empty Git repository in D:/a/git/git/t/trash directory.t7609-mergetool--lib/.git/\n  ++ . true/vimdiff\n  ./test-lib.sh: line 1021: true/vimdiff: No such file or directory\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 926f0d7b86c..3dd6b3a130e 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1195,7 +1195,7 @@ string(REPLACE \"@GIT_TEST_TEXTDOMAINDIR@\" \"'${CMAKE_BINARY_DIR}/po/build/locale'\n string(REPLACE \"@GIT_TEST_POPATH@\" \"'${CMAKE_BINARY_DIR}/po'\" git_build_options \"${git_build_options}\")\n string(REPLACE \"@GIT_TEST_TEMPLATE_DIR@\" \"'${CMAKE_BINARY_DIR}/templates/blt'\" git_build_options \"${git_build_options}\")\n string(REPLACE \"@GIT_TEST_GITPERLLIB@\" \"'${CMAKE_BINARY_DIR}/perl/build/lib'\" git_build_options \"${git_build_options}\")\n-string(REPLACE \"@GIT_TEST_MERGE_TOOLS_DIR@\" \"'${RUNTIME_PREFIX}'\" git_build_options \"${git_build_options}\")\n+string(REPLACE \"@GIT_TEST_MERGE_TOOLS_DIR@\" \"'${CMAKE_BINARY_DIR}/mergetools'\" git_build_options \"${git_build_options}\")\n string(REPLACE \"@RUNTIME_PREFIX@\" \"'${RUNTIME_PREFIX}'\" git_build_options \"${git_build_options}\")\n string(REPLACE \"@GITWEBDIR@\" \"'${GITWEBDIR}'\" git_build_options \"${git_build_options}\")\n string(REPLACE \"@USE_GETTEXT_SCHEME@\" \"\" git_build_options \"${git_build_options}\")\n-- \ngitgitgadget\n\n"},{"id":"509233","messageId":"e687da666eed9a25a717ef6d09cd0f471610b835.1734456721.git.gitgitgadget@gmail.com","threadId":"62657","inReplyTo":"pull.1840.git.1734456721.gitgitgadget@gmail.com","subject":"[PATCH 3/5] cmake: use the correct file name for the Perl header","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-17T17:31:59Z","receivedAt":"2024-12-17T17:32:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn e4b488049a5 (Makefile: extract script to massage Perl scripts,\n2024-12-06), the code was refactored that is used to transform the Perl\nscripts/modules to their final form.\n\nEven the CMake-based build was adjusted, but the change used the file\nname `PERL-HEADER` instead of the file name used by the Makefile-based\nbuild (same name but with the `GIT-` prefix). Let's adjust the former to\nthe latter.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 3dd6b3a130e..6f35cd66f39 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -867,7 +867,7 @@ list(TRANSFORM perl_modules REPLACE \"${CMAKE_SOURCE_DIR}/\" \"\")\n file(STRINGS ${CMAKE_SOURCE_DIR}/perl/header_templates/fixed_prefix.template.pl perl_header )\n string(REPLACE \"@PATHSEP@\" \":\" perl_header \"${perl_header}\")\n string(REPLACE \"@INSTLIBDIR@\" \"${INSTLIBDIR}\" perl_header \"${perl_header}\")\n-file(WRITE ${CMAKE_BINARY_DIR}/PERL-HEADER ${perl_header})\n+file(WRITE ${CMAKE_BINARY_DIR}/GIT-PERL-HEADER ${perl_header})\n \n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/GIT-VERSION-FILE\"\n \tCOMMAND \"${SH_EXE}\" \"${CMAKE_SOURCE_DIR}/GIT-VERSION-GEN\"\n@@ -888,7 +888,7 @@ foreach(script ${git_perl_scripts} ${perl_modules})\n \t\tCOMMAND \"${SH_EXE}\" \"${CMAKE_SOURCE_DIR}/generate-perl.sh\"\n \t\t\t\"${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS\"\n \t\t\t\"${CMAKE_BINARY_DIR}/GIT-VERSION-FILE\"\n-\t\t\t\"${CMAKE_BINARY_DIR}/PERL-HEADER\"\n+\t\t\t\"${CMAKE_BINARY_DIR}/GIT-PERL-HEADER\"\n \t\t\t\"${CMAKE_SOURCE_DIR}/${script}\"\n \t\t\t\"${CMAKE_BINARY_DIR}/${perl_gen_path}\"\n \t\tDEPENDS \"${CMAKE_SOURCE_DIR}/generate-perl.sh\"\n-- \ngitgitgadget\n\n"},{"id":"509231","messageId":"0f1808e37cc75ac6d2bd5f28deb7c91bfeedcba9.1734456721.git.gitgitgadget@gmail.com","threadId":"62657","inReplyTo":"pull.1840.git.1734456721.gitgitgadget@gmail.com","subject":"[PATCH 4/5] cmake: put the Perl modules into the correct location again","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-17T17:32:00Z","receivedAt":"2024-12-17T17:32:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn ccfba9e0c45 (Makefile: use \"generate-perl.sh\" to massage Perl\nlibrary, 2024-12-06), the previous strategy (which avoided spawning a\nshell script to transform the files) was replaced by the same\n`generate-perl.sh` invocation as for the Makefile-based build.\n\nThe only difference is that now the transformation tries to handle the\nPerl modules in-place (which ends up in empty files because the same\nfile is used as input and output via stdin/stdout redirection), and the\nPerl script cannot find them anymore because they are not in the\nexpected place.\n\nLet's put them into the expected place again, i.e. into\n`perl/build/lib/` instead of `perl/`.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n contrib/buildsystems/CMakeLists.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 6f35cd66f39..36f18ab2dd1 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -882,6 +882,10 @@ foreach(script ${git_perl_scripts} ${perl_modules})\n \tstring(REPLACE \".perl\" \"\" perl_gen_path \"${script}\")\n \n \tget_filename_component(perl_gen_dir \"${perl_gen_path}\" DIRECTORY)\n+\tif(script MATCHES \"\\.pm$\")\n+\t\tstring(REGEX REPLACE \"^perl\" \"perl/build/lib\" perl_gen_dir \"${perl_gen_dir}\")\n+\t\tstring(REGEX REPLACE \"^perl\" \"perl/build/lib\" perl_gen_path \"${perl_gen_path}\")\n+\tendif()\n \tfile(MAKE_DIRECTORY \"${CMAKE_BINARY_DIR}/${perl_gen_dir}\")\n \n \tadd_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/${perl_gen_path}\"\n-- \ngitgitgadget\n\n"},{"id":"509232","messageId":"c539b5200a72a512c800012660ca997b35232083.1734456721.git.gitgitgadget@gmail.com","threadId":"62657","inReplyTo":"pull.1840.git.1734456721.gitgitgadget@gmail.com","subject":"[PATCH 5/5] cmake/vcxproj: stop special-casing `remote-ext`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-17T17:32:01Z","receivedAt":"2024-12-17T17:32:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen the `vcxproj` target was introduced in `config.mak.uname` to allow\nbuilding Git with the Visual C toolchain, the `git remote-ext` command\nwas always executed in its dashed form. Therefore, it was impossible to\npass the test suite unless that command existed in its dashed form, and\nwe had to special-case this.\n\nLater, when the `vcxproj` target got out of fashion because Visual\nStudio gained native support for CMake builds, this special-casing was\ncopied without questioning it.\n\nBut as of 675df192c5f (transport-helper: do not run git-remote-ext etc.\nin dashed form, 2020-08-26), the reason for this special-casing no\nlonger exists. So let's just drop it.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n config.mak.uname                    | 4 ----\n contrib/buildsystems/CMakeLists.txt | 2 +-\n 2 files changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex d5112168a4c..b12d4e168ae 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -819,10 +819,6 @@ vcxproj:\n \tsed -i 's|\\(git\\)-\\([-a-z]*\\)\\.exe\"|\\1.exe\" \\2|g' \\\n \t\tbin-wrappers/git-{receive-pack,upload-archive}\n \tgit add -f $(test_bindir_programs)\n-\t# remote-ext is a builtin, but invoked as if it were external\n-\tsed 's|receive-pack|remote-ext|g' \\\n-\t\t<bin-wrappers/git-receive-pack >bin-wrappers/git-remote-ext\n-\tgit add -f bin-wrappers/git-remote-ext\n \n \t# Add templates\n \t$(MAKE) -C templates\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 36f18ab2dd1..802445c1eb0 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1082,7 +1082,7 @@ endif()\n \n #wrapper scripts\n set(wrapper_scripts\n-\tgit git-upload-pack git-receive-pack git-upload-archive git-shell git-remote-ext scalar)\n+\tgit git-upload-pack git-receive-pack git-upload-archive git-shell scalar)\n \n set(wrapper_test_scripts\n \ttest-fake-ssh test-tool)\n-- \ngitgitgadget\n"},{"id":"509241","messageId":"xmqqzfkuxcf7.fsf@gitster.g","threadId":"62657","inReplyTo":"pull.1840.git.1734456721.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/5] ps/build follow-ups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-17T20:49:32Z","receivedAt":"2024-12-17T20:49:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> These patches were required in the course of getting Git for Windows rebased\n> onto Git v2.48.0-rc0. They are based on ps/build.\n\nThanks.  Let me mark it to be fast-tracked to 'master'.\nQueued.\n>\n> Johannes Schindelin (5):\n>   cmake: better support for out-of-tree builds follow-up\n>   cmake(mergetools): better support for out-of-tree builds\n>   cmake: use the correct file name for the Perl header\n>   cmake: put the Perl modules into the correct location again\n>   cmake/vcxproj: stop special-casing `remote-ext`\n>\n>  config.mak.uname                    |  4 ----\n>  contrib/buildsystems/CMakeLists.txt | 15 +++++++++++----\n>  2 files changed, 11 insertions(+), 8 deletions(-)\n>\n>\n> base-commit: 904339edbd80ec5676616af6e072b41804c1c8eb\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1840%2Fdscho%2Fps-build-followups-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1840/dscho/ps-build-followups-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1840\n"},{"id":"509276","messageId":"Z2JjgJX28n9jRSFW@pks.im","threadId":"62657","inReplyTo":"xmqqzfkuxcf7.fsf@gitster.g","subject":"Re: [PATCH 0/5] ps/build follow-ups","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-18T05:54:08Z","receivedAt":"2024-12-18T05:54:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Dec 17, 2024 at 12:49:32PM -0800, Junio C Hamano wrote:\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n> \n> > These patches were required in the course of getting Git for Windows rebased\n> > onto Git v2.48.0-rc0. They are based on ps/build.\n> \n> Thanks.  Let me mark it to be fast-tracked to 'master'.\n> Queued.\n\nThanks for these fixups, Dscho. All of them look good to me.\n\nPatrick\n"}]}