Skip to content

Fix checkout methods that use notifyBlock/GTCheckoutStrategySafe #510

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Aug 27, 2015
Merged

Fix checkout methods that use notifyBlock/GTCheckoutStrategySafe #510

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0566639
Fix checkout methods utilizing notifyBlock/GTCheckoutStrategySafe
dleehr Aug 18, 2015
0e2ba10
Add tests for checkout+notify
dleehr Aug 19, 2015
3c17305
Cleanup checkout+notify tests
dleehr Aug 19, 2015
c1e58a6
Rework checkout methods to checkout the target treeish instead of head
dleehr Aug 19, 2015
8e6c27c
Rework checkout+notify tests to clarify expected behavior
dleehr Aug 19, 2015
5fa70c9
Convert nil check+return to single line
dleehr Aug 19, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fix checkout methods utilizing notifyBlock/GTCheckoutStrategySafe 
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.
  • Loading branch information
@dleehr
dleehr committed Aug 18, 2015
commit 05666399ce48bdde6c442a58df5e8ec4a9ac9197
10 changes: 4 additions & 6 deletions ObjectiveGit/GTRepository.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,17 +830,15 @@ - (BOOL)performCheckoutWithStrategy:(GTCheckoutStrategyType)strategy notifyFlags
}

- (BOOL)checkoutCommit:(GTCommit *)targetCommit strategy:(GTCheckoutStrategyType)strategy notifyFlags:(GTCheckoutNotifyFlags)notifyFlags error:(NSError **)error progressBlock:(GTCheckoutProgressBlock)progressBlock notifyBlock:(GTCheckoutNotifyBlock)notifyBlock {
BOOL success = [self moveHEADToCommit:targetCommit error:error];
BOOL success = [self performCheckoutWithStrategy:strategy notifyFlags:notifyFlags error:error progressBlock:progressBlock notifyBlock:notifyBlock];
if (success == NO) return NO;

return [self performCheckoutWithStrategy:strategy notifyFlags:notifyFlags error:error progressBlock:progressBlock notifyBlock:notifyBlock];
return [self moveHEADToCommit:targetCommit error:error];
}

- (BOOL)checkoutReference:(GTReference *)targetReference strategy:(GTCheckoutStrategyType)strategy notifyFlags:(GTCheckoutNotifyFlags)notifyFlags error:(NSError **)error progressBlock:(GTCheckoutProgressBlock)progressBlock notifyBlock:(GTCheckoutNotifyBlock)notifyBlock {
BOOL success = [self moveHEADToReference:targetReference error:error];
BOOL success = [self performCheckoutWithStrategy:strategy notifyFlags:notifyFlags error:error progressBlock:progressBlock notifyBlock:notifyBlock];
if (success == NO) return NO;

return [self performCheckoutWithStrategy:strategy notifyFlags:notifyFlags error:error progressBlock:progressBlock notifyBlock:notifyBlock];
return [self moveHEADToReference:targetReference error:error];
}

- (BOOL)checkoutCommit:(GTCommit *)target strategy:(GTCheckoutStrategyType)strategy error:(NSError **)error progressBlock:(GTCheckoutProgressBlock)progressBlock {
Expand Down