-
Notifications
You must be signed in to change notification settings - Fork 183
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
Add full clone fallback to sparse checkout #3661
Conversation
The following pipelines have been queued for testing: |
How do we detect that pipelines are falling back to full clone ? Is there any automated way to monitor that ? |
@@ -64,6 +95,9 @@ steps: | |||
Write-Host "git -c advice.detachedHead=false checkout $($repository.Commitish)" | |||
# This will use the default branch if repo.Commitish is empty | |||
git -c advice.detachedHead=false checkout $($repository.Commitish) | |||
if ($LASTEXITCODE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have any cases where people call sparse checkout multiple times and we fail on a try after the first and then revert back to a full clone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The step is idempotent so you should be able to add extra directories to an existing sparse checkout config, but the clone override is written as such that on any failure it will just pave over any existing repo files with the full clone.
We will be able to query for the warning message in our pipeline telemetry. |
I wanted to use our metric logging cmdlets here so we could track failures with a simpler query, but I would have to copy the code inline here, since we have the chicken/egg problem with external scripts in initial checkout. |
@weshaggard updated, but the diff is hard to read since I fixed some indentation. |
The following pipelines have been queued for testing: |
|
||
try { | ||
# Enable global override if there are sparse checkout issues | ||
if ('$(DisableSparseCheckout)' -ne 'true') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use the usual Skip naming here instead of disable? So SkipSparseCheckout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps both Disable
and Skip
aren't descriptive enough, as they only specify not doing something, as opposed to replacing sparse with a full clone.
Some other ideas:
- OverrideSparseCheckout
- UseFullClone
- CheckoutMode ==
sparse|full|clone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Override doesn't give any better details of what is actually happening. Maybe CheckoutMode but right now I think there is only 2 options I'm not sure what would be different between full and clone in your options there.
I don't have a strong feeling but I think Disable or Skip (slightly favoring skip for consistency) would still be the most descriptive of what we are trying to do which is to not run the sparse part of the checkout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah re: full vs. clone, I figured we would pick one or the other to mean "full clone." Let's go with skip for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One naming suggestion but otherwise looks good.
The following pipelines have been queued for testing: |
/check-enforcer override |
We sometimes have issues with the sparse checkout pipeline step. When it breaks, it breaks almost all pipelines. This change adds a failsafe to clone the full repository normally when sparse checkout fails.
For example, this recent failure highlighted the need for more resiliency with this core step:
#3606.