8000 Fix mergeBranchIntoCurrentBranch: for conflicted state conditions by slavikus · Pull Request #604 · libgit2/objective-git · GitHub
[go: up one dir, main page]

Skip to content

Fix mergeBranchIntoCurrentBranch: for conflicted state conditions #604

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

Closed
wants to merge 3 commits into from

Conversation

slavikus
Copy link
Contributor

checkout_opts passed to libgit2 missed a GIT_CHECKOUT_SAFE strategy, causing a dry run and only conflicted files being written out to the tree (losing all other non-conflicted ones).

This is because libgit2 has a safeguard check in git_merge call, auto-adding GIT_CHECKOUT_SAFE when no checkout_opts where specified, but since the Objective-Git code specified GIT_CHECKOUT_ALLOW_CONFLICTS flag, it wasn't applied, causing the behaviour outlined above:

checkout_strategy = given_checkout_opts ? given_checkout_opts->checkout_strategy : GIT_CHECKOUT_SAFE;

GTRepositoryStashApplyProgressGheckoutUntracked -> GTRepositoryStashApplyProgressСheckoutUntracked
checkout_opts passed to libgit2 missed a GIT_CHECKOUT_SAFE strategy, causing a dry run and only conflicted files being written out to the tree (losing all other non-conflicted ones).
@slavikus
Copy link
Contributor Author

Also added contents of PR #602 as it wasn't accepted earlier, sorry.

@slavikus
Copy link
Contributor Author

Looks like travis has a wrong version of libssh installed, so it fails for macOS builds :(

@slavikus
Copy link
Contributor Author

@tiennou you're best with these, would you please spend a few moments of your precious time on this one? :)

@tiennou
Copy link
Contributor
tiennou commented Dec 18, 2016

Would you be so kind to rebase that branch onto master ? I'm not fond of twisty histories 😉.

Apart from that, LGTM. I have a few concerns on that merge code though, but that's not on you.

@tiennou tiennou self-assigned this Dec 18, 2016
@slavikus
Copy link
Contributor Author

@tiennou, will do today. Should I close this pull request and make a new one instead?

As for the merge code itself, yep, it could use some improvement (like finding the ancestor at the very least), but I decided to go baby steps.

@tiennou
Copy link
Contributor
tiennou commented Dec 18, 2016

If that's easier for you, create a new one. Personally, I would just force-push the rebased branch to fix-merge-branch, but that might not work for you.

@slavikus
Copy link
Contributor Author
< 7E00 table class="d-block user-select-contain" data-paste-markdown-skip>

Created PR #605 , closing this one.

@slavikus slavikus closed this Dec 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0