#40648 closed defect (fixed)
cmake @2.8.11.2 does not set optimization flags in release
Reported by: | macports@… | Owned by: | cssdev |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | Cc: | cooljeanius (Eric Gallager), jeremyhu (Jeremy Huddleston Sequoia), johnsonsr@…, maehne (Torsten Maehne) | |
Port: | cmake |
Description (last modified by mf2k (Frank Schima))
Recently, projects generated with CMake as installed from MacPorts stopped getting optimization flags set in Release mode.
This appears to be caused by r110069 wherein the Portfile was configured to explicitly strip -O3 from the compiler modules shipped with CMake.
While it may well be true that when CMake is used to generate makefiles for a port, one should not set an optimization flag (I don't know), it does not seem to be the correct behavior for any non-MacPorts project that is using CMake installed by MacPorts. This seems to me to be a pretty standard use case- it's certainly what my team has been doing. This change makes it impossible to make a release build of software without additional hackery to force the optimization flag back in.
I'm not sure what the correct solution is. I'm hoping someone with a deeper knowledge has some ideas?
Attachments (5)
Change History (25)
comment:1 Changed 11 years ago by mf2k (Frank Schima)
Description: | modified (diff) |
---|---|
Keywords: | optimization release removed |
Owner: | changed from macports-tickets@… to css@… |
comment:3 Changed 11 years ago by cssdev
Cc: | egall@… jeremyhu@… added; egall@… removed |
---|---|
Status: | new → assigned |
Replying to macports@…:
This appears to be caused by r110069 wherein the Portfile was configured to explicitly strip -O3 from the compiler modules shipped with CMake.
I really don't understand why this change was made. (Suits me for letting some tickets timeout due to new family additions!)
I'm not sure what the correct solution is. I'm hoping someone with a deeper knowledge has some ideas?
Unless there's a clear case for the change, IMO the behavior of the upstream port should be maintained.
comment:4 follow-up: 5 Changed 11 years ago by cssdev
Cc: | egall@… added; egall@… removed |
---|
What was the original intent of r110069?
Was it to adjust release flags when a port specifies configure.optflags
? A better approach may be to set CMAKE_C_FLAGS_RELEASE
or CMAKE_CXX_FLAGS_RELEASE
for such ports.
comment:5 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)
Replying to css@…:
What was the original intent of r110069?
To properly honor ${configure.cflags} etc
Was it to adjust release flags when a port specifies
configure.optflags
? A better approach may be to setCMAKE_C_FLAGS_RELEASE
orCMAKE_CXX_FLAGS_RELEASE
for such ports.
If that's what you want to do, please make that fix in the cmake PortGroup.
comment:7 Changed 11 years ago by johnsonsr@…
The default cmake release flags absolutely should not be changed. If downstream macports codes should use different optimization flags, they should either not use "CMAKE_BUILD_TYPE:STRING=Release" or should individually override "CMAKE_C_FLAGS_RELEASE" and "CMAKE_CXX_FLAGS_RELEASE".
comment:9 Changed 11 years ago by cssdev
I'm not familiar with the operation of the portgroup, so I'll need some assistance with correcting it. I think we just need something like the following in cmake-1.0.tcl:
if {${configure.optflags} != ""} { configure.args-append -DCMAKE_C_FLAGS_RELEASE="${configure.optflags}" -DCMAKE_CXX_FLAGS_RELEASE="${configure.optflags}" }
What's a portgroup port using these flags? Does the portgroup need a version increment of some kind?
comment:10 Changed 11 years ago by johnsonsr@…
CSS, until you get an answer to your question, I think you should remove the '-O3' deletion from the post-patch scripts. The consequence of having a few cmake macports compiling with '-O3' is much less serious than the consequence of every user-installed code marked 'Release' unintentionally being compiled with -O0. Again, the current issue is really severe because upgrading their CMake installations in the future will not fix software they've already configured with CMake -- users will have to delete all their CMakeCache files to get the correct defaults for CMAKE_CXX_FLAGS_RELEASE.
comment:11 follow-up: 12 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)
The consequence of having a few cmake macports compiling with '-O3' is much less serious ...
The reason that I removed that was because -O3 introduced bugs which caused some ports to compile incorrectly. Please fix the PortGroup the way you want without regressing this issue.
comment:12 follow-up: 13 Changed 11 years ago by johnsonsr@…
Replying to jeremyhu@…:
The reason that I removed that was because -O3 introduced bugs which caused some ports to compile incorrectly.
It sounds like those ports individually need to have their configurations modified to set -D CMAKE_CXX_FLAGS_RELEASE="-O0"
. When a default autoconf variable causes a port to fail to compile, the "fix" isn't to patch the defaults in the autoconf port, is it?
comment:13 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)
Replying to johnsonsr@…:
Replying to jeremyhu@…:
The reason that I removed that was because -O3 introduced bugs which caused some ports to compile incorrectly.
It sounds like those ports individually need to have their configurations modified to set
-D CMAKE_CXX_FLAGS_RELEASE="-O0"
. When a default autoconf variable causes a port to fail to compile, the "fix" isn't to patch the defaults in the autoconf port, is it?
The fix is to make sure that those ports build with the correct optimization flags (configure.optflags) rather than -O3.
Changed 11 years ago by maehne (Torsten Maehne)
Attachment: | 0001-Fix-the-CMake-port-specific-part-of-ticket-40648.patch added |
---|
Changed 11 years ago by maehne (Torsten Maehne)
Attachment: | 0002-Improve-CMake-PortGroup-to-handle-the-configure-flags.patch added |
---|
Changed 11 years ago by maehne (Torsten Maehne)
Attachment: | 0003-Fix-CMake-PortGroup-debug-variant.patch added |
---|
Changed 11 years ago by maehne (Torsten Maehne)
comment:15 Changed 11 years ago by maehne (Torsten Maehne)
As a regular CMake user, I'm also affected by this annoying issue. Therefore, I had a look to the CMake port group to fix the issue with the following uploaded patches:
0001-Fix-the-CMake-port-specific-part-of-ticket-40648.patch
Fix the CMake-port-specific part of this ticket.
This reverts the changes made to the CMake Portfile in r110069 so that ports, which are configured by CMake, honor the configure.optflags set in the Portfile. Thus, Release builds are compiled with optimization by default again.
0002-Improve-CMake-PortGroup-to-handle-the-configure-flags.patch
Improve the CMake PortGroup so that it handles the configure.*flags.
The configure.cppflags, configure.optflags, configure.cflags, configure.cxxflags, configure.ldflags are handled by setting the equivalent CMAKE_*_FLAGS.
The configure.cppflags are added to the C/C++ compiler flags as CMake does not honor separately CPPFLAGS (it uses usually add_definitions() for that). The compiler flags for all build types (CMAKE_C_FLAGS, CMAKE_CXX_FLAGS) are used, as they are usually empty. Cf. also to CMake upstream ticket #12928 "CMake silently ignores CPPFLAGS" <http://www.cmake.org/Bug/view.php?id=12928>.
The configure.cflags contain configure.optflags by default. Therefore, they are set via the Release flags CMAKE_C_FLAGS_RELEASE, which would otherwise overrule the optimization flags, as they are set by default to "-O3 -NDEBUG". Therefore, be sure to add "-NDEBUG" to the configure.cflags if you want to turn off assertions in release builds!
The configure.cxxflags contain configure.optflags by default. Therefore, they are set via the Release flags CMAKE_CXX_FLAGS_RELEASE, which would otherwise overrule the optimization flags, as they are set by default to "-O3 -NDEBUG". Therefore, be sure to add "-NDEBUG" to the configure.cflags if you want to turn off assertions in release builds!
A port author has to be aware that a CMake script can always override these flags when it runs, as they are frequently set internally in function of other CMake build variables!
Attention: If the port author wants to be sure that no compiler flags are passed via configure.args to CMake, he has to set manually configure.optflags to "", as it is by default "-O2" and added to all language-specific flags. If he wants to prevent optimization, he should set configure.optflags to "-O0".
TODO: Handle the compiler flags specific to Objective-C, Fortran, and Java in the CMake PortGroup.
This patch does not fix the issue in the case of selecting the +debug variant of the CMake PortGroup for which two additional patches are necessary:
0003-Fix-CMake-PortGroup-debug-variant.patch
Change the CMAKE_BUILD_TYPE to Debug instead of debugFull for the +debug variant in the CMake PortGroup.
"debugFull" is not a standard build type in CMake so there are no default compiler flags set for it, which would turn on the inclusion of debug symbols. Instead, CMake provides the build type "Debug" for this purpose. Only KDE provides in its CMake build infrastructure support for a build type called "DebugFull", which turns off optimization, as KDE leaves it on for the "Debug" build type. Confer to <http://techbase.kde.org/Development/CMake/Addons_for_KDE> for further explanations.
0004-Handle-configure.cflags-and-configure.cxxflags-in-debug-variant-of-CMake-PortGroup.patch
Consider the configure.cflags and configure.cxxflags for +debug variant of the CMake PortsGroup.
The CMAKE_BUILD_TYPE "Debug" uses CMAKE_C_FLAGS_DEBUG and CMAKE_CXX_FLAGS_DEBUG to specify the compiler flags. These variables are set to "-g" plus the respective configure.cflags and configure.cxxflags. Be aware that configure.cflags and configure.cxxflags contain configure.optflags by default, which are not deleted for the Debug build. The port author has to set configure.optflags to "-O0" to turn off optimization.
Please have a look on it and test it with the ports, which motivated the change in r110069 in the first place. I don't have commit rights to apply these changes to the MacPorts SVN.
comment:16 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)
comment:17 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:18 Changed 11 years ago by maehne (Torsten Maehne)
@jeremyhu: Thanks for having applied so quickly my patches to the CMake PortGroup and the CMake port!
Your additional modification to honor configure.cxx_stdlib is incomplete, as it will be only considered for the Release build type and not the Debug build type used by the +debug variant of the PortGroup. You have to add ${cxx_stdlibflags
} also to the CMAKE_CXX_FLAGS_DEBUG
in the +debug variant.
comment:19 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)
Thanks for catching that. r112844
Changed 11 years ago by maehne (Torsten Maehne)
Attachment: | patch-cmake-portgroup.diff added |
---|
comment:20 Changed 11 years ago by maehne (Torsten Maehne)
Unfortunately, the modification to the CMake PortGroup proposed by this ticket have broken other ports using CMake to configure its sources for the build phase.
Unfortunately, the CMake man pages and <http://www.cmake.org/cmake/help/v2.8.12/cmake.html> do not mention at all that the environment variables CFLAGS, CXXFLAGS, and LDFLAGS get honored by CMake upon its first call in a new build directory. The book "Mastering CMake" (4th edition) by Ken Martin and Bill Hoffmann mentions it only briefly in Section 2.7 "Specifying the Compiler to CMake" and the end of Appendix A -- but you have to know what you're looking for.
Therefore, the previously proposed modification of the CMake PortGroup to honor configure.optflags and configure.cppflags can be simplified as other language-specific flags were already forwarded correctly to CMake via the above environment variables.
This should also fix the observed compilation problems of other ports using CMake to configure their sources for the build phase.
Could you please try out the patch-cmake-portgroup.diff
?
In the future, please Cc the port maintainers (
port info --maintainers cmake
).