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

🌌 Use build.rustflags (and variants) as a replacement for environment variable to enable spaces in paths #3

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

EmilioLaiso
Copy link

Fixes rust-mobile#128

As per the docs https://doc.rust-lang.org/cargo/reference/config.html#buildrustflags

There are four mutually exclusive sources of extra flags. They are checked in order, with the first one being used:

- CARGO_ENCODED_RUSTFLAGS environment variable.
- RUSTFLAGS environment variable.
- All matching target.<triple>.rustflags and target.<cfg>.rustflags config entries joined together.
- build.rustflags config value.

Using the build.rutflag approach allows for spaces in paths, as seen in https://github.com/rust-lang/docs.rs/blob/bcc1a2c9f63a62907abbe68dcbe3f73632a9a4de/crates/metadata/lib.rs#L256-L262

Additionally, as suggested in rust-mobile#128 (comment),
setting CC_SHELL_ESCAPED_FLAGS=1 and inserting quotes around paths that are passed to C(XX)FLAGS solves all the remaining issues (provided that cc-rs is updated to 1.1.11)

@EmilioLaiso
Copy link
Author

Should I link

rust-lang/cargo#14398
and
rust-mobile#186

?

@Jasper-Bekkers Jasper-Bekkers merged commit eb2f5c5 into for-traverse Sep 26, 2024
@MarijnS95
Copy link
Member

Should I link

rust-lang/cargo#14398
and
rust-mobile#186

?

Only if it's merged to xbuild main? Might want to reopen the PR there too for the larger community.

@EmilioLaiso EmilioLaiso deleted the for-traverse-fix-spaces-in-paths branch October 2, 2024 17:11
@maxded maxded mentioned this pull request Oct 17, 2024
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