fix(clean): Clean hosts builds with new layout #16300
Conversation
rustbot
added
Command-clean
S-waiting-on-review
|
CC @ranger-ross |
| let pkg_dir = format!("{}-*", pkg.name()); | ||
| clean_ctx.progress.on_cleaning_package(&pkg.name())?; | ||
|
|
||
| if clean_ctx.gctx.cli_unstable().build_dir_new_layout { |
There was a problem hiding this comment.
I duplicated clean.rs tests to verify -Zbuild-dir-new-layout. This uncovered several issues. I fixed most. There remains one which has test overlap with this so no test results changed. I decided to go ahead and get this update up rather than wait on all of them since this last one may be a bit nasty.
epage
force-pushed
the
clean
branch
2 times, most recently
from
12b9b45 to
0c3ed63
Compare
There was a problem hiding this comment.
Thanks!
Though I feel like the function is already complicated enough that we may want to separate old and new layout. Also, the extracted function should have comment telling that what it is going to clean. It is a bit annoying to trace the actual files getting removed.
Agreed, I am hoping the new layout will reduce the amount of code needed for clean. |
| let artifact_dir = layout | ||
| .artifact_dir() | ||
| .expect("artifact-dir was not locked during clean"); |
There was a problem hiding this comment.
I think we can't merge this until we un-revert #16299
There was a problem hiding this comment.
How come? Without that, artifact_dir() should always be Some
There was a problem hiding this comment.
Err nvm, I forgot we split that into 2 PRs
|
Depending on how short-lived it is, it is likely better to keep as one function, especially if #16264 moves forward as that will have a lot of shared code. |
rustbot
removed
the
S-waiting-on-review
### What does this PR try to resolve? A follow up to #16300. This doesn't split out any functions - The stuff before hand is quite significant - For #16264, we made add more before and some after - We could pull out only the branches of the `if` into functions but then we'd just move them back when this is done and seems like extra churn ### How to test and review this PR?
Update cargo submodule 14 commits in 2a7c4960677971f88458b0f8b461a866836dff59..bd979347d814dfe03bba124165dbce9554d0b4d8 2025-11-25 19:58:07 +0000 to 2025-12-02 16:03:50 +0000 - fix(completion): Put host-tuple before actual tuples (rust-lang/cargo#16327) - fix(lints): use plural form correctly (rust-lang/cargo#16324) - fix(completions): include `all` in `cargo tree --target` candidates (rust-lang/cargo#16322) - fix(lints): show lint error number (rust-lang/cargo#16320) - chore(deps): update compatible (rust-lang/cargo#16318) - chore(deps): update crate-ci/typos action to v1.40.0 (rust-lang/cargo#16316) - Do not lock the artifact-dir for check builds + fix uplifting (rust-lang/cargo#16307) - Properly validate crate names in `cargo install` (rust-lang/cargo#16314) - Support --filter-platform=host for cargo metadata rust-lang/cargo#9423 (rust-lang/cargo#16312) - Update to mdbook 0.5 (rust-lang/cargo#16292) - refactor(clean): Better divide old / new layout (rust-lang/cargo#16304) - update: silent failure on non-matching package specs with --breaking (rust-lang/cargo#16276) - fix(log): break timing-info message to multiple (rust-lang/cargo#16303) - fix(clean): Clean hosts builds with new layout (rust-lang/cargo#16300)
5 participants
What does this PR try to resolve?
When we have limited content do remove, we were cleaning in an inner loop and left off the host layout.
How to test and review this PR?
This is a follow up to #15947 that I noticed when looking at #16264.