#59835 closed defect (fixed)
cmake: strange libc++ "if" condition
Reported by: | michaelld (Michael Dickens) | Owned by: | michaelld (Michael Dickens) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | Cc: | kencu (Ken) | |
Port: | cmake |
Description
in < https://github.com/macports/macports-ports/commit/fcaeca417b5d60413f106f01c11dda06749c867b > this change was made to CMake Portfile:
if {!((${os.platform} eq "darwin" && ${os.major} < 10) || ${build_arch} eq "ppc" || ${build_arch} eq "ppc64")} { depends_lib-append port:libcxx configure.cxx_stdlib libc++ }
RJVB responds < https://github.com/macports/macports-ports/commit/cb7d64131b533a23695817b2aaa464e9e476ba78#commitcomment-36395605 >
I don't get this; why would you force a dependency on libc++ on non-Mac platforms?
I concur: what's up with this "if" statement? It seems very odd. Let's get it fixed & make CMake great again for impacted OSs!
Maybe with the current MP release we can remove this code entirely since I think all of the impacted OSX will use libc++ now anyway ... ?
Change History (15)
comment:1 Changed 5 years ago by kencu (Ken)
comment:2 Changed 5 years ago by kencu (Ken)
I can't presently build cmake against libstdc++ (our new c++11 version) using any compiler on 10.5 Intel.
It does build with gcc7 on PPC 10.4 and 10.5 with minor patches I have not yet uploaded.
I forget just now about 10.4 Intel. Previous cmake versions would build, haven't recently tried newer versions.
comment:3 follow-up: 9 Changed 5 years ago by michaelld (Michael Dickens)
If we keep the intent of this "if" contents, then based on Rene's comment, I'd think something like the following would make more sense:
if {(${os.platform} eq "darwin") && !((${os.major} < 10) || ${build_arch} eq "ppc" || ${build_arch} eq "ppc64")} { depends_lib-append port:libcxx configure.cxx_stdlib libc++ }
comment:4 Changed 5 years ago by michaelld (Michael Dickens)
I read the above to say -- assuming os.major == 10 means OSX 10.6 ... yes?
"For 10.6+ and not-PPC of either bit-size" ... which seems unnecessary since 10.6+ can't be PPC.
Hence can't this be reduced to something more like:
if {(${os.platform} eq "darwin") && (${os.major} >= 10)) { depends_lib-append port:libcxx configure.cxx_stdlib libc++ }
comment:5 Changed 5 years ago by michaelld (Michael Dickens)
But I'm also wondering whether this is required at all for, say, 10.14 or 10.15? Isn't using libc++
for the cxx_stdlib
already the default for those OSX versions? How far back does this requirement stand? I'm guessing it doesn't hurt to re-require using libc++
, but maybe it's not truly necessary for 10.8+ or 10.9+ since it's already the default for those OSX versions?
comment:7 Changed 5 years ago by michaelld (Michael Dickens)
(or just missing something in general?)
comment:8 Changed 5 years ago by kencu (Ken)
TBH I would just leave it as is, as it works fine, but tweak the <10 to <9.
sorry, on this ipad, can't type it all out just now.
that "!" operator is screwing with you, making it look wrong when it is right.
comment:9 Changed 5 years ago by kencu (Ken)
Replying to michaelld:
If we keep the intent of this "if" contents, then based on Rene's comment, I'd think something like the following would make more sense:
if {(${os.platform} eq "darwin") && !((${os.major} < 10) || ${build_arch} eq "ppc" || ${build_arch} eq "ppc64")} { depends_lib-append port:libcxx configure.cxx_stdlib libc++ }
that would be exactly the opposite of what we need :)
edit: hold on, you threw another "!" in ,the middle there, so now I'm double twisred again. I'd have to sort it out..
comment:10 follow-up: 13 Changed 5 years ago by kencu (Ken)
right. I see. It's the platform darwin bit that is wrong.
Yes, that should be out of this test, causes great confusion, and is wrong.
I would wrap this whole bit in a pkatform darwin block to simplify, but I see the issue with that now...
comment:11 Changed 5 years ago by michaelld (Michael Dickens)
that "!" just screws with one's mind, doesn't it! I think now you're getting what Rene was having an issue with ...
Maybe this could be moved to the following:
platform "darwin" { if {!((${os.major} < 10) || ${build_arch} eq "ppc" || ${build_arch} eq "ppc64")} { depends_lib-append port:libcxx configure.cxx_stdlib libc++ } }
... and i think that would make Rene happy (at least for this specific issue; not necessary in general, of course!) ... this seems like the more minimal change to me ...
comment:12 Changed 5 years ago by kencu (Ken)
yeah.
just please change it <9 instead, as outlined above.
comment:13 follow-up: 15 Changed 5 years ago by RJVB (René Bertin)
Replying to kencu:
I would wrap this whole bit in a pkatform darwin block to simplify, but I see the issue with that now...
Apart from pkatform
raising a syntax error 8-) what would be the issue with a properly spelled platform darwin
block?
Indeed, my gripe was with the platform test, esp. since combined with $os.major < 10
it will fire on Linux until a hypothetical 11-series kernel. Moving the platform test into its own if or equivalent would make the statement easier to read and easier to get right.
Unrelated:
I can't presently build cmake against libstdc++ (our new c++11 version) using any compiler on 10.5 Intel.
What if you build libc++ like it is done on Linux, that is against libstdc++? I cannot recall whether I did that using GCC instead of Clang, but I think it should be possible. Then, apply my libc++ patch for GCC (currently for GCC 7) and you can use g++-mp-X -stdlib=libc++
. If I had been more dedicated to this little project the patch would probably have made it into GCC already...
I assume that dependent code being built against libc++ doesn't see what ABI library libc++ uses behind the scenes (libc++ can work with at least 3).
comment:14 Changed 5 years ago by michaelld (Michael Dickens)
Owner: | set to michaelld |
---|---|
Resolution: | → fixed |
Status: | new → closed |
comment:15 Changed 5 years ago by kencu (Ken)
Replying to RJVB:
What if you build libc++ like it is done on Linux, that is against libstdc++? I cannot recall whether I did that using GCC instead of Clang, but I think it should be possible. Then, apply my libc++ patch for GCC (currently for GCC 7) and you can use
g++-mp-X -stdlib=libc++
. If I had been more dedicated to this little project the patch would probably have made it into GCC already... I assume that dependent code being built against libc++ doesn't see what ABI library libc++ uses behind the scenes (libc++ can work with at least 3).
I have in fact had libc++ running on PPC for a year or so, myself, and clang-5.0. Just waiting to see if/when it might be a good idea to unleash it on MacPorts. There are a few residual assembler hiccups that I think your gcc/libc++ enhancements might well fix.
$ port -v installed libcxx The following ports are currently installed: libcxx @5.0.1_4+emulated_tls-universal platform='darwin 9' archs='ppc' date='2019-01-15T21:53:44-0800'
this code works properly, but is a bit outdated, in that libc++ is to be used on 10.5 Intel as wel
That "not" operator "!" plays with your mind.
Please rewrite better if you like.
libc++ should be forced on for 10.5 Intel and up. 10.4 Intel and all PPC should not be forced, as they presently can't use libc++.