Add more information to wait-for-publish #11713
Conversation
|
r? @epage (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-console-output
ehuss
force-pushed
the
publish-waiting-extra-info
branch
from
ec2a69a
to
a814076
Compare
| /// If quiet, the source should not display any progress or status messages. | ||
| fn set_quiet(&mut self, quiet: bool); |
There was a problem hiding this comment.
Wait, don't we cache sources generally (and the "wait" logic has to workaround that)? I feel like mutating a cached item like this is pretty brittle (in an already brittle system) and I'd hope we can find a way to avoid making it worse.
There was a problem hiding this comment.
I'm not sure I follow. Sources are inherently mutable. I'm not sure I would say they are "cached" so much as they are lazily created as needed and held in the SourceMap. But by their nature they are accessed from the map and mutated as needed.
There was a problem hiding this comment.
As for the caching / lazy loading, I don't remember the details but I do remember running into a lot of problems implementing this because I was getting stale state due to caches that said "everything is loaded, no reason to talk to the server again".
But by their nature they are accessed from the map and mutated as needed.
From my quick glance, they are mutated as-needed in that you invalidate their cache and they reload. I didn't see any other real "state" to them that could leak from one operation to another. That is what I'm concerned about. As I said, I ran into a lot of problems with cargo assuming everything was only done once because commands are transient but when a command needed to do things multiple times, they broke. While I am not refreshed on all the details to map out whether its actually safe or not, this feels like one of those things that could easily cause people problems in the future.
src/cargo/ops/registry.rs
Outdated
| config.shell().note(format!( | ||
| "Waiting up to {max} seconds for `{short_pkg_description}` to be available at {source_description}.\n\ | ||
| You may press ctrl-c to skip waiting; the crate should be available shortly." | ||
| ))?; |
There was a problem hiding this comment.
When looking at the output, I felt this note was too busy
There was a problem hiding this comment.
Do you have any suggestions to improve it? I don't see any way to shorten the first sentence any more than it is, and I think it is important to inform the user what Cargo is about to do. The second sentence I think is pretty awkward, but I couldn't come up with anything much better. I think the previous text of (ctrl-c to wait asynchronously) was short and to-the-point, but not very clear to me on what that meant.
There was a problem hiding this comment.
I dropped the "up to 60 seconds" part to de-clutter since it didn't seem too terribly important for the user to know the exact value.
|
Thanks for going in and improving this! |
ehuss
force-pushed
the
publish-waiting-extra-info
branch
from
a814076
to
f7613ec
Compare
|
Note that this effectively resolves #11304. |
|
@epage Did you have any other questions or ideas for improvements here? I'd like to get this in 1.70 with plenty of time for testing. |
|
@bors r+ Sorry, seemed to either lose track of approving this or what I was blocking it on. I might have some nitpicks with it still but there is enough value being added that its not worth holding it up over them |
bors
removed
the
S-waiting-on-review
bors
added
the
S-waiting-on-bors
Add more information to wait-for-publish
This reworks the console output when waiting for a publish to be available:
* Shows a "Published" status to try to make it clear that the publish is complete, and that Cargo is moving to a separate phase.
* Removes the repeated status bars and updating messages, and uses a single progress bar to track the publish.
* Provides more of a description of why Cargo is waiting to try to make it clearer what is happening.
* Provides more information when a timeout happens to try to explain what might be happening.
* Shows a "Completed" message at the end to let the user know that everything is complete.
Comparing the output:
Before (with git):
```
Updating crates.io index
Packaging delay v0.0.2 (/Users/eric/Proj/rust/cargo/target/tmp/cit/t0/foo)
Packaged 3 files, 761.0B (569.0B compressed)
Uploading delay v0.4.27 (/Users/eric/Proj/rust/cargo/target/tmp/cit/t0/foo)
Updating crates.io index
Waiting on `delay` to propagate to crates.io index (ctrl-c to wait asynchronously)
Updating crates.io index
Updating crates.io index
Updating crates.io index
Updating crates.io index
Updating crates.io index
Updating crates.io index
Updating crates.io index
Updating crates.io index
Updating crates.io index
Updating crates.io index
```
Before (with sparse):
```
Updating crates.io index
Packaging delay v0.0.2 (/Users/eric/Proj/rust/cargo/target/tmp/cit/t0/foo)
Packaged 3 files, 761.0B (569.0B compressed)
Uploading delay v0.0.2 (/Users/eric/Proj/rust/cargo/target/tmp/cit/t0/foo)
Updating crates.io index
Waiting on `delay` to propagate to crates.io index (ctrl-c to wait asynchronously)
Fetch [=============================> ] 36 complete; 1 pending
```
New (git or sparse):
```
Updating crates.io index
Packaging delay v0.0.2 (/Users/eric/Proj/rust/cargo/target/tmp/cit/t0/foo)
Packaged 3 files, 761.0B (569.0B compressed)
Uploading delay v0.0.2 (/Users/eric/Proj/rust/cargo/target/tmp/cit/t0/foo)
Uploaded delay v0.0.2 (/Users/eric/Proj/rust/cargo/target/tmp/cit/t0/foo)
note: Waiting for `delay@0.0.2` to be available at registry `crates-io`.
You may press ctrl-c to skip waiting; the crate should be available shortly.
Waiting [===> ] 11/60
Published delay v0.0.2 (/Users/eric/Proj/rust/cargo/target/tmp/cit/t0/foo) has been successfully published to registry `crates-io`
```
(Note: In the last two cases the progress bar disappears when it is done, I have just included it here to illustrate.)
Fixes #11304
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
This adds the ability to silence Source status messages and progress bars. This is intended to be used with the publish "wait for available" system, which causes sources to repeatedly update which ends up spamming the terminal.
This level of detail isn't particularly important to the user and just clutters up the output.
ehuss
force-pushed
the
publish-waiting-extra-info
branch
from
fade07e
to
3c295cf
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Update cargo 11 commits in 4a3c588b1f0a8e2dc8dd8789dbf3b6a71b02ed49..15d090969743630bff549a1b068bcaa8174e5ee3 2023-03-14 14:05:36 +0000 to 2023-03-21 17:54:28 +0000 - docs(contrib): Move higher level resolver docs into doc comments (rust-lang/cargo#11870) - docs(contrib): Pull impl info out of architecture (rust-lang/cargo#11869) - Update curl-sys (rust-lang/cargo#11871) - Poll loop fixes (rust-lang/cargo#11624) - clippy: warn `disallowed_methods` for `std::env::var` and friends (rust-lang/cargo#11828) - Add --ignore-rust-version flag to cargo install (rust-lang/cargo#11859) - Handle case mismatches when looking up env vars in the Config snapshot (rust-lang/cargo#11824) - align semantics of generated vcs ignore files (rust-lang/cargo#11855) - Add more information to wait-for-publish (rust-lang/cargo#11713) - docs: Address warnings (rust-lang/cargo#11856) - docs(contrib): Create a file overview in the nightly docs (rust-lang/cargo#11850)
Update cargo 11 commits in 4a3c588b1f0a8e2dc8dd8789dbf3b6a71b02ed49..15d090969743630bff549a1b068bcaa8174e5ee3 2023-03-14 14:05:36 +0000 to 2023-03-21 17:54:28 +0000 - docs(contrib): Move higher level resolver docs into doc comments (rust-lang/cargo#11870) - docs(contrib): Pull impl info out of architecture (rust-lang/cargo#11869) - Update curl-sys (rust-lang/cargo#11871) - Poll loop fixes (rust-lang/cargo#11624) - clippy: warn `disallowed_methods` for `std::env::var` and friends (rust-lang/cargo#11828) - Add --ignore-rust-version flag to cargo install (rust-lang/cargo#11859) - Handle case mismatches when looking up env vars in the Config snapshot (rust-lang/cargo#11824) - align semantics of generated vcs ignore files (rust-lang/cargo#11855) - Add more information to wait-for-publish (rust-lang/cargo#11713) - docs: Address warnings (rust-lang/cargo#11856) - docs(contrib): Create a file overview in the nightly docs (rust-lang/cargo#11850)
This reworks the console output when waiting for a publish to be available:
Comparing the output:
Before (with git):
Before (with sparse):
New (git or sparse):
(Note: In the last two cases the progress bar disappears when it is done, I have just included it here to illustrate.)
Fixes #11304