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.

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

merge: add GIT_MERGE_TREE_FAIL_ON_CONFLICT #3482

Merged
merged 1 commit into from Oct 23, 2015

Conversation

Projects
None yet
3 participants
@ethomson
Copy link
Member

ethomson commented Oct 22, 2015

Provide a new merge option, GIT_MERGE_TREE_FAIL_ON_CONFLICT, which
will stop on the first conflict and fail the merge operation with
GIT_EMERGECONFLICT.

@ethomson

This comment has been minimized.

Copy link
Member

ethomson commented Oct 22, 2015

I had initially implemented this as an integer count - if we hit x conflicts, then we would fail. But that seemed really dumb and everybody would either set this to 0 or 1, so I settled on this instead. Let me know if you disagree.

/cc @vmg

@@ -49,6 +49,7 @@ typedef enum {
GIT_EINVALID = -21, /**< Invalid operation or input */
GIT_EUNCOMMITTED = -22, /**< Uncommitted changes in index prevented operation */
GIT_EDIRECTORY = -23, /**< The operation is not valid for a directory */
GIT_EMERGECONFLICT = -24, /**< A merge conflict exists and cannot continue */

This comment has been minimized.

@carlosmn

carlosmn Oct 22, 2015

Member

Odd alignment, did you use a tab instead of spaces here?

This comment has been minimized.

@ethomson

ethomson Oct 22, 2015

Member

Yes, thanks; fixed.

* continue resolving conflicts. The merge operation will fail with
* GIT_EMERGECONFLICT.
*/
GIT_MERGE_TREE_FAIL_ON_CONFLICT = (1 << 1),

This comment has been minimized.

@carlosmn

carlosmn Oct 22, 2015

Member

We should probably specify what happens to the resulting index (though presumably we don't do anything since we return an error, but it's good to be explicit).

This comment has been minimized.

@ethomson

ethomson Oct 22, 2015

Member

Updated this to be explicit.

merge: add GIT_MERGE_TREE_FAIL_ON_CONFLICT
Provide a new merge option, GIT_MERGE_TREE_FAIL_ON_CONFLICT, which
will stop on the first conflict and fail the merge operation with
GIT_EMERGECONFLICT.

@ethomson ethomson force-pushed the ethomson:merge_fail_on_conflict branch from 16d4aeb to 8683d31 Oct 22, 2015

@@ -40,7 +40,7 @@ int merge_trees_from_branches(
cl_git_pass(git_commit_tree(&our_tree, our_commit));
cl_git_pass(git_commit_tree(&their_tree, their_commit));

cl_git_pass(git_merge_trees(index, repo, ancestor_tree, our_tree, their_tree, opts));
error = git_merge_trees(index, repo, ancestor_tree, our_tree, their_tree, opts);

This comment has been minimized.

@carlosmn

carlosmn Oct 22, 2015

Member

Would this change (and the equivalent below) make the other users of merge_trees_from_branches() and _commits_ to stop making sure that the merge succeeded? Other users should be changed to wrap their call in cl_git_pass() since we don't anymore, no?

This comment has been minimized.

@ethomson

ethomson Oct 22, 2015

Member

Yes, sorry that's subtle. Other users had already been asserting success here (wrapping their calls in cl_git_pass). That was sort of silly as those calls could never fail, but there it is.

This comment has been minimized.

@carlosmn

carlosmn Oct 22, 2015

Member

Ok, just wanted to make sure this ramification had been considered.

@vmg

This comment has been minimized.

Copy link
Member

vmg commented Oct 23, 2015

You're an 👼 @ethomson

vmg added a commit that referenced this pull request Oct 23, 2015

Merge pull request #3482 from ethomson/merge_fail_on_conflict
merge: add GIT_MERGE_TREE_FAIL_ON_CONFLICT

@vmg vmg merged commit 7c0c21c into libgit2:master Oct 23, 2015

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment