Re: [PATCH v2] name_rev: add support for --cherry-picks

2018-07-27 Thread Junio C Hamano
Tejun Heo  writes:

> ... After all, calling the program twice isn't all that
> difficult.

As long as we all agree on that, I think we can move forward.
Because I think this ...

>> ... But it feels like
>> "reverse-map the cherry-picks" is orthogonal to the idea of name-rev.

... is a better way of saying what I've been feeling (i.e. the
feature indeed is useful, but does it belong to "name-rev"?), and
none among three of us would mind running "name-rev" to see the
simplest way to reach the primary commit you are interested in from
tags, and another "reverse-map the cherry-picks" command (and in
"git show -s --notes=reverse-cherry-pick" may be that command) to
get the data from that orthogonal feature.

And obviously, "git log --notes=reverse-cherry-pick" would give the
information if we take that "use notes to record reverse map for
cherry-picks" route; adding support for "name-rev --cherry-pick"
would not help such a use case.


Re: [PATCH v2] name_rev: add support for --cherry-picks

2018-07-27 Thread Tejun Heo
Hello, Jeff.

On Fri, Jul 27, 2018 at 4:47 AM Jeff King  wrote:
>  - the set of names might be distinct from the set of commits you'd want
>to traverse.  For instance, you might want to use "name-rev --tags",
>but find cherry-picks even on untagged branches (e.g., "--all").

Hmm... maybe but when I'm using --tags, I'm usually asking "when did
this get released?" and --cherry-picks seems to logically extend that
and fits such scenarios pretty well. ie. "Give me the release name and
all the aliases for this commit". We can play with the options to
support combinations of tagged/untagged but I'm not quite sure about
the usefulness. After all, calling the program twice isn't all that
difficult.

