Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to find executable under CMake 3.18 #1984

Closed
NeroBurner opened this issue Jul 16, 2020 · 6 comments · Fixed by #1985
Closed

Unable to find executable under CMake 3.18 #1984

NeroBurner opened this issue Jul 16, 2020 · 6 comments · Fixed by #1985
Labels
Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration.

Comments

@NeroBurner
Copy link
Contributor

NeroBurner commented Jul 16, 2020

Describe the bug
When upgrading CMake to 3.18.0 Catch2 tests are not found anymore. They get compiled, but cannot be executed. With CMake 3.17.3 everything works as expected

Expected behavior
test executables should be found and executed, also on CMake 3.18.0

Reproduction steps
Steps to reproduce the bug.

install CMake 3.18.0
create a minimal example using Catch2, for example https://github.com/NeroBurner/Catch2_minimial_example_cmake_3_18

#define CATCH_CONFIG_MAIN  // This tells Catch to provide a main() - only do this in one cpp file

// external libraries in this repository
#include <catch2/catch.hpp>

TEST_CASE("all is good")
{
    REQUIRE(true);
}
cmake -H. -B_build
cmkae --build _build
cmake --build _build --target test

Log with CMake 3.18.0

-- The CMake version used is 3.18.0
-- The C compiler identification is GNU 7.5.0
-- The CXX compiler identification is GNU 7.5.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- folder: Catch2_CMake_3_18_test
-- Set default build type to Release
-- Configuring done
-- Generating done
-- Build files have been written to: /home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build
Scanning dependencies of target test_yes
[ 50%] Building CXX object CMakeFiles/test_yes.dir/test_yes.cpp.o
[100%] Linking CXX executable test_yes
[100%] Built target test_yes
Running tests...
Test project /home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build
CMake Warning (dev) at CTestTestfile.cmake:7:
  Syntax Warning in cmake code at column 12

  Argument not separated from preceding token by whitespace.
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) at CTestTestfile.cmake:8:
  Syntax Warning in cmake code at column 24

  Argument not separated from preceding token by whitespace.
This warning is for project developers.  Use -Wno-dev to suppress it.

    Start 1: 
Could not find executable test_yes:all
Looked in the following places:
test_yes:all
test_yes:all
Release/test_yes:all
Release/test_yes:all
Debug/test_yes:all
Debug/test_yes:all
MinSizeRel/test_yes:all
MinSizeRel/test_yes:all
RelWithDebInfo/test_yes:all
RelWithDebInfo/test_yes:all
Deployment/test_yes:all
Deployment/test_yes:all
Development/test_yes:all
Development/test_yes:all
Unable to find executable: test_yes:all
1/1 Test #1:  .................................***Not Run   0.00 sec

0% tests passed, 1 tests failed out of 1

Label Time Summary:
test_yes    =   0.00 sec*proc (1 test)

Total Test time (real) =   0.00 sec

The following tests FAILED:
          1 -  (Not Run)
Errors while running CTest
Makefile:102: recipe for target 'test' failed
make: *** [test] Error 8

Logs with CMake 3.17.3

-- The CMake version used is 3.17.3
-- The C compiler identification is GNU 7.5.0
-- The CXX compiler identification is GNU 7.5.0
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc - works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ - works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- folder: Catch2_CMake_3_18_test
-- Set default build type to Release
-- Configuring done
-- Generating done
-- Build files have been written to: /home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build
Scanning dependencies of target test_yes
[ 50%] Building CXX object CMakeFiles/test_yes.dir/test_yes.cpp.o
[100%] Linking CXX executable test_yes
[100%] Built target test_yes
Running tests...
Test project /home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build
    Start 1: test_yes:all is good
1/1 Test #1: test_yes:all is good .............   Passed    0.00 sec

100% tests passed, 0 tests failed out of 1

Label Time Summary:
test_yes    =   0.00 sec*proc (1 test)

Total Test time (real) =   0.00 sec

Platform information:

  • OS: Ubuntu 18.04
  • Compiler+version: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0 as well as clang version 6.0.0-1ubuntu2
  • Catch version: v2.13.0 (also tested on 2.5.0 and 2.12.3)

Additional context

edit: cmake upstream issue: https://gitlab.kitware.com/cmake/cmake/-/issues/20965

@NeroBurner
Copy link
Contributor Author

Found an interesting difference in the generated CTestTestfile.cmake file

diff --git a/_build_cmake317/CTestTestfile.cmake b/_build_cmake318/CTestTestfile.cmake
index d085c28..21ea941 100644
--- a/_build_cmake317/CTestTestfile.cmake
+++ b/_build_cmake318/CTestTestfile.cmake
@@ -4,6 +4,6 @@
 # 
 # This file includes the relevant testing commands required for 
 # testing this directory and lists subdirectories to be tested as well.
-add_test("test_yes:all is good" "/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build/test_yes" "all is good")
-set_tests_properties("test_yes:all is good" PROPERTIES  FAIL_REGULAR_EXPRESSION "No tests ran" LABELS "test_yes" _BACKTRACE_TRIPLES "/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/external/catch2/contrib/ParseAndAddCatchTests.cmake;193;add_test;/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/external/catch2/contrib/ParseAndAddCatchTests.cmake;222;ParseAndAddCatchTests_ParseFile;/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/CMakeLists.txt;18;ParseAndAddCatchTests;/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/CMakeLists.txt;0;")
+add_test(""test_yes:all is good"" "/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build/test_yes" "all is good")
+set_tests_properties(""test_yes:all is good"" PROPERTIES  FAIL_REGULAR_EXPRESSION "No tests ran" LABELS "test_yes" _BACKTRACE_TRIPLES "/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/external/catch2/contrib/ParseAndAddCatchTests.cmake;193;add_test;/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/external/catch2/contrib/ParseAndAddCatchTests.cmake;222;ParseAndAddCatchTests_ParseFile;/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/CMakeLists.txt;18;ParseAndAddCatchTests;/home/nero/repos/external/Catch2_minimial_example_cmake_3_18/CMakeLists.txt;0;")
 subdirs("external/catch2")

Another cosmetic difference (unneeded / in the path)

diff --git a/_build_cmake317/Makefile b/_build_cmake318/Makefile
index 1730852..3981678 100644
--- a/_build_cmake317/Makefile
+++ b/_build_cmake318/Makefile
@@ -1,5 +1,5 @@
 # CMAKE generated file: DO NOT EDIT!
-# Generated by "Unix Makefiles" Generator, CMake Version 3.17
+# Generated by "Unix Makefiles" Generator, CMake Version 3.18
 
 # Default target executed when no arguments are given to make.
 default_target: all
@@ -43,10 +43,9 @@ default_target: all
 # Command-line flag to silence nested $(MAKE).
 $(VERBOSE)MAKESILENT = -s
 
-# Suppress display of executed commands.
+#Suppress display of executed commands.
 $(VERBOSE).SILENT:
 
-
 # A target that is always out of date.
 cmake_force:
 
@@ -111,7 +110,7 @@ test/fast: test
 
 # The main all target
 all: cmake_check_build_system
-       $(CMAKE_COMMAND) -E cmake_progress_start /home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build/CMakeFiles /home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build/CMakeFiles/progress.marks
+       $(CMAKE_COMMAND) -E cmake_progress_start /home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build/CMakeFiles /home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build//CMakeFiles/progress.marks
        $(MAKE) $(MAKESILENT) -f CMakeFiles/Makefile2 all
        $(CMAKE_COMMAND) -E cmake_progress_start /home/nero/repos/external/Catch2_minimial_example_cmake_3_18/_build/CMakeFiles 0
 .PHONY : all

@NeroBurner
Copy link
Contributor Author

created cmake upstream issue: https://gitlab.kitware.com/cmake/cmake/-/issues/20965

@NeroBurner
Copy link
Contributor Author

If I apply the following patch then CMake 3.18.0 works as expected, but fails on CMake 3.17.3

--- a/external/catch2/contrib/ParseAndAddCatchTests.cmake
+++ b/external/catch2/contrib/ParseAndAddCatchTests.cmake
@@ -190,23 +190,23 @@ function(ParseAndAddCatchTests_ParseFile SourceFile TestTarget)
             string(REPLACE "," "\\," Name ${Name})
 
             # Add the test and set its properties
-            add_test(NAME "\"${CTestName}\"" COMMAND ${OptionalCatchTestLauncher} $<TARGET_FILE:${TestTarget}> ${Name} ${AdditionalCatchParameters})
+            add_test(NAME "${CTestName}" COMMAND ${OptionalCatchTestLauncher} $<TARGET_FILE:${TestTarget}> ${Name} ${AdditionalCatchParameters})
             # Old CMake versions do not document VERSION_GREATER_EQUAL, so we use VERSION_GREATER with 3.8 instead
             if(PARSE_CATCH_TESTS_NO_HIDDEN_TESTS AND ${HiddenTagFound} AND ${CMAKE_VERSION} VERSION_GREATER "3.8")
                 ParseAndAddCatchTests_PrintDebugMessage("Setting DISABLED test property")
-                set_tests_properties("\"${CTestName}\"" PROPERTIES DISABLED ON)
+                set_tests_properties("${CTestName}" PROPERTIES DISABLED ON)
             else()
