Don't treat host/target duplicates as duplicates #10466
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Signed-off-by: hi-rustin <[email protected]>
Rustin170506
force-pushed
the
rustin-patch-tree
branch
from
3d79ee3 to
58906aa
Compare
|
Thanks for the PR, but I'm going to be stepping down from the Cargo team so I'm going to un-assign myself from this. The Cargo team will help review this when they get a chance. |
|
r? @ehuss Since you created the issue, you might be interested in looking at it. |
src/cargo/ops/tree/graph.rs
Outdated
| !seen_pkg.contains(&ignore_kind_package) | ||
| && seen_pkg.insert(ignore_kind_package) |
There was a problem hiding this comment.
Sorry I didn't say it clear in #10466 (comment). What do you think about dropping contains and using only insert operation here? This is only a little improvement and might not affect the performance but to me it's eye-pleasing 😆
| !seen_pkg.contains(&ignore_kind_package) | |
| && seen_pkg.insert(ignore_kind_package) | |
| seen_pkg.insert(ignore_kind_package) |
There was a problem hiding this comment.
I wonder if this would make this code less readable? Maybe determining if it exists is a better indication that we need to de-dupe?
There was a problem hiding this comment.
Oh I just noticed that we can push it further by collecting to a HashSet:
indexes.into_iter().map(....).collect::<HashSet<_>>().len() > 1Sorry about the back-and-forth, but this actually looks better and is a common dedup pattern I guess. We don't need that seen_pkg anymore.
There was a problem hiding this comment.
Great way! Addressed!
Thanks for your review! 💚 💙 💜 💛 ❤️
Rustin170506
force-pushed
the
rustin-patch-tree
branch
from
469314b to
cad9ea8
Compare
Signed-off-by: hi-rustin <[email protected]> Better code Signed-off-by: hi-rustin <[email protected]>
Rustin170506
force-pushed
the
rustin-patch-tree
branch
from
cad9ea8 to
c45c2a5
Compare
|
Thanks for your work! @bors r+ |
|
📌 Commit c45c2a5 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Update cargo 13 commits in 109bfbd055325ef87a6e7f63d67da7e838f8300b..1ef1e0a12723ce9548d7da2b63119de9002bead8 2022-03-17 21:43:09 +0000 to 2022-03-31 00:17:18 +0000 - Support `-Zmultitarget` in cargo config (rust-lang/cargo#10473) - doc: Fix document url for libcurl format (rust-lang/cargo#10515) - Fix wrong info in "Environment variables" docs (rust-lang/cargo#10513) - Use the correct flag in --locked --offline error message (rust-lang/cargo#10512) - Don't treat host/target duplicates as duplicates (rust-lang/cargo#10466) - Unstable --keep-going flag (rust-lang/cargo#10383) - Part 1 of RFC2906 - Packages can inherit fields from their root workspace (rust-lang/cargo#10497) - Remove unused profile support for -Zpanic-abort-tests (rust-lang/cargo#10495) - HTTP registry implementation (rust-lang/cargo#10470) - Add a notice about review capacity. (rust-lang/cargo#10501) - Add tests for ignoring symlinks (rust-lang/cargo#10047) - Update doc string for deps_of/compute_deps. (rust-lang/cargo#10494) - Consistently use crate::display_error on errors during drain (rust-lang/cargo#10394)
What does this PR try to resolve?
close #9519
Don't treat host/target duplicates as duplicates.
How should we test and review this PR?
cargo tree -d --target x86_64-unknown-linux-gnu