Improve handling of erroneous dirty library repo state after release checkout #42
Conversation
This file contains hidden or 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
The code for checking out tags was duplicated. Already fairly complex, it turns out to need even more code to work reliably. For this reason, it will be beneficial to move it to a function in the gitutils package to avoid code duplication and facilitate testing.
The engine's Git code is failing to get a clean checkout of releases under some circumstances. This resulted in the ssd1306@1.0.0 release being accepted even though the repository contains a .exe at that tag. However, the engine's checkout code fails to add the .exe when checking out this tag, leaving the repository in a dirty state that just so happens to allow it to pass validation. Although harmless in this particular situation, this could cause big problems if essential files went missing from releases, and so must be tested for.
Even though in theory it shouldn't ever be necessary to do a hard reset in this application, under certain circumstances, go-git can fail to complete checkout, while not even returning an error. The resulting dirty repository state would cause Library Manager to serve a corrupted release. A hard reset after each checkout ensures that the repository is at its tagged state.
Under certain circumstances, go-git can fail to complete checkout, while not even returning an error. While the clean and hard reset processes should ensure this is corrected, the fact that it is necessary casts doubt on the reliability of go-git. It's very important that the releases distributed by Library Manager match the repository, so it's better to reject any releases that can't be brought into a clean state.
It's not certain that there is any point in another try after a first unsuccessful cleaning attempt. However, go-git's checkout code also does a hard reset, but still leaves the repository dirty under certain circumstances. Running an identical hard reset procedure again gets the repository to a clean state, so this indicates a possibility that a retry might be successful.
silvanocerza
approved these changes
May 28, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Even though in theory it shouldn't ever be necessary to do a hard reset in this application, under certain circumstances, go-git can fail to complete checkout, while not even returning an error (go-git/go-git#99). The resulting dirty repository state would cause Library Manager to serve a corrupted release. A hard reset after each checkout ensures that the repository is at its tagged state.
The previous checkout behavior resulted in the ssd1306@1.0.0 release being indexed, even though the repository contains a
.exefile at that tag:https://github.com/lexus2k/ssd1306/tree/v1.0.0/tools
By chance, this incomplete checkout removed the
.exefile from the release, which allowed it to pass validation. Although harmless in this particular case, incomplete checkouts could cause big problems if essential files went missing from releases, and so must be avoided, by recovering when possible, and skipping the release when not possible.