[PATCH v2 00/11] cmake: document, fix on *nix, add CI
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Oct 27, 2022, 03:26 UTC
- Message-ID
- <cover-v2-00.11-00000000000-20221027T032622Z-avarab@gmail.com>
- In-Reply-To
- <cover-0.9-00000000000-20221021T091013Z-avarab@gmail.com>
This topig gets our tests from passing ~80% with ctest on *nix to passing 100%. Now on top of next's js/cmake-updates. Comments inline on patches:n
Ævar Arnfjörð Bjarmason (11): cmake: don't "mkdir -p" and "cd" in build instructions cmake: update instructions for portable CMakeLists.txt
These are new, and document how to use cmake+ctest, and make the instructions more OS-agnostic.
cmake: don't copy chainlint.pl to build directory
Same, except to rebase on js/cmake-updates.
cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4I tried to use the suggested "write_script" function, but couldn't get past cmake quoting issues. So the commit message is updated, but the change is the same.
cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh test-lib.sh: support a "GIT_TEST_BUILD_DIR" Makefile + cmake: use environment, not GIT-BUILD-DIR
Generalize the whole "test a build dir over there", and have cmake & make not step on each other's toes.
cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults
The test param defaults in js/cmake-updates are kept on Windows, all platforms benefit from having an easier way to provide custom test options.
cmake: copy over git-p4.py for t983[56] perforce test
Now at 101% tests passing :)
CI: add a "linux-cmake-test" to run cmake & ctest on linux
See the CI (and topic itself) at https://github.com/avar/git/actions/runs/3333306724/jobs/5515607569
.github/workflows/main.yml | 3 + .gitignore | 1 - Makefile | 1 - ci/run-build-and-tests.sh | 13 ++- contrib/buildsystems/CMakeLists.txt | 133 +++++++++++++++++++++------- t/README | 3 + t/lib-gettext.sh | 2 +- t/lib-gitweb.sh | 2 +- t/t7609-mergetool--lib.sh | 2 +- t/t9902-completion.sh | 14 +-- t/t9903-bash-prompt.sh | 2 +- t/test-lib.sh | 27 +++++- 12 files changed, 152 insertions(+), 51 deletions(-)
Range-diff against v1:
1: 16c99177e6d < -: ----------- cmake: don't copy chainlint.pl to build directory
-: ----------- > 1: 667a2bd5271 cmake: don't "mkdir -p" and "cd" in build instructions
-: ----------- > 2: 9e2470dcb95 cmake: update instructions for portable CMakeLists.txt
-: ----------- > 3: 2d7d9742a73 cmake: don't copy chainlint.pl to build directory
2: 2c1d194e590 ! 4: ea8a3feec81 cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
@@ Metadata
## Commit message ##
cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
- The cmake recipe added in [1] did not create the bin-wrappers/*
- directory, and thus fell back on running the tests with the equivalent
- of "--no-bin-wrappers".
+ 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.
- Thus 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]:
+ 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
- Or rather, this is what would have happened on *nix, but until [3]
- there wasn't any non-Windows support for "cmake". On Windows it didn't
- matter that the bin-wrappers weren't made executable, since there's no
- executable bit, instead the emulation layer looks at whether a file
- has a shebang.
+ 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.:
- But with [3] we've effectively used the semi-equivalent of
- "--no-bin-wrappers" unintentionally on *nix, and furthermore because
- we didn't make these executable
+ 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
- In addition, 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". I.e. the equivalent
- of this (in the "out" directory) would happen:
+ Fails around 20% of our testts 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'.
@@ Commit message
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.
+ our built "git-p4" wasn't executable, etc. There's also a few other
+ outstanding issues, which will be fixed in subsequent commits.
- This change should ideally use file(CHMOD ...), but that's much newer
- than our required cmake version[1].
+ This change should ideally use file(CHMOD ...), but the "file(CHMOD"
+ feature is much newer than our required cmake version[5].
- 1. a30e4c531d9 (Merge branch 'ss/cmake-build', 2020-08-11)
+ 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.
+
+ There was a suggestion of using a function to abstract this away[6],
+ which sounds good. But after spending too long trying to get all
+ combinations of "${content}" and ${content} (unqoted) in the function
+ and its callers working I wasn't able to fix the quoting issues it
+ introduced.
+
+ A lot of this is duplicated already, we can follow-up at some other
+ time with refactoring, and address any tricky quoting issues in
+ calling function with these parameters then.
+
+ 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. f31b6244950 (Merge branch 'yw/cmake-updates', 2022-06-07)
- 4. https://cmake.org/cmake/help/latest/command/file.html#chmod
+ 3. 2ea1d8b5563 (cmake: make it easier to diagnose regressions in CTest
+ runs, 2022-10-18)
+ 4. a30e4c531d9 (Merge branch 'ss/cmake-build', 2020-08-11)
+ 5. https://cmake.org/cmake/help/latest/command/file.html#chmod
+ 6. https://lore.kernel.org/git/0fda0e54-0432-7690-74a7-3d1a59923e0c@dunelm.org.uk/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
3: addaf73992f ! 5: 6387682db06 cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable
@@ Commit message
recipe has needed to copy various assets to that
"contrib/buildsystems/out" directory.
- Let's instead teach the test-lib.sh that there's such a thing as the
- "$GIT_SOURCE_DIR" distinct from the "$GIT_BUILD_DIR".
+ But we've only been doing this for the subsets of tests that run on
+ Windows, and which have otherwise been covered by that CI target. The
+ CI target builds (among other things) with "-DPERL_TESTS=OFF
+ -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON", see [1]. Furthermore, the
+ CI isn't testing from the "contrib/buildsystems/out" directory,
+ instead it clobbers the top-level MAkefile.
+
+ There was a recent commit to fix a subset of these issues, see
+ 6a83b5f0810 (cmake: copy the merge tools for testing, 2022-10-18).
+
+ Let's stop going for that approach, and instead teach the test-lib.sh
+ that there's such a thing as the "$GIT_SOURCE_DIR" distinct from the
+ "$GIT_BUILD_DIR".
Just as the "$TEST_DIRECTORY" always points to our actual "t"
directory (not the "[...]/out/t" cmake creates), this new
"$GIT_SOURCE_DIR" will always be the top-level source directory.
- So even though the "GIT_BUILD_DIR=(.*)" line in t/test-lib.sh will
- still be altered by CMakeLists.txt, that recipe will no longer need to
- copy over various things from our source directory, as the tests now
- know where to find those assets.
+ With this change we now pass 3/5 of the tests that we still had
+ failing with the fixes in the preceding commit.
+
+ 1. 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 ##
@@ contrib/buildsystems/CMakeLists.txt: if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
- "file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
- "string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
- "file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
+ #Setting the build directory in test-lib.sh before running tests
+ 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}/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()
@@ t/t9903-bash-prompt.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
## t/test-lib.sh ##
@@ t/test-lib.sh: then
- # elsewhere
- TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
+ echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
+ exit 1
fi
--GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
-+
-+# The GIT_{SOURCE,BUILD}_DIR is always the same, except when
-+# CMakeLists.txt replaces the "GIT_BUILD_DIR" line with the path to
-+# its build directory.
-+GIT_SOURCE_DIR="${TEST_DIRECTORY%/t}"
-+GIT_BUILD_DIR="$GIT_SOURCE_DIR"
+
- if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
++# For CMake the top-level source directory is different from our build
++# directory. With the top-level Makefile they're the same.
++GIT_SOURCE_DIR="$GIT_BUILD_DIR"
+ if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
then
- echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
+ GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
@@ t/test-lib.sh: then
make_valgrind_symlink $file
done
4: 52cd674d5b8 ! 6: 29a9811857f cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
@@ Commit message
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>
5: 6199a913c0a ! 7: a9ea3867f5f test-lib.sh: support a "GIT_TEST_BUILD_DIR"
@@ Commit message
[apply this change to git2]
(cd git2 && GIT_TEST_BUILD_DIR="$PWD/../git1" make -C t prove)
+ This facility and file-based instructions to have the test suite use
+ another build directory[2] are mutually exclusive, but in a subsequent
+ commit we'll make CMake use this instead.
+
1. 6720721e152 (test-lib.sh: Allow running the test suite against
installed git, 2009-03-16)
+ 2. 350a005e366 (cmake: avoid editing t/test-lib.sh, 2022-10-18)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ t/README: override the location of the dashed-form subcommands (what
## t/test-lib.sh ##
@@ t/test-lib.sh: fi
- # its build directory.
- GIT_SOURCE_DIR="${TEST_DIRECTORY%/t}"
- GIT_BUILD_DIR="$GIT_SOURCE_DIR"
+ # For CMake the top-level source directory is different from our build
+ # directory. With the top-level Makefile they're the same.
+ GIT_SOURCE_DIR="$GIT_BUILD_DIR"
+-if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
+if test -n "$GIT_TEST_BUILD_DIR"
+then
+ GIT_BUILD_DIR="$GIT_TEST_BUILD_DIR"
-+fi
-
- if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
++elif test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
then
+ GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
+ # On Windows, we must convert Windows paths lest they contain a colon
6: 45f1a4e6f93 < -: ----------- cmake: use GIT_TEST_BUILD_DIR instead of editing hack
7: fc9f036695f < -: ----------- cmake: support using GIT_TEST_OPTS from the environment
-: ----------- > 8: 51bb01b99d4 Makefile + cmake: use environment, not GIT-BUILD-DIR
-: ----------- > 9: 9f5276d79c9 cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults
8: 56102c2a4bf = 10: ef6a304368c cmake: copy over git-p4.py for t983[56] perforce test
9: b81f18dec61 = 11: 158a41ca7a4 CI: add a "linux-cmake-test" to run cmake & ctest on linux-- 2.38.0.1250.ge066ede4da3