Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake{,-devel}: fix build before 10.10 #16798

Merged
merged 1 commit into from Nov 26, 2022
Merged

Conversation

catap
Copy link
Contributor

@catap catap commented Nov 24, 2022

Description

cmake is required to build all clangs, and to prevent dependency loop I've used clang-11-bootstrap to build it on platforms which can't build it by compiler which is ships with it.

Closes: https://trac.macports.org/ticket/66340
Closes: https://trac.macports.org/ticket/66040

I've also made port lint --nitpick cmake-devel happy.

Type(s)
  • bugfix
Tested on

macOS 10.7.5 11G63 x86_64
Xcode 4.6.3 4H1503

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint --nitpick?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@macportsbot
Copy link

Notifying maintainers:
@mascguy for port cmake-devel.
@michaelld for port cmake-devel, cmake.

@catap
Copy link
Contributor Author

catap commented Nov 24, 2022

@michaelld to be clear about fix: cmake is used to build clang-15 for example, and without cmake it is impossible to build clang-15.

Here we have chicken and the egg issue, and clang-11-bootstrap (and gcc10-bootstrap) was made special to broke such dependency cycles and solve such issue.

Copy link
Contributor

@michaelld michaelld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start & I totally agree that not all compilers that claim to support C++11 actually do support it. My inclination is to bump the required compiler to C++14 or even C++17, as those are available on all platforms and macOS / OSX versions these days, and should supersede the C++11 issue. I don't have an OSX 10.7/8/9 box up and running, so I can't test this. Hoping others can comment on this overall issue.

compiler.cxx_standard 2011
# but not each C++2011 compiler can build it.
# on macOS before 10.10 use clang-11-bootstrap cause cmake is used to build all clangs.
if {${os.platform} eq "darwin" && ${os.major} < 14 && ${build_arch} ni [list ppc ppc64]} {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove platform darwin & move to this if clause? I think they do the same thing, and I prefer the current version

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelld am I right that you would like to have?

platform darwin {
    if {${os.major} < 14 && ${build_arch} ni [list ppc ppc64]} {
        configure.compiler.add_deps no

        depends_build-append   port:clang-11-bootstrap
        depends_skip_archcheck-append \
            clang-11-bootstrap

        configure.cc           ${prefix}/libexec/clang-11-bootstrap/bin/clang
        configure.cxx          ${prefix}/libexec/clang-11-bootstrap/bin/clang++

        configure.cxx_stdlib   libc++
        depends_lib-append     port:libcxx
    }
}

I haven't done that to keep code flatten, but I can rollback to this style.

# See: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Help/dev/source.rst
compiler.cxx_standard 2011
# but not each C++2011 compiler can build it.
# on macOS before 10.10 use clang-11-bootstrap cause cmake is used to build all clangs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking the clause ${build_arch} ni [list ppc ppc64] can be removed, yes? PPC/64 needs to be handled here, as it was before. I don't have a PPC/64 box running at the moment, so maybe @kencu can comment on this?

Copy link
Contributor Author

@catap catap Nov 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-11-bootstrap aren't available on ppc :) because PPC hasn't got clang.

So, this condition is mandatory. I may reverse it to ${build_arch} in [list i386 x86_64] if you would like.

@@ -7,6 +7,7 @@ if { ${os.platform} eq "darwin" && ${os.major} >= 20 && ${build_arch} eq "x86_64
universal_variant no
} else {
PortGroup muniversal 1.0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this extra space to make lint happy? Prefer to not have it, regardless of what lint says ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelld yeah, it does. Without it lint complains like:

Warning: Line 10 should be a newline (after PortGroup)

BTW, cmake port has the same empty line. I assume by the same reason :)

@catap
Copy link
Contributor Author

catap commented Nov 24, 2022

@michaelld the issue with the way to simple bump that clang which can build it requires cmake to be built :(

@catap catap requested a review from michaelld November 24, 2022 16:09
@catap
Copy link
Contributor Author

catap commented Nov 24, 2022

Let me rephrase it.

MacPorts contains a lot of clang, for example clang-15 which can build cmake without any issue.

But, here, the catch: clang-15 depends on cmake.

Frankly speaking the only clang which doesn't depends on this port is clang-11-bootstrap. It depends on cmake-bootstrap which can be build without libc++.

It was made to bootstrap libcxx. Now, it seems, that we need this compiler to bootstrap cmake which is required to bootstrap all another clangs.

@catap
Copy link
Contributor Author

catap commented Nov 25, 2022

@michaelld right now all ports which is used cmake aren't build by buildbots before 10.10 because of broken cmake.

May I ask you to move forward with this fix?

Frankly speaking I doubt that any another way to fix it possible.

@catap
Copy link
Contributor Author

catap commented Nov 25, 2022

@ryandesign , @kencu, @jmroot what do you think about this PR?

@jmroot
Copy link
Member

jmroot commented Nov 25, 2022

Just bumping the cxx_standard to 2014 should have the desired effect of excluding the system clang on 10.9 and older. The clang_dependency portgroup already takes care of the circular dependency problems. The one problem is Lion, where cmake currently forces clang-3.4, which doesn't work. Apparently this is because 3.4 has a fix for a Lion-specific libc++ bug which 3.7 (at least) doesn't have. It seems like we should bring that fix forward into all the clangs that don't have it.

@catap
Copy link
Contributor Author

catap commented Nov 25, 2022

@jmroot why not use clang-11-bootstrap instead which is used to build libcxx?

@jmroot
Copy link
Member

jmroot commented Nov 25, 2022

No need apparently; I just tried it and clang-3.7 built cmake OK on 10.7. I guess something changed in the 5 years since that workaround was added.

So the change needed is:

--- a/devel/cmake/Portfile
+++ b/devel/cmake/Portfile
@@ -41,7 +41,7 @@ checksums           rmd160  09373f97fee569133852498f0de6e9f6db10084b \
 revision            0
 
 gitlab.livecheck.regex {([0-9.]+)}
-compiler.cxx_standard 2011
+compiler.cxx_standard 2014
 
 platform darwin {
     if {!((${os.major} < 9) || ${build_arch} eq "ppc" ||
@@ -102,14 +102,6 @@ if {${os.platform} eq "darwin" && ${os.major} <= 10 &&
     set cmake_bootstrapping yes
 }
 
-# On Lion, Clang 3.3 produces bad stream reading code when using
-# libc++.  See https://trac.macports.org/ticket/44129 . Clang 3.4
-# works. But Clang 3.7 doesn't work.
-if {${os.platform} eq "darwin" && ${os.major} == 11 &&
-    ${configure.cxx_stdlib} eq "libc++"} {
-    compiler.whitelist macports-clang-3.4
-}
-
 # clang 3.4 can't build certain parts of the
 # cmake self-testing infrastructure
 # https://trac.macports.org/ticket/59782

@catap
Copy link
Contributor Author

catap commented Nov 26, 2022

@jmroot indeed, your way is cleaner 🤦

Pushed better version. Thanks.

Copy link
Contributor

@michaelld michaelld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ; thx!

@michaelld michaelld merged commit d9e3218 into macports:master Nov 26, 2022
@catap catap deleted the cmake branch November 27, 2022 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants