Please explain why this merge ignored changes from one branch in favor of another rather than showing a conflict

Your Excellencies:

I'm a new tech lead trying to figure out what caused a significant production issue. I want to know why the merge described below discards the content of a certain file from one branch and uses content from another branch, while saying that the file was "added by them". More importantly, I want to know how to prevent this in the future.

DETAILS

A dev wrote a feature across many files, including app/events/checkout_event.rb. Once merged and deployed, it was discovered that his changes to that file didn't make it to production... but many other changes from the same commit(s) did make it.

After much research, we have isolated the issue to a specific ref -- a merge. It appears that, during the merge where both branches had changed that file, git decided to throw out changes to a from one branch and to use the other branch.

I created three tags so my small brain didn't have to remember refs:
* "working" (aa29d0f4)
* other_parent (7f9ebfa5).
* broken (ccbeb88)

---- jw@logopolis:/projects/unity/atlantis$ git show broken

commit cbbeb883428a468e9c906c646becdf751ecff99a (broken)
Merge: 7f9ebfa5 aa29d0f4
Author: Johnathon Wright <jw@mustmodify.com>
Date:   Fri Oct 16 13:17:43 2020 -0400

    merge

... ----

'git show broken' doesn't show any changes to the file in question.

Here, you can see that the files were different:

---- jw@logopolis:/projects/unity/atlantis$ git ls-tree working app/events/checkout_event.rb

100644 blob 077013dc52f03b336143b892c1468516cae29dce app/events/checkout_event.rb ----

and

---- jw@logopolis:/projects/unity/atlantis$ git ls-tree other_parent app/events/checkout_event.rb

100644 blob 21b3d4e9b5ac4903c2aadbed28ab5b60eb77e8ce app/events/checkout_event.rb -----

Then I tried to reproduce the merge:

---- jw@logopolis:/projects/unity/atlantis$ git checkout working

HEAD is now at aa29d0f4 Track instances of price changing

jw@logopolis:/projects/unity/atlantis$ git merge other_parent

CONFLICT (rename/delete): spec/events/single_payment_event_spec.rb deleted in HEAD and renamed to spec/events/checkout_event_spec.rb in other_parent. Version other_parent of spec/events/checkout_event_spec.rb left in tree. Removing spec/commands/generate_ticket_and_send_command_spec.rb Auto-merging db/schema.rb CONFLICT (content): Merge conflict in db/schema.rb Auto-merging config/routes.rb Auto-merging app/models/user.rb CONFLICT (rename/delete): app/events/single_payment_event.rb deleted in HEAD and renamed to app/events/checkout_event.rb in other_parent. Version other_parent of app/events/checkout_event.rb left in tree. Auto-merging app/controllers/products_controller.rb Auto-merging app/controllers/priceline/price_points_controller.rb Automatic merge failed; fix conflicts and then commit the result.

jw@logopolis:/projects/unity/atlantis$ git status

HEAD detached at working
You have unmerged paths.
  (fix conflicts and run "git commit")
  (use "git merge --abort" to abort the merge)

Changes to be committed:

        new file:   app/controllers/criteria_sets_controller.rb
        modified:   app/controllers/priceline/price_points_controller.rb
        modified:   app/controllers/products_controller.rb
        modified:   app/events/search_hotels_event.rb
        modified:   app/forms/search_hotels_form.rb
        modified:   app/interfaces/priceline_api.rb
        modified:   app/models/priceline/criteria_set.rb
        modified:   app/models/user.rb
        modified:   app/queries/hotels_filter_query.rb
        new file:   app/views/products/waiting.html.erb
        modified:   app/workers/generate_booking_ticket_worker.rb
        modified:   app/workers/update_priceline_hotels_worker.rb
        modified:   config/routes.rb
        new file:
db/migrate/20201009200507_add_timestamps_to_pl_criteria_sets.rb
        new file:
db/migrate/20201009235952_add_completion_flag_to_criteria_sets.rb
        deleted:    spec/commands/generate_ticket_and_send_command_spec.rb
        modified:   spec/forms/search_hotels_form_spec.rb

Unmerged paths:
  (use "git add <file>..." to mark resolution)

        added by them:   app/events/checkout_event.rb
        both modified:   db/schema.rb
        added by them:   spec/events/checkout_event_spec.rb
----

note that even though app/events/checkout_event.rb exists in both the "working" and "other_parent" branches, git here says "added by them".

Note that "working" and "other_parent" both have parent nodes which rename the file in question from something else TO app/events/checkout_event.rb (same for the spec.) The two ancestors have different refs but the same datetime and commit message.

This might also be of interest, or perhaps not.

---- jw@logopolis:/projects/unity/atlantis$ git diff working..broken -- app/events/checkout_event.rb | grep "^-" | wc -l

158

jw@logopolis:/projects/unity/atlantis$ git diff working..broken -- app/events/checkout_event.rb | grep "^+" | wc -l

51 ----

Final bit of information:

---- jw@logopolis:/projects/unity/atlantis$ git --version

git version 2.17.1 ----

So, why does git say that this file was "added by them" instead of saying it had a conflict that needed merging?

It has been suggested that this might have something to do with some team members using merge and others using rebase. Or possibly rebasing shared history. I don't know how to determine whether that's the case or whether that might have caused this.

If so, I would appreciate seeing a set of instructions from a blank repo that cause this same thing to happen, so that I can help my team to understand the situation and how it can be avoided.

If it is not related to rebasing and then merging, any help would be appreciated in diagnosing this issue.

Thanks,

jw

Hi,

On Sat, Oct 24, 2020 at 2:39 PM Johnathon Wright <jw@mustmodify.com> wrote:

I'll respond despite not being one of the ones to whom your email was addressed... ;-)

Well, I've got bad news and good news for you, but mostly bad news...

This doesn't quite match what you show below. git _did_ report a conflict for this path, and left the user to resolve. Granted, there was content on the other side that git should have notified the user about with the conflict to make the conflict easier to resolve, but git did not "decide" how this path should be resolved and it explicitly marked it as conflicted.

All that said, I do lay the blame mostly at git's feet and not to your developers here. Being unable to correctly detect and report a rename/add/delete conflict and misreporting it as a rename/delete, with only one side's content shown does seem likely to lead to this kind of error. Someone would have to be very cautious in inspecting the conflicts to resolve this correctly.

That's because git doesn't show file changes within a merge regardless of whether someone tweaked the merge commit by modifying every single file. You would need to add --cc or something to see some of the differences. Or, when --remerge-diff is complete then you'd be able to use it, but it's still only half-baked.

This doesn't state what version of the file existed in the merge base, or merge bases. How many merge bases where there between these two commits? What version(s) of the file existed in that commit (or those commits)? (I think I can deduce the answer from the conflicts below, though, so don't worry too much about answering this.)

...

...

...

...

Yes, but in particular I think that git won't detect the rename on one of the two sides of history because the file has been modified too much.

It'd be more helpful to see the output of both (note: three dots, not two)
   git diff --name-status working...broken | grep -e checkout_event.rb
-e single_payment_event.rb
   git diff --name-status broken...working | grep -e checkout_event.rb
-e single_payment_event.rb

I suspect the output looks like
    R071    single_payment_event.rb   checkout_event.rb
for one of those two commands (with the number 071 likely changing),
and for the other it looks like
    A       checkout_event.rb
    D       single_payment_event.rb

In other words, the files were similar enough on one branch to detect the rename, but not on the other. You could tweak this with git diff --find-renames=<percentage> ..., and tweak it for the merge with git merge -Xfind-renames=<percentage>, but that's just attempting to work around the problem.

This is pretty old version of git so I'd like to encourage you to upgrade...but it won't help with this bug. This bug is present in all released git versions; it's test t6422.17 in our testsuite, marked as test_expect_failure.

Bug.

Nope, git bug. You can create a simple repository and trigger it pretty easily, as follows:

# Create testcase in new temporary repo git init stupid cd stupid/ seq 10 20 >original.file git add original.file git commit -m Initial git branch A git branch B git checkout A git mv original.file new.file seq 5 21 >new.file git add new.file git commit -m "Change on A" git checkout B git mv original.file new.file seq 10 50 >new.file git add new.file git commit -m "Change on B"

# Switch to branch A and merge B in git checkout A git merge B

Okay, so I said I had bad news and good news. The good news: this testcase passes if you use a special version of git -- the 'ort' branch of my fork of git at https://github.com/newren/git. I am working on pushing this upstream; see e.g. https://lore.kernel.org/git/a8d4825a323d5c1e7b2dc1edc8621c51c030ae1e.1603468885.git.gitgitgadget@gmail.com/ which marks t6422.17 as passing under the new ort merge backend. But unless you want to download someone's randomly tweaked git version with hundreds of patches, and build and use it, you might want to wait for it to go through the review process first. But, at least a solution is on it's way...

Hope that helps, Elijah