Skip to content
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

Wipe checkout directory on git checkout and git fetch failure and retry #2137

Merged
merged 9 commits into from
Jun 5, 2023

Conversation

triarius
Copy link
Contributor

@triarius triarius commented Jun 2, 2023

There are a certain class of errors from git in the defaultCheckoutPhase that would cause a retry, but before the retry, the checkout directory would be removed. This allows cleaning up in case an unclean exit from git had left the checkout directory in an unrecoverable state.

This PR adds failures in git fetch and git checkout when the exit code is 128 to that class of error. It also fixes the detection of errors for which the check directory needed to be clean, as previously, the detection would fail if the errors were wrapped, and they are wrapped in the defaultCheckoutPhase method.

I've done some refactoring to reduce the indentation level of the code, so I recommend reviewing with whitespace changes hidden.

@triarius triarius force-pushed the pdp-1070-clean-up-checkout-dir-when-git-fetch-fails branch 4 times, most recently from 6e3ca39 to 6bbd748 Compare June 3, 2023 03:55
@triarius triarius changed the title Wipe checkout directory on git fetch failure and retry Wipe checkout directory on git checkout and git fetch failure and retry Jun 3, 2023
@triarius triarius marked this pull request as ready for review June 4, 2023 22:36
@triarius triarius requested a review from a team June 4, 2023 22:36
if b.Config.Repository == "" {
b.shell.Commentf("Skipping checkout, BUILDKITE_REPO is empty")
break
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be on 1197. I've inverted the condition to decrease the indentation.

@triarius triarius force-pushed the pdp-1070-clean-up-checkout-dir-when-git-fetch-fails branch from 6bbd748 to 03edbe9 Compare June 5, 2023 02:29
Copy link
Contributor

@DrJosh9000 DrJosh9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this!

bootstrap/git.go Outdated Show resolved Hide resolved
bootstrap/git.go Outdated Show resolved Hide resolved
@@ -211,19 +213,19 @@ func (b *BootstrapTester) writeHookScript(m *bintest.Mock, name string, dir stri
body = "#!/bin/sh\n" + strings.Join(append([]string{m.Path}, args...), " ")
}

if err := os.MkdirAll(dir, 0700); err != nil {
if err := os.MkdirAll(dir, 0o700); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍👍

Co-authored-by: Josh Deprez <jd@buildkite.com>
@triarius triarius enabled auto-merge June 5, 2023 05:20
@triarius triarius merged commit a8b80af into main Jun 5, 2023
1 check passed
@triarius triarius deleted the pdp-1070-clean-up-checkout-dir-when-git-fetch-fails branch June 5, 2023 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants