Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Pass CFLAGS and CPPFLAGS to build process #134

Closed

Conversation

coldtobi
Copy link
Contributor

@coldtobi coldtobi commented Sep 22, 2022

When overriding compiler options in cmake CMake does not by default honor environment set compiler options, this has to be manually considered. In Debian, CFLAGS and CPPFLAGS are injected by the environment to set some archive-wide defaults, for example enabling hardening features.

(If the environment variables are not set, this patch has no effect.)

Patch origin:
https://salsa.debian.org/games-team/minetest/-/blob/master/debian/patches/enable_hardening.patch (minus some extra blank lines, which I missed when creating the patch for Debian))

When overrideing cmpiler options in cmake CMake does not by default honour
enviornment set compiler options, this has to be manually considered.
In Debian, CFLAGS and CPPFLAGS are injected by the enviornmnent to
set some archive-wide defaults, for example enabling hardening features.

(If the environment variables are not set, this patch has no effect.)

Patch origin:
https://salsa.debian.org/games-team/minetest/-/blob/master/debian/patches/enable_hardening.patch
(minus some extra blank lines, which I missed when creating the patch for Debian))
@numberZero
Copy link
Contributor

Does any CMake-based project does that?

@numberZero
Copy link
Contributor

numberZero commented Feb 21, 2023

Actually, this PR made me realize there is a problem. See #164 for details.

@coldtobi
Copy link
Contributor Author

Does any CMake-based project does that?

This is a known issue with CMake. Background: https://gitlab.kitware.com/cmake/cmake/-/issues/20631 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=653916

@numberZero
Copy link
Contributor

Can’t you simply cmake -DCMAKE_C_FLAGS="$CFLAGS $CPPFLAGS" -DCMAKE_CXX_FLAGS="$CXXFLAGS $CPPFLAGS"?

@Desour
Copy link
Member

Desour commented Apr 10, 2023

CFLAGS and CXXFLAGS are already used to initialize CMAKE_CXX_FLAGS and CMAKE_C_FLAGS (see https://cmake.org/cmake/help/latest/envvar/CXXFLAGS.html, https://cmake.org/cmake/help/latest/envvar/CFLAGS.html).
We should probably use those cmake variables, and also add the preprocessor flags (CPPFLAGS).

@numberZero
Copy link
Contributor

numberZero commented Apr 10, 2023

CFLAGS and CXXFLAGS are already used to initialize CMAKE_CXX_FLAGS and CMAKE_C_FLAGS (see https://cmake.org/cmake/help/latest/envvar/CXXFLAGS.html, https://cmake.org/cmake/help/latest/envvar/CFLAGS.html).

They are, but not CPPFLAGS. And specifying CMAKE_CXX_FLAGS explicitly on the command line overrides CXXFLAGS IIRC.

We should probably use those cmake variables

No, CMake does that itself.

and also add the preprocessor flags (CPPFLAGS).

If you really want to use that, add a cache variable (e.g. CPP_FLAGS), initialize it to CPPFLAGS, and add to compilation flags of everything (either via add_compile_options or non-cache CMAKE_CXX_FLAGS). Going without a dedicated cache variable would lead to very fragile builds.

Comment on lines +17 to +18
set(CMAKE_CXX_FLAGS_RELEASE "-O3 $ENV{CPPFLAGS}")
set(CMAKE_CXX_FLAGS_DEBUG "-g $ENV{CPPFLAGS}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fragile. Each time CMake runs, it rereads these environment variables.

@Desour
Copy link
Member

Desour commented May 14, 2023

@Desour Desour closed this May 14, 2023
@okias
Copy link
Contributor

okias commented Mar 7, 2024

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants