-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Improve dependency deduplication diagnostics #52080
Conversation
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.
Some minor nits if you care, otherwise r=me.
src/bootstrap/tool.rs
Outdated
println!(" `{}` enabled features {:?} at {:?}", | ||
prev.0, prev.2, prev.1); | ||
if cur.2 == prev.2 { | ||
continue; |
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.
You could use duplicates.drain_filter(...)
to avoid the double-checking.
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 considered that and then was like oh it's just during failure and copy paste is so convenient. I'll do it anyway.
src/bootstrap/tool.rs
Outdated
println!(" `{}` additionally enabled features {:?} at {:?}", | ||
cur.0, cur_extra, cur.1); | ||
println!(" `{}` additionally enabled features {:?} at {:?}", | ||
prev.0, prev_extra, prev.1); |
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.
These 6 lines could be simply
println!(" `{}` additionally enabled features {:?} at {:?}",
cur.0, &cur_features - &prev_features, cur.1);
println!(" `{}` additionally enabled features {:?} at {:?}",
prev.0, &prev_features - &cur_features, prev.1);
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.
oh. that's why -
didn't work... you need references -.-
@bors r+ rollup |
📌 Commit f352e98 has been approved by |
…nnytm Improve dependency deduplication diagnostics r? @kennytm this is obviously hard to test 😆 cc rust-lang#52072
Rollup of 14 pull requests Successful merges: - #51619 (rust: add initial changes to support powerpc64le musl) - #51793 (Fix variant background color on hover in search results) - #52005 (Update LLVM to bring in a wasm codegen fix) - #52016 (Deduplicate error reports for statics) - #52019 ([cross-lang-lto] Allow the linker to choose the LTO-plugin (which is useful when using LLD)) - #52030 (Any docs preposition change) - #52031 (Strenghten synchronization in `Arc::is_unique`) - #52033 ([Gardening] Update outdated comments: ByVal -> Scalar) - #52052 (Make verbose --version show if parallel queries are supported.) - #52055 (Include VS 2017 in error message.) - #52063 (Add a link to the rustc docs) - #52073 (Add a punch card to weird expressions test) - #52080 (Improve dependency deduplication diagnostics) - #51953 (enable Atomic*.{load,store} for ARMv6-M / MSP430) Failed merges:
…nnytm Improve dependency deduplication diagnostics r? @kennytm this is obviously hard to test 😆 cc rust-lang#52072
Rollup of 14 pull requests Successful merges: - #51619 (rust: add initial changes to support powerpc64le musl) - #51793 (Fix variant background color on hover in search results) - #52005 (Update LLVM to bring in a wasm codegen fix) - #52016 (Deduplicate error reports for statics) - #52019 ([cross-lang-lto] Allow the linker to choose the LTO-plugin (which is useful when using LLD)) - #52030 (Any docs preposition change) - #52031 (Strenghten synchronization in `Arc::is_unique`) - #52033 ([Gardening] Update outdated comments: ByVal -> Scalar) - #52055 (Include VS 2017 in error message.) - #52063 (Add a link to the rustc docs) - #52073 (Add a punch card to weird expressions test) - #52080 (Improve dependency deduplication diagnostics) - #52093 (rustc: Update tracking issue for wasm_import_module) - #52096 (Fix typo in cell.rs) Failed merges:
r? @kennytm
this is obviously hard to test 😆
cc #52072