Opened 10 months ago

Closed 10 months ago

Last modified 10 months 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 10 months 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 10 months 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 10 months 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 10 months 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 10 months 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 10 months ago by michaelld (Michael Dickens)

Am I missing something obvious?

comment:7 Changed 10 months ago by michaelld (Michael Dickens)

(or just missing something in general?)

comment:8 Changed 10 months 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.

Last edited 10 months ago by kencu (Ken) (previous) (diff)

comment:9 in reply to:  3 Changed 10 months 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 10 months ago by kencu (Ken) (previous) (diff)

comment:10 Changed 10 months 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 10 months 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 10 months ago by kencu (Ken)

yeah.

just please change it <9 instead, as outlined above.

comment:13 in reply to:  10 ; Changed 10 months 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 10 months 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 10 months 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.