Naming of targets in third_party is non-standard about approvaltests.cpp HOT 13 CLOSED

Evolving list of actions:

Fix this for:

• Catch2
• doctest
• Boost.ut

If possible, add warnings if user references old naming for:

• Decided not to do in this ticket - Catch2
• Decided not to do in this ticket - doctest
• Decided not to do in this ticket - Boost.ut

Other stuff

• Get this working with the starter project - Updating this is now part of the #85

from approvaltests.cpp.

@claremacrae, how about a developer warning when someone links against the old targets so that they can eventually be removed?

from approvaltests.cpp.

@athrun22 Good idea - do you have any idea where I should look to find out how to do that?

from approvaltests.cpp.

(Content moved to comment above)

from approvaltests.cpp.

@claremacrae This is an interesting predicament 🧐

CMake support for a library deprecation feature is actively being developed.

Off the top of my head, an interface library could be used to transitively link to the required library and an add_custom_command(TARGET) could be used to issue a warning message POST_BUILD using the cmake -E echo feature. I'll write up an example when I have time.

from approvaltests.cpp.

@claremacrae See below for an example for Catch2. Be warned that I have not had a chance to test it yet.

add_library(catch INTERFACE)
target_link_libraries(catch INTERFACE Catch2::Catch2)
add_custom_command(TARGET catch POST_BUILD COMMAND ${CMAKE_EXECUTABLE} ARGS -E echo "ApprovalTests: CMake target 'catch' is deprecated and may be removed in the future. Please link to the target 'Catch2::Catch2' instead.")

from approvaltests.cpp.

I thought I had got these all implemented correctly, and just needed to add the warnings...

Then I added these lines for a project that pulled Boost.ut in via FetchContent_Declare()

    if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
        # Turn off some checks for issues that should be fixed in ApprovalTests code
        target_compile_options(boost.ut INTERFACE
                -Wno-newline-eof
                -Wno-shadow-field-in-constructor
                -Wno-weak-vtables
                )
    endif()

And then I went to add the same lines to ApprovalTests.cpp/third_party/ut/CMakeLists.txt and got:

CMake Error at third_party/ut/CMakeLists.txt:12 (target_compile_options):
  target_compile_options can not be used on an ALIAS target.

So I need to check how I added the aliases - to ensure that I use the same combination of target and alias names as the library I am mimicking.

from approvaltests.cpp.

@claremacrae See below for an example for Catch2. Be warned that I have not had a chance to test it yet.

add_library(catch INTERFACE)
target_link_libraries(catch INTERFACE Catch2::Catch2)
add_custom_command(TARGET catch POST_BUILD COMMAND ${CMAKE_EXECUTABLE} ARGS -E echo "ApprovalTests: CMake target 'catch' is deprecated and may be removed in the future. Please link to the target 'Catch2::Catch2' instead.")

Thanks @jwillikers

I tried that out and got:

CMake Error at third_party/catch2/CMakeLists.txt:13 (add_custom_command):
  Target "catch" is an INTERFACE library that may not have PRE_BUILD,
  PRE_LINK, or POST_BUILD commands.

from approvaltests.cpp.

@claremacrae Oops... that does make sense. My other thought was to compile in a header file with the catch interface target that issued a compiler warning. I should be able to take a closer look at this soon.

from approvaltests.cpp.

Thanks!

from approvaltests.cpp.

@jwillikers on second thoughts, I'd quite like to separate out any warnings about using old target names to a separate ticket, if it's important to do, and to not include as part of this ticket...

That way, I can press on sooner with releasing recent improvements.

from approvaltests.cpp.

Remaining:

• Merge third_party_targets_70 branch to master

from approvaltests.cpp.

@claremacrae 💡 Great idea!

from approvaltests.cpp.