git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/6] cmake: guard OS-specific code & cleanup & chmod +x on *nix
@ 2022-12-19 18:39 Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 1/6] cmake: don't copy chainlint.pl to build directory Ævar Arnfjörð Bjarmason
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 18:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood, Victoria Dye,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

As with [1] and [2] submitted today these are extracted from the
ejected [2]. This is independent from [1] and [2]. These are all the
cmake changes I'll be submitting for now.

This series:

 - Cleans up some dead code in the recipe (1/6)
 - Adds a missing USE_LIBPCRE2 to GIT-BUILD-OPTIONS. We didn't see
   failures due to this because the win+VS job doesn't build with
   libpcre2 (2/6)
 - Wraps Windows-specific code in "if" checks for that platform
   (3-4/6). We already support cmake on *nix, this makes it clearer
   which bits are OS-specific
 - We had failing p4 tests due to omitting copying over
   git-p4.py. This is anotehr case where we didn't see a failure
   because the "win+VS" job doesn't test that part.
 - Do a "chmod +x" of the bin-wrappers, Windows didn't care, but on
   *nix running the tests would fail *a lot* due to this.

All in all on my *nix box this brings us from:

	80% tests passed, 194 tests failed out of 983

To:

	99% tests passed, 3 tests failed out of 983

The remaining failures are due to "gitweb" being broken, but that's a
general known shortcoming of the "cmake" recipe (which again, isn't
spotted by "win+VS" in CI because it skips those tests).

The remaining bits in [3] will get us to 100% passing. I still think
those are worthwhile, but there were some outstanding concerns about
them (e.g. changing how "GIT-BUILD-DIR" worked, and adding a *nix CI
job for cmake).

I don't think there's any outstanding comments or known or potential
concerns/breakages with these changes that have to be addressed, these
are all straightforward fixes.

CI & branch for this at [4] (where I have it on top of [1] and [2])

1. https://lore.kernel.org/git/patch-1.1-0fa41115261-20221219T102205Z-avarab@gmail.com
2. https://lore.kernel.org/git/cover-0.2-00000000000-20221219T102813Z-avarab@gmail.com/
3. https://lore.kernel.org/git/cover-v6-00.15-00000000000-20221206T001617Z-avarab@gmail.com/
4. https://github.com/avar/git/tree/avar/cmake-flags-and-os-specific

Ævar Arnfjörð Bjarmason (6):
  cmake: don't copy chainlint.pl to build directory
  cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
  cmake: increase test timeout on Windows only
  cmake: only look for "sh" in "C:/Program Files" on Windows
  cmake: copy over git-p4.py for t983[56] perforce test
  cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4

 contrib/buildsystems/CMakeLists.txt | 55 +++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 14 deletions(-)

Range-diff:
 1:  fc190b379cd <  -:  ----------- cmake: don't invoke msgfmt with --statistics
 2:  1a11aa233a3 <  -:  ----------- cmake: use "-S" and "-B" to specify source and build directories
 3:  b9ddb5db1d3 <  -:  ----------- cmake: update instructions for portable CMakeLists.txt
 4:  7b7850c00ee =  1:  95a6ce2f1c7 cmake: don't copy chainlint.pl to build directory
 7:  973c8038f54 =  2:  a1653607aaf cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
12:  4905ce5321d !  3:  2b5a9d2c628 cmake: increase test timeout on Windows only
    @@ Commit message
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## contrib/buildsystems/CMakeLists.txt ##
    -@@ contrib/buildsystems/CMakeLists.txt: if(NOT GIT_CTEST_SETS_BUILD_DIR)
    - endif()
    +@@ contrib/buildsystems/CMakeLists.txt: foreach(tsh ${test_scipts})
    + 		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
      endforeach()
      
     -# This test script takes an extremely long time and is known to time out even
