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

Don't use the value of SDKROOT if it doesn't match the target. #1047

Merged
merged 5 commits into from
May 6, 2024

Conversation

jfgoog
Copy link
Contributor

@jfgoog jfgoog commented May 2, 2024

The SDKROOT environment variable can be problematic on Mac. In cases where you are compiling for multiple targets, SDKROOT cannot be right for all of them. Furthermore, the system Python interpreter sets SDKROOT to the MacOSX SDK if not already set.

So, if you are using a Python script to run a build, and you need to build for multiple targets, the logic in the cc crate doesn't work. This is precisely what is happening with rustc itself, and so we can't upgrade the version of the cc crate used by the bootstrap code.

(Unsetting SDKROOT doesn't work either because the custom clang build that rustc uses for CI depends on it being set)

The SDKROOT environment variable can be problematic on Mac.
In cases where you are compiling for multiple targets, SDKROOT
cannot be right for all of them. Furthermore, the system Python
interpreter sets SDKROOT to the MacOSX SDK if not already set.

So, if you are using a Python script to run a build, and you need
to build for multiple targets, the logic in the cc crate doesn't work.
This is precisely what is happening with rustc itself, and so we can't
upgrade the version of the cc crate used by the bootstrap code.

(Unsetting SDKROOT doesn't work either because the custom clang
build that rustc uses for CI depends on it being set)
@jfgoog
Copy link
Contributor Author

jfgoog commented May 2, 2024

For more information, see rust-lang/rust#122504 and rust-lang/rust#124565

src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

I think this makes sense to do

src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thank you!

@NobodyXu NobodyXu merged commit 8521a7f into rust-lang:main May 6, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants