git cherry not marking commits with equivalent upstream

The documentation for git-cherry says it marks changes in the current
checkout that have an "equivalent" change in the upstream branch.  It
even says it's useful when feeding patches upstream by email instead of
git, which is what I'm doing (with CVS instead of email).  But it
doesn't seem to work for me.

I'll simulate cloning an upstream repo, creating and commiting a patch,
then sending it via email upstream to have it applied there, then
pulling the upstream commit (the upstream repo is a, mine is b):

    ~% mkdir a && cd a
    ~/a% git init
    Initialized empty Git repository in /home/andrew/a/.git/
    ~/a% touch a
    ~/a% git add a
    ~/a% git commit -m 1
    [master (root-commit) be4fa74] 1
     0 files changed, 0 insertions(+), 0 deletions(-)
     create mode 100644 a
    ~/a% git clone . ../b && cd ../b
    Initialized empty Git repository in /home/andrew/b/.git/
    ~/b% echo test > a
    ~/b% git add a
    ~/b% git commit -m 2
    [master 551e90a] 2
     1 files changed, 1 insertions(+), 0 deletions(-)
    ~/b% cd ../a
    ~/a% echo test > a
    ~/a% git add a
    ~/a% git commit -m 3
    [master bb13e6c] 3
     1 files changed, 1 insertions(+), 0 deletions(-)
    ~/a% cd ../b
    ~/b% git pull
    remote: Counting objects: 5, done.
    remote: Total 3 (delta 0), reused 0 (delta 0)
    Unpacking objects: 100% (3/3), done.
    From /home/andrew/a/.
       be4fa74..bb13e6c  master     -> origin/master
    Merge made by recursive.
    ~/b% cat a
    test

Now, I think I have two equivalent commits in repo b, one of which came
from repo a (upstream).  So I expect git-cherry to show the other commit
with a '-' instead of a '+'.  But no:

    ~/b% git log
    commit 27158bb3e5f7cf80a43eb7364a735f16c43e447c
    Merge: 551e90a bb13e6c
    Author: Andrew Pimlott [off-list ref]
    Date:   Thu Jul 1 12:25:21 2010 -0700

        Merge branch 'master' of /home/andrew/a/.

    commit bb13e6cea3a27a4450984b6d1d87f13d807d2d36
    Author: Andrew Pimlott [off-list ref]
    Date:   Thu Jul 1 12:25:18 2010 -0700

        3

    commit 551e90ac390a2a27152661b9cbe73845d237e008
    Author: Andrew Pimlott [off-list ref]
    Date:   Thu Jul 1 12:25:06 2010 -0700

        2

    commit be4fa741476176181947e96c5242003ffe4f4183
    Author: Andrew Pimlott [off-list ref]
    Date:   Thu Jul 1 12:24:42 2010 -0700

        1
    ~/b% git show bb13e6cea3a27a4450984b6d1d87f13d807d2d36 | git patch-id
    58105e2bbccf2799f480bf82bb76467ff0301c52 bb13e6cea3a27a4450984b6d1d87f13d807d2d36
    ~/b% git show 551e90ac390a2a27152661b9cbe73845d237e008 | git patch-id
    58105e2bbccf2799f480bf82bb76467ff0301c52 551e90ac390a2a27152661b9cbe73845d237e008
    ~/b% git cherry
    + 551e90ac390a2a27152661b9cbe73845d237e008

Is my undestanding of how this should work wrong?  Is there any way to get
the result I want?

Andrew
(Please Cc me on replies)

(forgot to mention I'm using git 1.7.1 from Debian package git-core
1:1.7.1-1)

On 2010.07.01 12:38:45 -0700, Andrew Pimlott wrote:

The documentation for git-cherry says it marks changes in the current
checkout that have an "equivalent" change in the upstream branch.  It
even says it's useful when feeding patches upstream by email instead of
git, which is what I'm doing (with CVS instead of email).  But it
doesn't seem to work for me.

I'll simulate cloning an upstream repo, creating and commiting a patch,
then sending it via email upstream to have it applied there, then
pulling the upstream commit (the upstream repo is a, mine is b):

    ~% mkdir a && cd a
    ~/a% git init
    Initialized empty Git repository in /home/andrew/a/.git/
    ~/a% touch a
    ~/a% git add a
    ~/a% git commit -m 1
    [master (root-commit) be4fa74] 1
     0 files changed, 0 insertions(+), 0 deletions(-)
     create mode 100644 a
    ~/a% git clone . ../b && cd ../b
    Initialized empty Git repository in /home/andrew/b/.git/
    ~/b% echo test > a
    ~/b% git add a
    ~/b% git commit -m 2
    [master 551e90a] 2
     1 files changed, 1 insertions(+), 0 deletions(-)
    ~/b% cd ../a
    ~/a% echo test > a
    ~/a% git add a
    ~/a% git commit -m 3
    [master bb13e6c] 3
     1 files changed, 1 insertions(+), 0 deletions(-)
    ~/a% cd ../b
    ~/b% git pull
    remote: Counting objects: 5, done.
    remote: Total 3 (delta 0), reused 0 (delta 0)
    Unpacking objects: 100% (3/3), done.
    From /home/andrew/a/.
       be4fa74..bb13e6c  master     -> origin/master
    Merge made by recursive.
    ~/b% cat a
    test

pull = fetch + merge, so your history in "b" looks like this:

  2 (origin/master)
 / \
1   M (master)
 \ /
  3

So "2" is common to both branches and thus ignored by cherry. If you
just fetch instead of merging, you get the result you expected:

doener@atjola:b (master) $ git fetch 
remote: Counting objects: 5, done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
From /home/doener/y/a
   4815708..dfbbb81  master     -> origin/master

doener@atjola:b (master) $ git cherry
- 2544e5a7f5d6b545d9e24ba9dcac74861b0bf15c

But once I merge:
doener@atjola:b (master) $ git merge origin/master
Merge made by recursive.

doener@atjola:b (master) $ git cherry
+ 2544e5a7f5d6b545d9e24ba9dcac74861b0bf15c

Björn

Although ‘git cherry’ is advertised simply to list commits from
<topic> that have not been merged into <upstream>, internally it
works by checking for patches in <upstream>..<topic> that do not
match a patch in <topic>..<upstream>.  This is fast because there
are not many patches to check, and it makes sense semantically
since even if a new patch patches an old patch from <upstream>,
it cannot be said to have been merged upstream unless it was
applied there at some point after the <topic> and <upstream>
branches diverged.

But.  If the <topic> branch later merges from <upstream>, it
throws a spanner in the works and for such a history ‘git cherry’
is not very useful at all.

Example:

 o---o---F---X'---G---U [upstream]
          \        \
           X----Y---M---T [topic]

Suppose the author of the ‘topic’ branch starts from upstream
commit F and makes a few changes.  One is applied upstream, and
additionally there is some other useful upstream change, so he
performs a merge to include the upstream updates into topic.
The expected output from ‘cherry’ is:

 + T
 + Y
 - X

Consider the author of a different branch, also called ‘topic’, but
this one starts from commit G.  Some infrastructure from an existing 
branch is needed, so first she merges that.  Then she adds her own
commit.  The expected output from ‘cherry’ is:

 + T
 + Y
 + X

since none of the new commits have been applied upstream since
the fork point.

‘cherry’ cannot distinguish between these two cases, in part because
it does not distinguish between parents in a merge commit.

Add a BUGS section to explain the problem.

Reported-by: Frédéric Brière <redacted>
Reported-by: Andrew Pimlott <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Hi Andrew,

Andrew Pimlott wrote:

The documentation for git-cherry says it marks changes in the current
checkout that have an "equivalent" change in the upstream branch.  It
even says it's useful when feeding patches upstream by email instead of
git, which is what I'm doing (with CVS instead of email).  But it
doesn't seem to work for me.

[...]

    ~/a% git commit -m 3
    [master bb13e6c] 3
     1 files changed, 1 insertions(+), 0 deletions(-)
    ~/a% cd ../b
    ~/b% git pull

[...]

    ~/b% git cherry
    + 551e90ac390a2a27152661b9cbe73845d237e008

I have not carefully read your example, but maybe this patch might help
explain it.  A correct solution for some cases might include a
‘git cherry --full’ option that scans the full upstream history for
equivalents to patches.

Thoughts?  Improvements?

 Documentation/git-cherry.txt |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
index fed115a..83e3bdc 100644
--- a/Documentation/git-cherry.txt
+++ b/Documentation/git-cherry.txt

@@ -59,6 +59,21 @@ OPTIONS
 <limit>::
 	Do not report commits up to (and including) limit.
 
+BUGS
+----
+'git cherry' does not cope well with merges from upstream on the
+working branch.  Any commits after the original fork point and
+before the latest merge from upstream will be reported as not found
+in <upstream>.
+
+                       ____*____*____*_____*__> <upstream>
+                      /                     \
+   original fork point                       \
+                      \__+__+__+__+__+__+__+__*_> <head>
+
+While these commits are part of upstream..head, their upstream
+counterparts are not in head..upstream.
+
 SEE ALSO
 --------
 linkgit:git-patch-id[1]

-- 
1.7.1.1