>  - instead of naming commits, you might want to see the information as
>you are viewing git-log output ("by the way, this was cherry-picked
>elsewhere, too")

name-rev --stdin sort of does this for commit names. I thought about
adding the support for --cherry-picks too but wasn't sure how to weave
in the result, but if we can figure that out this should work, right?

> So I kind of wonder if it would be more useful to have a command which
> incrementally updates a git-notes tree to hold the mapping, and then
> learn to consult it in various places. "git log --notes=reverse-cherry"
> would show it, though it might be worth teaching the notes-display code
> that certain types of notes contain object ids (so it would be
> interesting to recursively show their notes, or format them nicely,
> etc).
>
> I dunno. That is perhaps over-engineering. But it feels like
> "reverse-map the cherry-picks" is orthogonal to the idea of name-rev.

Hmm... to me, it makes logical sense because "name this commit" and
"also include aliases" seem to be a natural combination both in
semantics and use cases, but if there are better ways of tracking down
cherry-picks, I'm all ears.

Thanks.

-- 
tejun


Re: [PATCH v2] name_rev: add support for --cherry-picks

2018-07-27 Thread Jeff King
On Thu, Jul 26, 2018 at 12:01:34PM -0700, Junio C Hamano wrote:

> Tejun Heo  writes:
> 
> > I should have explained the use case better.
> 
> No, you did not need to.  I was not saying the feature is not useful.
> I was only saying that "explain where in the history X sits" command
> (i.e. "name-rev X" and "describe X") did not look like a good place
> to have that feature.

Just brainstorming a few reasons you might not want it tied to name-rev:

 - the set of names might be distinct from the set of commits you'd want
   to traverse.  For instance, you might want to use "name-rev --tags",
   but find cherry-picks even on untagged branches (e.g., "--all").

 - instead of naming commits, you might want to see the information as
   you are viewing git-log output ("by the way, this was cherry-picked
   elsewhere, too")

So I kind of wonder if it would be more useful to have a command which
incrementally updates a git-notes tree to hold the mapping, and then
learn to consult it in various places. "git log --notes=reverse-cherry"
would show it, though it might be worth teaching the notes-display code
that certain types of notes contain object ids (so it would be
interesting to recursively show their notes, or format them nicely,
etc).

I dunno. That is perhaps over-engineering. But it feels like
"reverse-map the cherry-picks" is orthogonal to the idea of name-rev.

-Peff


Re: [PATCH v2] name_rev: add support for --cherry-picks

2018-07-26 Thread Junio C Hamano
Tejun Heo  writes:

> I should have explained the use case better.

No, you did not need to.  I was not saying the feature is not useful.
I was only saying that "explain where in the history X sits" command
(i.e. "name-rev X" and "describe X") did not look like a good place
to have that feature.

>   Upstream, it's v4.17-rc1
> Backported and released through v4.16-prod-r2
>   Also backported to v4.16-prod-r1 (cuz the fix happened while r1 was 
> going through rc's)
>
> So, it extends the description of how the original commit is released
> (or given name) so that releases through cherry-picks are also
> included.

Perhaps.  I am not yet 100% convinced, but having a less hard time
trying to convince myself now ;-)

Thanks.


Re: [PATCH v2] name_rev: add support for --cherry-picks

2018-07-26 Thread Tejun Heo
Hello, Junio.

On Thu, Jul 26, 2018 at 08:12:45AM -0700, Junio C Hamano wrote:
> Tejun Heo  writes:
> 
> > From a6a88c3da252d69547ac8b463098fc4f4c03f322 Mon Sep 17 00:00:00 2001
> > From: Tejun Heo 
> > Date: Thu, 26 Jul 2018 04:14:52 -0700
> > Subject: [PATCH] name_rev: add support for --cherry-picks
> 
> The above belongs to the mail header, not the body.

Ah, right, sorry about that.

> >   $ git name-rev --cherry-picks 10f7ce0a0e524279f022
> >   10f7ce0a0e524279f022 master~1
> > d433e3b4d5a19b3d29e2c8349fe88ceade5f6190 branch1
> >   82cddd79f962de0bb1e7cdd95d48b4865816 branch2
> > 58a8d36b2532feb0a14b4fc2a50d587e64f38324 branch3
> > fa8b79edc5dfff21753c2ccfc1a1828336c4c070 branch4~1
> 
> "git name-rev X" asks "I want to know about X".  And the first line
> of the above tells us that 10f7ce is the first parent of the master
> branch.  What does the second line tell us?  10f7ce was created by
> cherry picking d433e3b4 which sits at the tip of branch1?
> 
> It appears that you are showing the reverse (d433e3, 58a8d3 and
> fa8b79 sit next to each other, but it cannot be that 10f7ce was
> created by cherry-picking these three).  I do not mean to say that

So, it means that d433e, 58a8d and fa8b7 are created by cherry picking
10f7c and 72cdd is created by cherry picking d433e.

> the reverse information is not useful thing to learn about the
> commit (i.e. "X got cherry-picked to all these places") but I am
> having a hard time convincing myself that the feature sits well in
> "describe" and "name-rev".

I should have explained the use case better.  Let's say I'm forking
off and maintaining prod releases.  We branch it off of a major
upstream version and keeps developing / backporting on that branch
reguarly cutting releases.  A lot of commits get cherry-picked back
from master tracking upstream but some are also cherry picked to
sub-release branches for quick-fix releases.  e.g.

  v4.16
 oooAoo..o master
   \.ooA'oo.o v4.16-prod
  \  \.oo v4.16-prod-r2
   \ .oA'' v4.16-prod-r1

Given a commit, it's useful to find out through which version that got
released, which is where "git-describe --contains" helps.  However,
when commits are backported through cherry-picks to prod branches as
in above, that commit is released through multiple versions and it's a
bit painful to hunt them down.  This is what --cherry-picks helps
with, so if now I do "git describe --contains --cherry-picks A", it'll
tell me that...

  Upstream, it's v4.17-rc1
Backported and released through v4.16-prod-r2
  Also backported to v4.16-prod-r1 (cuz the fix happened while r1 was going 
through rc's)

So, it extends the description of how the original commit is released
(or given name) so that releases through cherry-picks are also
included.

Thanks.

-- 
tejun


Re: [PATCH v2] name_rev: add support for --cherry-picks

2018-07-26 Thread Junio C Hamano
Tejun Heo  writes:

> From a6a88c3da252d69547ac8b463098fc4f4c03f322 Mon Sep 17 00:00:00 2001
> From: Tejun Heo 
> Date: Thu, 26 Jul 2018 04:14:52 -0700
> Subject: [PATCH] name_rev: add support for --cherry-picks

The above belongs to the mail header, not the body.

> It's often useful to track cherry-picks of a given commit.  Add
> --cherry-picks support to git-name-rev.  When specified, name_rev also
> shows the commits cherry-picked from the listed target commits with
> indentations.
>
>   $ git name-rev --cherry-picks 10f7ce0a0e524279f022
>   10f7ce0a0e524279f022 master~1
> d433e3b4d5a19b3d29e2c8349fe88ceade5f6190 branch1
>   82cddd79f962de0bb1e7cdd95d48b4865816 branch2
> 58a8d36b2532feb0a14b4fc2a50d587e64f38324 branch3
> fa8b79edc5dfff21753c2ccfc1a1828336c4c070 branch4~1

"git name-rev X" asks "I want to know about X".  And the first line
of the above tells us that 10f7ce is the first parent of the master
branch.  What does the second line tell us?  10f7ce was created by
cherry picking d433e3b4 which sits at the tip of branch1?

It appears that you are showing the reverse (d433e3, 58a8d3 and
fa8b79 sit next to each other, but it cannot be that 10f7ce was
created by cherry-picking these three).  I do not mean to say that
the reverse information is not useful thing to learn about the
commit (i.e. "X got cherry-picked to all these places") but I am
having a hard time convincing myself that the feature sits well in
"describe" and "name-rev".

> Note that branch2 is further indented because it's a nested cherry
> pick from d433e3b4d5a1.
>
> "git-describe --contains" is a wrapper around git-name-rev.  Also add
> --cherry-picks support to git-describe.
>
> v2: - Remove a warning message for a malformed cherry-picked tag.
>   There isn't much user can do about it.
> - Continue scanning cherry-pick tags until a working one is found
>   instead of aborting after trying the last one.  It might miss
>   nesting but still better to show than miss the commit entirely.