Re: git-log --cherry-pick gives different results when using tag or tag^{}

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> [+cc Junio, as the bug blames to him]
> ...
> I think what is happening is that we used to apply the SYMMETRIC_LEFT
> flag directly to the commit. Now we apply it to the tag, and it does not
> seem to get propagated. The patch below fixes it for me, but I have no
> idea if we actually need to be setting the other flags, or just
> SYMMETRIC_LEFT. I also wonder if the non-symmetric two-dot case needs to
> access any pointed-to commit and propagate flags in a similar way.

Thanks.

Where do we pass down other flags from tags to commits?  For
example, if we do this:

	$ git log ^v1.8.5 master

we mark v1.8.5 tag as UNINTERESTING, and throw that tag (not commit
v1.8.5^0) into revs->pending.objects[].  We do the same for 'master',
which is a commit.

Later, in prepare_revision_walk(), we call handle_commit() on them,
and unwrap the tag v1.8.5 to get v1.8.5^0, and then handles that
commit object with flags obtained from the tag object.  This code
only cares about UNINTERESTING and manually propagates it.

Perhaps that code needs to propagate at least SYMMETRIC_LEFT down to
the commit object as well, no?  With your patch, the topmost level
of tag object and the eventual commit object are marked with the
flag, but if we were dealing with a tag that points at another tag
that in turn points at a commit, the intermediate tag will not be
marked with SYMMETRIC_LEFT (nor UNINTERESTING for that matter),
which may not affect the final outcome, but it somewhat feels wrong.

How about doing it this way instead (totally untested, though)?

 revision.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index a68fde6..def070e 100644
--- a/revision.c
+++ b/revision.c
@@ -276,6 +276,7 @@ static struct commit *handle_commit(struct rev_info *revs,
 				return NULL;
 			die("bad object %s", sha1_to_hex(tag->tagged->sha1));
 		}
+		object->flags |= flags;
 	}
 
 	/*
@@ -287,7 +288,6 @@ static struct commit *handle_commit(struct rev_info *revs,
 		if (parse_commit(commit) < 0)
 			die("unable to parse commit %s", name);
 		if (flags & UNINTERESTING) {
-			commit->object.flags |= UNINTERESTING;
 			mark_parents_uninteresting(commit);
 			revs->limited = 1;
 		}