Excerpts from Björn Steinbrink's message of Thu Jul 01 13:41:51 -0700 2010:

pull = fetch + merge, so your history in "b" looks like this:

  2 (origin/master)
 / \
1   M (master)
 \ /
  3

So "2" is common to both branches and thus ignored by cherry.

Ok, it's unintuitive to me that 2 is not considered part of the upstream
branch just because I've merged it into mine, but that explains it.
Thanks!

However, I want to merge commits from upstream regularly and still
figure out what unmerged commits I have.  So how can I make this use
case work?  It sounds like I want to make git-cherry check against
everything upstream since the fork point, not just what I haven't
merged.  Would this be hard?

Andrew

Excerpts from Jonathan Nieder's message of Thu Jul 01 14:09:19 -0700 2010:

Example:

 o---o---F---X'---G---U [upstream]
          \        \
           X----Y---M---T [topic]

Suppose the author of the ‘topic’ branch starts from upstream
commit F and makes a few changes.  One is applied upstream, and
additionally there is some other useful upstream change, so he
performs a merge to include the upstream updates into topic.
The expected output from ‘cherry’ is:

 + T
 + Y
 - X

Consider the author of a different branch, also called ‘topic’, but
this one starts from commit G.  Some infrastructure from an existing 
branch is needed, so first she merges that.  Then she adds her own
commit.  The expected output from ‘cherry’ is:

 + T
 + Y
 + X

since none of the new commits have been applied upstream since
the fork point.

‘cherry’ cannot distinguish between these two cases

Thanks for the awesome explanation!  (I looked at the code but would not
have pulled this understanding.)  I would still say the first output is
the more reasonable: it's more likely (in my estimate) the wanted
result, and in the case where it's not it's at least easily
comprehended.  

Anyway, the doc patch helps, and I would love git cherry --full.

Andrew

Jonathan Nieder wrote:

                                   it makes sense semantically
since even if a new patch patches an old patch from <upstream>,

err, for 'patches' read 'matches'.  I was not trying to say
something that complicated. :)

Sorry for the noise.
Jonathan

Jonathan Nieder wrote:

+'git cherry' does not cope well with merges from upstream on the
+working branch.  Any commits after the original fork point and
+before the latest merge from upstream will be reported as not found
+in <upstream>.
+
+                       ____*____*____*_____*__> <upstream>
+                      /                     \
+   original fork point                       \
+                      \__+__+__+__+__+__+__+__*_> <head>
+
+While these commits are part of upstream..head, their upstream
+counterparts are not in head..upstream.

As Junio mentioned, the text and diagram are hard to reconcile since
“these commits” are not clearly explained to be “the commits after the
original fork point and before the latest merge, marked with a +”.

Actually, the whole text is kind of awkward, which is why I left this
hanging for so long[1].  Sorry.

Adding “(marked +)” after “these commits” sounds reasonable to me as
a quick fix.

[1] http://bugs.debian.org/575577

Jonathan Nieder venit, vidit, dixit 01.07.2010 23:09:

Although ‘git cherry’ is advertised simply to list commits from
<topic> that have not been merged into <upstream>, internally it
works by checking for patches in <upstream>..<topic> that do not
match a patch in <topic>..<upstream>.  This is fast because there

...and it says so in the very first line of the manpage.

are not many patches to check, and it makes sense semantically
since even if a new patch patches an old patch from <upstream>,
it cannot be said to have been merged upstream unless it was
applied there at some point after the <topic> and <upstream>
branches diverged.

But.  If the <topic> branch later merges from <upstream>, it
throws a spanner in the works and for such a history ‘git cherry’
is not very useful at all.

I actually think I've reported this before, but I don't mind :)
I keep (topic) branches for my git.git patches. git cherry is helpful
here. When a patch is applied, I merge the corresponding commit (i.e.
Junio's version) to my topic branch to mark it as applied and to have a
nice way of comparing the applied version to the submitted one. At that
point git cherry stops being useful.

Example:

 o---o---F---X'---G---U [upstream]
          \        \
           X----Y---M---T [topic]

Suppose the author of the ‘topic’ branch starts from upstream
commit F and makes a few changes.  One is applied upstream, and
additionally there is some other useful upstream change, so he
performs a merge to include the upstream updates into topic.
The expected output from ‘cherry’ is:

 + T
 + Y
 - X

Consider the author of a different branch, also called ‘topic’, but
this one starts from commit G.  Some infrastructure from an existing 
branch is needed, so first she merges that.  Then she adds her own
commit.  The expected output from ‘cherry’ is:

 + T
 + Y
 + X

since none of the new commits have been applied upstream since
the fork point.

‘cherry’ cannot distinguish between these two cases, in part because
it does not distinguish between parents in a merge commit.

Add a BUGS section to explain the problem.

This is not a bug. git cherry works exactly as described.

At worst, it is a misfeature.

git cherry would be more useful if you could specify a "limit" which is
an ancestor of "fork-point", not only descendants.

Thoughts?  Improvements?

Allow general "limit" :)

