From 9cf14984c0a71b1ccdff7db0699571bf5af1209b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 9 Aug 2022 21:50:20 +0200 Subject: [PATCH 1/4] cmake: align CTest definition with Git's CI runs In Git's CI runs, the Windows tests are run with `--no-bin-wrappers` and `--no-chain-lint`, mainly to win back some time caused by the serious performance penalty paid for the tests relying so heavily on POSIX shell scripting, which only works by using a POSIX emulation layer. Let's do the same when running the tests, say, in Visual Studio. While at it, enable the command trace via `-x` and verbose output via `-v`, otherwise it would be near impossible to diagnose any problems. Signed-off-by: Johannes Schindelin --- contrib/buildsystems/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 1b23f2440d8e45..4aee1e24342d9a 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1088,7 +1088,7 @@ file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh") #test foreach(tsh ${test_scipts}) add_test(NAME ${tsh} - COMMAND ${SH_EXE} ${tsh} + COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t) endforeach() From 86ab58b65081601678a24f99eb4afa34cccaefba Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 9 Aug 2022 21:54:07 +0200 Subject: [PATCH 2/4] cmake: copy the merge tools for testing Even when running the tests via CTest, t7609 and t7610 rely on more than only a few mergetools to be copied to the build directory. Let's make it so. Signed-off-by: Johannes Schindelin --- contrib/buildsystems/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 4aee1e24342d9a..fe606c179f7db5 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1078,7 +1078,8 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH}) #misc copies file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/) file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/) - file(COPY ${CMAKE_SOURCE_DIR}/mergetools/tkdiff DESTINATION ${CMAKE_BINARY_DIR}/mergetools/) + file(GLOB mergetools "${CMAKE_SOURCE_DIR}/mergetools/*") + file(COPY ${mergetools} DESTINATION ${CMAKE_BINARY_DIR}/mergetools/) file(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-prompt.sh DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/) file(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-completion.bash DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/) endif() From 951d32869fff93415d30509c7366409e6c27b500 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 9 Aug 2022 21:58:48 +0200 Subject: [PATCH 3/4] tests: explicitly skip `chmod` calls on Windows On Windows, we use a POSIX emulation layer, the MSYS2 runtime, to allow for running shell scripts (albeit at a hefty performance cost, Git's test suite takes ~700 seconds to complete on Linux, according to Git's CI runs, while it takes more than 6,000 seconds on Linux). This emulation layer has a funny quirk when it comes to `chmod` invocations: it pretends that it succeeded, when in reality it did not do a thing (because the Access Control Lists used in Windows' permission model are so different to Unix' default permission model that Git's test suite assumes to be in effect). Git's test suite relies on this quirk by assuming that the `chmod` calls in `test_chmod` and `test_write_script` simply succeed on Windows (without actually doing anything). However, this quirk is only in effect as long as `chmod` is run inside the pseudo Unix root directory structure or within the home directory. When run outside, such invocations fail like this: chmod: changing permissions of '': Invalid argument Now, when running Git's tests in, say, Visual Studio, we frequently are in a worktree where the `chmod` invocations would fail. Let's accommodate for that by explicitly skipping those `chmod` invocations on Windows. Signed-off-by: Johannes Schindelin --- t/test-lib-functions.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 6da7273f1d5fdd..7c63b22acab525 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -492,7 +492,10 @@ test_commit_bulk () { # of a file in the working directory and add it to the index. test_chmod () { - chmod "$@" && + if test_have_prereq !MINGW + then + chmod "$@" + fi && git update-index --add "--chmod=$@" } @@ -548,7 +551,10 @@ write_script () { echo "#!${2-"$SHELL_PATH"}" && cat } >"$1" && - chmod +x "$1" + if test_have_prereq !MINGW + then + chmod +x "$1" + fi } # Usage: test_hook [options] <<-\EOF From 0070712df701d04eb3c7356cc40613b0807432f6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 9 Aug 2022 22:15:53 +0200 Subject: [PATCH 4/4] add -p: avoid ambiguous signed/unsigned comparison In the interactive `add` operation, users can choose to jump to specific hunks, and Git will present the hunk list in that case. To avoid showing too many lines at once, only a maximum of 21 hunks are shown, skipping the "mode change" pseudo hunk. The comparison performed to skip the "mode change" pseudo hunk (if any) compares a signed integer `i` to the unsigned value `mode_change` (which can be 0 or 1 because it is a 1-bit type). According to section 6.3.1.8 of the C99 standard (see e.g. https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf), what should happen is an automatic conversion of the "lesser" type to the "greater" type, but since the types differ in signedness, it is ill-defined what is the correct "usual arithmetic conversion". Which means that Visual C's behavior can (and does) differ from GCC's: When compiling Git using the latter, `add -p`'s `goto` command shows no hunks by default because it casts a negative start offset to a pretty large unsigned value, breaking the "goto hunk" test case in `t3701-add-interactive.sh`. Let's avoid that by converting the unsigned bit explicitly to a signed integer. Signed-off-by: Johannes Schindelin --- add-patch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/add-patch.c b/add-patch.c index 509ca04456bd39..3524555e2b034b 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1547,7 +1547,7 @@ static int patch_update_file(struct add_p_state *s, strbuf_remove(&s->answer, 0, 1); strbuf_trim(&s->answer); i = hunk_index - DISPLAY_HUNKS_LINES / 2; - if (i < file_diff->mode_change) + if (i < (int)file_diff->mode_change) i = file_diff->mode_change; while (s->answer.len == 0) { i = display_hunks(s, file_diff, i);