Remove CMAKE_CXX_FLAGS from test targets

[mjcarroll]

🦟 Bug fix

Fixes #181

Summary

In general, we should never be setting or appending CMAKE_CXX_FLAGS, as these can be controlled via target specific flags.

Checklist

• Signed all commits for DCO
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

[richmattes]

Yes, the change looks good. You can reproduce #181 yourself (and confirm the fix) like so:

docker pull fedora:34
docker run --rm=true -it fedora:34 /bin/bash

dnf install -y gcc-c++ cmake make git eigen3-devel libuuid-devel vim

git clone https://github.com/ignitionrobotics/ign-cmake.git -b ign-cmake2
cmake -B ign-cmake-build -S ign-cmake -DCMAKE_INSTALL_PREFIX=/usr
cmake --build ign-cmake-build --target install -j 4

git clone https://github.com/ignitionrobotics/ign-math.git -b ign-math6
cmake -B ign-math-build -S ign-math -DCMAKE_INSTALL_PREFIX=/usr  -DBUILD_TESTING=OFF
cmake --build ign-math-build --target install -j 4

git clone https://github.com/ignitionrobotics/ign-common.git -b ign-common3
cmake -B ign-common-build -S ign-common -DCMAKE_INSTALL_PREFIX=/usr
cmake --build ign-common-build --target INTEGRATION_plugin

git --git-dir=ign-common/.git --work-tree=ign-common checkout remove_cxx_flags
cmake --build ign-common-build --target INTEGRATION_plugin

I did run into another problem while putting together a repro: #include <thread> is missing from ign_common/include/ignition/common/Util.hh. Reproduce with cmake --build ign-common-build --target UNIT_Time_TEST

[mjcarroll]

MacOS doesn't like this for some reason, digging...

[richmattes]

IGNDummyPlugins in test/plugins/CMakeLists.txt doesn't link against libignition_common3, and thus doesn't inherit any CXXFLAGS that override the the default C++ standard version. If AppleClang's default C++ standard version is older than C++11, then it's not finding the alignof operator and failing.

You can work around it by setting a project-wide CMAKE_CXX_STANDARD, or by setting the C++ standard version for the libraries in test/plugins/CMakeLists.txt with target_compile_features or by replacing add_library with an applicable ign-cmake helper function.

Enabling -DCMAKE_VERBOSE_MAKEFILE=ON in CI is helpful for debugging these kinds of errors.

[codecov]

Codecov Report

Merging #214 (4f65f57) into ign-common3 (7560ac9) will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           ign-common3     #214   +/-   ##
============================================
  Coverage        76.56%   76.56%           
============================================
  Files               73       73           
  Lines            10357    10357           
============================================
  Hits              7930     7930           
  Misses            2427     2427           

Impacted Files	Coverage Δ
include/ignition/common/Util.hh	100.00% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7560ac9...4f65f57. Read the comment docs.

[mjcarroll]

@richmattes Verified the latest on Fedora using your helpful reproduction.

The not including <thread> could potentially be justified because it's only in the macros and not actually used in the header, but I went ahead and added it as it shouldn't have any downstream impacts.

[mjcarroll]

Homebrew warnings are known, addressing that next...

[scpeters]

Homebrew warnings are known, addressing that next...

it was more than warnings:

https://build.osrfoundation.org/job/ignition_common-ci-pr_any-homebrew-amd64/1055/

[mjcarroll]

it was more than warnings:

This is why it's important to read 🙄

[scpeters]

I wish github actions had a yellow light instead of just red and green