We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 2935

Summary: [Patch] Don't override the default CMAKE_C_FLAGS
Product: SDL Reporter: Magnus Bjerke Vik <mbvett>
Component: buildAssignee: Ryan C. Gordon <icculus>
Status: ASSIGNED --- QA Contact: Sam Lantinga <slouken>
Severity: major    
Priority: P2 CC: icculus, mbvett
Version: 2.0.3   
Hardware: x86_64   
OS: Linux   
Attachments: Patch to remove CMAKE_C_FLAGS defaults

Description Magnus Bjerke Vik 2015-04-08 08:01:36 UTC
Created attachment 2111 [details]
Patch to remove CMAKE_C_FLAGS defaults

Overriding the default CMAKE_C_FLAGS causes SDL to fail detecting any headers and functions when using the Android CMake toolchain provided by https://github.com/taka-no-me/android-cmake

It also messes up the flags set by the CMAKE_BUILD_TYPE. For instance if CMAKE_BUILD_TYPE is set to Debug, then the -O3 flag will still be set.

I don't see any reason to override the default flags, the user should pick a CMAKE_BUILD_TYPE to get their desired flags (optimized, optimized with debug info, debug, etc.) My patch removes the default flags. I've kept the CFLAGS environment stuff since I don't know their purpose.
Comment 1 Ryan C. Gordon 2015-06-01 04:26:40 UTC
A better question might be why CMake should care about $CFLAGS at all...that is a configure idiom. CMake users that care about explicitly setting the equivalent of CFLAGS know that they should set CMAKE_C_FLAGS manually.

I'm inclined to say we should leave this alone for 2.0.4 and then remove the $CFLAGS check completely for 2.0.5 (maybe even with an error if it _is_ set to warn that this isn't the right way to do things in CMake).

Sam: does that sound reasonable?

--ryan.
Comment 2 Magnus Bjerke Vik 2015-06-01 06:19:43 UTC
(In reply to Ryan C. Gordon from comment #1)
> A better question might be why CMake should care about $CFLAGS at all...that
> is a configure idiom. CMake users that care about explicitly setting the
> equivalent of CFLAGS know that they should set CMAKE_C_FLAGS manually.
> 
> I'm inclined to say we should leave this alone for 2.0.4 and then remove the
> $CFLAGS check completely for 2.0.5 (maybe even with an error if it _is_ set
> to warn that this isn't the right way to do things in CMake).
> 
> Sam: does that sound reasonable?
> 
> --ryan.

I agree. The CFLAGS check should be removed.
Comment 3 Magnus Bjerke Vik 2016-02-24 20:59:27 UTC
2.0.4 was released a while back. Are we proceeding with this?