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
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Nov 3, 2022, 10:03 UTC
Message-ID
<4d6ff46f-afc1-62a6-923d-b793712a5276@dunelm.org.uk>
In-Reply-To
<patch-v3-09.12-8a660a2baa4-20221101T225022Z-avarab@gmail.com>
On 01/11/2022 22:51, Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
> 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?

Show 21 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.

Show 41 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)

> +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

Show 5 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.

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}) ?
Show 7 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
> +	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?
Best Wishes
Phillip
Show 8 quoted lines
>   #test
>   foreach(tsh ${test_scipts})
>   	add_test(NAME ${tsh}
> -		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
> +		COMMAND ${SH_EXE} ${tsh} ${GIT_TEST_OPTS}
>   		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>   	set_property(TEST ${tsh} APPEND PROPERTY ENVIRONMENT
>   		GIT_TEST_BUILD_DIR=${CMAKE_BINARY_DIR})
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 51 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.