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

portconfigure: Prioritize returning SDK with DEVELOPER_DIR set #135

Merged
merged 2 commits into from
Jul 19, 2019

Conversation

satraul
Copy link
Member

@satraul satraul commented Jul 17, 2019

  • move xcrun --sdk macosx to find macOS SDK to the top
  • export DEVELOPER_DIR so xcrun can find CommandLineTools SDK

Relevant: https://trac.macports.org/ticket/57143

xcrun --sdk macosx, which was previously reenabled as the final fallback by @jmroot, is moved to the top of the ::configure_get_sdkroot proc so the newest-available macOS SDK is returned first instead of the current OS version's SDKs. It was generally agreed on trac that the newest-available SDK should be used, so I assume this change is better and causes no problems.

The DEVELOPER_DIR change is done so a CommandLineTools SDK is returned if the port isn't Xcode-dependent. This fixes gmp failing to install in tracemode with the latest changes to master.

@jmroot
Copy link
Member

jmroot commented Jul 17, 2019

It definitely does cause problems, as indicated in the comment you deleted. Anything that detects functionality at build time will see things available in the SDK that are not available in the OS. This would be fine if everything used potentially weak-linked symbols correctly, but that's actually very rare in the world of software that isn't Mac-only.

@satraul
Copy link
Member Author

satraul commented Jul 17, 2019

It definitely does cause problems, as indicated in the comment you deleted. Anything that detects functionality at build time will see things available in the SDK that are not available in the OS. This would be fine if everything used potentially weak-linked symbols correctly, but that's actually very rare in the world of software that isn't Mac-only.

I don't fully understand weak-links yet but I've just read up about them. It means that we should still ideally return the target's OS version (which is the current OS) over the newest-available right?

I figure the solution would be to try xcrun --sdk ${sdk_version} first, then xcrun --sdk macosx, then the other checks. We would still be prioritizing DEVELOPER_DIR first (CLT>Xcode) over version (now current>newest-available). Prioritizing version over DEVELOPER_DIR would still cause conflicts with the Xcode tracemode changes.

@satraul satraul force-pushed the xcode/sdk branch 2 times, most recently from ba4da46 to 3ddc049 Compare July 17, 2019 09:23
@satraul satraul changed the title portconfigure: Return newest SDK with DEVELOPER_DIR set portconfigure: Prioritize returning SDK with DEVELOPER_DIR set Jul 17, 2019
@jmroot
Copy link
Member

jmroot commented Jul 18, 2019

Looking in the CLT location first is fine, but xcrun should remain the last resort because it can be quite slow. And yes the target OS version's SDK should be tried first and generic MacOSX after.

The lack of weak linking awareness isn't just theoretical by the way, there have been a number of tickets where users ran into problems because they had Xcode installed (with a later OS version's SDK) but not the CLT. This is one example: https://trac.macports.org/ticket/54893

@satraul satraul force-pushed the xcode/sdk branch 2 times, most recently from 4b0160a to 74aec32 Compare July 19, 2019 07:09
@satraul
Copy link
Member Author

satraul commented Jul 19, 2019

Alright thanks for the feedback, I've made some changes to favor checking with file over xcrun. Also refactored use_xcode so its set to true if CLT isn't installed. gmp sucessfully installs with the latest changes.

Copy link
Member

@neverpanic neverpanic left a comment

Choose a reason for hiding this comment

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

LG2M now, let's wait for CI.

@neverpanic neverpanic merged commit 25b1209 into macports:master Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants