Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#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)

0001-Fix-the-CMake-port-specific-part-of-ticket-40648.patch (812 bytes) - added by maehne (Torsten Maehne) 9 years ago.
0002-Improve-CMake-PortGroup-to-handle-the-configure-flags.patch (3.3 KB) - added by maehne (Torsten Maehne) 9 years ago.
0003-Fix-CMake-PortGroup-debug-variant.patch (625 bytes) - added by maehne (Torsten Maehne) 9 years ago.
0004-Handle-configure.cflags-and-configure.cxxflags-in-debug-variant-of-CMake-PortGroup.patch (958 bytes) - added by maehne (Torsten Maehne) 9 years ago.
patch-cmake-portgroup.diff (6.0 KB) - added by maehne (Torsten Maehne) 9 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 9 years ago by mf2k (Frank Schima)

Description: modified (diff)
Keywords: optimization release removed
Owner: changed from macports-tickets@… to css@…

In the future, please Cc the port maintainers (port info --maintainers cmake).

comment:2 Changed 9 years ago by cooljeanius (Eric Gallager)

Cc: egall@… added

Cc Me!

comment:3 in reply to:  description Changed 9 years ago by cssdev

Cc: egall@… jeremyhu@… added; egall@… removed
Status: newassigned

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 Changed 9 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 in reply to:  4 Changed 9 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 set CMAKE_C_FLAGS_RELEASE or CMAKE_CXX_FLAGS_RELEASE for such ports.

If that's what you want to do, please make that fix in the cmake PortGroup.

comment:6 Changed 9 years ago by johnsonsr@…

Cc: johnsonsr@… added

Cc Me!

comment:7 Changed 9 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:8 Changed 9 years ago by ryandesign (Ryan Schmidt)

Has duplicate #40711.

comment:9 Changed 9 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 9 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 Changed 9 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 in reply to:  11 ; Changed 9 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 in reply to:  12 Changed 9 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.

comment:14 Changed 9 years ago by maehne (Torsten Maehne)

Cc: Torsten.Maehne@… added

Cc Me!

Changed 9 years ago by maehne (Torsten Maehne)

Changed 9 years ago by maehne (Torsten Maehne)

Changed 9 years ago by maehne (Torsten Maehne)

comment:15 Changed 9 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 9 years ago by jeremyhu (Jeremy Huddleston Sequoia)

r112836 r112837 r112838 for the PortGroup.

It still needs to be updated to honor configure.cxx_stdlib.

comment:17 Changed 9 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Resolution: fixed
Status: assignedclosed

r112840 takes care of cxx_stdlib, and r112839 takes care of the port.

Last edited 9 years ago by jeremyhu (Jeremy Huddleston Sequoia) (previous) (diff)

comment:18 Changed 9 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 9 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Thanks for catching that. r112844

Changed 9 years ago by maehne (Torsten Maehne)

Attachment: patch-cmake-portgroup.diff added

comment:20 Changed 9 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?

Note: See TracTickets for help on using tickets.