Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

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

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++.

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 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:6 Changed 5 years ago by michaelld (Michael Dickens)

Am I missing something obvious?

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.

that "!" operator is screwing with you, making it look wrong when it is right.

Version 0, edited 5 years ago by kencu (Ken) (next)

comment:9 in reply to:  3 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..

Last edited 5 years ago by kencu (Ken) (previous) (diff)

comment:10 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 in reply to:  10 ; 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: newclosed

In b0c237895ad01a35d3ceb0c5a54acf665cec3585/macports-ports (master):

cmake: tweak explicitly using cxx_stdlib libc++ for just Darwin 9+

Closes: #59835

comment:15 in reply to:  13 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'
Note: See TracTickets for help on using tickets.