Example: CC='gcc -std=gnu11 -m32' The flags '-std=gnu11 -m32' are not honored by cmake (only 1 or 2 word CC is supported). The solution would be to move everything after the first two words to CFLAGS. The same should be done for CXX. Reproducible: Always
(In reply to Hristo Venev from comment #0) > CC='gcc -std=gnu11 -m32' Where does something like that come from?
Me testing for gcc 5.0 compatiblity + cmake-multilib
Where is GCC 5.0 and what is cmake-multilib? We need to be able to reproduce the problem before being able to fix it.
The first question about GCC 5.0 is irrelevant. Its position, if such a thing can be defined, has nothing to do with whether or not a C program is likely to compile under it (it will default to -std=gnu11 instead of -std=gnu89). The second question: cmake-multilib is an eclass. However I think the bug is in cmake-utils.eclass because it should be possible for CC to contain multiple words. Note that this is only one situation when this would happen. It is entirely possible that CC looks like this: CC='clang -target=armv6m-unknown-linux-gnu -mfloat-abi=soft' This is needed when I want to cross-compile with clang for a software floating point armv6m and buggy ebuilds/packages override CFLAGS. There can be other such cases.
Please provide your steps to reproduce.
(In reply to Hristo Venev from comment #4) > buggy ebuilds/packages override CFLAGS. Which packages, which ebuilds? I guess the root cause should be fixed first, before adding any workarounds to the eclasses.
The root cause is that cmake doesn't behave correctly when CC contains three or more words. CC='x86_64-pc-linux-gnu-gcc -std=gnu11' USE='abi_x86_32' emerge [package that inherits cmake-multilib] Some affected packages: dev-libs/yajl media-libs/openal media-libs/libraw media-libs/soxr media-libs/taglib media-libs/waffle media-libs/oyranos media-libs/libcuefile media-libs/game-music-emu media-libs/x265 media-gfx/graphite2 net-libs/libssh net-libs/libproxy x11-misc/virtualgl
CFLAGS do not belong in CC. Upstream does not support it, and I don't think we should add workarounds to either. If some package does not respect CFLAGS, please file a bug so it can be fixed properly.
(In reply to Michael Palimaka (kensington) from comment #8) > CFLAGS do not belong in CC. Upstream does not support it, and I don't think > we should add workarounds to either. > > If some package does not respect CFLAGS, please file a bug so it can be > fixed properly. Most of them will probably get closed with RESOLVED WONTFIX or RESOLVED INVALID (because they use strip-flags). Should I file a bug report for multilib.eclass? Namely multilib_toolchain_setup: 439 export CC="$(tc-getCC) $(get_abi_CFLAGS)" 440 export CXX="$(tc-getCXX) $(get_abi_CFLAGS)" 441 export LD="$(tc-getLD) $(get_abi_LDFLAGS)" Obviously CC, CXX and LD contain at least two words. Should I file a bug for the default/bsd/fbsd/amd64 and prefix/bsd/freebsd/*/x64 profiles, where CFLAGS_x86_fbsd="-m32 -DCOMPAT_32BIT -B/usr/lib32 -L/usr/lib32" Then, in anything using multilib.eclass for ABI=x86_fbsd the following would happen: export CC="gcc -m32 -DCOMPAT_32BIT -B/usr/lib32 -L/usr/lib32" export CXX="g++ -m32 -DCOMPAT_32BIT -B/usr/lib32 -L/usr/lib32" Note that on many profiles, including all default/linux/amd64/* for non-default ABIs CC and CXX will contain two words. For example, on the x86 abi on amd64: export CC="gcc -m32" export CXX="g++ -m32"
To ask what seems like an obvious question: Why does dev-util/cmake work with 2 words in CC, but not 3 words? Surely that's a bug in cmake that should be addressed. Or am I misunderstanding something?
(In reply to Mike Gilbert from comment #10) > To ask what seems like an obvious question: Why does dev-util/cmake work > with 2 words in CC, but not 3 words? Surely that's a bug in cmake that > should be addressed. > > Or am I misunderstanding something? I may be recalling it wrong but I think originally cmake did treat CC/CXX etc. as a single path and did not support parameters at all. I think this was changed (upstream or hacked downstream somehow?) to make it possible to use Gentoo multilib hackery. I don't know what is the most correct way of doing this. Maybe cmake-utils should split out the arguments from CC into CMAKE_C_FLAGS (and CMAKE_*_LINKER_FLAGS)?
I would rather undo this hackery.. Namely multilib_toolchain_setup: 439 export CC="$(tc-getCC) $(get_abi_CFLAGS)" 440 export CXX="$(tc-getCXX) $(get_abi_CFLAGS)" 441 export LD="$(tc-getLD) $(get_abi_LDFLAGS)" This loos wrong. We already have CFLAGS, CPPFLAGS, CXXFLAGS and LDFLAGS and those variables should be honored by build system. Where there's suddenly a need to abuse CC, CXX, LD? If we relax quoting rules, then we will get bugs for not supporting compiler path in location that contains space characters etc...
(In reply to Maciej Mrozowski from comment #12) that behavior is incidental. if cmake can't handle more than two "words" in its toolchain settings, then it's broken, end of story. looking for code that happens to trigger that bug is pointless. fix the issue at the root.
This actually seems to work for me when I invoke cmake outside of an ebuild. So I think there is some funky eclass action going on here that prevents this from working properly. I'll try to dig a bit deeper.
(In reply to Mike Gilbert from comment #14) > This actually seems to work for me when I invoke cmake outside of an ebuild. > So I think there is some funky eclass action going on here that prevents > this from working properly. > > I'll try to dig a bit deeper. Are you sure about that? I just tried and I can reproduce this locally. -DCMAKE_CXX_COMPILER="clang++;-m32;-stdlib=libc++" and the package gets " " instead of the two options. (In reply to SpanKY from comment #13) > (In reply to Maciej Mrozowski from comment #12) > > that behavior is incidental. if cmake can't handle more than two "words" in > its toolchain settings, then it's broken, end of story. looking for code > that happens to trigger that bug is pointless. fix the issue at the root. I agree that we should fix it at root. That is, stop passing flags in variable not intended for flags. (In reply to Maciej Mrozowski from comment #12) > If we relax quoting rules, then we will get bugs for not supporting compiler > path in location that contains space characters etc... The problem is unrelated to quoting rules. Lists in CMake are separated using semicolons; and if you pass semicolons as I pasted above, it still fails to handle more than two words.
From documentation: "CMAKE_<LANG>_COMPILER --------------------- The full path to the compiler for ``LANG``. This is the command that will be used as the ``<LANG>`` compiler. Once set, you can not change this variable." and the way it is used in CMakeDetermineCXXCompiler.cmake and other files (like get_filename_component from ${CMAKE_CXX_COMPILER}) it's rather unexpected for CMake if this parameter is set to a list. > I don't know what is the most correct way of doing this. Maybe cmake-utils should split out the arguments from CC into > CMAKE_C_FLAGS (and CMAKE_*_LINKER_FLAGS)? If vapier insists on abusing CC/CXX variable just because it happens to work in autotools, then we might not have a choice. > The problem is unrelated to quoting rules. Lists in CMake are separated using semicolons; and if you pass semicolons as I pasted above, > it still fails to handle more than two words. Yes, because like I asserted, this variable might not be supposed to be a list (documentation is not clear on that unfortunately). Then I would say it's exclusively about quoting. CMake does auto-quote variables on expansion itself. But what quoting rules are applied when constructing compiler command? I don't know. Take this code from CMakeCXXInformation.cmake, This is how final compiler invocation command is constructed # compile a C++ file into an object file if(NOT CMAKE_CXX_COMPILE_OBJECT) set(CMAKE_CXX_COMPILE_OBJECT "<CMAKE_CXX_COMPILER> <DEFINES> <INCLUDES> <FLAGS> -o <OBJECT> -c <SOURCE>") endif() In Gentoo we override it to: SET (CMAKE_CXX_COMPILE_OBJECT "<CMAKE_CXX_COMPILER> <DEFINES> ${includes} ${CPPFLAGS} <FLAGS> -o <OBJECT> -c <SOURCE>" CACHE STRING "C++ compile command" FORCE) since CMake doesn't know CPPFLAGS environment variable. Not splitting compiler path, defines, flags properly is relying on some internal behaviour, this is what I'm trying to say.
Created attachment 449062 [details, diff] 0001-cmake-utils.eclass-Fix-multi-arg-CC-CXX-FC-542530.patch What do you think of this patch? It does the obvious thing, i.e. moves variables around for CMake invocation. However, it really needs a tinderbox run for all multilib CMake packages.
(In reply to Michał Górny from comment #15) > Are you sure about that? I just tried and I can reproduce this locally. I was probably mistaken. (In reply to Michał Górny from comment #17) > What do you think of this patch? That's about what I had in mind for this as well. I think that code might break if CC contains quoted whitespace, but I'm not sure there's a sane way to deal with that.
Strangely enough, CMake seems to do some magic to support CC/CXX with 3 or more words. maciek@liwardyna ~/Projects/simgear/build $ export CXX="g++ -std=gnu11 -m32 -fomit-frame-pointer" maciek@liwardyna ~/Projects/simgear/build $ cmake -DCMAKE_VERBOSE_MAKEFILE=ON ../ -- The C compiler identification is GNU 4.9.3 -- The CXX compiler identification is GNU 4.9.3 -- Check for working C compiler: /usr/bin/cc -- Check for working C compiler: /usr/bin/cc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler: /usr/bin/g++ -- Check for working CXX compiler: /usr/bin/g++ -- works (...) But... maciek@liwardyna ~/Projects/simgear/build $ grep CXX_COMPILER CMakeCache.txt CMAKE_CXX_COMPILER:FILEPATH=/usr/bin/g++ CMAKE_CXX_COMPILER_ARG1:STRING= -std=gnu11 -m32 -fomit-frame-pointer //ADVANCED property for variable: CMAKE_CXX_COMPILER CMAKE_CXX_COMPILER-ADVANCED:INTERNAL=1 And indeed: maciek@liwardyna ~/Projects/simgear/build $ make | grep g++ cd /home/maciek/Projects/simgear/build/simgear && /usr/bin/g++ -std=gnu11 -m32 -fomit-frame-pointer -DHAVE_CONFIG_H -DHAVE_EXPAT_CONFIG_H -DHAVE_INTTYPES_H -DXML_STATIC -I/home/maciek/Projects/simgear/build/simgear -I/home/maciek/Projects/simgear/simgear/canvas/ShivaVG/include -I/home/maciek/Projects/simgear -I/home/maciek/Projects/simgear/build -I/home/maciek/Projects/simgear/3rdparty/expat -I/home/maciek/Projects/simgear/build/3rdparty/expat -I/usr/include/AL -I/home/maciek/Projects/simgear/3rdparty/utf8/source -I/home/maciek/Projects/simgear/3rdparty/udns -Wall -DBOOST_BIMAP_DISABLE_SERIALIZATION -O3 -DNDEBUG -std=gnu++98 -o CMakeFiles/SimGearCore.dir/debug/logstream.cxx.o -c /home/maciek/Projects/simgear/simgear/debug/logstream.cxx cc1plus: warning: command line option '-std=gnu11' is valid for C/ObjC but not for C++ (so not only it's passed but it's understood - in this case it's mismatched) So perhaps instead of: local toolchain_file=${BUILD_DIR}/gentoo_toolchain.cmake cat > ${toolchain_file} <<- _EOF_ || die SET (CMAKE_C_COMPILER $(tc-getCC)) SET (CMAKE_CXX_COMPILER $(tc-getCXX)) SET (CMAKE_Fortran_COMPILER $(tc-getFC)) _EOF_ and doing possibly complex parsing (as mgorny did in patch sent to -dev), we should just export CC=$(tc-getCC), CXX=$(tc-getCXX)..., before running cmake and be done with it?
If it works, then it works for me ;-). I wonder if there was a historical need of the complex version, if it was there to work around something or just the usual Gentoo nonsense hackery for the sake of making everything more complex than it needed to be.
What we do in Gentoo (using toolchain file to override CMAK_C{XX}_COMPILER) is in fact recommended (by upstream) and the right way. But as we can see it falls short in certain corner cases like this one. Side digression: when looking at our toolchain and build_rules files however, we have some variables that should not belong there. For instance I don't get why we have CMAKE_RANLIB and PKG_CONFIG_EXECUTABLE defined there (and not just passed to CMake directly. Same with CMAKE_FIND_ROOT_PATH* - could be just passed directly. Unfortunately when proper prefix support was added, eclass got unnecessarily uglier here and there.
Actually CMAKE_FIND_ROOT* do belong to toolchain file so retract this part, still prefix stuff should not be placed in build_rules file, even CMake documentation is quite specific about that. Anyway, I found a problem with tc-export approach - bootstrapped CMake (from dev-util/cmake) now fails to build - does not set linker properly: liwardyna cmake-3.5.2_build # cat Source/kwsys/CMakeFiles/cmsys_c.dir/link.txt "" qc libcmsys_c.a CMakeFiles/cmsys_c.dir/ProcessUNIX.c.o CMakeFiles/cmsys_c.dir/Base64.c.o CMakeFiles/cmsys_c.dir/EncodingC.c.o CMakeFiles/cmsys_c.dir/MD5.c.o CMakeFiles/cmsys_c.dir/Terminal.c.o CMakeFiles/cmsys_c.dir/System.c.o CMakeFiles/cmsys_c.dir/String.c.o
Maybe we should just set the *_ARG1 vars?
This is not official variable, some internal one and no idea whether available in bootstrapped CMake. What I wrote before was not procise, with tc-export, bootstrapped CMake builds fine, but it is not able to build final CMake (so should have written that bootstrapped CMake does not seem to support tc-export or uses some filtered env, needs more investigation).
Created attachment 452142 [details, diff] patch
I must have messed up something else with earlier attempt, it works.
Comment on attachment 452142 [details, diff] patch >+ # Bug 542530, export those instead of setting paths in toolchain file >+ tc-export CC CXX FC >+ That should actually be: local -x CC=$(tc-getCC) ... so that those variables don't leave the function scope.
I originally requested the toolchain file in bug #542530. It was a long time ago so it's hard to remember but I have a feeling that environment variables didn't work when cross-compiling. Perhaps CMake has changed in this regard. I'd really like to test it first to make sure so please wait till I find out tomorrow.
Created attachment 452210 [details, diff] 0001-cmake-utils.eclass-CMake-argument-passing-rework.patch
Created attachment 452212 [details, diff] 0002-cmake-utils.eclass-export-compilers-to-environment-i.patch You need both, in order.
Now let's invent case (with spaces in compiler path) that breaks cross-compiling on vapier's favourite build system...
Okay, I've tried cross-compiling media-libs/openal with these changes and it worked fine so all good as far as I'm concerned.
(In reply to Maciej Mrozowski from comment #30) > Created attachment 452212 [details, diff] [details, diff] > 0002-cmake-utils.eclass-export-compilers-to-environment-i.patch > > You need both, in order. Can we move forward with that, then?
I would like to address mentioned in -dev ml spurious CMake warnings first.. but temporarily (might be a couple of days) I do not have access to my Gentoo box. I could care less about warnings myself and we can easily fix them later by setting those variables only when they are actually referenced in buildsystem, so if you like, feel free to push those patches from -ml (or this bug) on my behalf.
(In reply to Maciej Mrozowski from comment #34) > I would like to address mentioned in -dev ml spurious CMake warnings first.. > but temporarily (might be a couple of days) I do not have access to my > Gentoo box. > I could care less about warnings myself and we can easily fix them later by > setting those variables only when they are actually referenced in > buildsystem, so if you like, feel free to push those patches from -ml (or > this bug) on my behalf. I was actually thinking of the second patch alone. It should work separately, correct?
No, I moved CMAKE_AR and CMAKE_RANLIB from build_rules to toolchain_file, context will not match.
(In reply to Maciej Mrozowski from comment #36) > No, I moved CMAKE_AR and CMAKE_RANLIB from build_rules to toolchain_file, > context will not match. I meant applying the solution for the patch. Is there any problem exporting those vars without the remaining movearounds?
You would need to remove setting corresponding vars from toolchain file, I don't know which setting (ENV var or variable in toolchain file) takes precedence.
commit 66483a93354c105dd01ec5595e9ffdc870856de8 Author: Michał Górny <mgorny@gentoo.org> AuthorDate: Sun Nov 27 12:26:19 2016 Commit: Michał Górny <mgorny@gentoo.org> CommitDate: Sun Nov 27 12:42:43 2016 cmake-utils.eclass: Export PKG_CONFIG as envvar rather than build rule commit 5132424e609fe2df9cd7916bc8dc4db70001b77d Author: Maciej Mrozowski <reavertm@gentoo.org> AuthorDate: Wed Nov 2 03:31:56 2016 Commit: Michał Górny <mgorny@gentoo.org> CommitDate: Sun Nov 27 12:42:39 2016 cmake-utils.eclass: export CC/CXX/FC to environment, #542530 commit 0f90bcc5ad56689347ef72c398cd0c952c35d143 Author: Michał Górny <mgorny@gentoo.org> AuthorDate: Sun Nov 27 12:17:22 2016 Commit: Michał Górny <mgorny@gentoo.org> CommitDate: Sun Nov 27 12:42:37 2016 cmake-utils.eclass: Move CMAKE_AR & CMAKE_RANLIB into toolchain defs Move CMAKE_AR & CMAKE_RANLIB definitions into the toolchain file. It seems to make more sense there than in build rules. This is technically most of what Maciej has done, except for the style changes and moving cache vars to CLI vars, as the latter triggers undesired unknown argument warnings.
Reverted because of #601292. Will have to figure out a better fix. Besides, while figuring this out, I think I've figured out how CMAKE_x_COMPILER works. You're supposed to put the path in the first element, and all options space-separated in the second one. That's what the code does for CC splitting.
https://gitlab.kitware.com/cmake/cmake/merge_requests/304 for the ASM issue.
The third version doesn't seem to cause any issues so far, so let's assume this is FIXED.
