Add some enhancements to `cargo clean` #12638

ehuss · 2023-09-07T05:19:14Z

What does this PR try to resolve?

This adds some enhancements to cargo clean that fell out as a result of some refactorings in #12634 for supporting an interface for cleaning from other places in cargo, and these were relatively easy to add and assist with testing in #12634.

The changes are:

Introduce some refactoring to offer a cleaning interface that can be used elsewhere in cargo.
Adds a --dry-run CLI option which will print what cargo clean will delete without deleting it. NOTE This PR makes the flag insta-stable. I don't figure there is too much that can be learned about it keeping it unstable, though we could change that. Add cache garbage collection #12634 has this flag gated with -Zgc.
Adds a summary line at the end of the cargo clean operation that indicates how much was deleted.

How should we test and review this PR?

Note that this PR also includes the changes from #12635 and #12637. Those commits can be dropped if those PRs are merged.

For the most part, this involves wrapping the cleaning operations in a CleanContext which provides an interface for performing cleaning operations. The dry run is just a flag that is checked at the deletion points. The summary data is also collected at those same points.

rustbot · 2023-09-07T05:19:18Z

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

bors · 2023-09-07T10:34:05Z

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

src/cargo/ops/cargo_clean.rs

weihanglo

Saw Ed is reviewing. I'll step back and take a rest 😁

src/bin/cargo/commands/clean.rs

src/cargo/ops/cargo_clean.rs

epage · 2023-09-07T14:21:58Z

NOTE This PR makes the flag insta-stable. I don't figure there is too much that can be learned about it keeping it unstable, though we could change that. #12634 has this flag gated with -Zgc.

Agreed that there isn't too much to learn. We also have a precedence for this and I'd love to see us extend that precedence further.

bors · 2023-09-11T08:11:35Z

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

epage

Feel free to r= me when its ready

This came from rust-lang#12638 and my many small frustrations from wanting to use `-n` and not being able to. We do not have any existing `-n` flags for this to be confused with. I would wager that `-n` is such an entrenched short flag in build tools that it would not make sense for us to use it with any other flag. For a survey of where `-n` is used as a short, see https://www.gnu.org/prep/standards/html_node/Option-Table.html#Option-Table

feat(cli): Add '-n' to dry-run This came from #12638 and my many small frustrations from wanting to use `-n` and not being able to. We do not have any existing `-n` flags for this to be confused with. I would wager that `-n` is such an entrenched short flag in build tools that it would not make sense for us to use it with any other flag. For a survey of where `-n` is used as a short, see https://www.gnu.org/prep/standards/html_node/Option-Table.html#Option-Table

This refactors some of the `cargo clean` code to wrap the "cleaning" operation in a `CleanContext` so that the context can be passed to other parts of cargo which can perform their own cleaning operations. There are some minor changes in the error messages to prepare for cleaning operations that aren't directly related to the build directory.

This adds a `--dry-run` option to have `cargo clean` display what it would delete without actually deleting it.

This adds a summary at the end when `cargo clean` finishes that displays how many files and bytes were removed.

And drop the `-n` short flag until we decide to commit to using it generally.

The previous status line was a little awkward in the way it combined both counts. I don't think showing the directories is particularly interesting, so they are only displayed when no files are deleted.

This makes it more consistent with other `--dry-run` commands, and makes it clearer to the user that cargo did not do anything.

The paths themselves aren't particularly interesting.

ehuss · 2023-09-20T02:25:40Z

@bors r=epage

bors · 2023-09-20T02:25:42Z

📌 Commit 655f75d has been approved by epage

It is now in the queue for this repository.

bors · 2023-09-20T02:25:48Z

⌛ Testing commit 655f75d with merge f57c80e...

bors · 2023-09-20T03:13:02Z

☀️ Test successful - checks-actions
Approved by: epage
Pushing f57c80e to master...

bors · 2023-09-20T03:13:03Z

☀️ Test successful - checks-actions
Approved by: epage
Pushing f57c80e to master...

bors · 2023-09-20T03:14:03Z

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