+DISCUSSION

+----
+'git cherry' does not cope well with merges from upstream on the
+working branch.  Any commits after the original fork point and

...which is the wrong way round anyways ;)

+before the latest merge from upstream will be reported as not found
+in <upstream>.
+
+                       ____*____*____*_____*__> <upstream>
+                      /                     \
+   original fork point                       \
+                      \__+__+__+__+__+__+__+__*_> <head>
+
+While these commits are part of upstream..head, their upstream
+counterparts are not in head..upstream.

git-cherry(1) never speaks about upstream..head nor head..upstream. It
uses "fork-point", and a merge creates a new "fork-point", i.e.
merge-base. I think it would be good to keep like that in order to avoid
that very misunderstanding. In fact, git cherry is about left and right
commits in upstream...head.

The second paragraph of "DESCRIPTION" may cause confusion when read
without the first one, though.

Michael

Michael J Gruber wrote:

Jonathan Nieder venit, vidit, dixit 01.07.2010 23:09:

quoted

Add a BUGS section to explain the problem.

This is not a bug. git cherry works exactly as described.

At worst, it is a misfeature.

Unix man pages have a history of using BUGS sections to describe
misfeatures and even unavoidable design constraints.

One nice effect is to encourage people to think of programs as
fixable.  But maybe it is bad PR. ;-)

git cherry would be more useful if you could specify a "limit" which is
an ancestor of "fork-point", not only descendants.

quoted

Thoughts?  Improvements?

Allow general "limit" :)

Hmm, I am not totally sure I understand.  Conceptually ‘git cherry’
currently does something like the following:

 1. List all commits limit..head and find their patch ids
    (limit defaults to upstream if not specified)

 2. List all commits head..upstream and find their patch ids

 3. For each commit listed in step 1, check if it is in the
    list from step 2 and print its commit id with a + or -
    accordingly.

Are you suggesting that the limit should replace head in
step 2?  Or something else?

git-cherry(1) never speaks about upstream..head nor head..upstream. It
uses "fork-point", and a merge creates a new "fork-point", i.e.
merge-base.

This explanation becomes problematic when there is more than one merge-base:
http://thread.gmane.org/gmane.comp.version-control.git/150067/focus=150093

Thank you for the comments.  I considered using the <limit> argument
to work around this but didn’t try the modification you suggest above.
I would be happy to find that it works (generalized for repos with
a more shallow history to --full).

Sleepily,
Jonathan

Jonathan Nieder venit, vidit, dixit 02.07.2010 10:18:

Michael J Gruber wrote:

quoted

Jonathan Nieder venit, vidit, dixit 01.07.2010 23:09:

quoted

quoted

Add a BUGS section to explain the problem.

This is not a bug. git cherry works exactly as described.

At worst, it is a misfeature.

Unix man pages have a history of using BUGS sections to describe
misfeatures and even unavoidable design constraints.

One nice effect is to encourage people to think of programs as
fixable.  But maybe it is bad PR. ;-)

quoted

git cherry would be more useful if you could specify a "limit" which is
an ancestor of "fork-point", not only descendants.

quoted

Thoughts?  Improvements?

Allow general "limit" :)

Hmm, I am not totally sure I understand.  Conceptually ‘git cherry’
currently does something like the following:

 1. List all commits limit..head and find their patch ids
    (limit defaults to upstream if not specified)

 2. List all commits head..upstream and find their patch ids

 3. For each commit listed in step 1, check if it is in the
    list from step 2 and print its commit id with a + or -
    accordingly.

Are you suggesting that the limit should replace head in
step 2?  Or something else?

I suggest that I was reading limit on the wrong branch :|
What I meant was specifiying a different lower limit in 2 would help:
then you could force git cherry to compare more commits, if you have a
rough idea about how far to go back. But even being able to say
"v1.6.0..upstream" here instead of head helps and is much more efficient
then going for --full.

quoted

git-cherry(1) never speaks about upstream..head nor head..upstream. It
uses "fork-point", and a merge creates a new "fork-point", i.e.
merge-base.

This explanation becomes problematic when there is more than one merge-base:
http://thread.gmane.org/gmane.comp.version-control.git/150067/focus=150093

I guess it always pays to read the full thread before jumping in... your
"prefork" there is what I meant above.

Michael