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

Re: [PATCH v3 09/12] cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 3, 2022, 15:43 UTC
Message-ID
<221103.86a6581129.gmgdl@evledraar.gmail.com>
In-Reply-To
<4d6ff46f-afc1-62a6-923d-b793712a5276@dunelm.org.uk>
On Thu, Nov 03 2022, Phillip Wood wrote:
Show 15 quoted lines
> On 01/11/2022 22:51, Ævar Arnfjörð Bjarmason wrote:
>> The rationale for adding "--no-bin-wrappers" and "--no-chain-lint" in
>> 2ea1d8b5563 (cmake: make it easier to diagnose regressions in CTest
>> runs, 2022-10-18) was those options slowed down the tests considerably
>> on Windows.
>> But since f31b6244950 (Merge branch 'yw/cmake-updates', 2022-06-07)
>> and with the preceding commits cmake and ctest are not
>> Windows-specific anymore.
>> So let's set those same options by default on Windows, but do so
>> with
>> the set() facility. As noted in cmake's documentation[1] this
>> integrates nicely with e.g. cmake-gui.
>
> Shouldn't there a documentation string for the variable if you want to
> support cmake-gui?

Yeah, I missed that. Now I've actually tested it locally with cmake-gui, and it works.

Show 23 quoted lines
>> On *nix we don't set any custom options. The change in 2ea1d8b5563
>> didn't discuss why Windows should have divergent defaults with "cmake"
>> and "make", but such reasons presumably don't apply on *nix. I for one
>> am happy with the same defaults as the tests have when running via the
>> Makefile.
>> With the "message()" addition we'll emit this when running cmake:
>> 	Generating hook-list.h
>> 	-- Using user-selected test options: -vixd
>> 	-- Configuring done
>> 	-- Generating done
>> 	-- Build files have been written to: /home/avar/g/git/contrib/buildsystems/out
>> Unfortunately cmake doesn't support a non-hacky way to pass
>> variables
>> to ctest without re-running cmake itself, so when re-running tests via
>> cmake and wanting to change the test defaults we'll need:
>> 	GIT_TEST_OPTS=-i cmake -S contrib/buildsystems -B
>> contrib/buildsystems/out &&
>> 	ctest --jobs=$(nproc) --test-dir contrib/buildsystems/out -R t0071 --verbose
>
> Rather than having to rerun cmake I think it would be nicer to use the
> shell to pass the test options when the tests are run so the user can 
> set their preferred defaults when running cmake but override them with
> GIT_TEST_OPTIONS when running ctest as I showed previously.

Yeah, I saw that, sorry about not directly addressing it. I tried it, but in the end I think I'd rather narrow down the scope in this series.

I.e. before it's hardcoded with no way to change it, after you can set it at build time.

cmake's model of viewing the world seems to really dislike the notion of dynamically setting test options for whatever reason, it's a FAQ about cmake/ctest. Your sh -c workaround is clever, and there's similar workarounds e.g. on stackoverflow.

But let's pursue that separately, and just go for the "way cmake likes it" in this series.