Update cargo 19 commits in b4ddf95ad9954118ac0dae835f2966394ad04c02..414d9e3a6d8096f3e276234ce220c868767a8792 2023-09-18 03:48:09 +0000 to 2023-09-22 07:03:57 +0000 - refactor(TomlManifest): fail when package_root is not a directory (rust-lang/cargo#12722) - Better suggestion for unsupported mode in build command (rust-lang/cargo#12693) - Update curl-sys to pull in curl 8.3.0 (rust-lang/cargo#12718) - chore(ci): Ignore patch version in MSRV (rust-lang/cargo#12716) - refactor: move cached crates.io SourceID to config module (rust-lang/cargo#12711) - fix: typos in registry authentication documentation (rust-lang/cargo#12714) - doc: mention unstable flag `-Z asymmetric-token` (rust-lang/cargo#12712) - fix: copy PDBs for EFI targets (rust-lang/cargo#12688) - infra: add auto-trigger rules for new labels (rust-lang/cargo#12713) - fix: use channel-specific link for registry auth error (rust-lang/cargo#12709) - Add some enhancements to `cargo clean` (rust-lang/cargo#12638) - chore: Fix typos (rust-lang/cargo#12707) - Shortest path (rust-lang/cargo#12678) - doc/reference/manifest: Adjust `keywords` description (rust-lang/cargo#12705) - Cargo add displays either feature list or summarized count (rust-lang/cargo#12702) - SemVer: Update documentation about removing optional dependencies (rust-lang/cargo#12687) - publish.py: Remove obsolete `sleep()` calls (rust-lang/cargo#12686) - generalise suggestion on abiguous spec (rust-lang/cargo#12685) - util/toml: Remove duplicate `serde(rename)` attributes (rust-lang/cargo#12682) r? ghost

rustbot assigned weihanglo Sep 7, 2023

rustbot added A-cli Area: Command-line interface, option parsing, etc. A-testing-cargo-itself Area: cargo's tests Command-clean S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 7, 2023

epage reviewed Sep 7, 2023

View reviewed changes

src/cargo/ops/cargo_clean.rs Outdated Show resolved Hide resolved

weihanglo reviewed Sep 7, 2023

View reviewed changes

src/bin/cargo/commands/clean.rs Outdated Show resolved Hide resolved

src/bin/cargo/commands/clean.rs Outdated Show resolved Hide resolved

epage reviewed Sep 7, 2023

View reviewed changes

src/cargo/ops/cargo_clean.rs Outdated Show resolved Hide resolved

epage reviewed Sep 7, 2023

View reviewed changes

src/cargo/ops/cargo_clean.rs Show resolved Hide resolved

epage reviewed Sep 7, 2023

View reviewed changes

src/cargo/ops/cargo_clean.rs Outdated Show resolved Hide resolved

ehuss force-pushed the clean-updates branch 2 times, most recently from e176917 to 5c46e80 Compare September 10, 2023 19:58

rustbot added A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation labels Sep 10, 2023

epage approved these changes Sep 12, 2023

View reviewed changes

epage mentioned this pull request Sep 12, 2023

feat(cli): Add '-n' to dry-run #12660

Merged

weihanglo approved these changes Sep 13, 2023

View reviewed changes

ehuss added 6 commits September 19, 2023 18:14

Add a --dry-run option to cargo clean.

45c5394

This adds a `--dry-run` option to have `cargo clean` display what it would delete without actually deleting it.

Add a summary to cargo clean.

495ed7e

This adds a summary at the end when `cargo clean` finishes that displays how many files and bytes were removed.

Use existing arg_dry_run function.

e7e354c

And drop the `-n` short flag until we decide to commit to using it generally.

Use Self type.

e9110aa

Separately track files and directories removed.

ebee726

The previous status line was a little awkward in the way it combined both counts. I don't think showing the directories is particularly interesting, so they are only displayed when no files are deleted.

ehuss added 3 commits September 19, 2023 18:24

Add a warning to cargo clean --dry-run

f61d42d

This makes it more consistent with other `--dry-run` commands, and makes it clearer to the user that cargo did not do anything.

Don't display paths with cargo clean --dry-run without --verbose

c204734

The paths themselves aren't particularly interesting.

Add cargo --dry-run to the documentation.

655f75d

ehuss force-pushed the clean-updates branch from 5c46e80 to 655f75d Compare September 20, 2023 01:26

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 Sep 20, 2023

bors merged commit f57c80e into rust-lang:master Sep 20, 2023
18 of 20 checks passed

bors mentioned this pull request Sep 20, 2023

Ensure package IDs are unique when cleaning. #12708

Closed

weihanglo mentioned this pull request Sep 22, 2023

Update cargo rust-lang/rust#116066

Merged

ehuss added this to the 1.74.0 milestone Sep 30, 2023

ehuss mentioned this pull request Oct 19, 2023

Add cache garbage collection #12634

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add some enhancements to `cargo clean` #12638

Add some enhancements to `cargo clean` #12638

ehuss commented Sep 7, 2023

rustbot commented Sep 7, 2023

bors commented Sep 7, 2023

weihanglo left a comment

epage commented Sep 7, 2023

bors commented Sep 11, 2023

epage left a comment

ehuss commented Sep 20, 2023

bors commented Sep 20, 2023

bors commented Sep 20, 2023

bors commented Sep 20, 2023

bors commented Sep 20, 2023

bors commented Sep 20, 2023

Add some enhancements to cargo clean #12638

Add some enhancements to cargo clean #12638

Conversation

ehuss commented Sep 7, 2023

What does this PR try to resolve?

How should we test and review this PR?

rustbot commented Sep 7, 2023

bors commented Sep 7, 2023

weihanglo left a comment

Choose a reason for hiding this comment

epage commented Sep 7, 2023

bors commented Sep 11, 2023

epage left a comment

Choose a reason for hiding this comment

ehuss commented Sep 20, 2023

bors commented Sep 20, 2023

bors commented Sep 20, 2023

bors commented Sep 20, 2023

bors commented Sep 20, 2023

bors commented Sep 20, 2023

Add some enhancements to `cargo clean` #12638

Add some enhancements to `cargo clean` #12638