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

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

[Tejun Heo]

From aefa07bc66bb4a116eb84eb46d7f070f9632c990 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
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.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 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

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e027fb8c4..13a229bd7 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -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
diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index 5cb0eb085..df16c4a89 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -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
 --------
 
diff --git a/builtin/describe.c b/builtin/describe.c
index 1e87f68d5..94c84004d 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -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) {
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 0eb440359..7b21556ad 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -7,9 +7,13 @@
 #include "parse-options.h"
 #include "sha1-lookup.h"
 #include "commit-slab.h"
+#include "trailer.h"
+#include "object-store.h"
 
 #define CUTOFF_DATE_SLOP 86400 /* one day */
 
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
+
 typedef struct rev_name {
 	const char *tip_name;
 	timestamp_t taggerdate;
@@ -19,9 +23,12 @@ typedef struct rev_name {
 } rev_name;
 
 define_commit_slab(commit_rev_name, struct rev_name *);
+define_commit_slab(commit_cherry_picks, struct object_array *);
 
 static timestamp_t cutoff = TIME_MAX;
 static struct commit_rev_name rev_names;
+static struct commit_cherry_picks cherry_picks;
+static int do_cherry_picks = 0;
 
 /* How many generations are maximally preferred over _one_ merge traversal? */
 #define MERGE_TRAVERSAL_WEIGHT 65535
@@ -38,6 +45,26 @@ static void set_commit_rev_name(struct commit *commit, struct rev_name *name)
 	*commit_rev_name_at(&rev_names, commit) = name;
 }
 
+static struct object_array *get_commit_cherry_picks(struct commit *commit)
+{
+	struct object_array **slot =
+		commit_cherry_picks_peek(&cherry_picks, commit);
+
+	return slot ? *slot : NULL;
+}
+
+static struct object_array *get_create_commit_cherry_picks(struct commit *commit)
+{
+	struct object_array **slot =
+		commit_cherry_picks_at(&cherry_picks, commit);
+
+	if (!*slot) {
+		*slot = xmalloc(sizeof(struct object_array));
+		**slot = (struct object_array)OBJECT_ARRAY_INIT;
+	}
+	return *slot;
+}
+
 static int is_better_name(struct rev_name *name,
 			  const char *tip_name,
 			  timestamp_t taggerdate,
@@ -76,6 +103,47 @@ static int is_better_name(struct rev_name *name,
 	return 0;
 }
 
+static void record_cherry_pick(struct commit *commit)
+{
+	enum object_type type;
+	unsigned long size;
+	void *buffer;
+	struct trailer_info info;
+	int i;
+
+	buffer = read_object_file(&commit->object.oid, &type, &size);
+	trailer_info_get(&info, buffer);
+
+	/* when nested, the last trailer describes the latest cherry-pick */
+	for (i = info.trailer_nr - 1; i >= 0; i--) {
+		const int prefix_len = sizeof(cherry_picked_prefix) - 1;
+		char *line = info.trailers[i];
+
+		if (!strncmp(line, cherry_picked_prefix, prefix_len)) {
+			struct object_id from_oid;
+			struct object *from_object;
+			struct commit *from_commit;
+			struct object_array *from_cps;
+
+			if (get_oid_hex(line + prefix_len, &from_oid)) {
+				fprintf(stderr, "Could not get sha1 from %s", line);
+				break;
+			}
+
+			from_object = parse_object(&from_oid);
+			if (!from_object || from_object->type != OBJ_COMMIT)
+				break;
+
+			from_commit = (struct commit *)from_object;
+			from_cps = get_create_commit_cherry_picks(from_commit);
+			add_object_array(&commit->object, NULL, from_cps);
+			break;
+		}
+	}
+
+	free(buffer);
+}
+
 static void name_rev(struct commit *commit,
 		const char *tip_name, timestamp_t taggerdate,
 		int generation, int distance, int from_tag,
@@ -91,6 +159,10 @@ static void name_rev(struct commit *commit,
 	if (commit->date < cutoff)
 		return;
 
+	/* if a cherry pick we see for the first time, remember it */
+	if (do_cherry_picks && !name)
+		record_cherry_pick(commit);
+
 	if (deref) {
 		tip_name = to_free = xstrfmt("%s^0", tip_name);
 
@@ -402,6 +474,32 @@ static void name_rev_line(char *p, struct name_ref_data *data)
 	strbuf_release(&buf);
 }
 
+static void show_cherry_picks(struct object *obj, int always,
+			      int allow_undefined, int name_only, int level)
+{
+	struct object_array *cps;
+	int i;
+
+	if (obj->type != OBJ_COMMIT)
+		return;
+
+	cps = get_commit_cherry_picks((struct commit *)obj);
+	if (!cps)
+		return;
+
+	for (i = 0; i < cps->nr; i++) {
+		struct object *cherry_pick = cps->objects[i].item;
+		int j;
+
+		for (j = 0; j < level; j++)
+			fputs("  ", stdout);
+
+		show_name(cherry_pick, NULL, always, allow_undefined, name_only);
+		show_cherry_picks(cherry_pick, always, allow_undefined,
+				  name_only, level + 1);
+	}
+}
+
 int cmd_name_rev(int argc, const char **argv, const char *prefix)
 {
 	struct object_array revs = OBJECT_ARRAY_INIT;
@@ -420,6 +518,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
 		OPT_BOOL(0, "always",     &always,
 			   N_("show abbreviated commit object as fallback")),
+		OPT_BOOL(0, "cherry-picks", &do_cherry_picks, N_("include cherry-picked commits")),
 		{
 			/* A Hidden OPT_BOOL */
 			OPTION_SET_INT, 0, "peel-tag", &peel_tag, NULL,
@@ -430,6 +529,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 	};
 
 	init_commit_rev_name(&rev_names);
+	init_commit_cherry_picks(&cherry_picks);
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
 	if (all + transform_stdin + !!argc > 1) {
@@ -464,10 +564,9 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 
-		if (commit) {
+		if (commit)
 			if (cutoff > commit->date)
 				cutoff = commit->date;
-		}
 
 		if (peel_tag) {
 			if (!commit) {
@@ -506,9 +605,17 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 		}
 	} else {
 		int i;
-		for (i = 0; i < revs.nr; i++)
-			show_name(revs.objects[i].item, revs.objects[i].name,
-				  always, allow_undefined, data.name_only);
+		for (i = 0; i < revs.nr; i++) {
+			struct object *obj = revs.objects[i].item;
+			const char *name = revs.objects[i].name;
+
+			show_name(obj, name, always, allow_undefined,
+				  data.name_only);
+
+			if (do_cherry_picks)
+				show_cherry_picks(obj, always, allow_undefined,
+						  data.name_only, 1);
+		}
 	}
 
 	UNLEAK(revs);
diff --git a/t/t6121-describe-cherry-picks.sh b/t/t6121-describe-cherry-picks.sh
new file mode 100755
index 000000000..838e0acc0
--- /dev/null
+++ b/t/t6121-describe-cherry-picks.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+test_description='git describe should show cherry-picks correctly
+
+           C
+ o----o----x
+      |\ 
+      | .--o
+      |\  C1
+      | .--o
+       \  C2
+        .--o
+          C3
+
+C1 and C3 are cherry-picks from C, and C2 from C1.  Verify git desribe
+handles c and its cherry-picks correctly.
+'
+. ./test-lib.sh
+
+GIT_AUTHOR_EMAIL=bogus_email_address
+export GIT_AUTHOR_EMAIL
+
+test_expect_success \
+    'prepare repository with topic branches with cherry-picks' \
+    'test_tick &&
+     echo First > A &&
+     git update-index --add A &&
+     git commit -m "Add A." &&
+
+     test_tick &&
+     git checkout -b T1 master &&
+     git checkout -b T2 master &&
+     git checkout -b T3 master &&
+     git checkout master &&
+
+     test_tick &&
+     echo Second > B &&
+     git update-index --add B &&
+     git commit -m "Add B." &&
+
+     test_tick &&
+     git checkout -f T1 &&
+     rm -f B &&
+     git cherry-pick -x master &&
+
+     test_tick &&
+     git checkout -f T2 &&
+     rm -f B &&
+     git cherry-pick -x T1 &&
+
+     test_tick &&
+     git checkout -f T3 &&
+     rm -f B &&
+     git cherry-pick -x master
+'
+
+test_expect_success 'Verify describing cherry-picks' '
+     git describe --contains --all --cherry-picks master >actual &&
+     echo -e "master\n  T1\n    T2\n  T3" >expect &&
+     test_cmp expect actual
+'
+
+test_done
-- 
2.17.1

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

[Tejun Heo]

From a6a88c3da252d69547ac8b463098fc4f4c03f322 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
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.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 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

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e027fb8c4..13a229bd7 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -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
diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index 5cb0eb085..df16c4a89 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -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
 --------
 
diff --git a/builtin/describe.c b/builtin/describe.c
index 1e87f68d5..94c84004d 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -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) {
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 0eb440359..adffae0fe 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -7,9 +7,13 @@
 #include "parse-options.h"
 #include "sha1-lookup.h"
 #include "commit-slab.h"
+#include "trailer.h"
+#include "object-store.h"
 
 #define CUTOFF_DATE_SLOP 86400 /* one day */
 
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
+
 typedef struct rev_name {
 	const char *tip_name;
 	timestamp_t taggerdate;
@@ -19,9 +23,12 @@ typedef struct rev_name {
 } rev_name;
 
 define_commit_slab(commit_rev_name, struct rev_name *);
+define_commit_slab(commit_cherry_picks, struct object_array *);
 
 static timestamp_t cutoff = TIME_MAX;
 static struct commit_rev_name rev_names;
+static struct commit_cherry_picks cherry_picks;
+static int do_cherry_picks = 0;
 
 /* How many generations are maximally preferred over _one_ merge traversal? */
 #define MERGE_TRAVERSAL_WEIGHT 65535
@@ -38,6 +45,26 @@ static void set_commit_rev_name(struct commit *commit, struct rev_name *name)
 	*commit_rev_name_at(&rev_names, commit) = name;
 }
 
+static struct object_array *get_commit_cherry_picks(struct commit *commit)
+{
+	struct object_array **slot =
+		commit_cherry_picks_peek(&cherry_picks, commit);
+
+	return slot ? *slot : NULL;
+}
+
+static struct object_array *get_create_commit_cherry_picks(struct commit *commit)
+{
+	struct object_array **slot =
+		commit_cherry_picks_at(&cherry_picks, commit);
+
+	if (!*slot) {
+		*slot = xmalloc(sizeof(struct object_array));
+		**slot = (struct object_array)OBJECT_ARRAY_INIT;
+	}
+	return *slot;
+}
+
 static int is_better_name(struct rev_name *name,
 			  const char *tip_name,
 			  timestamp_t taggerdate,
@@ -76,6 +103,45 @@ static int is_better_name(struct rev_name *name,
 	return 0;
 }
 
+static void record_cherry_pick(struct commit *commit)
+{
+	enum object_type type;
+	unsigned long size;
+	void *buffer;
+	struct trailer_info info;
+	int i;
+
+	buffer = read_object_file(&commit->object.oid, &type, &size);
+	trailer_info_get(&info, buffer);
+
+	/* when nested, the last trailer describes the latest cherry-pick */
+	for (i = info.trailer_nr - 1; i >= 0; i--) {
+		const int prefix_len = sizeof(cherry_picked_prefix) - 1;
+		char *line = info.trailers[i];
+
+		if (!strncmp(line, cherry_picked_prefix, prefix_len)) {
+			struct object_id from_oid;
+			struct object *from_object;
+			struct commit *from_commit;
+			struct object_array *from_cps;
+
+			if (get_oid_hex(line + prefix_len, &from_oid))
+				continue;
+
+			from_object = parse_object(&from_oid);
+			if (!from_object || from_object->type != OBJ_COMMIT)
+				continue;
+
+			from_commit = (struct commit *)from_object;
+			from_cps = get_create_commit_cherry_picks(from_commit);
+			add_object_array(&commit->object, NULL, from_cps);
+			break;
+		}
+	}
+
+	free(buffer);
+}
+
 static void name_rev(struct commit *commit,
 		const char *tip_name, timestamp_t taggerdate,
 		int generation, int distance, int from_tag,
@@ -91,6 +157,10 @@ static void name_rev(struct commit *commit,
 	if (commit->date < cutoff)
 		return;
 
+	/* if a cherry pick we see for the first time, remember it */
+	if (do_cherry_picks && !name)
+		record_cherry_pick(commit);
+
 	if (deref) {
 		tip_name = to_free = xstrfmt("%s^0", tip_name);
 
@@ -402,6 +472,32 @@ static void name_rev_line(char *p, struct name_ref_data *data)
 	strbuf_release(&buf);
 }
 
+static void show_cherry_picks(struct object *obj, int always,
+			      int allow_undefined, int name_only, int level)
+{
+	struct object_array *cps;
+	int i;
+
+	if (obj->type != OBJ_COMMIT)
+		return;
+
+	cps = get_commit_cherry_picks((struct commit *)obj);
+	if (!cps)
+		return;
+
+	for (i = 0; i < cps->nr; i++) {
+		struct object *cherry_pick = cps->objects[i].item;
+		int j;
+
+		for (j = 0; j < level; j++)
+			fputs("  ", stdout);
+
+		show_name(cherry_pick, NULL, always, allow_undefined, name_only);
+		show_cherry_picks(cherry_pick, always, allow_undefined,
+				  name_only, level + 1);
+	}
+}
+
 int cmd_name_rev(int argc, const char **argv, const char *prefix)
 {
 	struct object_array revs = OBJECT_ARRAY_INIT;
@@ -420,6 +516,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
 		OPT_BOOL(0, "always",     &always,
 			   N_("show abbreviated commit object as fallback")),
+		OPT_BOOL(0, "cherry-picks", &do_cherry_picks, N_("include cherry-picked commits")),
 		{
 			/* A Hidden OPT_BOOL */
 			OPTION_SET_INT, 0, "peel-tag", &peel_tag, NULL,
@@ -430,6 +527,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 	};
 
 	init_commit_rev_name(&rev_names);
+	init_commit_cherry_picks(&cherry_picks);
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
 	if (all + transform_stdin + !!argc > 1) {
@@ -464,10 +562,9 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 
-		if (commit) {
+		if (commit)
 			if (cutoff > commit->date)
 				cutoff = commit->date;
-		}
 
 		if (peel_tag) {
 			if (!commit) {
@@ -506,9 +603,17 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 		}
 	} else {
 		int i;
-		for (i = 0; i < revs.nr; i++)
-			show_name(revs.objects[i].item, revs.objects[i].name,
-				  always, allow_undefined, data.name_only);
+		for (i = 0; i < revs.nr; i++) {
+			struct object *obj = revs.objects[i].item;
+			const char *name = revs.objects[i].name;
+
+			show_name(obj, name, always, allow_undefined,
+				  data.name_only);
+
+			if (do_cherry_picks)
+				show_cherry_picks(obj, always, allow_undefined,
+						  data.name_only, 1);
+		}
 	}
 
 	UNLEAK(revs);
diff --git a/t/t6121-describe-cherry-picks.sh b/t/t6121-describe-cherry-picks.sh
new file mode 100755
index 000000000..838e0acc0
--- /dev/null
+++ b/t/t6121-describe-cherry-picks.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+test_description='git describe should show cherry-picks correctly
+
+           C
+ o----o----x
+      |\ 
+      | .--o
+      |\  C1
+      | .--o
+       \  C2
+        .--o
+          C3
+
+C1 and C3 are cherry-picks from C, and C2 from C1.  Verify git desribe
+handles c and its cherry-picks correctly.
+'
+. ./test-lib.sh
+
+GIT_AUTHOR_EMAIL=bogus_email_address
+export GIT_AUTHOR_EMAIL
+
+test_expect_success \
+    'prepare repository with topic branches with cherry-picks' \
+    'test_tick &&
+     echo First > A &&
+     git update-index --add A &&
+     git commit -m "Add A." &&
+
+     test_tick &&
+     git checkout -b T1 master &&
+     git checkout -b T2 master &&
+     git checkout -b T3 master &&
+     git checkout master &&
+
+     test_tick &&
+     echo Second > B &&
+     git update-index --add B &&
+     git commit -m "Add B." &&
+
+     test_tick &&
+     git checkout -f T1 &&
+     rm -f B &&
+     git cherry-pick -x master &&
+
+     test_tick &&
+     git checkout -f T2 &&
+     rm -f B &&
+     git cherry-pick -x T1 &&
+
+     test_tick &&
+     git checkout -f T3 &&
+     rm -f B &&
+     git cherry-pick -x master
+'
+
+test_expect_success 'Verify describing cherry-picks' '
+     git describe --contains --all --cherry-picks master >actual &&
+     echo -e "master\n  T1\n    T2\n  T3" >expect &&
+     test_cmp expect actual
+'
+
+test_done
-- 
2.17.1

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

[Junio C Hamano]

Tejun Heo <tj@kernel.org> writes:

> From a6a88c3da252d69547ac8b463098fc4f4c03f322 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> 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
>       82cddd79f962de0bb1e7cdd95d48b48633335816 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.

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

[Tejun Heo]

Hello, Junio.

On Thu, Jul 26, 2018 at 08:12:45AM -0700, Junio C Hamano wrote:
> Tejun Heo <tj@kernel.org> writes:
> 
> > From a6a88c3da252d69547ac8b463098fc4f4c03f322 Mon Sep 17 00:00:00 2001
> > From: Tejun Heo <tj@kernel.org>
> > 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
> >       82cddd79f962de0bb1e7cdd95d48b48633335816 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
 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

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

[Junio C Hamano]

Tejun Heo <tj@kernel.org> 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

[Jeff King]

On Thu, Jul 26, 2018 at 12:01:34PM -0700, Junio C Hamano wrote:

> Tejun Heo <tj@kernel.org> 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

[Tejun Heo]

Hello, Jeff.

On Fri, Jul 27, 2018 at 4:47 AM Jeff King <peff@peff.net> 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

[Junio C Hamano]

Tejun Heo <tj@kernel.org> 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.