Added an option for range-v3 to build tests #1535

adembudak · 2024-05-27T12:11:49Z

Added range_v3_tests build option and set it off by default.

eli-schwartz

What's the advantage of the option? e.g. there aren't any external dependencies and you can restrict which tests to run by using the subproject name as a suite.

eli-schwartz · 2024-05-27T12:16:34Z

subprojects/packagefiles/range-v3/meson_options.txt

@@ -0,0 +1 @@
+option('range_v3_tests', type : 'boolean', value : false)


defaulting tests off means they aren't run in the wrapdb CI

meson options are namespaces already, this option would get used as -Drange-v3:range_v3_tests=true. Don't include project names inside option names. Yes, I know you need it in cmake...

i wanted to keep the variable name the same with the original repo (in upstream repo CMake configuration), but i agree having it just tests would have a better ergonomics and also following the current conventions of the Wrap files.

Still defaults to false?

Umm, yes. Don't build the tests by default.

Again, you need to make it build in CI. Previously it did so because it was enabled ~~by default~~ always.

See README.md.

adembudak · 2024-05-27T13:44:54Z

What's the advantage of the option? e.g. there aren't any external dependencies and you can restrict which tests to run by using the subproject name as a suite.

The library files are header only which doesn't need to compile, enabled tests (400+ of them) need to compiled, waiting less and spending less CPU cycles would be some of the reason i guess :)

eli-schwartz

When fixing a review request in one commit, please use git commit --amend && git push --force to update. :) It is one logical commit, not two.

Per README.md, the releases.json file contains information about the release, and should have the revision incremented when making changes to the meson.build files.

eli-schwartz · 2024-05-27T13:59:37Z

subprojects/packagefiles/range-v3/meson.build

+if get_option('tests')
+  subdir('test')
+endif
+


There's stray whitespace at the end of the files you edited?

eli-schwartz · 2024-05-27T14:00:06Z

subprojects/packagefiles/range-v3/meson_options.txt

@@ -0,0 +1 @@
+option('range_v3_tests', type : 'boolean', value : false)


Still defaults to false?

eli-schwartz · 2024-05-27T14:01:00Z

The library files are header only which doesn't need to compile, enabled tests (400+ of them) need to compiled, waiting less and spending less CPU cycles would be some of the reason i guess :)

Ah right, those are still compiled by default even if the tests themselves don't run...

adembudak · 2024-05-27T14:14:15Z

Changed range_v3_tests to tests. Not sure whether it should be tests or test though. I chose the former but any feedback is greatly appreciated.

benoit-pierre · 2024-05-27T14:17:14Z

tests appears to be the more common choice:

▹ grep -wl "'test'" subprojects/packagefiles/*/meson_options.txt | wc -l
4
▹ grep -wl "'tests'" subprojects/packagefiles/*/meson_options.txt | wc -l
45

jpakkane · 2024-06-09T21:21:36Z

Please do not have merge commits in your branch, keep your branch up to date using rebase instead.

adembudak · 2024-06-09T21:34:13Z

Please do not have merge commits in your branch, keep your branch up to date using rebase instead.

Thanks, will keep it in mind next time.

eli-schwartz · 2024-06-09T22:21:23Z

There are two problems:

the issue with merge commits isn't "please remember next time", but rather "we can't merge the PR until the merge commit is removed"
the merge commit also contains significant logic changes to the range-v3 wrap, which is one of the reasons the policy exists to not merge a merge commit

P.S. Sorry for not noticing the request to review.

eli-schwartz · 2024-06-09T22:21:32Z

[1/479] Compiling C++ object subprojects/range-v3-0.12.0/test/action/range.v3.test.act.concepts.exe.p/cont_concepts.cpp.obj
FAILED: subprojects/range-v3-0.12.0/test/action/range.v3.test.act.concepts.exe.p/cont_concepts.cpp.obj 
"clang-cl" "-Isubprojects\range-v3-0.12.0\test\action\range.v3.test.act.concepts.exe.p" "-Isubprojects\range-v3-0.12.0\test\action" "-I..\subprojects\range-v3-0.12.0\test\action" "-I..\subprojects\range-v3-0.12.0\include" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/W2" "/EHsc" "/std:c++17" "/permissive-" "/Od" "/Zi" "/Fdsubprojects\range-v3-0.12.0\test\action\range.v3.test.act.concepts.exe.p\cont_concepts.cpp.pdb" /Fosubprojects/range-v3-0.12.0/test/action/range.v3.test.act.concepts.exe.p/cont_concepts.cpp.obj "/c" ../subprojects/range-v3-0.12.0/test/action/cont_concepts.cpp
In file included from ../subprojects/range-v3-0.12.0/test/action/cont_concepts.cpp:10:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\array:8:
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\yvals_core.h(898,1): error: static assertion failed: error STL1000: Unexpected compiler version, expected Clang 17.0.0 or newer.
_EMIT_STL_ERROR(STL1000, "Unexpected compiler version, expected Clang 17.0.0 or newer.");
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\yvals_core.h(519,44): note: expanded from macro '_EMIT_STL_ERROR'
#define _EMIT_STL_ERROR(NUMBER, MESSAGE)   static_assert(false, "error " #NUMBER ": " MESSAGE)
                                           ^             ~~~~~
1 error generated.

What does this mean???

Added `tests` build option and set it off by default.

adembudak · 2024-06-10T00:01:54Z

