Github showing an incorrect diff between main branch and PR branch

Question

I am trying to figure out why the diff shown in the "compare files" does not actually reflect the changes I have been making. This seems to be a new problem in the last few days; I have used the "compare files" view MANY times before at work etc and it has always reflected the correct main branch, whereas now it has not.

Here is what is currently in my main branch (a test repo to figure out what the hell the problem is):

// this is the entirety of main.js
function addition(a, b) {
  return a + b
}

And here is what I am merging into it:

// this is the entirety of main.js
const difference = (num1, num2) => (
  num1 - num2
);

In my pull request, I would expect to see a merge conflict, since I am adding to the first few lines of my code. And I am getting the little bar that says "this branch has conflicts that must be resolved". However, when I look at the compare files view, this is what I see:

The view does not recognize that in the main branch, there is clearly already a function there:

I cannot come up with a reason why this might be so. Is it a hiccup with the "compare files" view? Could it possibly have something to do with the way that I have taken the branch off of main?

For me, this is not a huge deal; pulling / rebasing main into my current branch and handling the merge conflicts in my text editor is totally fine. I ask this because I am working with students who are new to git, and very new to group git in particular, and I would like to know why this unexpected behavior is happening so that I can explain it to them.

If you would like to check out the repo itself, here it is.

Thank you!

user_1256452 · Answer

Cloning that repository, then fetching the pull request:

$ git clone https://github.com/pmacaluso3/merge_conflicts
Cloning into 'merge_conflicts'...
remote: Enumerating objects: 18, done.
remote: Counting objects: 100% (18/18), done.
remote: Compressing objects: 100% (15/15), done.
remote: Total 18 (delta 1), reused 13 (delta 0), pack-reused 0
Unpacking objects: 100% (18/18), done.
$ cd merge_conficts
$ git fetch origin refs/pull/2/head:pr2
From https://github.com/pmacaluso3/merge_conflicts
 * [new ref]         refs/pull/2/head -> pr2

gets us ready to observe the commit graph. Note that I have used the name pr2 for my local branch to refer to your pull request #2, which is the one you find puzzling:

$ git log --all --decorate --oneline --graph
*   7b4d901 (HEAD -> main, origin/main, origin/HEAD) Merge pull request #3 from pmacaluso3/testing-merge-conflicts
|\  
| *   8941f5a (origin/testing-merge-conflicts) Merge branch 'main' into testing-merge-conflicts
| |\  
| |/  
|/|   
* |   5f144f2 Merge pull request #1 from pmacaluso3/pete/addition
|\ \  
| * | bffd4d1 addition!
|/ /  
| * 660b014 testing
|/  
| * 0bb0d7f (origin/j/difference, pr2) diff
|/  
* 0283a25 initial

So we see that the commit in my local pr2 branch—which is also named j/difference in your repository, hence named origin/h/difference in mine, though I did not know that until I fetched refs/pull/2/head from GitHub—is commit 0bb0d7f, which has, as its parent, commit 0283a25.

If we look at what's in 0283a25, we see that there are two empty files:

$ git show 0283a25 | sed 's/@/ /'
commit 0283a257e0e1802b57dcbbaccc7860a1b2de9dfa
Author: famousPete 
Date:   Wed Aug 26 13:37:10 2020 -0400

    initial

diff --git a/README.md b/README.md
new file mode 100644
index 0000000..45b983b
--- /dev/null
+++ b/README.md
 @ -0,0 +1 @@
+hi
diff --git a/main.js b/main.js
new file mode 100644
index 0000000..e69de29

Hence, your PR does indeed tell Git to add three lines to an empty main.js.

This is what the view on GitHub shows as well.

In my pull request, I would expect to see a merge conflict, since I am adding to the first few lines of my code.

The original proposed merge is of 0bb0d7f with 0283a25. This merge is successful. GitHub have done it and we can obtain it:

$ git fetch origin refs/pull/2/merge:temp
remote: Enumerating objects: 1, done.
remote: Counting objects: 100% (1/1), done.
remote: Total 1 (delta 0), reused 0 (delta 0), pack-reused 0
Unpacking objects: 100% (1/1), done.
From https://github.com/pmacaluso3/merge_conflicts
 * [new ref]         refs/pull/2/merge -> temp
$ git log --decorate --oneline --graph temp
*   00e1eb0 (temp) Merge 0bb0d7f5b28f8817564f3acff0ca95a1622ea27d into 0283a257e0e1802b57dcbbaccc7860a1b2de9dfa
|\  
| * 0bb0d7f (origin/j/difference, pr2) diff
|/  
* 0283a25 initial

And I am getting the little bar that says "this branch has conflicts that must be resolved".

If you merge now, to main, then yes, there would be a merge conflict. But until someone rebases this particular commit on (the tip of) main, that merge conflict is left to whoever might try to merge this commit to main. It would be nice if GitHub could do a diff against the current tip of main instead of just against the merge base it chose earlier.

user_157957 · Answer

The simple answer is that the view you're looking at in Github doesn't show what you think it shows.

When you have made commits on two branches (I'll call them main and feature), you have a commit graph looking something like this:

            d -- e -- f   <- main
           /
a -- b -- c
           \
            g -- h -- i   <- feature

When you propose to merge feature into main, what you are aiming to end up with is this:

            d -- e -- f
           /           \
a -- b -- c             j  <- main   
           \           /
            g -- h -- i

Note that main and feature are pointers to commits, and do not "own" any changes; and each commit contains a snapshot of the entire repository at a point in time.

In order to create a two-way diff, we need to pick two of these snapshots to compare; there are several such comparisons we could view for this proposed merge:

main before and after the merge: compare f with a proposed version of j
main and feature before the merge: compare f directly with i
the changes that are in feature that aren't already in main: compare c (the last commit reachable from both branches) with i
the changes that are in main that aren't already in feature: compare c with f

You have assumed that Github will show you option 1. Since the merge creates conflicts, the proposed version of j would have to include some representation of these, as though you'd created a commit where you didn't resolve them.

What Github is actually showing you is option 3: regardless of what's happened on main, it only shows the changes that were made on feature. You can see this most clearly when you have more than one commit on feature: it will allow you to filter the view to individual commits, which wouldn't make much sense if it was comparing to a proposed merge commit j.

There are a few reasons they might have chosen this way:

Historically, a "pull request" was a series of patches sent by e-mail. The person sending the e-mail didn't know what the recipient's main branch looked like.
When reviewing a change, you can add inline comments against particular line numbers. If line 100 became line 150 because a non-conflicting change to main added 50 lines further up the file, the comment's new location would have to be calculated.
If there is a conflict, as in this case, a proposed version of commit j would have to include some kind of markup for those conflicts. Just viewing that as a two-way diff would result in confusing annotations, e.g. that you were adding the code <<<<<<<<<<<<<<<<<<<<<<. A more customised UI would be possible, but tricky to design so that it's easy to understand.

Some UIs will allow you to visually resolve conflicts in a three-pane view, which is actually cramming in a lot of information: the outer panels show comparisons 3 and 4 above (the changes made on each of the two branches), while the centre panel shows comparison 1 (the current attempt to merge them). An important thing about these views is that the centre panel is interactive - it shows the result of you resolving the conflicts, not just a textual representation of them.

Github showing an incorrect diff between main branch and PR branch

2 Answers