Do not mask CMAKE_CXX_FLAGS environment variable

[tartina]

Thr user's own CMAKE_CXX_FLAGS got lost using $ENV assignement

[tartina]

It was exported by rpm macro from Fedora build system %cmake. I got it only removing the ENV part. Also its not possible to choose a release with debug build type.

Il Ven 7 Giu 2024, 10:46 theGreatWhiteShark @.***> ha scritto:

@.**** commented on this pull request.

Hey @tartina https://github.com/tartina ,

I'm puzzled. I introduced these ENV modifiers for that very reason: to not mask the user's variable.

The way I understand cmake ${VAR} does access a variable defined within the CMakeLists.txt or any other file directly imported. $ENV{VAR}, however, does access a variable defined on the system.

When I add these debug statements into CMakeLists.txt

color_message("${_escape}[1;34m CMAKE_CXX_FLAGS: ${CMAKE_CXX_FLAGS} ${_escape}[0m") color_message("${_escape}[1;34m ENV(CMAKE_CXX_FLAGS): $ENV{CMAKE_CXX_FLAGS} ${_escape}[0m")

I get

$ CMAKE_CXX_FLAGS="-test-flag" cmake .. [...] CMAKE_CXX_FLAGS: ENV(CMAKE_CXX_FLAGS): -test-flag [...]

How would you alter/provide CXX flags?

— Reply to this email directly, view it on GitHub https://github.com/hydrogen-music/hydrogen/pull/1978#pullrequestreview-2104001042, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABB6LURZEV2XVYJGWXFV4HLZGFXOJAVCNFSM6AAAAABI3POQK2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMBUGAYDCMBUGI . You are receiving this because you were mentioned.Message ID: @.***>

[tartina]

Sorry I was partially wrong.

The Fedora macro defines CXXFLAGS, not CMAKE_CXX_FLAGS which is not defined in the environment. But cmake docs say that CMAKE_CXX_FLAGS is initialized from some internal configs and CXXFLAGS. So using ENV you are deleting this initialization, if CMAKE_CXX_FLAGS env variable is empty. It should not be defined as env variable according to the docs

The correct title for this PR should be do not mask user's CXXFLAGS env variable