Skip to content

refactor(source): More RecursivePathSource clean up #14231

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Jul 11, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented Jul 10, 2024

What does this PR try to resolve?

This is a follow up to #13993 and #14169 and is part of my work towards #10752.

How should we test and review this PR?

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added Command-read-manifest S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2024
@epage epage marked this pull request as draft July 10, 2024 20:35
@epage epage marked this pull request as ready for review July 10, 2024 20:51
epage added a commit to epage/cargo that referenced this pull request Jul 10, 2024
This came up in rust-lang#14231
```
---- expected: tests/testsuite/lto.rs:611:27
++++ actual:   stderr
   1    1 | [FRESH] registry-shared v0.0.1
   2    2 | [FRESH] registry v0.0.1
   3    3 | [COMPILING] bar v0.0.0 ([ROOT]/foo/bar)
   4    4 | [RUNNING] `rustc --crate-name bar [..]-C lto [..]--test [..]`
   5    5 | [RUNNING] `rustc --crate-name b [..]-C lto [..]--test [..]`
   6      - [FINISHED] `release` profile [optimized] target(s) in [ELAPSED]s
   7    6 | [RUNNING] `[ROOT]/foo/target/release/deps/bar-[HASH][EXE]`
   8    7 | [RUNNING] `[ROOT]/foo/target/release/deps/b-[HASH][EXE]`
   9    8 | [DOCTEST] bar
  10    9 | [RUNNING] `rustdoc --edition=2015 --crate-type cdylib --crate-type rlib --crate-name bar --test [..]-C lto [..]
       10 + [FINISHED] `release` profile [optimized] target(s) in 1m 00s

Update with SNAPSHOTS=overwrite
```
bors added a commit that referenced this pull request Jul 10, 2024
fix(test): Redact elapsed time in the minutes time frame

### What does this PR try to resolve?

This came up in #14231
```
---- expected: tests/testsuite/lto.rs:611:27
++++ actual:   stderr
   1    1 | [FRESH] registry-shared v0.0.1
   2    2 | [FRESH] registry v0.0.1
   3    3 | [COMPILING] bar v0.0.0 ([ROOT]/foo/bar)
   4    4 | [RUNNING] `rustc --crate-name bar [..]-C lto [..]--test [..]`
   5    5 | [RUNNING] `rustc --crate-name b [..]-C lto [..]--test [..]`
   6      - [FINISHED] `release` profile [optimized] target(s) in [ELAPSED]s
   7    6 | [RUNNING] `[ROOT]/foo/target/release/deps/bar-[HASH][EXE]`
   8    7 | [RUNNING] `[ROOT]/foo/target/release/deps/b-[HASH][EXE]`
   9    8 | [DOCTEST] bar
  10    9 | [RUNNING] `rustdoc --edition=2015 --crate-type cdylib --crate-type rlib --crate-name bar --test [..]-C lto [..]
       10 + [FINISHED] `release` profile [optimized] target(s) in 1m 00s

Update with SNAPSHOTS=overwrite
```

### How should we test and review this PR?

### Additional information
@epage epage force-pushed the git-clean branch 2 times, most recently from cb606d6 to 4c705a4 Compare July 11, 2024 18:34
@bors
Copy link
Contributor

bors commented Jul 11, 2024

☔ The latest upstream changes (presumably #14238) made this pull request unmergeable. Please resolve the merge conflicts.

epage added 5 commits July 11, 2024 16:46
This prepares the way for moving `walk` from `read_packages` into here.
This is tied to the `Source` and only used there.
Primary benefit: looking to track more state in `RecursivePathSource`
and this is a stepping stone.

Minor benefits:
- Cleaner
- Avoid re-allocating
- Faster lookup for `download`
Copy link
Member

@Muscraft Muscraft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Muscraft
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 11, 2024

📌 Commit 848dd7f has been approved by Muscraft

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2024
@bors
Copy link
Contributor

bors commented Jul 11, 2024

⌛ Testing commit 848dd7f with merge cf38b96...

@bors
Copy link
Contributor

bors commented Jul 11, 2024

☀️ Test successful - checks-actions
Approved by: Muscraft
Pushing cf38b96 to master...

@bors bors merged commit cf38b96 into rust-lang:master Jul 11, 2024
20 checks passed
@epage epage deleted the git-clean branch July 12, 2024 00:57
bors added a commit that referenced this pull request Jul 15, 2024
fix(source): Don't warn about unreferenced duplicate packages

### What does this PR try to resolve?

This also improves the message, consolidating multiple duplicates and saying which was loaded instead, as it naturally fell out of the design

Fixes #10752

### How should we test and review this PR?

### Additional information

We're still subject to #13724 and fully load every manifest, even if we don't use it.  I'm exploring that topic at https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Redundant.20code.20in.20.60GitSouce.60.3F/near/450783427

This change builds on
- #13993
- #14169
- #14231
- #14234
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
Update cargo

31 commits in 154fdac39ae9629954e19e9986fd2cf2cdd8d964..a2b58c3dad4d554ba01ed6c45c41ff85390560f2
2024-07-07 01:28:23 +0000 to 2024-07-16 00:52:02 +0000
- chore(ci): bump CI tools (rust-lang/cargo#14257)
- test: migrate fetch and list_availables to snapbox (rust-lang/cargo#14214)
- chore: downgrade to jobserver@0.1.28 (rust-lang/cargo#14254)
- perf(source): Don't `du` on every git source load (rust-lang/cargo#14252)
- fix(source): Don't warn about unreferenced duplicate packages (rust-lang/cargo#14239)
- feat(test): Add cargo_test to test-support prelude (rust-lang/cargo#14243)
- Add workflow to publish Cargo automatically (rust-lang/cargo#14202)
- test: migrate implicit_features to snapbox (rust-lang/cargo#14245)
- test: migrate build-std/main to snapbox (rust-lang/cargo#14241)
- test: migrate check_cfg to snapbox (rust-lang/cargo#14235)
- refactor(source): More RecursivePathSource clean up (rust-lang/cargo#14231)
- Add more profiling traces (rust-lang/cargo#14238)
- fix(overrides): Don't warn on duplicate packages from using '..' (rust-lang/cargo#14234)
- fix(test): Redact elapsed time in the minutes time frame (rust-lang/cargo#14233)
- test: Migrate lto tests to snapbox (rust-lang/cargo#14209)
- fix: Ensure dep/feature activates the dependency on 2024 (rust-lang/cargo#14221)
- chore(docs): update index of reference (rust-lang/cargo#14228)
- test: migrate test to snapbox (rust-lang/cargo#14226)
- chore: remove duplicate words (rust-lang/cargo#14229)
- docs(contrib): Document things I look for in RFCs (rust-lang/cargo#14222)
- docs(ref): Note MSRV for features in the docs (rust-lang/cargo#14224)
- test(progress): Resolve flakiness (rust-lang/cargo#14223)
- fix(test): Reduce over-prescription to the caller (rust-lang/cargo#14217)
- refactor: move get_source_id out of registry (rust-lang/cargo#14218)
- fix: rename to `rustdoc::broken_intra_doc_links` (rust-lang/cargo#14215)
- test: migrate member_errors, multitarget and new to snapbox (rust-lang/cargo#14210)
- test: migrate generate_lockfile and glob_targets to snapbox (rust-lang/cargo#14200)
- test: Ensure --list test works with cargo-bloat (rust-lang/cargo#14213)
- dont make new constant InternedString in hot path (rust-lang/cargo#14211)
- Fix compatible_with_older_cargo test. (rust-lang/cargo#14212)
- test: migrate metabuild, metadata and net_config to snapbox (rust-lang/cargo#14162)
@rustbot rustbot added this to the 1.81.0 milestone Jul 17, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Command-read-manifest S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants