Added required_features for issue #1570. #3667
Conversation
Based on PR rust-lang#2056 by @tsurai and PR rust-lang#2325 by @JanLikar
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
This looks fantastic and quite thorough to me, thanks @jbendig! I think with updates to the manifest docs this is good to go! cc @rust-lang/tools, a neat feature being added to cargo! |
|
Oh and the Travis tests look to be failing, but you can just add |
|
Thanks @alexcrichton! I used your I added an explanation for required-features as a sub-section to Configuring a target. This seems like the best place because the toml block already there is more [lib] specific and The [features] section seems more about features in general. Any thoughts or preferences? |
src/cargo/ops/cargo_compile.rs
Outdated
| let mut compatible_targets = Vec::with_capacity(targets.len()); | ||
| for (target, profile) in targets.drain(0..) { | ||
| if target.is_lib() || match target.required_features() { | ||
| Some(f) => !f.iter().any(|f| !features.contains(f)), |
There was a problem hiding this comment.
Perhaps instead of the double-negation here we could go for f.iter().all(...)?
There was a problem hiding this comment.
Sure, that's definitely more readable.
|
Looks fantastic! Can you also add one more tests which emulates |
|
Good idea. I'll try and make these changes later today. |
…equire at least one feature that's not selected.
|
📌 Commit d43fd2e has been approved by |
|
☀️ Test successful - status-appveyor, status-travis |
|
Saw this on TWiR: how does this interact with |
|
@pthariensflame it shouldn't interact directly, this reads activated features after they've been decided and then dictates whether the binaries/examples are compiled |
|
Oh, ok. |
Based on PR #2056 by @tsurai and PR #2325 by @JanLikar
I tried to fix most everything that was talked about in the previous pull requests. Docs still need to be updated though.