[1/479] Compiling C++ object subprojects/range-v3-0.12.0/test/action/range.v3.test.act.concepts.exe.p/cont_concepts.cpp.obj
FAILED: subprojects/range-v3-0.12.0/test/action/range.v3.test.act.concepts.exe.p/cont_concepts.cpp.obj 
"clang-cl" "-Isubprojects\range-v3-0.12.0\test\action\range.v3.test.act.concepts.exe.p" "-Isubprojects\range-v3-0.12.0\test\action" "-I..\subprojects\range-v3-0.12.0\test\action" "-I..\subprojects\range-v3-0.12.0\include" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/W2" "/EHsc" "/std:c++17" "/permissive-" "/Od" "/Zi" "/Fdsubprojects\range-v3-0.12.0\test\action\range.v3.test.act.concepts.exe.p\cont_concepts.cpp.pdb" /Fosubprojects/range-v3-0.12.0/test/action/range.v3.test.act.concepts.exe.p/cont_concepts.cpp.obj "/c" ../subprojects/range-v3-0.12.0/test/action/cont_concepts.cpp
In file included from ../subprojects/range-v3-0.12.0/test/action/cont_concepts.cpp:10:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\array:8:
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\yvals_core.h(898,1): error: static assertion failed: error STL1000: Unexpected compiler version, expected Clang 17.0.0 or newer.
_EMIT_STL_ERROR(STL1000, "Unexpected compiler version, expected Clang 17.0.0 or newer.");
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\yvals_core.h(519,44): note: expanded from macro '_EMIT_STL_ERROR'
#define _EMIT_STL_ERROR(NUMBER, MESSAGE)   static_assert(false, "error " #NUMBER ": " MESSAGE)
                                           ^             ~~~~~
1 error generated.

What does this mean???

Seems like something related with clang-cl version, the runner has version 16.0.6:

range-v3| Project name: range-v3
range-v3| Project version: 0.12.0
range-v3| C++ compiler for the host machine: clang-cl (clang-cl 16.0.6)

eli-schwartz · 2024-06-10T00:28:20Z

Aha:
actions/runner-images#10001
actions/runner-images#10018

eli-schwartz requested changes May 27, 2024

View reviewed changes

adembudak requested a review from eli-schwartz May 27, 2024 13:51

eli-schwartz reviewed May 27, 2024

View reviewed changes

adembudak force-pushed the master branch 2 times, most recently from 585929d to 9156bb4 Compare May 27, 2024 14:31

adembudak requested a review from eli-schwartz May 27, 2024 14:32

Added an option for range-v3 to build tests

982fae4

Added `tests` build option and set it off by default.

adembudak force-pushed the master branch from 53314ad to 982fae4 Compare June 9, 2024 23:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added an option for range-v3 to build tests #1535

Added an option for range-v3 to build tests #1535

adembudak commented May 27, 2024

eli-schwartz left a comment

eli-schwartz May 27, 2024

adembudak May 27, 2024 •

edited

Loading

eli-schwartz May 27, 2024

adembudak May 27, 2024

eli-schwartz May 27, 2024

adembudak commented May 27, 2024

eli-schwartz left a comment

eli-schwartz May 27, 2024

adembudak May 27, 2024 •

edited

Loading

eli-schwartz May 27, 2024

eli-schwartz commented May 27, 2024

adembudak commented May 27, 2024

benoit-pierre commented May 27, 2024

jpakkane commented Jun 9, 2024

adembudak commented Jun 9, 2024 •

edited

Loading

eli-schwartz commented Jun 9, 2024

eli-schwartz commented Jun 9, 2024

adembudak commented Jun 10, 2024 •

edited

Loading

eli-schwartz commented Jun 10, 2024

		@@ -0,0 +1 @@
		option('range_v3_tests', type : 'boolean', value : false)

Added an option for range-v3 to build tests #1535

Are you sure you want to change the base?

Added an option for range-v3 to build tests #1535

Conversation

adembudak commented May 27, 2024

eli-schwartz left a comment

Choose a reason for hiding this comment

eli-schwartz May 27, 2024

Choose a reason for hiding this comment

adembudak May 27, 2024 • edited Loading

Choose a reason for hiding this comment

eli-schwartz May 27, 2024

Choose a reason for hiding this comment

adembudak May 27, 2024

Choose a reason for hiding this comment

eli-schwartz May 27, 2024

Choose a reason for hiding this comment

adembudak commented May 27, 2024

eli-schwartz left a comment

Choose a reason for hiding this comment

eli-schwartz May 27, 2024

Choose a reason for hiding this comment

adembudak May 27, 2024 • edited Loading

Choose a reason for hiding this comment

eli-schwartz May 27, 2024

Choose a reason for hiding this comment

eli-schwartz commented May 27, 2024

adembudak commented May 27, 2024

benoit-pierre commented May 27, 2024

jpakkane commented Jun 9, 2024

adembudak commented Jun 9, 2024 • edited Loading

eli-schwartz commented Jun 9, 2024

eli-schwartz commented Jun 9, 2024

adembudak commented Jun 10, 2024 • edited Loading

eli-schwartz commented Jun 10, 2024

adembudak May 27, 2024 •

edited

Loading

adembudak May 27, 2024 •

edited

Loading

adembudak commented Jun 9, 2024 •

edited

Loading

adembudak commented Jun 10, 2024 •

edited

Loading