Show 44 quoted lines
>> 1. https://cmake.org/cmake/help/latest/command/set.html#set-cache-entry
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>   contrib/buildsystems/CMakeLists.txt | 46 +++++++++++++++++++++++++++--
>>   1 file changed, 44 insertions(+), 2 deletions(-)
>> diff --git a/contrib/buildsystems/CMakeLists.txt
>> b/contrib/buildsystems/CMakeLists.txt
>> index f0de37b35a1..6a3240d4ffa 100644
>> --- a/contrib/buildsystems/CMakeLists.txt
>> +++ b/contrib/buildsystems/CMakeLists.txt
>> @@ -49,7 +49,7 @@ To use this in Visual Studio:
>>     Open the worktree as a folder. Visual Studio 2019 and later will
>> detect
>>   the CMake configuration automatically and set everything up for you,
>> -ready to build. You can then run the tests in `t/` via a regular Git Bash.
>> +ready to build. See "== Running the tests ==" below for running the tests.
>>     Note: Visual Studio also has the option of opening
>> `CMakeLists.txt`
>>   directly; Using this option, Visual Studio will not find the source code,
>> @@ -74,6 +74,35 @@ empty(default) :
>>     NOTE: -DCMAKE_BUILD_TYPE is optional. For multi-config
>> generators like Visual Studio
>>   this option is ignored
>> +
>> +== Running the tests ==
>> +
>> +Once we've built in "contrib/buildsystems/out" the tests can be run at
>> +the top-level (note: not the generated "contrib/buildsystems/out/t/"
>> +drectory). If no top-level build is found (as created with the
>> +Makefile) the t/test-lib.sh will discover the git in
>> +"contrib/buildsystems/out" on e.g.:
>> +
>> +	(cd t && ./t0001-init.sh)
>> +	setup: had no ../git, but found & used cmake built git in ../contrib/buildsystems/out/git
>> +	[...]
>> +
>> +The tests can also be run with ctest, e.g. after building with "cmake"
>> +and "make" or "msbuild" run, from the top-level e.g.:
>> +
>> +	ctest --test-dir contrib/buildsystems/out --jobs="$(nproc)"--output-on-failure
>
> CmakeLists.txt claims we only require v3.14 which does not appear to
> support --test-dir (see 
> https://cmake.org/cmake/help/v3.14/manual/ctest.1.html)

Thanks, I missed that, updated docs etc. as appropriate in the incoming re-roll.

Show 7 quoted lines
>> +Options can be passed by setting GIT_TEST_OPTIONS before invoking
>> +cmake. E.g. on a Linux system with systemd the tests can be sped up by
>> +using a ramdisk for the scratch files:
>
> Doesn't the systemd wiki warn against using /run for things like this
> as to avoid running out of space. I thought our usual recommendation
> was to use --root=/dev/shm
Used /dev/shm in the updated docs.
Show 8 quoted lines
>> +	GIT_TEST_OPTS="--root=/run/user/$(id -u)/ctest" cmake -S contrib/buildsystems -B contrib/buildsystems/out
>> +	[...]
>> +	-- Using user-selected test options: --root=/run/user/1001/ctest
>> +
>> +Then running the tests with "ctest" (here with --jobs="$(nproc)"):
>
> I think it would be helpful to show setting --jobs at configure time
> as it makes running the tests simpler.

mm, you mean turn it into a set(...) variable? Sounds useful, or ideally grabbing some pre-set cmake idea of the parallelism.

But for now I'd like to just leave it as "maybe try running this", rather than integrate it into the whole cmake/ctest chain.

Show 10 quoted lines
>> +	ctest --jobs=$(nproc) --test-dir contrib/buildsystems/out
>>   ]]
>>   cmake_minimum_required(VERSION 3.14)
>>   @@ -1110,10 +1139,23 @@ endif()
>>     file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
>>   +string(COMPARE NOTEQUAL "$ENV{GIT_TEST_OPTS}" ""
>> HAVE_USER_GIT_TEST_OPTS)
>> +if(HAVE_USER_GIT_TEST_OPTS)
>
> if (DEFINED ENV{GIT_TEST_OPTS}) ?
Thanks! That's much better.
Show 9 quoted lines
>> +	set(GIT_TEST_OPTS "$ENV{GIT_TEST_OPTS}")
>> +	message(STATUS "Using user-selected test options: ${GIT_TEST_OPTS}")
>> +elseif(WIN32)
>> +	set(GIT_TEST_OPTS "--no-bin-wrappers --no-chain-lint -vx")
>> +	message(STATUS "Using Windowns-specific default test options: ${GIT_TEST_OPTS}")
>> +else()
>> +	set(GIT_TEST_OPTS "")
>
> I'd like to see us setting -vx here so users get debugging logs

I think it might make sense to do that by default, but let's consider that separately, and if we're going to do that we should set that default for the t/Makefile (or t/test-lib.h), not just cmake.

The reason it has these settings now is because of Windows-specific trade-offs in the already landed topic.

But since the topic of this series is to port it to run nicely on *nix I don't see why we'd have it as a goal to have "cmake" / "ctest" behave differently than "make" or "make test".

Except of course cases where some inherent differente between the toolchains suggests that we should.

But in this case I don't see why that's the case, if I run a full "ctest" run and have failures, I might wish I've got logs, but the same goes for:

     make test GIT_TEST_OPTS="--verbose-log -vx"
Show 5 quoted lines
>> +	message(STATUS "No custom test options selected, set e.g. GIT_TEST_OPTS=\"-vixd\"")
>> +endif()
>> +separate_arguments(GIT_TEST_OPTS)
>
> What rules does this use for separating arguments?

