Do not supply --crate-version flag to rustdoc if present in RUSTDOCFLAGS #8014
Conversation
|
r? @ehuss (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/cargo/core/compiler/mod.rs
Outdated
| fn crate_versions_requested(bcx: &BuildContext<'_, '_>) -> bool { | ||
| bcx.config.cli_unstable().crate_versions | ||
| } |
There was a problem hiding this comment.
I don't think this necessarily needs to be a separate function, I think it would be best just to put the check inline. I can appreciate having the function names clearly express what is being done, but wrapping single expressions like this tends to significantly increase the total lines of code.
src/cargo/core/compiler/mod.rs
Outdated
| fn crate_version_flag_in_rustdocflags(bcx: &BuildContext<'_, '_>, unit: &Unit<'_>) -> bool { | ||
| bcx.rustdocflags_args(unit) | ||
| .iter() | ||
| .any(|flag| flag == RUSTDOC_CRATE_VERSION_FLAG) |
There was a problem hiding this comment.
I think these checks will need to be starts_with since you can have something like --crate-version=foo.
tests/testsuite/doc.rs
Outdated
| let mut doc_html = String::new(); | ||
| File::open(&doc_file) | ||
| .unwrap() | ||
| .read_to_string(&mut doc_html) | ||
| .unwrap(); |
There was a problem hiding this comment.
| let mut doc_html = String::new(); | |
| File::open(&doc_file) | |
| .unwrap() | |
| .read_to_string(&mut doc_html) | |
| .unwrap(); | |
| let doc_html = fs::read_to_string(&doc_file).unwrap(); |
tests/testsuite/doc.rs
Outdated
| } | ||
|
|
||
| #[cargo_test] | ||
| fn crate_versions_flag_is_overridden_by_rustdoc_extra_arguments() { |
There was a problem hiding this comment.
I don't think it is necessary to define a whole separate test. To avoid the repetition, at the bottom of the other test, call p.build_dir().rm_rf(); and then p.cargo("rustdoc …"), and then check the results again (can maybe put the asserts in a closure so they aren't repeated).
src/cargo/core/compiler/mod.rs
Outdated
| && !crate_version_flag_in_rustdocflags(bcx, unit) | ||
| && !crate_version_flag_in_extra_args(bcx, unit) |
There was a problem hiding this comment.
I'm thinking it might be a little simpler just to check the args of the ProcessBuilder instead of re-fetching the extra args/rustdocflags.
So, move the call to add_crate_versions_if_requested after the args are added to the builder, and call something like this:
rustdoc.get_args().iter().any(|flag| {
flag.to_str().map_or(false, |s| s.starts_with(RUSTDOC_CRATE_VERSION_FLAG))
})
aleksator
force-pushed
the
crate_version_in_rustdocflags
branch
from
010f77f
to
3c11603
Compare
|
Applied the suggestions as separate commits for easier reviewing, will squash after approval |
…AGS or extra compiler arguments
aleksator
force-pushed
the
crate_version_in_rustdocflags
branch
from
3c11603
to
c9e3141
Compare
|
@bors r+ |
|
📌 Commit c9e3141 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-azure |
Update cargo. 8 commits in 7019b3ed3d539db7429d10a343b69be8c426b576..8a0d4d9c9abc74fd670353094387d62028b40ae9 2020-03-17 21:02:00 +0000 to 2020-03-24 17:57:04 +0000 - Re-implement proc-macro feature decoupling. (rust-lang/cargo#8028) - Remove unused transitive dependencies: miniz_oxide, adler32 (rust-lang/cargo#8023) - Fix bug with -Zfeatures=dev_dep and `check --profile=test`. (rust-lang/cargo#8027) - Remove Config from CompileOptions. (rust-lang/cargo#8021) - Add `rustless.org` to documented blocklist. (rust-lang/cargo#7922) - Print colored warnings when build script panics (rust-lang/cargo#8017) - Do not supply --crate-version flag to rustdoc if present in RUSTDOCFLAGS (rust-lang/cargo#8014) - Add proc-macro to index, and new feature resolver. (rust-lang/cargo#8003)
Update cargo. 8 commits in 7019b3ed3d539db7429d10a343b69be8c426b576..8a0d4d9c9abc74fd670353094387d62028b40ae9 2020-03-17 21:02:00 +0000 to 2020-03-24 17:57:04 +0000 - Re-implement proc-macro feature decoupling. (rust-lang/cargo#8028) - Remove unused transitive dependencies: miniz_oxide, adler32 (rust-lang/cargo#8023) - Fix bug with -Zfeatures=dev_dep and `check --profile=test`. (rust-lang/cargo#8027) - Remove Config from CompileOptions. (rust-lang/cargo#8021) - Add `rustless.org` to documented blocklist. (rust-lang/cargo#7922) - Print colored warnings when build script panics (rust-lang/cargo#8017) - Do not supply --crate-version flag to rustdoc if present in RUSTDOCFLAGS (rust-lang/cargo#8014) - Add proc-macro to index, and new feature resolver. (rust-lang/cargo#8003)
Checks for the flag in extra compiler arguments as well