Merge Conflict Tool merges source and target branches in reverse

Changed title: Merge Conflict Tool source and target branches reversed → Merge Conflict Tool merges source and target branches in reverse

I'm pretty sure this is intentional. The tool isn't meant to fix merge conflicts when merging the MR, but to fix merge conflicts so that the branch can be worked on further. What you're asking for is support for rebasing, whereas we currently use a merge strategy for conflict resolution.

Thank you for the comprehensive issue by the way, very helpful when diagnosing issues

@connorshea I hope this current behavior is not intentional.

When I tried to merge branch_b into master, but the end result is master is merged into branch_b. This caused some issues in my project where I merge several features into a temporary branch for testing, those feature branches now include all branches that were merged into that temporary branch.

When you encounter a conflict, you are given a choice: Resolve in gitlab (new) or resolve manually. I would expect both of these options to do the same thing (merge source into target), but instead they do opposite things and that is not made clear.

https://about.gitlab.com/2016/09/06/resolving-merge-conflicts-from-the-gitlab-ui/ This issue is shown in the blog post showing it off near "Here's an example conflict in a schema.rb" (note the "master into expiration-date-on-memberships" message below the conflict of his attempt to merge that branch into master) however this blog post doesn't continue to show what is supposed to happen after you click "Commit conflict resolution"

Hm, I guess you may be right... after closely reading the "How does that work?" section, it is indeed doing that I hoped it would not do. That's unfortunate :( I wish this was made more clear before I encouraged developers to try the tool expecting it would work the way we manually resolve conflicts.

Also not asking for rebasing support here (although that would be AWESOME) I was just expecting this would behave more like

git checkout target && git merge --no-ff source

@devdaniel Leaving this as I still think it's relevant, but rebasing isn't what you're suggesting so feel free to disregard this comment:

I personally avoid using the merge conflict resolution UI for this reason, as I prefer to rebase rather than merge to keep the history clean. It may be useful to support either strategy, the problem being that rebasing requires force pushing to the branch which may override another developer's commits if you were resolving conflicts when they pushed a new commit.

@devdaniel I've never merged using the --no-ff flag, I've always resolved conflicts using a rebase, so I never really thought about this.

I assume there's a reason for not using this? @smcgivern @DouweM @fatihacet

Right, I wish there was a way to disable the tool for now. I've had to do a lot of rebasing after developers were using this tool to resolve conflicts for merges to QA branches and then some of those feature branches were merged to master. We ended up with things on master we did not intend. Worst part is that the reverse merge happens behind the scenes, you don't get to see the end result until after it's already done.

Unless you read this blog post carefully, this UI is very misleading.

@devdaniel thanks for reporting!

The idea behind this is that the author can resolve the conflicts in the UI. The author will most likely (although not always) have access to the source branch, but is much less likely to have access to the target branch - and this is the case for most MRs to the GitLab CE!

As @connorshea says, this isn't necessarily meant to be the same as accepting the merge request - although I can see how it would be useful - it's just to stop the MR being in an umergeable state. But this is definitely inconsistent with the 'merge manually' link: could you add that point to https://gitlab.com/gitlab-org/gitlab-ce/issues/21135 please?

I think your workflow sounds like one for a company I used to work for. Does it go something like this?