Bad ones :( I've updated the commit message accordingly, but kept this, I couldn't find some non-crappy way to e.g. handle spaces in parameters on the cmake version we require.

Previous: Phillip WoodNext: Ævar Arnfjörð Bjarmason
Message 52 of 113 in “cmake: fix *nix & general issues, no test-lib.sh editing, ctest in CI”
  1. 0/9 cmake: fix *nix & general issues, no test-lib.sh editing, ctest in CIÆvar Arnfjörð Bjarmason, Oct 21, 2022
  2. 1/9 cmake: don't copy chainlint.pl to build directoryÆvar Arnfjörð Bjarmason, Oct 21, 2022
  3. 2/9 cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4Ævar Arnfjörð Bjarmason, Oct 21, 2022
  4. Phillip WoodOct 21, 2022
  5. Ævar Arnfjörð BjarmasonOct 21, 2022
  6. 4/9 cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.shÆvar Arnfjörð Bjarmason, Oct 21, 2022
  7. 5/9 test-lib.sh: support a "GIT_TEST_BUILD_DIR"Ævar Arnfjörð Bjarmason, Oct 21, 2022
  8. 6/9 cmake: use GIT_TEST_BUILD_DIR instead of editing hackÆvar Arnfjörð Bjarmason, Oct 21, 2022
  9. Phillip WoodOct 21, 2022
  10. Ævar Arnfjörð BjarmasonOct 21, 2022
  11. Phillip WoodOct 25, 2022
  12. 3/9 cmake & test-lib.sh: add a $GIT_SOURCE_DIR variableÆvar Arnfjörð Bjarmason, Oct 21, 2022
  13. 7/9 cmake: support using GIT_TEST_OPTS from the environmentÆvar Arnfjörð Bjarmason, Oct 21, 2022
  14. Phillip WoodOct 21, 2022
  15. Ævar Arnfjörð BjarmasonOct 21, 2022
  16. Phillip WoodOct 25, 2022
  17. Ævar Arnfjörð BjarmasonOct 25, 2022
  18. 8/9 cmake: copy over git-p4.py for t983[56] perforce testÆvar Arnfjörð Bjarmason, Oct 21, 2022
  19. 9/9 CI: add a "linux-cmake-test" to run cmake & ctest on linuxÆvar Arnfjörð Bjarmason, Oct 21, 2022
  20. Johannes SchindelinOct 21, 2022
  21. Phillip WoodOct 21, 2022
  22. Ævar Arnfjörð BjarmasonOct 21, 2022
  23. Phillip WoodOct 25, 2022
  24. 00/11 cmake: document, fix on *nix, add CIÆvar Arnfjörð Bjarmason, Oct 27, 2022
  25. 01/11 cmake: don't "mkdir -p" and "cd" in build instructionsÆvar Arnfjörð Bjarmason, Oct 27, 2022
  26. 02/11 cmake: update instructions for portable CMakeLists.txtÆvar Arnfjörð Bjarmason, Oct 27, 2022
  27. Eric SunshineOct 27, 2022
  28. 03/11 cmake: don't copy chainlint.pl to build directoryÆvar Arnfjörð Bjarmason, Oct 27, 2022
  29. 05/11 cmake & test-lib.sh: add a $GIT_SOURCE_DIR variableÆvar Arnfjörð Bjarmason, Oct 27, 2022
  30. 04/11 cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4Ævar Arnfjörð Bjarmason, Oct 27, 2022
  31. 06/11 cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.shÆvar Arnfjörð Bjarmason, Oct 27, 2022
  32. 08/11 Makefile + cmake: use environment, not GIT-BUILD-DIRÆvar Arnfjörð Bjarmason, Oct 27, 2022
  33. 07/11 test-lib.sh: support a "GIT_TEST_BUILD_DIR"Ævar Arnfjörð Bjarmason, Oct 27, 2022
  34. 09/11 cmake: support GIT_TEST_OPTS, abstract away WIN32 defaultsÆvar Arnfjörð Bjarmason, Oct 27, 2022
  35. 10/11 cmake: copy over git-p4.py for t983[56] perforce testÆvar Arnfjörð Bjarmason, Oct 27, 2022
  36. 11/11 CI: add a "linux-cmake-test" to run cmake & ctest on linuxÆvar Arnfjörð Bjarmason, Oct 27, 2022
  37. 00/12 cmake: document, fix on *nix, add CIÆvar Arnfjörð Bjarmason, Nov 1, 2022
  38. 01/12 cmake: don't "mkdir -p" and "cd" in build instructionsÆvar Arnfjörð Bjarmason, Nov 1, 2022
  39. Phillip WoodNov 3, 2022
  40. 02/12 cmake: update instructions for portable CMakeLists.txtÆvar Arnfjörð Bjarmason, Nov 1, 2022
  41. Eric SunshineNov 1, 2022
  42. Phillip WoodNov 3, 2022
  43. Ævar Arnfjörð BjarmasonNov 3, 2022
  44. 03/12 cmake: don't copy chainlint.pl to build directoryÆvar Arnfjörð Bjarmason, Nov 1, 2022
  45. 04/12 cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4Ævar Arnfjörð Bjarmason, Nov 1, 2022
  46. 05/12 cmake & test-lib.sh: add a $GIT_SOURCE_DIR variableÆvar Arnfjörð Bjarmason, Nov 1, 2022
  47. 06/12 cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.shÆvar Arnfjörð Bjarmason, Nov 1, 2022
  48. 07/12 test-lib.sh: support a "GIT_TEST_BUILD_DIR"Ævar Arnfjörð Bjarmason, Nov 1, 2022
  49. 08/12 Makefile + cmake: use environment, not GIT-BUILD-DIRÆvar Arnfjörð Bjarmason, Nov 1, 2022
  50. 09/12 cmake: support GIT_TEST_OPTS, abstract away WIN32 defaultsÆvar Arnfjörð Bjarmason, Nov 1, 2022
  51. Phillip WoodNov 3, 2022
  52. Ævar Arnfjörð BjarmasonNov 3, 2022
  53. 11/12 cmake: copy over git-p4.py for t983[56] perforce testÆvar Arnfjörð Bjarmason, Nov 1, 2022
  54. 10/12 cmake: increase test timeout on Windows onlyÆvar Arnfjörð Bjarmason, Nov 1, 2022
  55. 12/12 CI: add a "linux-cmake-test" to run cmake & ctest on linuxÆvar Arnfjörð Bjarmason, Nov 1, 2022
  56. 00/14 cmake: document, fix on *nix, add CIÆvar Arnfjörð Bjarmason, Nov 3, 2022
  57. 02/14 cmake: use "-S" and "-B" to specify source and build directoriesÆvar Arnfjörð Bjarmason, Nov 3, 2022
  58. 05/14 cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4Ævar Arnfjörð Bjarmason, Nov 3, 2022
  59. 04/14 cmake: don't copy chainlint.pl to build directoryÆvar Arnfjörð Bjarmason, Nov 3, 2022
  60. 01/14 cmake: don't invoke msgfmt with --statisticsÆvar Arnfjörð Bjarmason, Nov 3, 2022
  61. 03/14 cmake: update instructions for portable CMakeLists.txtÆvar Arnfjörð Bjarmason, Nov 3, 2022
  62. 06/14 cmake & test-lib.sh: add a $GIT_SOURCE_DIR variableÆvar Arnfjörð Bjarmason, Nov 3, 2022
  63. 07/14 cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.shÆvar Arnfjörð Bjarmason, Nov 3, 2022
  64. 13/14 cmake: copy over git-p4.py for t983[56] perforce testÆvar Arnfjörð Bjarmason, Nov 3, 2022
  65. 09/14 Makefile + cmake: use environment, not GIT-BUILD-DIRÆvar Arnfjörð Bjarmason, Nov 3, 2022
  66. 08/14 test-lib.sh: support a "GIT_TEST_BUILD_DIR"Ævar Arnfjörð Bjarmason, Nov 3, 2022
  67. 11/14 cmake: increase test timeout on Windows onlyÆvar Arnfjörð Bjarmason, Nov 3, 2022
  68. 12/14 cmake: only look for "sh" in "C:/Program Files" on WindowsÆvar Arnfjörð Bjarmason, Nov 3, 2022
  69. 14/14 CI: add a "linux-cmake-test" to run cmake & ctest on linuxÆvar Arnfjörð Bjarmason, Nov 3, 2022
  70. 10/14 cmake: support GIT_TEST_OPTS, abstract away WIN32 defaultsÆvar Arnfjörð Bjarmason, Nov 3, 2022
  71. Taylor BlauNov 5, 2022
  72. Phillip WoodNov 8, 2022
  73. 00/15 cmake: document, fix on *nix, add CIÆvar Arnfjörð Bjarmason, Dec 2, 2022
  74. 01/15 cmake: don't invoke msgfmt with --statisticsÆvar Arnfjörð Bjarmason, Dec 2, 2022
  75. 03/15 cmake: update instructions for portable CMakeLists.txtÆvar Arnfjörð Bjarmason, Dec 2, 2022
  76. 02/15 cmake: use "-S" and "-B" to specify source and build directoriesÆvar Arnfjörð Bjarmason, Dec 2, 2022
  77. Eric SunshineDec 3, 2022
  78. 04/15 cmake: don't copy chainlint.pl to build directoryÆvar Arnfjörð Bjarmason, Dec 2, 2022
  79. 05/15 cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4Ævar Arnfjörð Bjarmason, Dec 2, 2022
  80. 07/15 cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.shÆvar Arnfjörð Bjarmason, Dec 2, 2022
  81. 06/15 cmake & test-lib.sh: add a $GIT_SOURCE_DIR variableÆvar Arnfjörð Bjarmason, Dec 2, 2022
  82. 08/15 Makefile + test-lib.sh: don't prefer cmake-built to make-built gitÆvar Arnfjörð Bjarmason, Dec 2, 2022
  83. 09/15 test-lib.sh: support a "GIT_TEST_BUILD_DIR"Ævar Arnfjörð Bjarmason, Dec 2, 2022
  84. Eric SunshineDec 3, 2022
  85. 10/15 cmake: optionally be able to run tests before "ctest"Ævar Arnfjörð Bjarmason, Dec 2, 2022
  86. 11/15 cmake: support GIT_TEST_OPTS, abstract away WIN32 defaultsÆvar Arnfjörð Bjarmason, Dec 2, 2022
  87. Eric SunshineDec 3, 2022
  88. Ævar Arnfjörð BjarmasonDec 3, 2022
  89. Eric SunshineDec 3, 2022
  90. 12/15 cmake: increase test timeout on Windows onlyÆvar Arnfjörð Bjarmason, Dec 2, 2022
  91. 13/15 cmake: only look for "sh" in "C:/Program Files" on WindowsÆvar Arnfjörð Bjarmason, Dec 2, 2022
  92. 14/15 cmake: copy over git-p4.py for t983[56] perforce testÆvar Arnfjörð Bjarmason, Dec 2, 2022
  93. 15/15 CI: add a "linux-cmake-test" to run cmake & ctest on linuxÆvar Arnfjörð Bjarmason, Dec 2, 2022
  94. Eric SunshineDec 3, 2022
  95. 00/15 cmake: document, fix on *nix, add CIÆvar Arnfjörð Bjarmason, Dec 6, 2022
  96. 01/15 cmake: don't invoke msgfmt with --statisticsÆvar Arnfjörð Bjarmason, Dec 6, 2022
  97. 02/15 cmake: use "-S" and "-B" to specify source and build directoriesÆvar Arnfjörð Bjarmason, Dec 6, 2022
  98. 04/15 cmake: don't copy chainlint.pl to build directoryÆvar Arnfjörð Bjarmason, Dec 6, 2022
  99. 03/15 cmake: update instructions for portable CMakeLists.txtÆvar Arnfjörð Bjarmason, Dec 6, 2022
  100. 05/15 cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4Ævar Arnfjörð Bjarmason, Dec 6, 2022
  101. 06/15 cmake & test-lib.sh: add a $GIT_SOURCE_DIR variableÆvar Arnfjörð Bjarmason, Dec 6, 2022
  102. 07/15 cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.shÆvar Arnfjörð Bjarmason, Dec 6, 2022
  103. 08/15 Makefile + test-lib.sh: don't prefer cmake-built to make-built gitÆvar Arnfjörð Bjarmason, Dec 6, 2022
  104. 09/15 test-lib.sh: support a "GIT_TEST_BUILD_DIR"Ævar Arnfjörð Bjarmason, Dec 6, 2022
  105. 10/15 cmake: optionally be able to run tests before "ctest"Ævar Arnfjörð Bjarmason, Dec 6, 2022
  106. 11/15 cmake: support GIT_TEST_OPTS, abstract away WIN32 defaultsÆvar Arnfjörð Bjarmason, Dec 6, 2022
  107. 12/15 cmake: increase test timeout on Windows onlyÆvar Arnfjörð Bjarmason, Dec 6, 2022
  108. 14/15 cmake: copy over git-p4.py for t983[56] perforce testÆvar Arnfjörð Bjarmason, Dec 6, 2022
  109. 13/15 cmake: only look for "sh" in "C:/Program Files" on WindowsÆvar Arnfjörð Bjarmason, Dec 6, 2022
  110. 15/15 CI: add a "linux-cmake-test" to run cmake & ctest on linuxÆvar Arnfjörð Bjarmason, Dec 6, 2022
  111. Phillip WoodDec 7, 2022
  112. Ævar Arnfjörð BjarmasonDec 8, 2022
  113. Phillip WoodNov 3, 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.