Fix checkout methods that use notifyBlock/GTCheckoutStrategySafe #510
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
Currently, the performCheckout methods move HEAD to the new ref/commit before calling git_checkout_head. This is incorrect behavior. When git_checkout_head is called, it reports changes between the workdir and HEAD per-file to the notifyBlock, allowing the block to abort the checkout based on a reason (dirty/conflict/etc). However, since HEAD has already been moved to a different ref/commit, the notifyBlock is notified about changes between the current workdir and the requested ref/commit. This results in a dirty state for any file that differs between the old ref and the new ref, regardless of whether or not any change was made in the workdir. The correct behavior (e.g. with GTCheckoutStrategySafe) is to notify of changes that will cause data loss in the workdir. This change leaves HEAD in position until after the checkout, correcting the checkout behavior. When GTCheckoutStrategySafe is used, notifyBlock is only invoked as expected (e.g. dirty files that would be overwritten on checkout). Also, HEAD is only moved if the checkout succeeds, preventing other confusing behavior.
|
Nice work! 👾 |
phatblat
added a commit
that referenced
this pull request
Aug 27, 2015
Fix checkout methods that use notifyBlock/GTCheckoutStrategySafe
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.
Previously,
performCheckoutmethod moved HEAD to the target, then calledgit_checkout_head(). This does not match behavior in git core, and prevents thecheckoutStrategyandnotifyCallbackparameters from being used correctly. The only way to reliably checkout a branch or commit was to use GTCheckoutStrategyForce, which throws away conflicts. Also, the HEAD could be moved even if the checkout fails, resulting in a confused state.This PR corrects the checkout behavior with two changes:
git_checkout_tree()is used instead ofgit_checkout_head(). The tree that is checked out is the target object (commit or ref).Included tests demonstrate the correct behavior catching a conflicted file. The notifyBlock is called to report the conflict, and with GTCheckoutStrategySafe, the checkout fails because of the conflict as expected.