git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 6/6] cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Dec 19, 2022, 18:39 UTC
Message-ID
<patch-6.6-3724cad82e0-20221219T183623Z-avarab@gmail.com>
In-Reply-To
<cover-0.6-00000000000-20221219T183623Z-avarab@gmail.com>

Since the cmake file was made to run on *nix in [1] running the tests with "ctest" broken, because we'd attempt to invoke our bin-wrappers/, but they didn't have the executable bit.

In the best case, the "t/test-lib.sh" would be unable to find "bin-wrappers/git", and we'd fall back on "GIT_EXEC_PATH=$GIT_BUILD_DIR" using the fallback behavior added in [2]:

	$ ./t0001-init.sh
	<GIT_BUILD_DIR>/t/../contrib/buildsystems/out/bin-wrappers/git is not executable; using GIT_EXEC_PATH

This was recently somewhat swept under the rug in [3], as ctest would run them with "--no-bin-wrappers". But still with [3], running e.g.:

	cmake -S contrib/buildsystems -B contrib/buildsystems/out -DCMAKE_BUILD_TYPE=Debug &&
	make -C contrib/buildsystems/out &&
	ctest --test-dir contrib/buildsystems/out --jobs="$(nproc)" --output-on-failure

Fails around 20% of our tests on *nix. So even with [3] we'd fail any test that needed to invoke one of our built shell, perl or Python scripts on *nix. E.g. t0012-help.sh would fail on a test that tried to invoke "git web--browse". The equivalent of this (in the "out" directory) would happen:

	$ ./git --exec-path=$PWD web--browse
	git: 'web--browse' is not a git command. See 'git --help'.
Which we can fix by "chmod +x"-ing the built "git-web--browse":
	$ chmod +x git-web--browse
	$ ./git --exec-path=$PWD web--browse
	usage: git web--browse [--browser=browser|--tool=browser] [--config=conf.var] url/file ...

The same goes for e.g. the "git-p4" tests, which would fail because our built "git-p4" wasn't executable, etc. There's also a few other outstanding issues, which will be fixed in subsequent commits.

Ideally we'd use the file(CHMOD ...) form everywhere, but that syntax was introduced in cmake 3.19[4], whereas we only require 3.14. Let's provide a fallback behind a version check, so that we'll eventually be able to delete the "else" part. Both forms result in the same file modes.

Before this change:
	80% tests passed, 196 tests failed out of 977
After:
	99% tests passed, 5 tests failed out of 977
The remaining failures will be addressed in subsequent commits.

As this isn't needed on Windows let's skip this there. There's also an unconfirmed (it works in CI) report[5] that invoking the "chmod" command fails in some scenarios.

1. f31b6244950 (Merge branch 'yw/cmake-updates', 2022-06-07)
2. e4597aae659 (run test suite without dashed git-commands in PATH, 2009-12-02)
3. 2ea1d8b5563 (cmake: make it easier to diagnose regressions in CTest
   runs, 2022-10-18)
4. https://cmake.org/cmake/help/latest/command/file.html#chmod
5. https://lore.kernel.org/git/87f22a55-ee84-2f76-7b9b-924a97f44f89@dunelm.org.uk/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
write script
---
 contrib/buildsystems/CMakeLists.txt | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 29b73ecbbbc..74b094ae5dc 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -847,6 +847,21 @@ add_custom_command(OUTPUT ${git_links} ${git_http_links}
 		DEPENDS git git-remote-http)
 add_custom_target(git-links ALL DEPENDS ${git_links} ${git_http_links})
 
+function(write_script path content)
+	file(WRITE ${path} ${content})
+
+	if(WIN32)
+		message(TRACE "skipping chmod +x '${path}' on Windows")
+	elseif("${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION}" VERSION_GREATER_EQUAL "3.19")
+		file(CHMOD ${path} FILE_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE)
+	else()
+		execute_process(COMMAND chmod +x ${path}
+				RESULT_VARIABLE CHILD_ERROR)
+		if(CHILD_ERROR)
+			message(FATAL_ERROR "failed to chmod +x '${path}': '${CHILD_ERROR}'")
+		endif()
+	endif()
+endfunction()
 
 #creating required scripts
 set(SHELL_PATH /bin/sh)
@@ -872,7 +887,7 @@ foreach(script ${git_shell_scripts})
 	string(REPLACE "# @@BROKEN_PATH_FIX@@" "" content "${content}")
 	string(REPLACE "@@PERL@@" "${PERL_PATH}" content "${content}")
 	string(REPLACE "@@PAGER_ENV@@" "LESS=FRX LV=-c" content "${content}")
-	file(WRITE ${CMAKE_BINARY_DIR}/${script} ${content})
+	write_script(${CMAKE_BINARY_DIR}/${script} "${content}")
 endforeach()
 
 #perl scripts
@@ -887,13 +902,13 @@ foreach(script ${git_perl_scripts})
 	file(STRINGS ${CMAKE_SOURCE_DIR}/${script}.perl content NEWLINE_CONSUME)
 	string(REPLACE "#!/usr/bin/perl" "#!/usr/bin/perl\n${perl_header}\n" content "${content}")
 	string(REPLACE "@@GIT_VERSION@@" "${PROJECT_VERSION}" content "${content}")
-	file(WRITE ${CMAKE_BINARY_DIR}/${script} ${content})
+	write_script(${CMAKE_BINARY_DIR}/${script} "${content}")
 endforeach()
 
 #python script
 file(STRINGS ${CMAKE_SOURCE_DIR}/git-p4.py content NEWLINE_CONSUME)
 string(REPLACE "#!/usr/bin/env python" "#!/usr/bin/python" content "${content}")
-file(WRITE ${CMAKE_BINARY_DIR}/git-p4 ${content})
+write_script(${CMAKE_BINARY_DIR}/git-p4 "${content}")
 file(COPY ${CMAKE_SOURCE_DIR}/git-p4.py DESTINATION ${CMAKE_BINARY_DIR}/)
 
 #perl modules
@@ -1032,20 +1047,20 @@ foreach(script ${wrapper_scripts})
 	file(STRINGS ${CMAKE_SOURCE_DIR}/wrap-for-bin.sh content NEWLINE_CONSUME)
 	string(REPLACE "@@BUILD_DIR@@" "${CMAKE_BINARY_DIR}" content "${content}")
 	string(REPLACE "@@PROG@@" "${script}${EXE_EXTENSION}" content "${content}")
-	file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/${script} ${content})
+	write_script(${CMAKE_BINARY_DIR}/bin-wrappers/${script} "${content}")
 endforeach()
 
 foreach(script ${wrapper_test_scripts})
 	file(STRINGS ${CMAKE_SOURCE_DIR}/wrap-for-bin.sh content NEWLINE_CONSUME)
 	string(REPLACE "@@BUILD_DIR@@" "${CMAKE_BINARY_DIR}" content "${content}")
 	string(REPLACE "@@PROG@@" "t/helper/${script}${EXE_EXTENSION}" content "${content}")
-	file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/${script} ${content})
+	write_script(${CMAKE_BINARY_DIR}/bin-wrappers/${script} "${content}")
 endforeach()
 
 file(STRINGS ${CMAKE_SOURCE_DIR}/wrap-for-bin.sh content NEWLINE_CONSUME)
 string(REPLACE "@@BUILD_DIR@@" "${CMAKE_BINARY_DIR}" content "${content}")
 string(REPLACE "@@PROG@@" "git-cvsserver" content "${content}")
-file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/git-cvsserver ${content})
+write_script(${CMAKE_BINARY_DIR}/bin-wrappers/git-cvsserver "${content}")
 
 #options for configuring test options
 option(PERL_TESTS "Perform tests that use perl" ON)

-- 
2.39.0.1071.g97ce8966538
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 6 of 7 in “cmake: guard OS-specific code & cleanup & chmod +x on *nix”
  1. 0/6 cmake: guard OS-specific code & cleanup & chmod +x on *nixÆvar Arnfjörð Bjarmason, Dec 19, 2022
  2. 1/6 cmake: don't copy chainlint.pl to build directoryÆvar Arnfjörð Bjarmason, Dec 19, 2022
  3. 2/6 cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.shÆvar Arnfjörð Bjarmason, Dec 19, 2022
  4. 3/6 cmake: increase test timeout on Windows onlyÆvar Arnfjörð Bjarmason, Dec 19, 2022
  5. 4/6 cmake: only look for "sh" in "C:/Program Files" on WindowsÆvar Arnfjörð Bjarmason, Dec 19, 2022
  6. 6/6 cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4Ævar Arnfjörð Bjarmason, Dec 19, 2022
  7. 5/6 cmake: copy over git-p4.py for t983[56] perforce testÆvar Arnfjörð Bjarmason, Dec 19, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.