git: do not validate submodules of fresh checkouts #14605
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
rustbot
added
A-git
src/cargo/sources/git/utils.rs
Outdated
| Some(co) => co, | ||
| None => GitCheckout::clone_into(dest, self, rev, gctx)?, | ||
| None => { | ||
| let checkout = GitCheckout::clone_into(dest, self, rev, gctx)?; | ||
| checkout.update_submodules(gctx)?; | ||
| checkout | ||
| } | ||
| }; | ||
| checkout.update_submodules(gctx)?; | ||
|
|
There was a problem hiding this comment.
is_fresh is determined by the presence of the CHECKOUT_READY_LOCK file. That file is written in reset which is called at the end of clone_into. As-written in this PR, if the submodule update fails or gets interrupted, we won't be able to detect that and recover on the next invocation. We need to make sure that CHECKOUT_READY_LOCK is not written to until after the submodule update to make sure we can recover gracefully.
|
We likely should update |
| } | ||
|
|
||
| impl CheckoutGuard { | ||
| #[must_use] |
There was a problem hiding this comment.
Should this be put on the struct itself?
|
Could you update the commits to be how you want them reviewed and merged? For example, I could see this being
|
Fixes #14603
What does this PR try to resolve?
As is, we unconditionally validate freshness of the submodules of a checkout, even though we could assume that a fresh checkout has to have up-to-date submodules as well.
How should we test and review this PR?
N/A
Additional information
N/A