This repository has been archived by the owner on Sep 9, 2020. It is now read-only.
gps: Adaptively clean dirty git repos #1279
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Oct 17, 2017
sdboyer
force-pushed
the
git-autorecover
branch
from
90abc13 to
7e96bb2
Compare
|
ugh. windows failures. like, actual ones on the new tests. oh frabjous day! |
sdboyer
force-pushed
the
git-autorecover
branch
from
e441dd7 to
4043ee9
Compare
|
Results of some testing I could do on Windows: Windows Version: Windows 10 Fall creators update (Build 1709) Go version: 1.8.5 and 1.9.2 (Tested on both) Go Env variables: Default. Changed no environment variable explicitly. Result: The test case passed for both the git-autorecover branch and the 7e96bb2 commit for both Go 1.8 and 1.9 |
|
so yeah, i don't know why that error occurred on appveyor earlier (and i can't reproduce because the parallelism is killing it - i have to get rid of that tonight) but this appears to work on windows. which is tony-the-tiger-level grrrrrrreat news |
Instead of forcing the user to clean up dirty git repositories, we can take at least basic steps to doing it ourselves - or, if we detect problems, instructing the user to fix it. The overhead introduced here is a `git status` call, which will be non-negligible on larger repos, but it's probably worth it for the resilient behavior.
This covers three basic cases - untracked files, modified files, and some corruption in the .git directory. The first two are plausible; the third is less so, as we don't know much about what real git failure patterns could look like in the .git directory itself. However, it's an adequate test inasmuch as it triggers failure in the basic git calls we make, thereby triggering the recovery procedure.
sdboyer
force-pushed
the
git-autorecover
branch
from
4043ee9 to
123ba6b
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this do / why do we need it?
Instead of forcing the user to clean up dirty git repositories, we can
take at least basic steps to doing it ourselves - or, if we detect
problems, instructing the user to fix it.
The overhead introduced here is a
git statuscall, which will benon-negligible on larger repos, but it's probably worth it for the
resilient behavior.
Eventually, once we get to #534, we'll want to issue a warning when we undertake a cleanup.
This needs tests. Also, let's delay merging this until after the next release so that it can percolate on tip for a while.
Might want to also refactor
maybeGitSource.try()to do this check earlier, andos.RemoveAll()then try again if it doesn't work.What should your reviewer look out for in this PR?
Possible failure modes. It'd be great if this attempt at making things better didn't inadvertently make things worse!
Do you need help or clarification on anything?
Which issue(s) does this PR fix?
fixes #1016
fixes #618