11 ms·

> the merged commit can actually be literally anything That cannot be stressed enough. Years ago a searched for a weird bug and could not find it. Of course I

> the merged commit can actually be literally anything That cannot be stressed enough. Years ago a searched for a weird bug and could not find it. Of course I did not look at merge conflicts because "they don't really introduce anything new". But it turned out that a merge commit had introduced an arbitrary line, not related to its parents. Also the opposite is true: You can make a conflict free merge of two correct parents and the result is broken code. It does not seem to happen very often in real life, but it's a risk one should be aware of. I wonder how many merge commits there are in the Linux source tree that introduce something new by conflict resolution or break something existing without conflict resolution involved. At work we are a small enough organization that we don't really merge, all our merges are fast-forward. But that means you move the risks above to rebasing.

jahnu 3y ago
> You can make a conflict free merge of two correct parents and the result is broken code. It does not seem to happen very often in real life, but it's a risk one should be aware of. Correct and I would say even more than that; in a system more than minimal complexity you need to run your tests on your branch with main merged into it before you merge your branch into main. Too often CI is set up to only test HEAD of the new branch.
- layer8 3y ago
  Yes! It is driving me insane that this is not the default setup. If you do the merge of main into the branch yourself, there is a race condition where by the time the tests are completed and you want to merge the branch into main, main has changed again, so you have to repeat. Exactly because of that it would be nice if CI guaranteed a correct sequencing of merging and testing.
- IshKebab 3y ago
  You need to use a merge queue (aka merge train). It should be completely standard at this point (though it unfortunately isn't). Doesn't help that it's a Gitlab Premium feature.
  - usr1106 3y ago
    Ah, that's what a merge train is. I always found the term confusing. We pay for Premium, but we don't use merge trains. We use only fast forward merging, so I guess it's a no-op in that case.
    - IshKebab 3y ago
      It's not a nop. Merge queues solve the following issue: 1. Author 0 makes branch_0 and tests it in CI. It passes. 2. Author 1 makes branch_1 and tests it in CI. It passes. 3. They both merge their branches. This can break `master`, irrespective of the kind of merging you use. Fundamentally two people can make changes that are valid (pass CI) independently but not when combined. That kind of breakage gets more common the more different people you have working on a project. If you just have a handful of people it's relatively rare so you can just accept `master` breaking occasionally. If you have a few dozen people or more then it's pretty much a requirement. I don't think there are any serious downsides to enabling it.
      - usr1106 3y ago
        If you are limited to fast forward merging only one of your authors can merge. The second one needs to rebase and their updated MR will go through CI again, now on top of the first author's commit that has already been merged.
        mkbosmans 3y ago
        I think that is unique to Gitlab though. For example Azure DevOps will rebase the branch for you when completing a pull request.
        IshKebab 3y ago
        Ah I see what you mean. Yeah that is roughly equivalent to a manual merge queue. It's going to fall apart on medium/large teams because you'll constantly be rebasing and rerunning CI.
juped 3y ago
lots; the tongue-in-cheek name is "evil merge" but it's honestly pretty common precisely because, as you say, the mere textual merge could be a broken result. "conflict" is a bad name because it sounds like some kind of error; it's just a place where a simple algorithm can't make a judgment call so a human has to. and it's not the only place. the only reason an "evil merge" is "evil" is because the tooling breaks down a little around it (however! if you look at diffs for commits, nearly all tooling will show you the useful information here, e.g. in gitk it's color coded bold black)
usr1106 3y ago
Oops, that was a slip: > Of course I did not look at merge conflicts Should of course been "merge commits". Sorry, too late to edit.

jahnu 3y ago

> You can make a conflict free merge of two correct parents and the result is broken code. It does not seem to happen very often in real life, but it's a risk one should be aware of. Correct and I would say even more than that; in a system more than minimal complexity you need to run your tests on your branch with main merged into it before you merge your branch into main. Too often CI is set up to only test HEAD of the new branch.

layer8 3y ago
Yes! It is driving me insane that this is not the default setup. If you do the merge of main into the branch yourself, there is a race condition where by the time the tests are completed and you want to merge the branch into main, main has changed again, so you have to repeat. Exactly because of that it would be nice if CI guaranteed a correct sequencing of merging and testing.
IshKebab 3y ago
You need to use a merge queue (aka merge train). It should be completely standard at this point (though it unfortunately isn't). Doesn't help that it's a Gitlab Premium feature.
- usr1106 3y ago
  Ah, that's what a merge train is. I always found the term confusing. We pay for Premium, but we don't use merge trains. We use only fast forward merging, so I guess it's a no-op in that case.
  - IshKebab 3y ago
    It's not a nop. Merge queues solve the following issue: 1. Author 0 makes branch_0 and tests it in CI. It passes. 2. Author 1 makes branch_1 and tests it in CI. It passes. 3. They both merge their branches. This can break `master`, irrespective of the kind of merging you use. Fundamentally two people can make changes that are valid (pass CI) independently but not when combined. That kind of breakage gets more common the more different people you have working on a project. If you just have a handful of people it's relatively rare so you can just accept `master` breaking occasionally. If you have a few dozen people or more then it's pretty much a requirement. I don't think there are any serious downsides to enabling it.
    - usr1106 3y ago
      If you are limited to fast forward merging only one of your authors can merge. The second one needs to rebase and their updated MR will go through CI again, now on top of the first author's commit that has already been merged.
      - mkbosmans 3y ago
        I think that is unique to Gitlab though. For example Azure DevOps will rebase the branch for you when completing a pull request.
      - IshKebab 3y ago
        Ah I see what you mean. Yeah that is roughly equivalent to a manual merge queue. It's going to fall apart on medium/large teams because you'll constantly be rebasing and rerunning CI.

juped 3y ago

lots; the tongue-in-cheek name is "evil merge" but it's honestly pretty common precisely because, as you say, the mere textual merge could be a broken result. "conflict" is a bad name because it sounds like some kind of error; it's just a place where a simple algorithm can't make a judgment call so a human has to. and it's not the only place. the only reason an "evil merge" is "evil" is because the tooling breaks down a little around it (however! if you look at diffs for commits, nearly all tooling will show you the useful information here, e.g. in gitk it's color coded bold black)

usr1106 3y ago

Oops, that was a slip: > Of course I did not look at merge conflicts Should of course been "merge commits". Sorry, too late to edit.