13:  6c6b530965d =  4:  c23f659c054 cmake: only look for "sh" in "C:/Program Files" on Windows
14:  563f1b9b045 !  5:  70a7f3e19b2 cmake: copy over git-p4.py for t983[56] perforce test
    @@ contrib/buildsystems/CMakeLists.txt
     @@ contrib/buildsystems/CMakeLists.txt: endforeach()
      file(STRINGS ${CMAKE_SOURCE_DIR}/git-p4.py content NEWLINE_CONSUME)
      string(REPLACE "#!/usr/bin/env python" "#!/usr/bin/python" content "${content}")
    - write_script(${CMAKE_BINARY_DIR}/git-p4 "${content}")
    + file(WRITE ${CMAKE_BINARY_DIR}/git-p4 ${content})
     +file(COPY ${CMAKE_SOURCE_DIR}/git-p4.py DESTINATION ${CMAKE_BINARY_DIR}/)
      
      #perl modules
 5:  82ecb797915 !  6:  3724cad82e0 cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
    @@ contrib/buildsystems/CMakeLists.txt: foreach(script ${git_perl_scripts})
      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
    - file(GLOB_RECURSE perl_modules "${CMAKE_SOURCE_DIR}/perl/*.pm")
     @@ contrib/buildsystems/CMakeLists.txt: 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}")
 6:  1f326944a07 <  -:  ----------- cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable
 8:  b8448c7a8a6 <  -:  ----------- Makefile + test-lib.sh: don't prefer cmake-built to make-built git
 9:  5135e40969e <  -:  ----------- test-lib.sh: support a "GIT_TEST_BUILD_DIR"
10:  65204463730 <  -:  ----------- cmake: optionally be able to run tests before "ctest"
11:  e25992b16f1 <  -:  ----------- cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults
15:  917a884eb65 <  -:  ----------- CI: add a "linux-cmake-test" to run cmake & ctest on linux
-- 
2.39.0.1071.g97ce8966538


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/6] cmake: don't copy chainlint.pl to build directory
  2022-12-19 18:39 [PATCH 0/6] cmake: guard OS-specific code & cleanup & chmod +x on *nix Ævar Arnfjörð Bjarmason
@ 2022-12-19 18:39 ` Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 2/6] cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh Ævar Arnfjörð Bjarmason
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 18:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood, Victoria Dye,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

When [1] made this copy of "chainlint.sed" the script was invoked in
the test-lib.sh as:

	[...] sed -f "$GIT_BUILD_DIR/t/chainlint.sed" [...]

But when [2] replaced "chainlint.sed" with a "chainlint.pl" it also
replaced that "$GIT_BUILD_DIR" with "$TEST_DIRECTORY", invoking it as:

	"$PERL_PATH" "$TEST_DIRECTORY/chainlint.pl" "$0"

So this line could have been deleted in [2] but wasn't. Let's do that
now.

1. 7f5397a07c6 (cmake: support for testing git when building out of
   the source tree, 2020-06-26)
2. 23a14f30166 (test-lib: replace chainlint.sed with chainlint.pl,
   2022-09-01)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/buildsystems/CMakeLists.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 80290edd72a..c641e9349c9 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1106,7 +1106,6 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
 	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
 		"file(WRITE ${CMAKE_SOURCE_DIR}/GIT-BUILD-DIR \"${CMAKE_BINARY_DIR}\")")
 	#misc copies
-	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.pl DESTINATION ${CMAKE_BINARY_DIR}/t/)
 	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)
 	file(GLOB mergetools "${CMAKE_SOURCE_DIR}/mergetools/*")
 	file(COPY ${mergetools} DESTINATION ${CMAKE_BINARY_DIR}/mergetools/)
-- 
2.39.0.1071.g97ce8966538


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/6] cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
  2022-12-19 18:39 [PATCH 0/6] cmake: guard OS-specific code & cleanup & chmod +x on *nix Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 1/6] cmake: don't copy chainlint.pl to build directory Ævar Arnfjörð Bjarmason
@ 2022-12-19 18:39 ` Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 3/6] cmake: increase test timeout on Windows only Ævar Arnfjörð Bjarmason
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 18:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood, Victoria Dye,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

When we build with libpcre2 which cmake has supported since [1] we
need to set "USE_LIBPCRE2='YesPlease'" (or similar) in
"GIT-BUILD-OPTIONS".

Without this e.g. t7810-grep.sh will fail, as it has tests that rely
on the behavior of !PCRE2. The reason this hasn't been noticed is that
the Windows CI doesn't have access to libpcre2.

