A closer look at "ps -A -o cmd | grep g++" during an ebuild based on cmake-utils showed me this: /usr/lib/ccache/bin/i686-pc-linux-gnu-g++ The compiler is g++, of course -march=prescott -O2 -ggdb These are my CXXFLAGS from make.conf -O3 -DNDEBUG -DQT_NO_DEBUG From CMAKE_CXX_FLAGS_RELEASE -I/usr/include/qt4 ... And so the command continues This reference of CMAKE_CXX_FLAGS_RELEASE I found in CMakeCache.txt in the ebuild's build directory. I guess the "-O3 -DNDEBUG" come from the cmake setup, from /usr/share/cmake/Modules/Platform/gcc.cmake to be precise, as I don't see any reference to -O3 in the source tree. I would think that according to Gentoo philosophy, the Settings from make.conf should override any other. Therefore at least the -O3 should be disabled by the eclass. Especially users specifying -Os probably don't want -O3. Maybe this could be done by setting the crrespronding INIT variables on the command line like "-DCMAKE_C{,XX}_FLAGS_RELEASE_INIT=-DNDEBUG", I don't know.
There are 5 default CMAKE_BUILD_TYPE s: "" (None) Debug Release RelWithDebInfo <-- Deb refers to Debian, though I'm not sure what that is ;-) MinSizeRel If you don't want the Release flags, then you should do my_cmake_args="${my_cmake_args} -DCMAKE_BUILD_TYPE=''" in your ebuild. I don't think it is in gentoo philosophy to strip away the flags a developer has chosen on a per file or per project basis. This would lead to suboptimal performance and possible breakage. However, a lot, if not most, of cmake developers are new and do not use the Release build type, so a default built type of None is probably better. Then change to Release on a per package basis.
(In reply to comment #1) > There are 5 default CMAKE_BUILD_TYPE s: > "" (None) > Debug > Release > RelWithDebInfo <-- Deb refers to Debian, though I'm not sure what that is ;-) > MinSizeRel OK, I see. _common_configure_code uses Release unless the debug flag is part of both IUSE and USE. Looking again at /usr/share/cmake/Modules/Platform/gcc.cmake I see: None => "" Debug => "-g" MinSizeRel => "-Os -DNDEBUG" Release => "-O3 -DNDEBUG" RelWithDebInfo => "-O2 -g" So I guess RelWithDebInfo is a release version with debug information, nothing about debian. And MinSizeRel is a release optimized for minimal size. > If you don't want the Release flags, then you should do > my_cmake_args="${my_cmake_args} -DCMAKE_BUILD_TYPE=''" > in your ebuild. Thanks for mentioning this setting, now I see where to look in the eclass for code related to this issue. > I don't think it is in gentoo philosophy to strip away the flags a developer > has chosen on a per file or per project basis. This would lead to suboptimal > performance and possible breakage. My argument to prefer -Os over -O3 e.g. in memory constrained environments still holds, though. There are quite a lot of packages for which configure would specify some default CFLAGS based on the maintainer's taste. In Gentoo the admin configured flags almost always take precedence, excepting flags known to break builds. While one can clearly ague about this policy, I see no reason to handle cmake any different from autotooled packages. > However, a lot, if not most, of cmake developers are new and do not use the > Release build type, so a default built type of None is probably better. Then > change to Release on a per package basis. This would mean that cmake would not add any flags of its own accord, so the ones from make.conf take effect, right? Sounds sensible to me. You could also introduce CMAKE_BUILD_TYPE as a variable in /etc/make.conf and the make.defaults files of the profiles, and overridable from the command line environment. That way people could choose between a CFLAGS based configuration and a CMAKE_BUILD_TYPE based one.
actually it is exactly Gentoo philosophy to not allow upstream to force CFLAGS (such as specific optimizations). regardless, this isnt an "upstream" thing. this is *cmake* adding -O3 to *other packages*. this isnt random packages themselves adding -O3 to their own build. this is clearly broken. what does CMAKE_BUILD_TYPE accomplish exactly ? just setting CFLAGS ? if so, we should change it to "None" all the time and change the logic to: use debug || append-cppflags -DNDEBUG
> what does CMAKE_BUILD_TYPE accomplish exactly ? just setting CFLAGS ? if so, > we should change it to "None" all the time and change the logic to: > use debug || append-cppflags -DNDEBUG > This sounds reasonable. the CMAKE_BUILD_TYPE effects compiler and linker flags. There are the standard ones as mentioned, but there can also be custom ones. The default behavior is to have no CMAKE_BUILD_TYPE. If you do have CMAKE_BUILD_TYPE=Release, it does not ignore the flags specified with CFLAGS, but the BUILD_TYPE flags get appended after the normal flags, so -O3 would override -Os, if you were trying to force -Os.
Created attachment 169955 [details, diff] cmake-utils.eclass.patch test this out for me then please
Created attachment 169969 [details, diff] ceases making CMAKE_BUILD_TYPE=Release default, but still adds -DNDEBUG I think this is closer to what we want. BUILD_TYPE=None,i.e. BUILD_TYPE='', will be specified by default and does not need to be explicitly defined. BUILD_TYPE=Debug with 'use debug' is still needed to get '-g'. NDEBUG refers to no-debug so it should be defined at release. If an ebuild would want to undefine this, they would have to call cmake-utils_src_configurein or cmake-utils_src_configuerout explicitly instead of cmake-utils_src_configure.
Comment on attachment 169969 [details, diff] ceases making CMAKE_BUILD_TYPE=Release default, but still adds -DNDEBUG this is incorrect for two reasons: -DNDEBUG is a CPPFLAG, not CFLAG and setting Debug as the type sets CFLAGS to "-O2 -g"
(In reply to comment #7) > (From update of attachment 169969 [details, diff] [edit]) > this is incorrect for two reasons: -DNDEBUG is a CPPFLAG, not CFLAG and setting > Debug as the type sets CFLAGS to "-O2 -g" > Martin was right in correcting me that RelWithDebInfo has nothing to do with debian, and stands for Release with Debug Info. But it is the RelWithDebInfo that gives you "-O2 -g", not Debug. Debug gives you "-g" only. This applies to both C and C++. -DNDEBUG is a preprocessor macro definition. It is used in the default cmake release flags for both C and C++. It will only affect the project if they do something like #ifndef NDEBUG < some debug stuff > #endif in their source code. If you would like to visually see the commands that are generated do EXTRA_ECONF="-DCMAKE_VERBOSE_MAKEFILE=ON" emerge <a cmake configured ebuild>
Comment on attachment 169969 [details, diff] ceases making CMAKE_BUILD_TYPE=Release default, but still adds -DNDEBUG all of which is irrelevant. the patch i posted already does the correct thing. a release target only changes flags and therefore should be ignored.
(In reply to comment #9) > (From update of attachment 169969 [details, diff] [edit]) > all of which is irrelevant. the patch i posted already does the correct thing. > a release target only changes flags and therefore should be ignored. > No. Your patch strips out debug code when someone is trying to enable debugging and does not enable debugging symbols in the binaries.
Created attachment 169987 [details, diff] cmake-utils.eclass.patch then you should have pointed out that the logic in my patch was inverted instead of trying to push a patch you've already been told is wrong
http://sources.gentoo.org/eclass/cmake-utils.eclass?r1=1.10&r2=1.12
I did say so, but you did not follow. It is still wrong. 1) NDEBUG, if you are going to define it, should be defined for both the C++ and C case. It also should not matter if the ebuild defines 'debug' in IUSE. 2) If you have the 'debug' USE flag, you need '-g' for debugging symbols. You can do it with -DCMAKE_BUILD_TYPE=Debug or append-flags=-g. Either way could be done, though the former is better incase a project has specific debug flags that they use.
it is defined regardless of compiler. that is the point of CPPFLAGS. as i already said, USE=debug does not affect compiler flags. if you want debug symbols, then you had better set appropriate -g/-O flags in your CFLAGS/CXXFLAGS yourself.
(In reply to comment #14) > it is defined regardless of compiler. that is the point of CPPFLAGS. Not all compilers have the same flags. That is why using CMAKE_BUILD_TYPE is a good thing -- it adjusts to compilers. But that is besides the point. You are not following again. I was talking about languages, i.e. C and C++, which are different from compilers, i.e. gcc and icc. > as i already said, USE=debug does not affect compiler flags. if you want debug > symbols, then you had better set appropriate -g/-O flags in your > CFLAGS/CXXFLAGS yourself. That defeats the point of an eclass or a USE flag.
it doesnt defeat it at all. in fact, your method would place arbitrary limitations on people and make builds harder. as for different compilers, i dont think that's really worth talking about. if a compiler does not accept the -DFOO form, then pretty much everything is going to break. if you want to build stuff with Gentoo, your compiler better accept -D flags for defines.
(In reply to comment #12) > http://sources.gentoo.org/eclass/cmake-utils.eclass?r1=1.10&r2=1.12 Looks good. Just one thought: there may be projects out there with some additional case destinction based on CMAKE_BUILD_TYPE coded into their cmake files. Would it be easy for ebuilds to override the setting of CMAKE_BUILD_TYPE, or should the eclass provide some mechanism of overriding that definition by setting some variable? (In reply to comment #15) > I was talking about languages, i.e. C and C++ I (and I think SpanKY as well) have the impression you take CPPFLAGS to mean "C Plus Plus flags" where in fact it means "C PreProcessor flags". As both C and C++ employ the same preprocessor, and it is up to this preprocessor to deal with macro expansion, this should work. If cmake passes CPPFLAGS along to the compiler, that is. The C++ only flags would be CXXFLAGS, not CPPFLAGS.
hmm, good point ... how about something like: - echo -DCMAKE_BUILD_TYPE=None + echo -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE:-None}
Hi, we will be probbably reviewing some parts of this eclasses later. Because it sets wrong build type for kde ebuilds which bothers us a lot. Btw That last patch looks good. But redefinition in ebuild is possible now too. As i said there is something veird. :]
http://sources.gentoo.org/eclass/cmake-utils.eclass?r1=1.12&r2=1.13
I think it's a bit workaround for the problem and not really satisfactory. Let's summarize. CMAKE_BUILD_TYPE is just cmake control variable - some kind of preset. There are a few additional variables associated like CMAKE_C(XX)?_FLAGS_(DEBUG|MINSIZEREL|RELEASE|RELWITHDEBINFO) (debinfo means debug info..) Same applies to linker flags. Selecting build type - applies default-cmake-built-in set of those above - UNLESS overwritten manually by developer in CMakeLists.txt (what is often the case btw). CMAKE_C(XX)?_FLAGS are NOT effective C(XX)FLAGS - they are just APPENDED to those set by build type (if one is set). Of course they can be as well overriden by developer in CMakeLists.txt but they will not OVERRIDE all C(XX)?FLAGS appended to compiler. Resulting C(XX)?FLAGS passed to compiler = CMAKE_C(XX)?_FLAGS + CMAKE_C(XX)?_FLAGS_build_type Goals (correct me if I'm wrong) : - favor make.conf defined C(XX)?FLAGS - preserve the gentoo user's way of building package (either "release" or "debug") There is another option as well - to provide choice (via USE flag) - whether to enforce Gentoo way (default) or upstream (hardcoded in CMakeLists.txt) way - for packages that do it. The situation is: *** some packages check/set CMAKE_BUILD_TYPE and/or mangle with configuration/CFLAGS IF (CMAKE_BUILD_TYPE MATCHES Debug) do sth, not necessarily mangle with CFLAGS, but copy some files, add_definitions(), build optional targets ENDIF (CMAKE_BUILD_TYPE MATCHES Debug) This is the worst case - in this situation one can't simply play with CMAKE_BUILD_TYPE in order TO UNSET C(XX)?FLAGS - but this is what cmake-utils patch is doing at the moment. ** some packages check for CMAKE_BUILD_TYPE and if not set, then they set it to their factory default - in this case it's not much of a problem with build type (as application just taken care of itself) - still C(XX)?FLAGS to go. ** some packages do not set/check either CMAKE_BUILD_TYPE or preferred C(XX)?FLAGS, leaving the default - this is the easiest case. My conclusions of above would be: - problem needs to addressed directly - applications shouldn't build in undefined way (like now - for example because of build type not set) My suggestions: - set CMAKE_BUILD_TYPE=RELEASE unless stated otherwise (via USE="debug") this determines build type - no more undefined behavior (like recent regressions in kde ebuilds) - set CMAKE_C(XX)?_FLAGS to C(XX)?FLAGS from make.conf this tries to enforce gentoo user's will - set CMAKE_C(XX)?_FLAGS_build_type to -DNDEBUG for build_types MINSIZEREL and RELEASE and -DDEBUG for DEBUG and RELWITHDEBINFO why? first - to override cmake defaults (like -O3 etc) - second to preserve definitions for "debug" builds. Compiler flags are stripped from there as user is responsible for setting them in make.conf and no definitions should be allowed in make.conf, right? - the hardest part - strip hardcoded SET (CMAKE_C(XX)?_FLAGS_(|DEBUG|MINSIZEREL|RELEASE|RELWITHDEBINFO) (so those default as well) from CMakeLists.txt from packages that are known to do it and break in some way - this has to be done ebuild per ebuild. Strip undonditional SET (CMAKE_BUILD_TYPE .*) - those not claused by IF (NOT CMAKE_BUILD_TYPE) as well. The above is optional - just to ensure that EVERY package builds the way WE want. Here's my rant. I would vote for this optional USE flag controlling whether to enforce gentoo policy (default) or not.
errata: - set CMAKE_BUILD_TYPE=Release unless stated otherwise (via USE="debug" - then set CMAKE_BUILD_TYPE=Debug)) If eclass that inherits has different approach (like kde eclasses could have for example) - just leave it to them to override this setting by DebugFull or whatever. But important - to override - build type has to set by cmake-utils and vice versa.
I agree with Maciej. Ignoring CMAKE_BUILD_TYPE looses functionality and optimizations. Haphazardly adding -DNDEBUG could case breakage. One idea to deal with the 'hard part', which may become unmanageable reading through and attempting to fix the CMakeLists.txt, is the following: Run cmake with the given CMAKE_BUILD_TYPE and other args. Let the developer do whatever wacky things she may want to do with the flags. Next, parse the resulting CMakeCache.txt for the CMAKE_C(XX)?_FLAGS_(|DEBUG|MINSIZEREL|RELEASE|RELWITHDEBINFO) and strip out -O. -march=* -mcpu=* -m32 -64 with some sed or awk magic. Next run cmake again. I think this should work, provided the developer is not using FORCE with set(). Another option would be to hack the cmake modules so the user specified flags come after the BUILD_TYPE flags, but that would be unsavory.
Created attachment 170382 [details, diff] patch Patch implementing my suggestion. Test it please. Btw, I added support for CMAKE_VERBOSE_MAKEFILE - for testing purpose I used CMAKE_COMPILE_VERBOSE variable - fix it if it's not desired behaviour for this variable.
@thewtex Unfortunately 'hard part' cannot be done this way - CMakeCache.txt is not resulting config - it's the same you can see running ccmake - input config (the same which is set by my patch). As a test, create simple CMakeLists.txt and add SET(CMAKE_CXX_FLAGS "-DNASTY") - you won't see that in CMakeCache.txt but it will be applied. Only patching CMakeLists.txt files for possible definitions can be done IMHO, but for specific ebuilds.
Reopening, issue still pending. We will look on it soon.
attempting to dynamically parse a build file and run sed/awk on flags is broken. it's a dead end path that will simply waste people's time.
Parsing CMakeCache.txt or defining the variables initially has the same effect, though the latter is much simpiler. Maciej's patch looks good.
Created attachment 170772 [details, diff] user specified flags trump package flags Here is a patch where the user specified flags trump the program specified flags by adding the flags after the package flags. CFLAGS, CXXFLAGS, and LDFLAGS are addressed. Adjustment of the pragrams CMakeLists.txt should not be necessary. The CMAKE_BUILD_TYPE independent and CMAKE_BUILD_TYPE flags are retained and their original order is maintained. The default build type is Release, though it can be changed with mycmakeargs="${mycmakeargs} -DCMAKE_BUILD_TYPE=<mybuildtype>" We could make the CMAKE_BUILD_TYPE an optional argument to cmake-utils_src_configure. This patch has been tested on C and C++ packages and on cmake-2.4.8 and cmake-2.6.2
Thanks for the patches. I hope we have it fixed now. Finaly! :P Just one mistake in that patch was usage of || instead of && ;]
good catch on the && and thanks for reviewing the patch
