-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add [target.'cfg(...)']
syntax for rustc(doc)flags in .cargo/config
#3854
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
src/cargo/ops/cargo_rustc/context.rs
Outdated
@@ -872,11 +873,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> { | |||
} | |||
|
|||
pub fn rustflags_args(&self, unit: &Unit) -> CargoResult<Vec<String>> { | |||
env_args(self.config, &self.build_config, unit.kind, "RUSTFLAGS") | |||
env_args(self.config, &self.build_config, &self.target_info, unit.kind, "RUSTFLAGS") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may want to switch between target_info
and host_info
based off unit.kind
Thanks for the PR! Just one minor nit but otherwise looks good to me |
src/cargo/ops/cargo_rustc/context.rs
Outdated
@@ -178,6 +178,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { | |||
-> CargoResult<()> { | |||
let rustflags = env_args(self.config, | |||
&self.build_config, | |||
&self.target_info, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the same match should go here too right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure! Thanks.
@tee-too looks great to me although I think there's still that one spot to add a match? Maybe the matches could become a helper method as well perhaps? Other than that r=me, you may wish to rebase to clean up the commit history as well but I'm fine either way. |
@alexcrichton rebased and cleaned up. |
@tee-too oh I think there's still one more location for the |
(commented above in the PR comments) |
Allow to use the Rust `cfg(...)` syntax to configure rust(doc)flags. The flags are concatenated when a build matches several `cfg`, or several `cfg` and a $triple. Fix #3499.
@alexcrichton I think I understood where was the missing match. Sorry for that. |
@bors: r+ Thanks! |
📌 Commit e6e6f10 has been approved by |
Add `[target.'cfg(...)']` syntax for rustc(doc)flags in .cargo/config Allow to use the Rust `cfg(...)` syntax to configure rust(doc)flags. The flags are concatenated when a build matches several `cfg`, or several `cfg` and a $triple. Fix #3499.
☀️ Test successful - status-appveyor, status-travis |
Allow to use the Rust
cfg(...)
syntax to configure rust(doc)flags.The flags are concatenated when a build matches several
cfg
, orseveral
cfg
and a $triple.Fix #3499.