With this the remaining two failures we had left after the preceding
step are resolved, but note that that test run didn't include the
git-p4 tests, which a subsequent commit will address).

1. 80431510a2b (cmake: add pcre2 support, 2022-05-24)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/buildsystems/CMakeLists.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index c641e9349c9..040f5f31230 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1086,6 +1086,9 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PYTHON_PATH='${PYTHON_PATH}'\
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "TAR='${TAR}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_CURL='${NO_CURL}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_EXPAT='${NO_EXPAT}'\n")
+if(PCRE2_FOUND)
+	file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "USE_LIBPCRE2='YesPlease'\n")
+endif()
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PERL='${NO_PERL}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PTHREADS='${NO_PTHREADS}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_UNIX_SOCKETS='${NO_UNIX_SOCKETS}'\n")
-- 
2.39.0.1071.g97ce8966538


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/6] cmake: increase test timeout on Windows only
  2022-12-19 18:39 [PATCH 0/6] cmake: guard OS-specific code & cleanup & chmod +x on *nix Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 1/6] cmake: don't copy chainlint.pl to build directory Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 2/6] cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh Ævar Arnfjörð Bjarmason
@ 2022-12-19 18:39 ` Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 4/6] cmake: only look for "sh" in "C:/Program Files" on Windows Ævar Arnfjörð Bjarmason
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 18:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood, Victoria Dye,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

Per c858750b41c (cmake: increase time-out for a long-running test,
2022-10-18) the reason to set a custom timeout for
t7112-reset-submodule.sh is Windows-specific. Let's only do that on
Windows then.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/buildsystems/CMakeLists.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 040f5f31230..d45b9c8e00a 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1125,8 +1125,10 @@ foreach(tsh ${test_scipts})
 		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
 endforeach()
 
-# This test script takes an extremely long time and is known to time out even
-# on fast machines because it requires in excess of one hour to run
-set_tests_properties("${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh" PROPERTIES TIMEOUT 4000)
+if(WIN32)
+	# This test script takes an extremely long time and is known to time out even
+	# on fast machines because it requires in excess of one hour to run
+	set_tests_properties("${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh" PROPERTIES TIMEOUT 4000)
+endif()
 
 endif()#BUILD_TESTING
-- 
2.39.0.1071.g97ce8966538


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/6] cmake: only look for "sh" in "C:/Program Files" on Windows
  2022-12-19 18:39 [PATCH 0/6] cmake: guard OS-specific code & cleanup & chmod +x on *nix Ævar Arnfjörð Bjarmason
                   ` (2 preceding siblings ...)
  2022-12-19 18:39 ` [PATCH 3/6] cmake: increase test timeout on Windows only Ævar Arnfjörð Bjarmason
@ 2022-12-19 18:39 ` Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 5/6] cmake: copy over git-p4.py for t983[56] perforce test Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 6/6] cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4 Ævar Arnfjörð Bjarmason
  5 siblings, 0 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 18:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood, Victoria Dye,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

Guard the finding of "SH_EXE" in "C:\Program Files" with a check for
whether we're on Windows.

This Windows-specific code was first added in [1], and later expanded
on [2], but since some of that was added this build recipe has been
made portable outside of Windows.

1. 72b6eeb81b1 (cmake: do find Git for Windows' shell interpreter,
   2020-09-28)
2. 476e54b1c60 (cmake: support local installations of git,
   2022-07-27).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/buildsystems/CMakeLists.txt | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index d45b9c8e00a..560a15ed35a 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -107,10 +107,17 @@ if(USE_VCPKG)
 	set(CMAKE_TOOLCHAIN_FILE ${VCPKG_DIR}/scripts/buildsystems/vcpkg.cmake CACHE STRING "Vcpkg toolchain file")
 endif()
 
-find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin" "$ENV{LOCALAPPDATA}/Programs/Git/bin")
-if(NOT SH_EXE)
-	message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
-			"On Windows, you can get it as part of 'Git for Windows' install at https://gitforwindows.org/")
+if(WIN32)
+	find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin" "$ENV{LOCALAPPDATA}/Programs/Git/bin")
+	if(NOT SH_EXE)
+		message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
+				"You can get it as part of 'Git for Windows' install at https://gitforwindows.org/")
+	endif()
+else()
+	find_program(SH_EXE sh)
+	if(NOT SH_EXE)
+		message(FATAL_ERROR "cannot find 'sh' in '$PATH'")
+	endif()
 endif()
 
 #Create GIT-VERSION-FILE using GIT-VERSION-GEN