Developers branch from a master branch.
They create MRs from their feature branches to a testing branch (let's say integration, for the sake of argument).
Once the branch is merged to integration, it's available for testing somewhere with the other features currently on integration.
Once it's been tested there, it can have an MR from the feature branch to master.

So at step 2, if people are resolving conflicts, they're pulling in a bunch of other changes (everything else in integration) which they don't necessarily want to do.

I don't think we're going to provide an option to disable this, or prevent people from merging into the source branch. What we could do, if the user has access to merge to the target branch, is provide a more prominent button to accept the merge request and resolve the conflicts at the same time (assuming MR approvals, build status, and other merge blockers aren't in place). The feature would work the same, the branches would just be switched. Making sure these two options are clearly defined could be tricky, but it sounds reasonable - and avoids the 'double merge' when you just want to resolve and accept the MR in one step.

/cc @JobV @DouweM

Added ~480950 ~13921 labels

mentioned in issue #21135 (closed)

@smcgivern Thanks for your reply... and you are right, that workflow is exactly how we operate and that issue is what we encountered. Unfortunately, we didn't see the actual commit when resolving the conflicts. GitLab seems to just do it in the back-end and then the front-end just shows the source branch was updated and you need to refresh the page, which then shows the "accept merge" button again. As a result we didn't catch what was going on right away and stuff leaked to master we didn't want. We were working on a long-term feature at the time and had to rebase about a month of feature branches on several repositories

What we could do, if the user has access to merge to the target branch, is provide a more prominent button to accept the merge request and resolve the conflicts at the same time

This is what I am really looking for, and we thought this is what was going to happen when we saw we could resolve conflicts in gitlab directly.

@devdaniel I think that's reasonable. I'd love to make both options possible, if we can explain them clearly enough. The double merge thing is - even ignoring the problems you've run into - much less elegant than a single step process.

As a result we didn't catch what was going on right away and stuff leaked to master we didn't want. We were working on a long-term feature at the time and had to rebase about a month of feature branches on several repositories

That sucks, I'm sorry this feature ended up causing you a bunch of hassle. Hopefully we can figure something out

Another user just ran into similar problems because of this behavior. They also asked if the feature can be disabled or if changes to the source branch can be shown explicitly. https://gitlab.zendesk.com/agent/tickets/38008

Added ~113220 label

@smcgivern @connorshea Just trying to understand here so bear with me

You both say the tool isn't meant to fix merge conflicts when merging the MR, but to fix merge conflicts so that the branch can be worked on further.

Why is that? Why is it not mean to fix merge conflicts when merging the MR? Sorry if I missed this bit in the above conversation.

@tauriedavis the idea is that you would fix the merge conflicts and merge the branch separately.

Let's say I'm working on Feature A and stop for a week due to the company prioritizing Feature B. I come back to Feature A one week later and it has conflicts due to the changes that have been made since I last touched it. I want to resolve the conflicts without merging since Feature A is still incomplete. That's why these are separate actions.

I misspoke when I said it's not meant to be used with a merge. What I should have said is that it can be used to resolve conflicts right before a merge, but that's not what it's exclusively meant to do. Hence the current behavior.

No need to apologize for asking questions, especially with git being the confusing mess it can be :)

@tauriedavis we also lose the ability to 'merge if build succeeds' if we merge into the target branch directly. Builds will run on the target branch, but you could create a broken target branch quite easily.

Thanks! This is making more sense. It sounds like if the user has access to merge to the target branch, then we could provide the option to switch target/source. I'll spend some time looking into how we can make this distinction more clear.

Reassigned to @tauriedavis

we also lose the ability to 'merge if build succeeds' if we merge into the target branch directly

Just thinking about this a bit more - this doesn't necessarily have to be the case, but it could be confusing. We could:

Keep the ref to the current source branch head (we do this anyway).
Reset the source branch to match the target branch.
Merge the ref from 1 into the new source branch.
Set 'merge when build succeeds' to true (if enabled).

But that's a lot of steps where something can go wrong. This also needs the user doing the resolution to have access to both the source branch and the target branch. The last merge at step 4 will also be kind of pointless, but at least it would be consistent.

Mentioned in issue #22903 (closed)

I have not had time to look into this. Going to unassign in case someone else has time to pick it up or it is scheduled.

Assignee removed

I also have the same issue with my team when there's merge conflict when merging a feature branch into the testing branch, which would end up bloating the feature branch with all unrelated changes that have been merged into the testing branch earlier. Worse thing is when the feature branch is merged into the production branch, bringing with it all changes that are not intended to deploy yet.

So, +1 to this issue report.

Added ~874211 and removed ~480950 labels

Could we create a “limbo” state where the user can resolve conflicts through the merge conflict editor without merging the target branch into the source branch? I know this doesn't work like this using git locally, but maybe we could create this interface in GitLab. The target branch changes wouldn't be merged but simply viewed in the merge conflict editor.

@pedroms so you mean create the resolutions, but don't commit them? It is possible, but it's potentially very fiddly, and I'm not sure how that would help. What would you want to use that for?

@smcgivern in my mind, you would create the resolutions and commit them to the source branch. What would be different from today is that you wouldn't be forced to merge the target branch into the source branch to create those resolutions. You would be able to see the changes from the target branch in the merge conflict editor but these changes wouldn't actually be merged. That's why I was calling it a “limbo” state.

@pedroms you need to merge the other branch in order to create the resolutions, though Otherwise you will still get a conflict as both branches touched the same line. You might also end up with the code in an inconsistent state, because your resolution might call a method that's in the other branch but not the source branch, which we definitely want to avoid.

@smcgivern Oh, I understand… I think the issue here is mainly about users not knowing that resolving conflicts in GitLab merges the target branch into the source branch — which is something you might not want. The only indication that this will happen is in the automatic commit message (“master into branch_b”). I'm not sure how we can make that more apparent… @tauriedavis suggestions?

Added ~19492 label

Mentioned in issue #24298 (closed)

Mentioned in issue #24857 (moved)

requested by customer: https://gitlab.my.salesforce.com/0016100000W4Jpd

mentioned in issue #26123 (closed)

mentioned in issue #27309 (closed)

Another customer request: https://gitlab.zendesk.com/agent/tickets/57916

mentioned in issue #27624 (closed)

I think one issue with this is that the button "resolve conflict" is located in the area where the "merge" button is if there is no conflict. That both buttons are in the same area might increase the probability of someone thinking "it's going to resolve AND then merge" similar to what would happen if you'd issue a git merge foo on the CLI.

mentioned in issue #25424 (closed)

Yeah, good thought @michaelsauter. I could see that increasing confusion. I've added that as something to think about while we work on the issue for iterating the merge widget https://gitlab.com/gitlab-org/gitlab-ce/issues/25424

mentioned in issue #30798 (closed)

To be honest I can't really say if this is confusing, because I could see both ways clearly, but I certainly got confused in #30798 (closed) (argh, silly me)

Before we could make both ways work, I am unsure if we could make this more clear.

I feel that this could be based on #4176 (closed), and it could be similar to what @pedroms said in https://gitlab.com/gitlab-org/gitlab-ce/issues/21947#note_17426982 regarding the limbo state.

So how it would look like would be that we're introducing a "preview" merge, and we resolve the conflicts there. Then we merge that "preview" branch instead of the original source branch if desired.

It could be like we have a "preview merge" button, and if it's a clean merge then it would be like #4176 (closed), otherwise we resolve the conflicts there with web UI. Buttons might be:

Preview merge (always shows, and resolve conflicts if there's a conflict)
Merge preview (only if a preview merge was generated)
Merge SOURCE BRANCH (only if it's clean)
Resolve conflicts in SOURCE BRANCH (only if there's a conflict)

And replace SOURCE BRANCH with the real branch name here. Be more explicit about what we're going to merge.

Would this be less confusing?

And maybe have a checkbox for "merge only if pipeline passed" so that we avoid the combination of action buttons.

mentioned in issue #34496 (closed)

mentioned in issue #33572 (closed)

@pedroms I hope that the target branch does not merge into the source branch, please adopt our advice and provide the next solution

Totally agree on the confusing default behavior. The best would be to be able to chose what to do after resolving the merge conflict.

Possibility to check "Merge source into destination" and "Merge destination into source".

Hope changes will come soon because I like the idea of merge requests but they are currently not usable in our workflow.

We use the "Hotfix branches" workflow (http://nvie.com/posts/a-successful-git-branching-model/#hotfix-branches), the hotfix branch need to merge to "master" branch and "develop" branch.

When we merge the hotfix to develop is conflicted, resolve conflicts on the web ui editor, the "develop" merge back to "hotfix".

And then the "hotfix" is merge to "master", let to the "develop" branch is indirectly merge to "master", the whole workflow is broken!

Could I disable the "Resolve Conflicts" Button?
Let me chose the branch merge direction

Zendesk ticket: https://gitlab.zendesk.com/agent/tickets/82955

@smcgivern @victorwu can you take a look at this one again? A customer is having the problem with two long-standing branches that shouldn't be merged into each other:

We have a requirement on my team to keep two distinct branches, occasionally cherry-picking a commit from the source branch to merge into the target branch. To prepare for this, we did one last merge from the source branch into the target branch, where the target branch now has many differences. The merge went well from source to target, adding the expected one thing, but then Gitlab automatically merged the target branch into the source! Being a legacy branch the majority of our customers use, this broke a lot of things for us until we were able to revert. How might we avoid this default behavior in the future? There was no warning or anything

@harishsr I'm not clear from the ticket about how this happened automatically? It sounds like they used the merge conflict editor, which does say that it will merge the target branch into the source branch.

As I've said above, if we did this the other way around, we'd skip a bunch of controls:

Merge when pipeline succeeds wouldn't work, because the pipeline would be on the target branch, not the source branch, so your merge may introduce a job failure.
Similarly, approvals would be affected, because no-one will have approved the merged code.
If you have git hooks running on branches, they would run on the already-merged branch.
Probably some others I've forgotten.

tl;dr: I think the discussion above is pretty clear on why it works this way. I'm happy to entertain the option of doing it the other way, but I can't think of any proposal (and I haven't seen any) that works consistently with the other tools we have around MRs.

For those who are waiting for the decision on this issue, please take a look and support my feature request for interim solution by disabling the resolve conflict operation via UI: #24857 (moved)

+1

I understand everything written above and now I know the behavior is even documented, but it is very far from intuitive and, in my opinion, it easily leads to mistakes. I fell for this one yesterday and only realized it 24h later, making my life even harder. Needs to be fixed.

I've also +1'd #24857 (moved).

Fun fact: I don't mean to be rude or anything like that, but I've found this issue by searching "gitlab ui conflict f**k up". It was one of the top links.

I think the workflow is correct but confuses too many people. In spite of my training efforts, both developers and protected branch masters get this muddled up. Unfortunately, as much as I am a control freak I can't solve the problem by being the only master! I look to gitlab to enforce as much of the workflow as possible.

What I want my team to do is always, always deal with merge conflicts in topic branches, get a clean commit ready, then create a merge request from topic to release branch. In the worst case that they still have merge conflicts, gitlab's merge support helps the team to resolve them in the topic branch before proceeding to merge.

All would be well...

However, my team gets muddled up when merging bugfixes from older to newer releases. They set MR from e.g release 1.1 to release 1.2, resolve conflicts, then getting the changes going in the "wrong" direction -- from 1.2 to 1.1. They then overreact by avoiding the merge conflict tool as being weird in some way. Well, I don't think we need to avoid or disable the merge conflict tool altogether.

What we really want is to protect stable branches from these accidental "backward" merges.

My suggestion would be to split up the protected branch setting "Allowed to merge" into two settings: "Allow to merge from" and "Allowed to merge to". I'd set "Allow to merge from" to "no one". That way, developers and masters both have no choice: gitlab wouldn't let them pick that protected branch as a merge source -- they'd need a topic branch first. Combined with "Allowed to push" set to "no one", protected branches would only be modified by being the destination of Gitlab merge requests. That'd be absolutely spot on.

Thoughts?

I'm tempted to create a feature request for this. I'd greatly prefer it to disabling the merge conflict tool altogether.

@rplatt that's an interesting idea, thanks! This would only apply to conflict resolution, though, right? The 'merge' in protected branches settings typically refers to a merge request.

Please do create a separate feature request for that.

@smcgivern I was just writing up the feature request, and got to thinking about alternatives.

I really like the way the editor protects branches from push. Automatically proposing to branch with something like "patch-1", and offering to create the merge request, is really neat. I was thinking how it'd be cool to have something similar for conflict resolution. But then I realized an opposite problem...

Let's say during a merge request someone uses the editor; a few minor changes were necessary after conflict resolution due to changes in an API. Those changes don't belong in the older source branch, which uses the old API, any more than the merge does! If the source branch is meant to be left untouched by the merge request, then all edits and conflict resolution need to be on a temporary branch.

So...

Would it be a more complete solution, if the merge request workflow created a temporary working branch where appropriate? Like the editor does for protection against pushes, but at the level of the merge request?

Most powerful would be to create a working branch on first edit/conflict resolution, but to also have a nice button on the merge request view to create a working branch to push from local repositories. Further edits, and the merge, use the working branch in place of the source branch.

This wouldn't be limited to protected branches either, so would keep things easier and solve even more of the cases listed in this issue's comments. It's also a bit friendlier, as it doesn't block the use of the conflict resolution ui for any merges. The downside, I'm guessing it's more work!

Sorry if that was long-winded! What do you think of this versus my previous suggestion? I'll create the feature request(s) accordingly :)

@rplatt that's interesting, and is slightly related to my comment above: https://gitlab.com/gitlab-org/gitlab-ce/issues/21947#note_16342778

We don't want an MR to become disconnected from the branch you can access through other git clients, though. The only time we can do that kind of juggling is on merging it, I think. If someone wants to have more control over the way their commits are applied, they can always do that manually, after all.

@smcgivern that's true. What do you think about the reasoning, that enforcing a correct branch is a thing for the whole merge request though, not just the conflict resolution? So that all edits and changes as part of the merge request are done in the right place? I'd suggest that a new option to protect branches would actually prevent even allowing the protected branch to be a source of a merge request, rather than let someone create the merge request only to find that it must be closed due to disallowing conflict resolution?

mentioned in issue #40712 (closed)

@rplatt I think a 'correct branch' is entirely dependent on your particular workflow.

I think protected a branch from being the source of an MR, or from being used as the 'theirs' side in conflict resolution, is pretty consistent, though.

You might also be interested in https://gitlab.com/gitlab-org/gitlab-ce/issues/40712.

This was just added to GitHub and is somewhat related:

Source

Merge Conflict Tool merges source and target branches in reverse

Summary

Steps to reproduce

Expected behavior

Actual behavior

Relevant logs and/or screenshots

Designs

Child items ...

Activity