name_rev: add support for --cherry-picks

From aefa07bc66bb4a116eb84eb46d7f070f9632c990 Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Thu, 26 Jul 2018 04:14:52 -0700

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
      82cddd79f962de0bb1e7cdd95d48b48633335816 branch2
    58a8d36b2532feb0a14b4fc2a50d587e64f38324 branch3
    fa8b79edc5dfff21753c2ccfc1a1828336c4c070 branch4~1

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.

---
 Documentation/git-describe.txt   |   5 ++
 Documentation/git-name-rev.txt   |   4 ++
 builtin/describe.c               |   7 +-
 builtin/name-rev.c               | 117 +++++++++++++++++++++++++++++--
 t/t6121-describe-cherry-picks.sh |  63 +++++++++++++++++
 5 files changed, 190 insertions(+), 6 deletions(-)
 create mode 100755 t/t6121-describe-cherry-picks.sh

index e027fb8c4..13a229bd7 100644
@@ -60,6 +60,11 @@ OPTIONS
 	the tag that comes after the commit, and thus contains it.
 	Automatically implies --tags.
 
+--cherry-picks::
+	Also show the commits cherry-picked from the target commits.
+	Cherry-picks are shown indented below their from-commmits.
+	Can only be used with --contains.
+
 --abbrev=<n>::
 	Instead of using the default 7 hexadecimal digits as the
 	abbreviated object name, use <n> digits, or as many digits

index 5cb0eb085..df16c4a89 100644
@@ -61,6 +61,10 @@ OPTIONS
 --always::
 	Show uniquely abbreviated commit object as fallback.
 
+--cherry-picks::
+	Also show the commits cherry-picked from the target commits.
+	Cherry-picks are shown indented below their from-commmits.
+
 EXAMPLES
 --------
 

index 1e87f68d5..94c84004d 100644
@@ -528,9 +528,10 @@ static void describe(const char *arg, int last_one)
 
 int cmd_describe(int argc, const char **argv, const char *prefix)
 {
-	int contains = 0;
+	int contains = 0, cherry_picks = 0;
 	struct option options[] = {
 		OPT_BOOL(0, "contains",   &contains, N_("find the tag that comes after the commit")),
+		OPT_BOOL(0, "cherry-picks", &cherry_picks, N_("also include cherry-picks with --contains")),
 		OPT_BOOL(0, "debug",      &debug, N_("debug search strategy on stderr")),
 		OPT_BOOL(0, "all",        &all, N_("use any ref")),
 		OPT_BOOL(0, "tags",       &tags, N_("use any tag, even unannotated")),

@@ -570,6 +571,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 
 	if (longformat && abbrev == 0)
 		die(_("--long is incompatible with --abbrev=0"));
+	if (cherry_picks && !contains)
+		die(_("--cherry-picks can only be used with --contains"));
 
 	if (contains) {
 		struct string_list_item *item;

@@ -579,6 +582,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 		argv_array_pushl(&args, "name-rev",
 				 "--peel-tag", "--name-only", "--no-undefined",
 				 NULL);
+		if (cherry_picks)
+			argv_array_push(&args, "--cherry-picks");
 		if (always)
 			argv_array_push(&args, "--always");
 		if (!all) {

2.17.1

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

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
      82cddd79f962de0bb1e7cdd95d48b48633335816 branch2
    58a8d36b2532feb0a14b4fc2a50d587e64f38324 branch3
    fa8b79edc5dfff21753c2ccfc1a1828336c4c070 branch4~1

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.

---
 Documentation/git-describe.txt   |   5 ++
 Documentation/git-name-rev.txt   |   4 ++
 builtin/describe.c               |   7 +-
 builtin/name-rev.c               | 115 +++++++++++++++++++++++++++++--
 t/t6121-describe-cherry-picks.sh |  63 +++++++++++++++++
 5 files changed, 188 insertions(+), 6 deletions(-)
 create mode 100755 t/t6121-describe-cherry-picks.sh

index e027fb8c4..13a229bd7 100644
@@ -60,6 +60,11 @@ OPTIONS
 	the tag that comes after the commit, and thus contains it.
 	Automatically implies --tags.
 
+--cherry-picks::
+	Also show the commits cherry-picked from the target commits.
+	Cherry-picks are shown indented below their from-commmits.
+	Can only be used with --contains.
+
 --abbrev=<n>::
 	Instead of using the default 7 hexadecimal digits as the
 	abbreviated object name, use <n> digits, or as many digits

index 5cb0eb085..df16c4a89 100644
@@ -61,6 +61,10 @@ OPTIONS
 --always::
 	Show uniquely abbreviated commit object as fallback.
 
+--cherry-picks::
+	Also show the commits cherry-picked from the target commits.
+	Cherry-picks are shown indented below their from-commmits.
+
 EXAMPLES
 --------
 

index 1e87f68d5..94c84004d 100644
@@ -528,9 +528,10 @@ static void describe(const char *arg, int last_one)
 
 int cmd_describe(int argc, const char **argv, const char *prefix)
 {
-	int contains = 0;
+	int contains = 0, cherry_picks = 0;
 	struct option options[] = {
 		OPT_BOOL(0, "contains",   &contains, N_("find the tag that comes after the commit")),
+		OPT_BOOL(0, "cherry-picks", &cherry_picks, N_("also include cherry-picks with --contains")),
 		OPT_BOOL(0, "debug",      &debug, N_("debug search strategy on stderr")),
 		OPT_BOOL(0, "all",        &all, N_("use any ref")),
 		OPT_BOOL(0, "tags",       &tags, N_("use any tag, even unannotated")),

@@ -570,6 +571,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 
 	if (longformat && abbrev == 0)
 		die(_("--long is incompatible with --abbrev=0"));
+	if (cherry_picks && !contains)
+		die(_("--cherry-picks can only be used with --contains"));
 
 	if (contains) {
 		struct string_list_item *item;

@@ -579,6 +582,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 		argv_array_pushl(&args, "name-rev",
 				 "--peel-tag", "--name-only", "--no-undefined",
 				 NULL);
+		if (cherry_picks)
+			argv_array_push(&args, "--cherry-picks");
 		if (always)
 			argv_array_push(&args, "--always");
 		if (!all) {

2.17.1

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

"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".

Hello, Junio.

Ah, right, sorry about that.

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

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
 o----o----o----A----o----o..........................................o master
       \.----o----o....A'----o----o.................o v4.16-prod
              \          \.----o----o v4.16-prod-r2
               \ .----o----A'' 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

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.

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

Thanks.

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

Hello, Jeff.

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.

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?

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

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

... 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.