Support windows MSYS2 environments #1704

LebedevRI · 2023-11-23T00:00:38Z

No description provided.

LebedevRI · 2023-11-23T02:37:55Z

Hurray, understanding that weird Exit code 0xc0000135 was fun, that unblocks me elsewhere.

@dmah42 all yours, pls merge :)

dmah42 · 2023-11-23T08:47:19Z

include/benchmark/benchmark.h

@@ -746,13 +746,13 @@ class BENCHMARK_EXPORT State {
  // have been called previously.
  //
  // NOTE: KeepRunning may not be used after calling either of these functions.
-  BENCHMARK_ALWAYS_INLINE StateIterator begin();
-  BENCHMARK_ALWAYS_INLINE StateIterator end();
+  inline BENCHMARK_ALWAYS_INLINE StateIterator begin();


should we just have a definition for BENCHMARK_ALWAYS_INLINE for WINDOWS && !CLANG or something?

The issue is that the definition was marked as inline, while declaration was not.
In windows, that effectively results in two different function declarations.
So i don't think this is BENCHMARK_ALWAYS_INLINE-specific in any way,
and thus i don't see why BENCHMARK_ALWAYS_INLINE should be altered.

it reads a bit surprisingly to have an "always inline" attribute but then have to supplement it with "inline". i assumed you needed this because on some platforms "always inline" defines to "".

As far as i understand it, BENCHMARK_ALWAYS_INLINE is an optimization hint
that does not contain the same binding semantical meaning (on the symbol visibility)
as the inline one.

dmah42 · 2023-11-23T08:48:19Z

test/CMakeLists.txt

+  add_test(${ARGV})
+  if(WIN32 AND BUILD_SHARED_LIBS)
+    cmake_parse_arguments(TEST "" "NAME" "" ${ARGN})
+    set_tests_properties(${TEST_NAME} PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$<TARGET_FILE_DIR:benchmark::benchmark>")


is this something users of the library have had to do or are you introducing it with this change? (i don't use windows platforms so i have no idea...)

This is exactly the same thing you were previously doing in

benchmark/.github/workflows/build-and-test.yml

Lines 105 to 109 in c8ef1ee

- name: setup test environment

# Make sure gmock and benchmark DLLs can be found

run: >

echo "$((Get-Item .).FullName)/_build/bin/${{ matrix.build_type }}" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append;

echo "$((Get-Item .).FullName)/_build/src/${{ matrix.build_type }}" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append;

(i'm just moving it into the right place)

i don't remember adding that.. anyway, it's fine, just seems like an odd windows only thing for cmake to need.

dmah42 · 2023-11-23T08:48:45Z

test/CMakeLists.txt

-    add_test(NAME ${name} COMMAND ${name})
+    benchmark_add_test(NAME ${name} COMMAND ${name})
+    if(WIN32 AND BUILD_SHARED_LIBS)
+      set_tests_properties(${name} PROPERTIES


this feels like something CMake should be taking care of with the dependencies, no?

I don't know, this is just how things work on windows i guess.
I agree that it's very broken, and i don't use windows myself too.

Maybe helps with ``` D:\a\_temp\msys64\ucrt64\bin\g++.exe -DHAVE_STD_REGEX -DHAVE_STEADY_CLOCK -DTEST_BENCHMARK_LIBRARY_HAS_NO_ASSERTIONS -ID:/a/benchmark/benchmark/include -Wall -Wextra -Wshadow -Wfloat-equal -Wold-style-cast -Werror -pedantic -pedantic-errors -fstrict-aliasing -Wno-deprecated-declarations -Wno-deprecated -Wstrict-aliasing -Wno-unused-variable -std=c++11 -fvisibility=hidden -fno-keep-inline-dllexport -UNDEBUG -MD -MT test/CMakeFiles/benchmark_test.dir/benchmark_test.cc.obj -MF test\CMakeFiles\benchmark_test.dir\benchmark_test.cc.obj.d -o test/CMakeFiles/benchmark_test.dir/benchmark_test.cc.obj -c D:/a/benchmark/benchmark/test/benchmark_test.cc In file included from D:/a/benchmark/benchmark/test/benchmark_test.cc:1: D:/a/benchmark/benchmark/include/benchmark/benchmark.h:1007:37: error: 'bool benchmark::State::KeepRunningInternal(benchmark::IterationCount, bool)' redeclared without dllimport attribute after being referenced with dll linkage [-Werror] 1007 | inline BENCHMARK_ALWAYS_INLINE bool State::KeepRunningInternal(IterationCount n, | ^~~~~ ```

``` [27/110] Building CXX object test/CMakeFiles/spec_arg_verbosity_test.dir/spec_arg_verbosity_test.cc.obj FAILED: test/CMakeFiles/spec_arg_verbosity_test.dir/spec_arg_verbosity_test.cc.obj D:\a\_temp\msys64\clang32\bin\clang++.exe -DHAVE_STD_REGEX -DHAVE_STEADY_CLOCK -DHAVE_THREAD_SAFETY_ATTRIBUTES -DTEST_BENCHMARK_LIBRARY_HAS_NO_ASSERTIONS -ID:/a/benchmark/benchmark/include -Wall -Wextra -Wshadow -Wfloat-equal -Wold-style-cast -Werror -pedantic -pedantic-errors -Wshorten-64-to-32 -fstrict-aliasing -Wno-deprecated-declarations -Wno-deprecated -Wstrict-aliasing -Wthread-safety -Wno-unused-variable -std=c++11 -fvisibility=hidden -fvisibility-inlines-hidden -UNDEBUG -MD -MT test/CMakeFiles/spec_arg_verbosity_test.dir/spec_arg_verbosity_test.cc.obj -MF test\CMakeFiles\spec_arg_verbosity_test.dir\spec_arg_verbosity_test.cc.obj.d -o test/CMakeFiles/spec_arg_verbosity_test.dir/spec_arg_verbosity_test.cc.obj -c D:/a/benchmark/benchmark/test/spec_arg_verbosity_test.cc In file included from D:/a/benchmark/benchmark/test/spec_arg_verbosity_test.cc:5: D:/a/benchmark/benchmark/include/benchmark/benchmark.h:999:44: error: 'benchmark::State::KeepRunning' redeclared inline; 'dllimport' attribute ignored [-Werror,-Wignored-attributes] 999 | inline BENCHMARK_ALWAYS_INLINE bool State::KeepRunning() { | ^ D:/a/benchmark/benchmark/include/benchmark/benchmark.h:1003:44: error: 'benchmark::State::KeepRunningBatch' redeclared inline; 'dllimport' attribute ignored [-Werror,-Wignored-attributes] 1003 | inline BENCHMARK_ALWAYS_INLINE bool State::KeepRunningBatch(IterationCount n) { | ^ D:/a/benchmark/benchmark/include/benchmark/benchmark.h:1075:60: error: 'benchmark::State::begin' redeclared inline; 'dllimport' attribute ignored [-Werror,-Wignored-attributes] 1075 | inline BENCHMARK_ALWAYS_INLINE State::StateIterator State::begin() { | ^ D:/a/benchmark/benchmark/include/benchmark/benchmark.h:1078:60: error: 'benchmark::State::end' redeclared inline; 'dllimport' attribute ignored [-Werror,-Wignored-attributes] 1078 | inline BENCHMARK_ALWAYS_INLINE State::StateIterator State::end() { | ^ ```

…error We get ever so slightly different results on windows with GCC. ``` 71: Test command: D:\a\benchmark\benchmark\_build\test\statistics_gtest.exe 71: Working Directory: D:/a/benchmark/benchmark/_build/test 71: Test timeout computed to be: 10000000 71: Running main() from gmock_main.cc 71: [==========] Running 4 tests from 1 test suite. 71: [----------] Global test environment set-up. 71: [----------] 4 tests from StatisticsTest 71: [ RUN ] StatisticsTest.Mean 71: [ OK ] StatisticsTest.Mean (0 ms) 71: [ RUN ] StatisticsTest.Median 71: [ OK ] StatisticsTest.Median (0 ms) 71: [ RUN ] StatisticsTest.StdDev 71: [ OK ] StatisticsTest.StdDev (0 ms) 71: [ RUN ] StatisticsTest.CV 71: D:/a/benchmark/benchmark/test/statistics_gtest.cc:31: Failure 71: Expected equality of these values: 71: benchmark::StatisticsCV({2.5, 2.4, 3.3, 4.2, 5.1}) 71: Which is: 0.32888184094918088 71: 0.32888184094918121 71: [ FAILED ] StatisticsTest.CV (0 ms) 71: [----------] 4 tests from StatisticsTest (0 ms total) ```

LebedevRI · 2023-11-23T14:48:16Z

@dmah42 thank you!

Might it be time for .4 release?

LebedevRI force-pushed the msys branch 8 times, most recently from 18c56b9 to 799b943 Compare November 23, 2023 02:17

LebedevRI changed the title ~~[CI] Attempt to add windows MSYS2-based coverage~~ Support windows MSYS2 environments Nov 23, 2023

LebedevRI requested a review from dmah42 November 23, 2023 02:18

LebedevRI force-pushed the msys branch 2 times, most recently from 2727a17 to 1addb18 Compare November 23, 2023 02:26

dmah42 reviewed Nov 23, 2023

View reviewed changes

dmah42 approved these changes Nov 23, 2023

View reviewed changes

LebedevRI added 5 commits November 23, 2023 17:39

[CI] Attempt to add windows MSYS2-based coverage

89916f4

Fix DLL path discovery for tests

dcb9400

LebedevRI force-pushed the msys branch from 1addb18 to dcb9400 Compare November 23, 2023 14:39

LebedevRI merged commit 1e96bb0 into google:main Nov 23, 2023
79 of 80 checks passed

LebedevRI deleted the msys branch November 23, 2023 14:47

LebedevRI mentioned this pull request Jan 9, 2024

32 bit action #1162

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support windows MSYS2 environments #1704

Support windows MSYS2 environments #1704

LebedevRI commented Nov 23, 2023

LebedevRI commented Nov 23, 2023

dmah42 Nov 23, 2023

LebedevRI Nov 23, 2023

dmah42 Nov 23, 2023

LebedevRI Nov 23, 2023

dmah42 Nov 23, 2023

LebedevRI Nov 23, 2023

LebedevRI Nov 23, 2023

dmah42 Nov 23, 2023

dmah42 Nov 23, 2023

LebedevRI Nov 23, 2023

LebedevRI commented Nov 23, 2023

	- name: setup test environment
	# Make sure gmock and benchmark DLLs can be found
	run: >
	echo "$((Get-Item .).FullName)/_build/bin/${{ matrix.build_type }}" \| Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append;
	echo "$((Get-Item .).FullName)/_build/src/${{ matrix.build_type }}" \| Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append;

Support windows MSYS2 environments #1704

Support windows MSYS2 environments #1704

Conversation

LebedevRI commented Nov 23, 2023

LebedevRI commented Nov 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LebedevRI commented Nov 23, 2023