From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
"Phillip Wood" <phillip.wood123@gmail.com>,
"Victoria Dye" <vdye@github.com>,
"Eric Sunshine" <ericsunshine@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 6/6] cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
Date: Mon, 19 Dec 2022 19:39:24 +0100 [thread overview]
Message-ID: <patch-6.6-3724cad82e0-20221219T183623Z-avarab@gmail.com> (raw)
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
prev parent reply other threads:[~2022-12-19 18:39 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Ævar Arnfjörð Bjarmason [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=patch-6.6-3724cad82e0-20221219T183623Z-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=ericsunshine@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood123@gmail.com \
--cc=vdye@github.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).