libgit2 / libgit2sharp

Git + .NET = ❤
http://libgit2.github.com
MIT License
3.18k stars 887 forks source link

Add a FileConflictStrategy property to CheckoutOptions #868

Open nulltoken opened 9 years ago

nulltoken commented 9 years ago

CherryPickOptions, MergeOptions, RevertOptions already expose such property.

Let's also make this option available when performing a simple checkout

Related: https://stackoverflow.com/questions/26878695/what-is-the-libgit2sharp-equivalent-of-git-checkout-ours

sitereactor commented 9 years ago

I've tried having a go at this, but can't seem to get around LibGit2Sharp.MergeConflictException in my tests. I'm hoping someone might be able to give me a bit of guidance.

First, my assumption of how the FileConflictStrategy would be used in CheckoutOptions (related to the StackOverflow post linked above) outlined in a simple scenario: You have two branches with the same file, which, when merged, results in a merge conflict. So the index is in a "merging state" and you want to resolve it by going through the conflicts and picking CheckoutFileConflictStrategy.Ours or CheckoutFileConflictStrategy.Theirs for each of them using CheckoutPaths(string committishOrBranchSpec, IEnumerable<string> paths, CheckoutOptions checkoutOptions) and then Stage and Commit. I'd expect the CheckoutPaths call to be something like this repo.CheckoutPaths(branch.CanonicalName, new[] { conflict.Theirs.Path }, new CheckoutOptions() { FileConflictStrategy = CheckoutFileConflictStrategy.Theirs });

In CheckoutOptions I have used a IConvertableToGitCheckoutOpts.CheckoutStrategy similar to the one in MergeOptions. But I always end up with a LibGit2Sharp.MergeConflictException - even when returning CheckoutStrategy.GIT_CHECKOUT_USE_THEIRS | CheckoutStrategy.GIT_CHECKOUT_ALLOW_CONFLICTS as the CheckoutStrategy or CheckoutStrategy.GIT_CHECKOUT_SKIP_UNMERGED | CheckoutStrategy.GIT_CHECKOUT_USE_THEIRS.

So in short, I'm not sure how to deal with the state of the repository when checking out files.

nulltoken commented 9 years ago

@sitereactor Hmmm. Could you please open a PR with your current code? It might be easier to help :wink:

sitereactor commented 9 years ago

I did some more research and figured I should probably be using GIT_CHECKOUT_FORCE, GIT_CHECKOUT_SKIP_UNMERGED and GIT_CHECKOUT_USE_THEIRS. It works as expected now, so gonna polish the tests a bit and summit a PR.

sushihangover commented 9 years ago

Seeing that the attached PR is not merged and going back to the original question on StackoverFlow which boils down to how you would do a:

Since this only applies when you have current unmerged paths (conflicts) while the repo has an active merge state, would it make sense to add a checkout extension to Conflicts.[Ours|Theirs], i.e:

repo.Index.Conflicts[0].Ours.Checkout();
repo.Index.Conflicts[1].Theirs.Checkout();

or a new variant of CheckoutPaths that accepts a Conflict Ours/Theirs IndexEntry:

foreach (var conflict in repo.Index.Conflicts) {
   repo.CheckoutPaths(conflict.Theirs)
}
jaccarmac commented 6 years ago

I'd love to see the ability to do checkouts leaving merge conflicts. What's necessary to finish this PR? (Or add support for the squash option on merge.) I'll gladly chip in with a little direction.

ethomson commented 6 years ago

To clarify: this isn't leaving merge conflicts (we do that now). This is taking one explicit side (ours / theirs) during checkout.

If you could take a look and perform a code review on that PR #940 would be good. I'll try to take a look soon as well.

jaccarmac commented 6 years ago

Understood. Will look at that PR, however I am also talking specifically about implementing the equivalent to git checkout -m, which is what I thought this issue targeted. Currently checking out in LibGit2Sharp's API can only act like checkout alone or checkout -f which discards "our" changes.

ethomson commented 6 years ago

I see - this is actually not something that libgit2 supports at the moment, would you mind opening an issue over there?

jaccarmac commented 6 years ago

I will. How should I phrase it given GitCheckoutOpts, which is what CheckoutTree ultimately passes to the proxy method, has a GIT_CHECKOUT_CONFLICT_STYLE_MERGE variant?

ethomson commented 6 years ago

We would want a bit on the strategy, something like GIT_CHECKOUT_MERGE that indicates that we would actually do a merge between the checkout target and the worktree contents.

This is not orthogonal to GIT_CHECKOUT_CONFLICT_STYLE_MERGE, which overrides the merge.conflictStyle configuration option. You could in fact use both, which would indicate that you want git checkout -m emulation and to not use diff3 format on the conflicts that result.

jaccarmac commented 6 years ago

I see. So the strategy bit is the change that needs to be applied on libgit2, and then both that and the existing conflict style bit need to be added to the LibGit2Sharp API?

ethomson commented 6 years ago

Yes to the first part. The existing conflict style bit doesn't really need to be added, it's quite a specialized use case. (Respecting the defaults is probably 👍 in the general case.)

jaccarmac commented 6 years ago

All right, thanks for the direction. I guess there's not a place for a new issue here until the merge-support is in the C library. I'll still review the existing code and apologies for the sidetrack.