-                set_tests_properties("\"${CTestName}\"" PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran"
+                set_tests_properties("${CTestName}" PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran"
                                                         LABELS "${Labels}")
             endif()
             set_property(
               TARGET ${TestTarget}
               APPEND
-              PROPERTY ParseAndAddCatchTests_TESTS "\"${CTestName}\"")
+              PROPERTY ParseAndAddCatchTests_TESTS "${CTestName}")
             set_property(
               SOURCE ${SourceFile}
               APPEND
-              PROPERTY ParseAndAddCatchTests_TESTS "\"${CTestName}\"")
+              PROPERTY ParseAndAddCatchTests_TESTS "${CTestName}")
         endif()

@NeroBurner
Copy link
Contributor Author

got a workaround working on both CMake 3.18 and CMake 3.17.3, add_test(NAME) handling seemingly changed

--- a/external/catch2/contrib/ParseAndAddCatchTests.cmake
+++ b/external/catch2/contrib/ParseAndAddCatchTests.cmake
@@ -189,24 +189,29 @@ function(ParseAndAddCatchTests_ParseFile SourceFile TestTarget)
             # Escape commas in the test spec
             string(REPLACE "," "\\," Name ${Name})
 
+            # Work around CMake 3.18.0 change in `add_test()`, before the escaped quotes were neccessary,
+            # beginning with CMake 3.18.0 the escaped double quotes confuse the call
+            if(${CMAKE_VERSION} VERSION_LESS "3.18")
+                set(CTestName "\"${CTestName}\"")
+            endif()
             # Add the test and set its properties
-            add_test(NAME "\"${CTestName}\"" COMMAND ${OptionalCatchTestLauncher} $<TARGET_FILE:${TestTarget}> ${Name} ${AdditionalCatchParameters})
+            add_test(NAME "${CTestName}" COMMAND ${OptionalCatchTestLauncher} $<TARGET_FILE:${TestTarget}> ${Name} ${AdditionalCatchParameters})
             # Old CMake versions do not document VERSION_GREATER_EQUAL, so we use VERSION_GREATER with 3.8 instead
             if(PARSE_CATCH_TESTS_NO_HIDDEN_TESTS AND ${HiddenTagFound} AND ${CMAKE_VERSION} VERSION_GREATER "3.8")
                 ParseAndAddCatchTests_PrintDebugMessage("Setting DISABLED test property")
-                set_tests_properties("\"${CTestName}\"" PROPERTIES DISABLED ON)
+                set_tests_properties("${CTestName}" PROPERTIES DISABLED ON)
             else()
-                set_tests_properties("\"${CTestName}\"" PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran"
+                set_tests_properties("${CTestName}" PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran"
                                                         LABELS "${Labels}")
             endif()
             set_property(
               TARGET ${TestTarget}
               APPEND
-              PROPERTY ParseAndAddCatchTests_TESTS "\"${CTestName}\"")
+              PROPERTY ParseAndAddCatchTests_TESTS "${CTestName}")
             set_property(
               SOURCE ${SourceFile}
               APPEND
-              PROPERTY ParseAndAddCatchTests_TESTS "\"${CTestName}\"")
+              PROPERTY ParseAndAddCatchTests_TESTS "${CTestName}")
         endif()

NeroBurner added a commit to NeroBurner/Catch2 that referenced this issue Jul 16, 2020
With CMake 3.18.0 the `add_test(NAME)` handling changed. The escaped
double quotes confuse the new call. Work around this upstream change.

fixes: catchorg#1984
@NeroBurner
Copy link
Contributor Author

relevant change in documentation https://cmake.org/cmake/help/v3.18/command/add_test.html

- Adds a test called <name>. The test name may not contain spaces, quotes, or other characters special in CMake syntax. The options are:
+ Adds a test called <name>. The test name may contain arbitrary characters except for double-quotes. However, if it contains spaces or semicolons it must be enclosed in double-quotes. The options are:

@NeroBurner
Copy link
Contributor Author

got a response on the CMake-Issue by Brad King
https://gitlab.kitware.com/cmake/cmake/-/issues/20965#note_799472

This was likely caused by !4754 (merged) fixing the same problem that Catch2 was trying to work around with extra escape sequences. However, what Catch2 was doing before was not officially supported. In CMake 3.17 and below, the add_test command documentation states:

The test name may not contain spaces, quotes, or other characters special in CMake syntax.

Therefore Catch2 should be updated to be aware that CMake 3.18 and above allow the special characters.

horenmar pushed a commit that referenced this issue Jul 17, 2020
With CMake 3.18.0 the `add_test(NAME)` handling changed. The escaped
double quotes confuse the new call. Work around this upstream change.

fixes: #1984
@horenmar horenmar added the Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration. label Jul 17, 2020
horenmar pushed a commit that referenced this issue Jul 26, 2020
With CMake 3.18.0 the `add_test(NAME)` handling changed. The escaped
double quotes confuse the new call. Work around this upstream change.

fixes: #1984
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants