Skip to content
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

Sparse-registry fetching total size keeps increasing #10820

Closed
pickfire opened this issue Jul 3, 2022 · 8 comments · Fixed by #11068
Closed

Sparse-registry fetching total size keeps increasing #10820

pickfire opened this issue Jul 3, 2022 · 8 comments · Fixed by #11068
Labels
A-sparse-registry Area: http sparse registries C-bug Category: bug

Comments

@pickfire
Copy link
Contributor

pickfire commented Jul 3, 2022

Problem

From original thread https://internals.rust-lang.org/t/call-for-testing-cargo-sparse-registry/16862/20?u=pickfire

One thing I noticed is that the fetching need to go through several loops now, previously it just fetch until 100% then done, but now when it reached 100%, then it had to fetch another time again, and the total stuff needed to fetch which keeps increasing (1 -> 10 -> 50 -> 100 -> 130) resulting in the progress decreasing, feels weird.

I feel like the total should be - until there is a definite answer to the total number that needs to fetch then only show it rather than keep increasing it.

Steps

  1. rustup update nightly
  2. cargo +nightly -Z sparse-registry update

Possible Solution(s)

A way to improve that in the registry would be to on publish, provide an optional expected_dep_forest_size from the number of transitive dependencies used at publish time. This number could be used to provide a reasonable progress estimate quickly.

Notes

No response

Version

No response

@pickfire pickfire added the C-bug Category: bug label Jul 3, 2022
@ehuss ehuss added the A-sparse-registry Area: http sparse registries label Jul 3, 2022
@arlosi
Copy link
Contributor

arlosi commented Jul 6, 2022

This is expected behavior, but I understand it's confusing to have progress go backwards.

@rustbot modify labels: -C-bug +C-enhancement

@rustbot
Copy link
Collaborator

rustbot commented Jul 6, 2022

Error: The feature relabel is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@arlosi
Copy link
Contributor

arlosi commented Jul 6, 2022

Some additional ideas on what we could do.

Remove the progress bar

  • But keep the X/Y status. Y will still change as deps are discovered.
Fetching (35/38)

Include an estimate of the dependency graph size when publishing

  • Dependency graph size can change depending on features, platform, etc.
  • Would require running a batch job to generate for all existing crate versions
  • Increases metadata size

Attempt to run the resolver using cached metadata to get an estimate

  • Only works if the local cache is fresh
  • Wastes CPU time

Use the lockfile to get an estimate of the total number of dependencies

  • Only works if a lockfile is present.

@pickfire
Copy link
Contributor Author

pickfire commented Jul 7, 2022

But keep the X/Y status. Y will still change as deps are discovered.

That sounds pretty much the same issue to me, it's basically a progress bar that never ends, I rather it be X/- or X/∞ if we don't know it yet.

@arlosi
Copy link
Contributor

arlosi commented Jul 7, 2022

Maybe instead of 35/38 something like:

Fetching (35 completed; 3 downloading)

@pickfire
Copy link
Contributor Author

pickfire commented Jul 8, 2022

Or maybe X/N+3 then until it is confirmed, X/Y

@arlosi
Copy link
Contributor

arlosi commented Jul 8, 2022

What do you mean by confirmed? Each dependency that's fetched could potentially require additional dependencies that aren't fetched yet.

@pickfire
Copy link
Contributor Author

pickfire commented Jul 9, 2022

Confirmed as in cargo knows that there won't be any additional dependencies.

@bors bors closed this as completed in bd99c04 Sep 9, 2022
bors added a commit that referenced this issue Dec 18, 2022
Enable triagebot's relabel functionality

### What does this PR try to resolve?

This fixes the following failure that rustbot currently posts whenever someone tries to use "<b>`@</b><b>rustbot</b>` label" in this repository.

> **Error**: The feature `relabel` is not enabled in this repository.
> To enable it add its section in the `triagebot.toml` in the root of the repository.

Unauthenticated relabel has been enabled in rust-lang/rust for nearly 4 years. People overwhelmingly use it in good faith.

<br>

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

Compare against https://github.com/rust-lang/rust/blob/1.66.0/triagebot.toml.

Also skim through the 7 pages of labels on https://github.com/rust-lang/cargo/labels, whether it makes sense the ones I decided to allow arbitrary GitHub users to apply.

<br>

### Additional information

Attempted uses of "<b>`@</b><b>rustbot</b>` label", that failed, but this PR would allow:

- #10343 (comment)
- #10243 (comment)
- #9982 (comment)
- #9128 (comment)
- #9067 (comment)
- #8441 (comment)
- #11432 (comment)
- #8841 (comment)
- #10820 (comment)
- #10572 (comment)
- #9114 (comment)
- #8980 (comment)
- #9064 (comment)
- #8726 (comment)
- #8089 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sparse-registry Area: http sparse registries C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants