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: restore no-default-rpath usage on Tiger #11736

Merged
merged 1 commit into from Aug 1, 2021
Merged

cmake: restore no-default-rpath usage on Tiger #11736

merged 1 commit into from Aug 1, 2021

Conversation

kencu
Copy link
Contributor

@kencu kencu commented Jul 30, 2021

cmake is gradually removing support for Tiger
at present, only the default adding of an -rpath
to all library and executables is troublesome however

this reverts:
https://gitlab.kitware.com/cmake/cmake/-/commit/4aed96e2309e64571aabfda263b3ca4c4f62d619

closes: https://trac.macports.org/ticket/63261

this needs someone to actually test it on Tiger to be 100% certain that it does the needed changes, but it looks to be correct.

What changes newer and future versions of cmake might make that make supporting Tiger difficult remain to be seen.

cmake is gradually removing support for Tiger
at present, only the default adding of an -rpath
to all library and executables is troublesome however

this reverts:
https://gitlab.kitware.com/cmake/cmake/-/commit/4aed96e2309e64571aabfda263b3ca4c4f62d619

closes: https://trac.macports.org/ticket/63261
@macportsbot
Copy link

Notifying maintainers:
@michaelld for port cmake.

@macportsbot macportsbot added by: member Created by a member with commit rights maintainer: requires approval labels Jul 30, 2021
@kencu
Copy link
Contributor Author

kencu commented Jul 30, 2021

@ryandesign , @evanmiller -- please feel free to test as you both have noted this issue so far.

@evanmiller
Copy link
Contributor

Do we know whether DARWIN_MAJOR_VERSION refers to the host OS version or target OS version (i.e. -mmacosx-version-min=)? I'm wondering if the logic ought to be modified to test the target version (to facilitate 10.5 -> 10.4 compiles).

@mojca
Copy link
Member

mojca commented Jul 30, 2021

I decided to be bold and opened an upstream request:

@mojca
Copy link
Member

mojca commented Jul 30, 2021

Oh, I'm sorry. I see that you already reported it upstream. I wasn't reading all the references close enough.

@kencu
Copy link
Contributor Author

kencu commented Jul 30, 2021

darwin major version will relate to the host os. (Edit: That actually might be more complicated, now that I read more around it. By default, it appears to relate to the host os.)

Let's for now restore what we had, which is an easy step that gets everyone working again.

Building on 10.5 for deployment on 10.4 will still work (the LC_RPATH load command will be ignored by Tiger once it is in there), but this is overall not well supported in MP anyway due to the many assumptions in Portfiles and open-source software that they will run on the building system. We can work on specific issues as we cross them, if needed.

Can you confirm for Michael you built this and it works on Tiger now? Then we can push I think.

@evanmiller
Copy link
Contributor

@kencu OK, restoring the old behavior for MP makes sense. I have not built or tested on Tiger.

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 ; I haven't tested, but since it impacts just Tiger / 10.4, I think this is fine either way. I will note from the noted CMake commit, that it specifically notes not just deprecation of OSX 10.4 or older, not removing support ... if this is the only real change that adds support back, then I think it's fine that we include it in MP & I don't see why they don't do the same upstream ...

Since CMake 3.19, we no longer support macOS SDKs older than 10.5,
which corresponds to Xcode 3. Supporting older Xcode versions for
device platforms is also not realistic. We therefore expect the -rpath
linker option should always be supported now.

@michaelld
Copy link
Contributor

@evanmiller Can you confirm that this change allows current CMake to build and run on Tiger / 10.4?

@evanmiller
Copy link
Contributor

Confirmed fixed on 10.4.11.

@kencu kencu merged commit 5149624 into macports:master Aug 1, 2021
@kencu kencu deleted the cmaketigerfix branch September 25, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
by: member Created by a member with commit rights maintainer: requires approval
5 participants