git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>,
	Johannes Sixt <j6t@kdbg.org>,
	vincent.guittot@linaro.org
Subject: Re: [Bug report] git diff stat shows unrelated diff
Date: Fri, 15 Feb 2019 08:09:33 -0800	[thread overview]
Message-ID: <CABPp-BFpj8DBxgsqe9Rqnzb4vx5fPqNc+sUj8LzrFMj2bacoBQ@mail.gmail.com> (raw)
In-Reply-To: <20190215064013.s7yfkmfvfmwfmepc@vireshk-i7>

Hi Viresh,

On Thu, Feb 14, 2019 at 10:40 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 14-02-19, 13:23, Elijah Newren wrote:
> > I think you're getting tripped up by double-dot vs triple-dot with
> > diff being different than log:
> >
> > `git diff D..E` means the same thing as `git diff D E`, i.e. diff the
> > two commits D and E.
>
> Right, so both the branches have at least until rc2 (though
> pm/linux-next had until rc6), why will the diff D..E show the diff
> between rc1 and rc2 ?

I don't think it does; it includes the diff between rc2 and rc6.
First, `git diff D..E` will show you roughly the same thing as if you
have two clones of linux, one with D checked out, one with E checked
out, and then you run 'diff -ru linux-copy1/ linux-copy2/'.

But let's go back to your example, you had

Branch A: commit 55538fbc79e9
Branch B: commit a4f342b9607d
Branch C: commit 7c139d3f0f99
Branch pm/linux-next: unspecified, but I'll guess commit a06639e89e4

Now, using git describe on each of those commits in order shows us
where in history they are:

A: v5.0-rc2-2-g55538fbc79e9
B: v5.0-rc1-2-ga4f342b9607d
C: v5.0-rc2-5-g7c139d3f0f99
D: v5.0-rc6-84-ga06639e89e47

Now, you said you ran
   git diff -M --stat pm/linux-next..7c139d3f0f99
and that it included the diff between rc1 and rc2.  I think you
actually saw pieces of the diff from rc2 to rc6 and assumed it was the
diff from rc1 to rc2.  Besides the fact that pm/linux-next is based on
rc6 and 7c139d3f0f99 is based on rc2, here's another way to verify
that:

$ git diff --name-only v5.0-rc1 v5.0-rc2 | sort > early
$ git diff --name-only v5.0-rc2 v5.0-rc6 | sort > late
$ comm -23 early late > early-only
$ comm -13 early late > late-only
$ wc -l *-only
  407 early-only
 1149 late-only
 1556 total

So, here "early-only" is a list of files that changed between rc1 and
rc2 that did not change between rc2 and rc6.  "late-only" is a list of
files that did not change between rc1 and rc2, but did change between
rc2 and rc6.  No, let's compare that to your diff:

$ git diff --name-only a06639e89e4..7c139d3f0f99 | sort > changes
$ comm -12 changes early-only | wc -l
6
$ comm -12 changes late-only | wc -l
1148

So, the files listed in your diff only included 6 files that were
unique to early-only, and included 1148 files that were unique to
late-only.  So, your diff looks an awful lot like the diff between rc2
and rc6, and not much at all like the diff between rc1 and rc2.

> > `git diff D...E` means the same thing as `git diff $(git merge-base D E) E`
>
> I get exactly the same result with both .. and ... in this particular
> case and that's why I wonder if everything is okay or not.

With `git diff D..E` it doesn't matter much which order you put D and
E in, other than flipping '-' lines for '+' lines.  With `git diff
D...E` it makes a huge difference.  Compare:

$ git diff --shortstat 7c139d3f0f99..a06639e89e4
 1466 files changed, 15417 insertions(+), 7313 deletions(-)
$ git diff --shortstat a06639e89e4..7c139d3f0f99
 1466 files changed, 7313 insertions(+), 15417 deletions(-)

So, as I said, swapping with double-dot only changes '-' and '+'
lines.  In contrast:

$ git diff --shortstat 7c139d3f0f99...a06639e89e4
 1463 files changed, 15401 insertions(+), 7165 deletions(-)
$ git diff --shortstat a06639e89e4...7c139d3f0f99
 4 files changed, 148 insertions(+), 16 deletions(-)

You get a really small diff in one direction, but huge in the other.
The reason for this is that commit C (7c139d3f0f99) is really close to
the merge base, but pm/linux-next (a06639e89e4) is really far from it
-- it includes rc6.

> The problem in this case is:
> - PM tree had rc1,2,3,4,5,6 merged earlier into it.
> - Then I got merged one of my branches which was based of rc1 into
>   pm/linux-next.
> - And now I am trying to send pull request for another branch which is
>   a merge of the earlier branch (which got merged, based of rc1) and
>   second branch that has more stuff over rc2.
> - The most recent merge commit common between my branch and
>   pm/linux-next becomes the earlier branch which was based of rc1.
> - Now I expect ... to return the diff between rc1 and rc2 as it will
>   diff against the most recent merge.
> - But I expected ... to not include rc1..rc2 diff.
>
> > There are some people for whom this state of affairs makes sense.  I
> > am not one of them, and I suspect you aren't either.  The arguments
> > made by those who feel this makes sense seem reasonable to me in the
> > moment when they present them, but I have never been able to remember
> > these arguments longer than briefly.  It just doesn't stick with me.
> > The only thing I seem to be able to retain is the following:  "git
> > diff D..E is totally useless and should be an error because (1) it
> > doesn't do what I expect and (2) for folks that want the behavior
> > currently gotten with that syntax can instead just use a space instead
> > of a double dot."
>
> Okay but git request-pull uses .. and not ... and that's where I saw
> the issue in the first place.

I haven't used request-pull myself and I'm not familiar with the code,
so I don't know if you might have just passed it the wrong arguments
or if it has a bug.  I'll have to leave it to someone else to comment
on that (though they may need to know what arguments you passed to
that command).


Hope that helps,
Elijah

  reply	other threads:[~2019-02-15 16:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-14  8:22 [Bug report] git diff stat shows unrelated diff Viresh Kumar
2019-02-14 18:42 ` Johannes Sixt
2019-02-14 21:23 ` Elijah Newren
2019-02-14 22:10   ` Junio C Hamano
2019-02-15 18:52     ` Denton Liu
2019-02-15 19:25       ` Elijah Newren
2019-02-15 20:12         ` Junio C Hamano
2019-02-15 22:48           ` Philip Oakley
2019-02-15 23:32             ` Junio C Hamano
2019-02-16 12:47               ` Philip Oakley
2019-02-17  3:34                 ` Junio C Hamano
2019-02-17 23:34                   ` Philip Oakley
2019-02-18  0:21                     ` Junio C Hamano
2019-02-15 19:28       ` Junio C Hamano
2019-02-15  6:40   ` Viresh Kumar
2019-02-15 16:09     ` Elijah Newren [this message]
2019-02-18  4:34       ` Viresh Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABPp-BFpj8DBxgsqe9Rqnzb4vx5fPqNc+sUj8LzrFMj2bacoBQ@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).