threads / patch / 62657

patch, 5 partsps/build follow-ups

Subject: [PATCH 0/5] ps/build follow-ups

## tl;dr

8 messages between Dec 17, 2024 and Dec 18, 2024. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Johannes Schindelin via GitGitGadget· Dec 17, 2024, 17:31 UTC · lore

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· Dec 17, 2024, 17:31 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 1/5] cmake: better support for out-of-tree builds follow-up

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 contrib/buildsystems/CMakeLists.txt | 3 +++
 1 file changed, 3 insertions(+)

Show changes to contrib/buildsystems/CMakeLists.txt +3 −0

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· Dec 17, 2024, 17:31 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 2/5] cmake(mergetools): better support for out-of-tree builds

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 contrib/buildsystems/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to contrib/buildsystems/CMakeLists.txt +1 −1

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· Dec 17, 2024, 17:31 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 3/5] cmake: use the correct file name for the Perl header

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 contrib/buildsystems/CMakeLists.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Show changes to contrib/buildsystems/CMakeLists.txt +2 −2

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· Dec 17, 2024, 17:32 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 4/5] cmake: put the Perl modules into the correct location again

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 contrib/buildsystems/CMakeLists.txt | 4 ++++
 1 file changed, 4 insertions(+)

Show changes to contrib/buildsystems/CMakeLists.txt +4 −0

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· Dec 17, 2024, 17:32 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 5/5] cmake/vcxproj: stop special-casing `remote-ext`

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 config.mak.uname                    | 4 ----
 contrib/buildsystems/CMakeLists.txt | 2 +-
 2 files changed, 1 insertion(+), 5 deletions(-)

Show changes to 2 files +1 −5

config.mak.uname, contrib/buildsystems/CMakeLists.txt

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-receive-pack >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· Dec 17, 2024, 20:49 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 0/5] ps/build follow-ups

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> 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.

Show 17 quoted 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-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· Dec 18, 2024, 05:54 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/5] ps/build follow-ups

On Tue, Dec 17, 2024 at 12:49:32PM -0800, Junio C Hamano wrote:

Show 8 quoted lines

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> 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