-- 
2.39.0.1071.g97ce8966538


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/6] cmake: copy over git-p4.py for t983[56] perforce test
  2022-12-19 18:39 [PATCH 0/6] cmake: guard OS-specific code & cleanup & chmod +x on *nix Ævar Arnfjörð Bjarmason
                   ` (3 preceding siblings ...)
  2022-12-19 18:39 ` [PATCH 4/6] cmake: only look for "sh" in "C:/Program Files" on Windows Ævar Arnfjörð Bjarmason
@ 2022-12-19 18:39 ` Ævar Arnfjörð Bjarmason
  2022-12-19 18:39 ` [PATCH 6/6] cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4 Ævar Arnfjörð Bjarmason
  5 siblings, 0 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 18:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood, Victoria Dye,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

Since [1] two git-p4 tests have relied on there being a "git-p4.py" in
the build directory, but the cmake recipe was not updated to account
for this. Let's copy the "git-p4.py" over.

We could also change the test to e.g. grab the built "git-p4" and
alter its shebang, which would be friendly to GIT_TEST_INSTALLED, but
let's just do the bare minimum here to get cmake+ctest working without
altering the test itself.

The reason this hasn't been caught by "vs-build" and "vs-test" is
because those tests added in [2] invoke "cmake" with
"-DPYTHON_TESTS=OFF", and therefore we'd skip this part of the git-p4
tests before getting past the "do we have python?" check. Even if we
got past that the Windows CI wouldn't have a "p4" or "p4d" binary
installed, so we'd skip the tests anyway.

In a subsequent commit we'll run "cmake" and "ctest" in CI with
"ubuntu-latest", so we'll need this "git-p4.py" file.

1. f7b5ff607fa (git-p4: improve encoding handling to support
   inconsistent encodings, 2022-04-30)
2. 4c2c38e800f (ci: modification of main.yml to use cmake for vs-build
   job, 2020-06-26)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/buildsystems/CMakeLists.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 560a15ed35a..29b73ecbbbc 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -894,6 +894,7 @@ endforeach()
 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})
+file(COPY ${CMAKE_SOURCE_DIR}/git-p4.py DESTINATION ${CMAKE_BINARY_DIR}/)
 
 #perl modules
 file(GLOB_RECURSE perl_modules "${CMAKE_SOURCE_DIR}/perl/*.pm")
-- 
2.39.0.1071.g97ce8966538


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 6/6] cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
  2022-12-19 18:39 [PATCH 0/6] cmake: guard OS-specific code & cleanup & chmod +x on *nix Ævar Arnfjörð Bjarmason
                   ` (4 preceding siblings ...)
  2022-12-19 18:39 ` [PATCH 5/6] cmake: copy over git-p4.py for t983[56] perforce test Ævar Arnfjörð Bjarmason
@ 2022-12-19 18:39 ` Ævar Arnfjörð Bjarmason
  5 siblings, 0 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 18:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood, Victoria Dye,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-12-19 18:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-19 18:39 [PATCH 0/6] cmake: guard OS-specific code & cleanup & chmod +x on *nix Ævar Arnfjörð Bjarmason
2022-12-19 18:39 ` [PATCH 1/6] cmake: don't copy chainlint.pl to build directory Ævar Arnfjörð Bjarmason
2022-12-19 18:39 ` [PATCH 2/6] cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh Ævar Arnfjörð Bjarmason
2022-12-19 18:39 ` [PATCH 3/6] cmake: increase test timeout on Windows only Ævar Arnfjörð Bjarmason
2022-12-19 18:39 ` [PATCH 4/6] cmake: only look for "sh" in "C:/Program Files" on Windows Ævar Arnfjörð Bjarmason
2022-12-19 18:39 ` [PATCH 5/6] cmake: copy over git-p4.py for t983[56] perforce test Ævar Arnfjörð Bjarmason
2022-12-19 18:39 ` [PATCH 6/6] cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4 Ævar Arnfjörð Bjarmason

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).