[PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Tejun Heo]

Hello,

Some trailers refer to other commits.  Let's call them xrefs
(cross-references).  For example, a cherry pick trailer points to the
source commit.  It is sometimes useful to build a reverse map of these
xrefs - ie. source -> cherry-pick instead of cherry-pick -> source.

This, e.g, can answer which releases a commit ended up in on
repositories which cherry-picks fixes back from the development
branch.  Let's say the repository looks like the following.

	  D'---E'' release-B
	 /
	C'      E' release-D
       /       /
  A---B---C---D---E master

where the followings cherry-picks took place.

  C -> C'
  D -> D'
  E -> E' -> E''

Using the reverse-mapping in this patchset, we can get the following
annotated log which succinctly shows which commit ended up where.

  $ git log --notes=xref-cherry-picks --oneline | git name-rev --name-only --stdin
  4b165af commit E
  Notes (xref-cherry-picks):
      Cherry-picked-to: release-D
      Cherry-picked-to:   release-B

  82bf1f3 commit D
  Notes (xref-cherry-picks):
      Cherry-picked-to: release-B~1

  364eccf commit C
  Notes (xref-cherry-picks):
      Cherry-picked-to: release-B~2

  ed3adf3 commit B
  ddd1bf2 commit A

This patchset implements generic trailer cross-reference
reverse-mapping and the necessary hooks and samples to keep
cherry-pick reverse-maps up-to-date.

diffstat follows.  Thanks.

 Documentation/git-cherry-pick.txt           |    5 
 Documentation/git-fetch.txt                 |    5 
 Documentation/git-notes.txt                 |    8 
 Documentation/git-reverse-trailer-xrefs.txt |  145 +++++++++++++++
 Documentation/githooks.txt                  |   23 ++
 Makefile                                    |    1 
 builtin.h                                   |    1 
 builtin/fetch.c                             |   72 +++++++
 builtin/reverse-trailer-xrefs.c             |  160 +++++++++++++++++
 builtin/revert.c                            |   14 +
 git.c                                       |    1 
 notes-merge.c                               |    9 
 notes-utils.c                               |    2 
 notes-utils.h                               |    3 
 notes.c                                     |  260 +++++++++++++++++++++++++++-
 notes.h                                     |    9 
 t/t3321-notes-xref-cherry-picks.sh          |  124 +++++++++++++
 templates/hooks--post-cherry-pick.sample    |    8 
 templates/hooks--post-fetch.sample          |   30 +++
 trailer.c                                   |  105 +++++++++++
 trailer.h                                   |   38 ++++
 21 files changed, 1015 insertions(+), 8 deletions(-)

--
tejun

[PATCH 1/5] trailer: Implement a helper to reverse-map trailer xrefs

[Tejun Heo]

From: Tejun Heo <htejun@fb.com>

Some trailers refer to other commits.  Let's call them xrefs
(cross-references).  For example, a cherry pick trailer points to the
source commit.  It is sometimes useful to build a reverse map of these
xrefs - ie. source -> cherry-pick instead of cherry-pick -> source.

This patch implements trailer helpers to enable such reverse maps.

The helpers can process arbitrary trailers and once the xrefs are
recorded, the reverse-mapping can be iterated per source commit using
trailer_rev_xrefs_for_each().

Signed-off-by: Tejun Heo <htejun@fb.com>
---
 trailer.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 trailer.h |  38 ++++++++++++++++++++
 2 files changed, 143 insertions(+)

diff --git a/trailer.c b/trailer.c
index 0796f326b..15744600b 100644
--- a/trailer.c
+++ b/trailer.c
@@ -2,6 +2,7 @@
 #include "config.h"
 #include "string-list.h"
 #include "run-command.h"
+#include "object-store.h"
 #include "commit.h"
 #include "tempfile.h"
 #include "trailer.h"
@@ -1170,3 +1171,107 @@ void format_trailers_from_commit(struct strbuf *out, const char *msg,
 	format_trailer_info(out, &info, opts);
 	trailer_info_release(&info);
 }
+
+implement_static_commit_slab(trailer_rxrefs_slab, struct object_array *);
+
+static struct object_array *get_trailer_rxrefs(
+			struct trailer_rev_xrefs *rxrefs, struct commit *commit)
+{
+	struct object_array **slot =
+		trailer_rxrefs_slab_peek(&rxrefs->slab, commit);
+
+	return slot ? *slot : NULL;
+}
+
+static struct object_array *get_create_trailer_rxrefs(
+			struct trailer_rev_xrefs *rxrefs, struct commit *commit)
+{
+	struct object_array **slot =
+		trailer_rxrefs_slab_at(&rxrefs->slab, commit);
+
+	if (*slot)
+		return *slot;
+
+	add_object_array(&commit->object, oid_to_hex(&commit->object.oid),
+			 &rxrefs->dst_commits);
+	*slot = xmalloc(sizeof(struct object_array));
+	**slot = (struct object_array)OBJECT_ARRAY_INIT;
+	return *slot;
+}
+
+void trailer_rev_xrefs_init(struct trailer_rev_xrefs *rxrefs,
+			    const char *trailer_prefix)
+{
+	rxrefs->trailer_prefix = xstrdup(trailer_prefix);
+	rxrefs->trailer_prefix_len = strlen(trailer_prefix);
+	init_trailer_rxrefs_slab(&rxrefs->slab);
+	rxrefs->dst_commits = (struct object_array)OBJECT_ARRAY_INIT;
+}
+
+/* record the reverse mapping of @commit's xref trailer */
+void trailer_rev_xrefs_record(struct trailer_rev_xrefs *rxrefs,
+			      struct commit *commit)
+{
+	struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
+	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, &opts);
+
+	/* when nested, the last trailer describes the latest cherry-pick */
+	for (i = info.trailer_nr - 1; i >= 0; i--) {
+		char *line = info.trailers[i];
+
+		if (starts_with(line, rxrefs->trailer_prefix)) {
+			struct object_id dst_oid;
+			struct object *dst_object;
+			struct commit *dst_commit;
+			struct object_array *src_objs;
+			char cherry_hex[GIT_MAX_HEXSZ + 1];
+
+			if (get_oid_hex(line + rxrefs->trailer_prefix_len,
+					&dst_oid))
+				continue;
+
+			dst_object = parse_object(the_repository, &dst_oid);
+			if (!dst_object || dst_object->type != OBJ_COMMIT)
+				continue;
+
+			dst_commit = (struct commit *)dst_object;
+			src_objs = get_create_trailer_rxrefs(rxrefs, dst_commit);
+
+			oid_to_hex_r(cherry_hex, &commit->object.oid);
+			add_object_array(&commit->object, cherry_hex, src_objs);
+			break;
+		}
+	}
+
+	free(buffer);
+}
+
+void trailer_rev_xrefs_release(struct trailer_rev_xrefs *rxrefs)
+{
+	clear_trailer_rxrefs_slab(&rxrefs->slab);
+	object_array_clear(&rxrefs->dst_commits);
+	free(rxrefs->trailer_prefix);
+}
+
+void trailer_rev_xrefs_next(struct trailer_rev_xrefs *rxrefs, int *idx_p,
+			    struct commit **dst_commit_p,
+			    struct object_array **src_objs_p)
+{
+	if (*idx_p >= rxrefs->dst_commits.nr) {
+		*dst_commit_p = NULL;
+		*src_objs_p = NULL;
+		return;
+	}
+
+	*dst_commit_p = (struct commit *)
+		rxrefs->dst_commits.objects[*idx_p].item;
+	*src_objs_p = get_trailer_rxrefs(rxrefs, *dst_commit_p);
+	(*idx_p)++;
+}
diff --git a/trailer.h b/trailer.h
index b99773964..07090a11f 100644
--- a/trailer.h
+++ b/trailer.h
@@ -2,6 +2,8 @@
 #define TRAILER_H
 
 #include "list.h"
+#include "object.h"
+#include "commit-slab.h"
 
 struct strbuf;
 
@@ -99,4 +101,40 @@ void trailer_info_release(struct trailer_info *info);
 void format_trailers_from_commit(struct strbuf *out, const char *msg,
 				 const struct process_trailer_options *opts);
 
+/*
+ * Helpers to reverse trailers referencing to other commits.
+ *
+ * Some trailers, e.g. "(cherry picked from...)", references other commits.
+ * The following helpers can be used to reverse map those references.  See
+ * builtin/reverse-trailer-xrefs.c for a usage example.
+ */
+declare_commit_slab(trailer_rxrefs_slab, struct object_array *);
+
+struct trailer_rev_xrefs {
+	char *trailer_prefix;
+	int trailer_prefix_len;
+	struct trailer_rxrefs_slab slab;
+	struct object_array dst_commits;
+};
+
+void trailer_rev_xrefs_init(struct trailer_rev_xrefs *rxrefs,
+			    const char *trailer_prefix);
+void trailer_rev_xrefs_record(struct trailer_rev_xrefs *rxrefs,
+			      struct commit *commit);
+void trailer_rev_xrefs_release(struct trailer_rev_xrefs *rxrefs);
+
+void trailer_rev_xrefs_next(struct trailer_rev_xrefs *rxrefs,
+			    int *idx_p, struct commit **dst_commit_p,
+			    struct object_array **src_objs_p);
+
+/*
+ * Iterate the recorded reverse mappings - @dst_commit was pointed to by
+ * commits in @src_objs.
+ */
+#define trailer_rev_xrefs_for_each(rxrefs, idx, dst_commit, src_objs)		\
+	for ((idx) = 0,								\
+	     trailer_rev_xrefs_next(rxrefs, &(idx), &(dst_commit), &(src_objs));\
+	     (dst_commit);							\
+	     trailer_rev_xrefs_next(rxrefs, &(idx), &(dst_commit), &(src_objs)))
+
 #endif /* TRAILER_H */
-- 
2.17.1

[PATCH 2/5] notes: Implement special handlings for refs/notes/xref-

[Tejun Heo]

From: Tejun Heo <htejun@fb.com>

Some trailers refer to other commits.  Let's call them xrefs
(cross-references).  For example, a cherry pick trailer points to the
source commit.  It is sometimes useful to build a reverse map of these
xrefs - ie. source -> cherry-pick instead of cherry-pick -> source.

These reverse-maps will be recorded in special notes whose refs start
with refs/notes/xref-.  This patch implements the following special
handling for the xref notes.

* When xref notes are appended to an existing one, both parts get
  parsed and dead or dupliate references are dropped, so that the
  merged note contains only valid and unique xrefs.

* When xref notes are formatted for printing, the formatter recurses
  into each xref and prints the nested xrefs with increasing
  indentation to show the comprehensive xref chains.

The latter part will be documented by a future patch with the actual
use case.

Signed-off-by: Tejun Heo <htejun@fb.com>
---
 Documentation/git-notes.txt |   8 ++
 notes-merge.c               |   9 ++
 notes-utils.c               |   2 +
 notes-utils.h               |   3 +-
 notes.c                     | 260 +++++++++++++++++++++++++++++++++++-
 notes.h                     |   9 ++
 6 files changed, 288 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index df2b64dbb..872919ad4 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -88,6 +88,10 @@ the command can read the input given to the `post-rewrite` hook.)
 	Append to the notes of an existing object (defaults to HEAD).
 	Creates a new notes object if needed.
 
+	If the notes ref starts with `/refs/notes/xref-`, each
+	line is expected to contain `<tag>: <commit-id>` and
+	only lines with reachable unique commit IDs are kept.
+
 edit::
 	Edit the notes for a given object (defaults to HEAD).
 
@@ -273,6 +277,10 @@ Note that if either the local or remote version contain duplicate lines
 prior to the merge, these will also be removed by this notes merge
 strategy.
 
+"cat_xrefs" is similar to "union" but expects each line to contain
+`<tag>: <commit-id>`. Only lines with reachable unique commit IDs are
+kept.
+
 
 EXAMPLES
 --------
diff --git a/notes-merge.c b/notes-merge.c
index bd05d50b0..3fe2389a1 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -464,6 +464,15 @@ static int merge_one_change(struct notes_merge_options *o,
 			die("failed to concatenate notes "
 			    "(combine_notes_cat_sort_uniq)");
 		return 0;
+	case NOTES_MERGE_RESOLVE_CAT_XREFS:
+		if (o->verbosity >= 2)
+			printf("Concatenating unique and valid cross-references "
+			       "in local and remote notes for %s\n",
+			       oid_to_hex(&p->obj));
+		if (add_note(t, &p->obj, &p->remote, combine_notes_cat_xrefs))
+			die("failed to concatenate notes "
+			    "(combine_notes_cat_xrefs)");
+		return 0;
 	}
 	die("Unknown strategy (%i).", o->strategy);
 }
diff --git a/notes-utils.c b/notes-utils.c
index 14ea03178..db6363c39 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -70,6 +70,8 @@ int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s)
 		*s = NOTES_MERGE_RESOLVE_UNION;
 	else if (!strcmp(v, "cat_sort_uniq"))
 		*s = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
+	else if (!strcmp(v, "cat_xrefs"))
+		*s = NOTES_MERGE_RESOLVE_CAT_XREFS;
 	else
 		return -1;
 
diff --git a/notes-utils.h b/notes-utils.h
index 540830652..b604f9b51 100644
--- a/notes-utils.h
+++ b/notes-utils.h
@@ -28,7 +28,8 @@ enum notes_merge_strategy {
 		NOTES_MERGE_RESOLVE_OURS,
 		NOTES_MERGE_RESOLVE_THEIRS,
 		NOTES_MERGE_RESOLVE_UNION,
-		NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ
+		NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ,
+		NOTES_MERGE_RESOLVE_CAT_XREFS,
 };
 
 struct notes_rewrite_cfg {
diff --git a/notes.c b/notes.c
index 25cdce28b..835466787 100644
--- a/notes.c
+++ b/notes.c
@@ -9,6 +9,7 @@
 #include "tree-walk.h"
 #include "string-list.h"
 #include "refs.h"
+#include "hashmap.h"
 
 /*
  * Use a non-balancing simple 16-tree structure with struct int_node as
@@ -996,8 +997,12 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
 	if (!notes_ref)
 		notes_ref = default_notes_ref();
 
-	if (!combine_notes)
-		combine_notes = combine_notes_concatenate;
+	if (!combine_notes) {
+		if (starts_with(notes_ref, "refs/notes/xref-"))
+			combine_notes = combine_notes_cat_xrefs;
+		else
+			combine_notes = combine_notes_concatenate;
+	}
 
 	t->root = (struct int_node *) xcalloc(1, sizeof(struct int_node));
 	t->first_non_note = NULL;
@@ -1189,6 +1194,76 @@ void free_notes(struct notes_tree *t)
 	memset(t, 0, sizeof(struct notes_tree));
 }
 
+/*
+ * Parse a "[TAG:]HEX" line.  @line is trimmed.  If @tag_p is not NULL and
+ * TAG exists, the string is split.  Returns the pointer to OID and updates
+ * *@tag_p to point to TAG.
+ */
+static char *parse_xref(char *line, char **tag_p)
+{
+	char *p, *hex;
+
+	/* ltrim */
+	while (isspace(*line))
+		line++;
+
+	p = strchr(line, ':');
+	if (p) {
+		if (tag_p) {
+			/* split and store TAG */
+			*tag_p = line;
+			*p = '\0';
+		}
+		/* trim whitespaces after ':' */
+		p++;
+		while (isspace(*p))
+			p++;
+		hex = p;
+	} else {
+		if (tag_p)
+			*tag_p = NULL;
+		hex = line;
+	}
+
+	/* rtrim */
+	p = hex;
+	while (*p != '\0' && !isspace(*p))
+		p++;
+	*p = '\0';
+	return hex;
+}
+
+static void walk_xrefs(const char *tree_ref, struct object_id *from_oid,
+		       struct strbuf *sb, int level)
+{
+	struct object_array xrefs = OBJECT_ARRAY_INIT;
+	struct string_list lines = STRING_LIST_INIT_DUP;
+	int i;
+
+	read_xref_note(tree_ref, from_oid, &xrefs, &lines);
+
+	for (i = 0; i < xrefs.nr; i++) {
+		char *line = lines.items[i].string;
+		char *tag;
+
+		parse_xref(line, &tag);
+		strbuf_addf(sb, "    %s%s%*s%s\n",
+			    tag ?: "", tag ? ": " : "", 2 * level, "",
+			    xrefs.objects[i].name);
+		if (xrefs.objects[i].item) {
+			if (level < 32)
+				walk_xrefs(tree_ref, &xrefs.objects[i].item->oid,
+					   sb, level + 1);
+			else
+				warning("xref nested deeper than %d levels, terminating walk",
+					level);
+		}
+	}
+
+	object_array_clear(&xrefs);
+	string_list_clear(&lines, 0);
+}
+
 /*
  * Fill the given strbuf with the notes associated with the given object.
  *
@@ -1208,6 +1283,7 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
 	char *msg, *msg_p;
 	unsigned long linelen, msglen;
 	enum object_type type;
+	int format_xrefs;
 
 	if (!t)
 		t = &default_notes_tree;
@@ -1250,6 +1326,8 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
 		}
 	}
 
+	format_xrefs = !raw && starts_with(t->ref, "refs/notes/xref-");
+
 	for (msg_p = msg; msg_p < msg + msglen; msg_p += linelen + 1) {
 		linelen = strchrnul(msg_p, '\n') - msg_p;
 
@@ -1257,6 +1335,14 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
 			strbuf_addstr(sb, "    ");
 		strbuf_add(sb, msg_p, linelen);
 		strbuf_addch(sb, '\n');
+
+		if (format_xrefs) {
+			struct object_id oid;
+
+			msg_p[linelen] = '\0';
+			if (!get_oid_hex(parse_xref(msg_p, NULL), &oid))
+				walk_xrefs(t->ref, &oid, sb, 1);
+		}
 	}
 
 	free(msg);
@@ -1309,3 +1395,173 @@ void expand_loose_notes_ref(struct strbuf *sb)
 		expand_notes_ref(sb);
 	}
 }
+
+/*
+ * Parse a cross-referencing note.
+ *
+ * @note contains lines of "[TAG:]HEX" pointing to other commits.  Read the
+ * target commits and add the objects to @result.  If @result_lines is not
+ * NULL, it should point to a strdup'ing string_list.  The verbatim note
+ * lines matching the target commits are appened to the list.
+ */
+static void parse_xref_note(const char *note, unsigned long size,
+			    const struct object_id *commit_oid,
+			    struct object_array *result,
+			    struct string_list *result_lines)
+{
+	struct strbuf **lines, **pline;
+
+	lines = strbuf_split_buf(note, size, '\n', 0);
+
+	for (pline = lines; *pline; pline++) {
+		struct strbuf *line = *pline;
+		const char *target_hex;
+		struct object_id target_oid;
+		struct object *target_obj;
+
+		target_hex = parse_xref(line->buf, NULL);
+		if (get_oid_hex(target_hex, &target_oid)) {
+			if (commit_oid)
+				warning("read invalid sha1 on %s: %s",
+					oid_to_hex(commit_oid), line->buf);
+			continue;
+		}
+
+		target_obj = parse_object(the_repository, &target_oid);
+		if (!target_obj || target_obj->type != OBJ_COMMIT) {
+			if (commit_oid)
+				warning("read invalid commit on %s: %s",
+					oid_to_hex(commit_oid), line->buf);
+			continue;
+		}
+
+		add_object_array(target_obj, target_hex, result);
+		if (result_lines) {
+			assert(result_lines->strdup_strings);
+			string_list_append(result_lines, line->buf);
+		}
+	}
+
+	strbuf_list_free(lines);
+}
+
+struct notes_tree_entry {
+	struct hashmap_entry ent;
+	struct notes_tree tree;
+};
+
+static int notes_tree_cmp(const void *hashmap_cmp_fn_data,
+			  const void *entry, const void *entry_or_key,
+			  const void *keydata)
+{
+	const struct notes_tree_entry *e1 = entry;
+	const struct notes_tree_entry *e2 = entry_or_key;
+
+	return strcmp(e1->tree.ref, e2->tree.ref);
+}
+
+/*
+ * Read and parse a cross-referencing note.
+ *
+ * Read the @notes_ref note of @commit_oid and parse it with
+ * parse_xref_note().
+ */
+void read_xref_note(const char *notes_ref, const struct object_id *commit_oid,
+		    struct object_array *result,
+		    struct string_list *result_lines)
+{
+	static struct hashmap *notes_tree_map = NULL;
+	unsigned hash = memhash(notes_ref, strlen(notes_ref));
+	struct notes_tree_entry key, *ent;
+	const struct object_id *note_oid;
+	unsigned long size;
+	enum object_type type;
+	char *note;
+
+	if (!notes_tree_map) {
+		notes_tree_map = xcalloc(1, sizeof(struct hashmap));
+		hashmap_init(notes_tree_map, notes_tree_cmp, NULL, 0);
+	}
+
+	hashmap_entry_init(&key.ent, hash);
+	key.tree.ref = (char *)notes_ref;
+	ent = hashmap_get(notes_tree_map, &key, NULL);
+	if (!ent) {
+		ent = xcalloc(1, sizeof(struct notes_tree_entry));
+		init_notes(&ent->tree, notes_ref, NULL, 0);
+		hashmap_entry_init(&ent->ent, hash);
+		hashmap_put(notes_tree_map, ent);
+	}
+
+	note_oid = get_note(&ent->tree, commit_oid);
+	if (!note_oid)
+		return;
+
+	note = read_object_file(note_oid, &type, &size);
+	if (!size) {
+		free(note);
+		return;
+	}
+
+	parse_xref_note(note, size, commit_oid, result, result_lines);
+	free(note);
+}
+
+/*
+ * Combine a xref note in @new_oid into @cur_oid.  Unreachable or duplicate
+ * xrefs are dropped.  This is the default combine_notes callback for
+ * refs/notes/xref-.
+ */
+int combine_notes_cat_xrefs(struct object_id *cur_oid,
+			    const struct object_id *new_oid)
+{
+	char *cur_msg = NULL, *new_msg = NULL;
+	unsigned long cur_len, new_len;
+	enum object_type cur_type, new_type;
+	struct object_array xrefs = OBJECT_ARRAY_INIT;
+	struct string_list lines = STRING_LIST_INIT_DUP;
+	struct strbuf output = STRBUF_INIT;
+	int i, j, cur_nr, ret;
+
+	/* read in both note blob objects */
+	if (!is_null_oid(new_oid))
+		new_msg = read_object_file(new_oid, &new_type, &new_len);
+	if (!new_msg || !new_len || new_type != OBJ_BLOB) {
+		free(new_msg);
+		return 0;
+	}
+	if (!is_null_oid(cur_oid))
+		cur_msg = read_object_file(cur_oid, &cur_type, &cur_len);
+	if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
+		free(cur_msg);
+		free(new_msg);
+		oidcpy(cur_oid, new_oid);
+		return 0;
+	}
+
+	/* parse xrefs and de-dup */
+	parse_xref_note(cur_msg, cur_len, NULL, &xrefs, &lines);
+	cur_nr = xrefs.nr;
+	parse_xref_note(new_msg, new_len, NULL, &xrefs, &lines);
+
+	for (i = 0; i < cur_nr; i++)
+		for (j = cur_nr; j < xrefs.nr; j++)
+			if (!strcmp(xrefs.objects[i].name,
+				    xrefs.objects[j].name))
+				lines.items[j].string[0] = '\0';
+
+	/* write out the combined object */
+	for (i = 0; i < lines.nr; i++)
+		if (lines.items[i].string[0] != '\0')
+			strbuf_addf(&output, "%s\n", lines.items[i].string);
+
+	ret = write_object_file(output.buf, output.len, blob_type, cur_oid);
+
+	strbuf_release(&output);
+	object_array_clear(&xrefs);
+	string_list_clear(&lines, 0);
+	free(cur_msg);
+	free(new_msg);
+
+	return ret;
+}
diff --git a/notes.h b/notes.h
index 414bc6855..09ca92f36 100644
--- a/notes.h
+++ b/notes.h
@@ -2,6 +2,7 @@
 #define NOTES_H
 
 #include "string-list.h"
+#include "object.h"
 
 struct object_id;
 struct strbuf;
@@ -317,4 +318,12 @@ void expand_notes_ref(struct strbuf *sb);
  */
 void expand_loose_notes_ref(struct strbuf *sb);
 
+/* For refs/notes/xref- */
+void read_xref_note(const char *notes_ref, const struct object_id *commit_oid,
+		    struct object_array *result,
+		    struct string_list *result_lines);
+
+int combine_notes_cat_xrefs(struct object_id *cur_oid,
+			    const struct object_id *new_oid);
+
 #endif
-- 
2.17.1

[PATCH 3/5] notes: Implement git-reverse-trailer-xrefs

[Tejun Heo]

From: Tejun Heo <htejun@fb.com>

Some trailers refer to other commits.  Let's call them xrefs
(cross-references).  For example, a cherry pick trailer points to the
source commit.  It is sometimes useful to build a reverse map of these
xrefs - ie. source -> cherry-pick instead of cherry-pick -> source.

This patch implements git-reverse-trailer-xrefs which can process a
given trailer for the specified commits and generate and update the
matching refs/notes/xref- notes.

The command reverse-maps trailers beginning with --trailer-prefix and
stores them in --ref notes with each line tagged with --tag.  It also
has a refs/notes/xref-cherry-picks preset for the three params.

Signed-off-by: Tejun Heo <htejun@fb.com>
---
 Documentation/git-reverse-trailer-xrefs.txt | 136 +++++++++++++++++
 Makefile                                    |   1 +
 builtin.h                                   |   1 +
 builtin/reverse-trailer-xrefs.c             | 160 ++++++++++++++++++++
 git.c                                       |   1 +
 5 files changed, 299 insertions(+)
 create mode 100644 Documentation/git-reverse-trailer-xrefs.txt
 create mode 100644 builtin/reverse-trailer-xrefs.c

diff --git a/Documentation/git-reverse-trailer-xrefs.txt b/Documentation/git-reverse-trailer-xrefs.txt
new file mode 100644
index 000000000..20d260486
--- /dev/null
+++ b/Documentation/git-reverse-trailer-xrefs.txt
@@ -0,0 +1,136 @@
+git-reverse-trailer-xrefs(1)
+============================
+
+NAME
+----
+git-reverse-trailer-xrefs - Record reverse-map of trailer commit references into notes
+
+SYNOPSIS
+--------
+[verse]
+`git reverse_trailer_xrefs` --xref-cherry-picks [--clear] [<options>] [<commit-ish>...]
+`git reverse_trailer_xrefs` --trailer-prefix=<prefix> --ref=<notes-ref> [--tag=<tag>] [<options>] [<commit-ish>...]
+`git reverse_trailer_xrefs` --ref=<notes-ref> --clear [<options>] [<commit-ish>...]
+
+
+DESCRIPTION
+-----------
+Record or clear reverse-map of trailer commit references in the
+specified notes ref.
+
+Some commit trailers reference other commits. For example,
+`git-cherry-pick -x` adds the following trailer to record the source
+commit.
+----------
+(cherry picked from commit <source-commit-id>)
+----------
+The reverse mappings of such cross references can be useful. For
+cherry-picks, it would allow finding all the cherry-picked commits of
+a given source commit.  `git-reverse-trailer-xrefs` can be used to
+create and maintain such reverse mappings in notes.
+
+When used with `--xref-cherry-picks`, the cherry-pick trailers are
+parsed from the specified commits and the reverse mappings are
+recorded in the `refs/notes/xref-cherry-picks` notes of the source
+commits in the following format.
+----------
+Cherry-picked-to: <destination-commit-id>
+----------
+
+When a note with its notes ref starting with `refs/notes/xref-` is
+formatted to be displayed with the commit for, e.g., `git-show` or
+`git-log`, the destination commit is followed recursively and the
+matching notes are shown with increasing level of indentations.
+
+`--trailer-prefix`, `--notes` and `--tag` can be used to use a custom
+set of trailer, notes ref and reverse mapping tag.
+
+OPTIONS
+-------
+<commit-ish>...::
+	Commit-ish object names to describe.  Defaults to HEAD if omitted.
+
+--xref-cherry-picks::
+	Use the preset to reverse map `git-cherry-pick -x`
+	trailers. `--trailer-prefix` is set to `(cherry-picked from
+	commit `, `--notes` is set to `refs/notes/xref-cherry-picks`
+	and `--tag` is set to `Cherry-picked-to`. This option can't be
+	specified with the three preset options.
+
+--trailer-prefix=<prefix>::
+	Process trailers which start with <prefix>. It is matched
+	character-by-character and should be followed by the
+	referenced commit ID. When there are multiple matching
+	trailers, the last one is used.
+
+--notes=<notes-ref>::
+	The notes ref to use for the reverse mapping. While this can
+	be any notes ref, it is recommented to use ones starting with
+	`refs/notes/xref-` as they are recognized as cross-referencing
+	notes and handled specially when updating and showing.
+
+--tag=<tag>::
+	Optional tag to use when generating reverse reference
+	notes. If specified, each note line is formatted as `<tag>:
+	<commit-id>`; otherwise, `<commit-id>`.
+
+--clear::
+	Instead of creating reverse mapping notes, clear them from the
+	specified commits.
+
+
+EXAMPLES
+--------
+
+Assume the following history. Development is happening in "master" and
+releases are branched off and fixes are cherry-picked into them.
+
+------------
+	D'---E'' release-B
+       /
+      C'      E' release-D
+     /       /
+A---B---C---D---E master
+------------
+
+The following cherry-picks took place.
+
+------------
+C -> C'
+D -> D'
+E -> E' -> E''
+------------
+
+The reverse mappings for all commits can be created using the
+following command.
+
+------------
+$ git reverse-trailer-xrefs --all --xref-cherry-picks
+------------
+
+With the notes added, where each commit ended up can be easily
+determined.
+
+------------
+$ git log --notes=xref-cherry-picks --oneline | git name-rev --name-only --stdin
+4b165af commit E
+Notes (xref-cherry-picks):
+    Cherry-picked-to: release-D
+    Cherry-picked-to:   release-B
+
+82bf1f3 commit D
+Notes (xref-cherry-picks):
+    Cherry-picked-to: release-B~1
+
+364eccf commit C
+Notes (xref-cherry-picks):
+    Cherry-picked-to: release-B~2
+
+ed3adf3 commit B
+ddd1bf2 commit A
+------------
+
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 1a44c811a..3c23ecf9d 100644
--- a/Makefile
+++ b/Makefile
@@ -1086,6 +1086,7 @@ BUILTIN_OBJS += builtin/multi-pack-index.o
 BUILTIN_OBJS += builtin/mv.o
 BUILTIN_OBJS += builtin/name-rev.o
 BUILTIN_OBJS += builtin/notes.o
+BUILTIN_OBJS += builtin/reverse-trailer-xrefs.o
 BUILTIN_OBJS += builtin/pack-objects.o
 BUILTIN_OBJS += builtin/pack-redundant.o
 BUILTIN_OBJS += builtin/pack-refs.o
diff --git a/builtin.h b/builtin.h
index 6538932e9..51089e258 100644
--- a/builtin.h
+++ b/builtin.h
@@ -195,6 +195,7 @@ extern int cmd_multi_pack_index(int argc, const char **argv, const char *prefix)
 extern int cmd_mv(int argc, const char **argv, const char *prefix);
 extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
 extern int cmd_notes(int argc, const char **argv, const char *prefix);
+extern int cmd_reverse_trailer_xrefs(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_redundant(int argc, const char **argv, const char *prefix);
 extern int cmd_patch_id(int argc, const char **argv, const char *prefix);
diff --git a/builtin/reverse-trailer-xrefs.c b/builtin/reverse-trailer-xrefs.c
new file mode 100644
index 000000000..0edb3b91a
--- /dev/null
+++ b/builtin/reverse-trailer-xrefs.c
@@ -0,0 +1,160 @@
+#include "builtin.h"
+#include "cache.h"
+#include "strbuf.h"
+#include "repository.h"
+#include "config.h"
+#include "commit.h"
+#include "blob.h"
+#include "notes.h"
+#include "notes-utils.h"
+#include "trailer.h"
+#include "revision.h"
+#include "list-objects.h"
+#include "object-store.h"
+#include "parse-options.h"
+
+static const char * const reverse_trailer_xrefs_usage[] = {
+	N_("git reverse_trailer_xrefs --xref-cherry-picks [--clear] [<options>] [<commit-ish>...]"),
+	N_("git reverse_trailer_xrefs --trailer-prefix=<prefix> --notes=<notes-ref> --tag=<tag> [<options>] [<commit-ish>...]"),
+	N_("git reverse_trailer_xrefs --notes=<notes-ref> --clear [<options>] [<commit-ish>...]"),
+	NULL
+};
+
+#define CHERRY_PICKED_PREFIX	"(cherry picked from commit "
+#define CHERRY_PICKS_REF	"refs/notes/xref-cherry-picks"
+#define CHERRY_PICKED_TO_TAG	"Cherry-picked-to"
+
+static int verbose;
+
+static void clear_trailer_xref_note(struct commit *commit, void *data)
+{
+	struct notes_tree *tree = data;
+	int status;
+
+	status = remove_note(tree, commit->object.oid.hash);
+
+	if (verbose) {
+		if (status)
+			fprintf(stderr, "Object %s has no note\n",
+				oid_to_hex(&commit->object.oid));
+		else
+			fprintf(stderr, "Removing note for object %s\n",
+				oid_to_hex(&commit->object.oid));
+	}
+}
+
+static void record_trailer_xrefs(struct commit *commit, void *data)
+{
+	trailer_rev_xrefs_record(data, commit);
+}
+
+static int note_trailer_xrefs(struct notes_tree *tree,
+			      struct commit *dst_commit,
+			      struct object_array *src_objs, const char *tag)
+{
+	char dst_hex[GIT_MAX_HEXSZ + 1];
+	struct strbuf note = STRBUF_INIT;
+	struct object_id note_oid;
+	int i, ret;
+
+	oid_to_hex_r(dst_hex, &dst_commit->object.oid);
+
+	for (i = 0; i < src_objs->nr; i++) {
+		const char *hex = src_objs->objects[i].name;
+
+		if (tag)
+			strbuf_addf(&note, "%s: %s\n", tag, hex);
+		else
+			strbuf_addf(&note, "%s\n", tag);
+		if (verbose)
+			fprintf(stderr, "Adding note %s -> %s\n", dst_hex, hex);
+	}
+
+	ret = write_object_file(note.buf, note.len, blob_type, &note_oid);
+	strbuf_release(&note);
+	if (ret)
+		return ret;
+
+	ret = add_note(tree, &dst_commit->object.oid, &note_oid, NULL);
+	return ret;
+}
+
+static int reverse_trailer_xrefs(struct rev_info *revs, int clear,
+				 const char *trailer_prefix,
+				 const char *notes_ref, const char *tag)
+{
+	struct notes_tree tree = { };
+	int i, ret;
+
+	init_notes(&tree, notes_ref, NULL, NOTES_INIT_WRITABLE);
+
+	if (prepare_revision_walk(revs))
+		die("revision walk setup failed");
+
+	if (clear) {
+		traverse_commit_list(revs, clear_trailer_xref_note, NULL, &tree);
+	} else {
+		struct trailer_rev_xrefs rxrefs;
+		struct commit *dst_commit;
+		struct object_array *src_objs;
+
+		trailer_rev_xrefs_init(&rxrefs, trailer_prefix);
+		traverse_commit_list(revs, record_trailer_xrefs, NULL, &rxrefs);
+
+		trailer_rev_xrefs_for_each(&rxrefs, i, dst_commit, src_objs) {
+			ret = note_trailer_xrefs(&tree, dst_commit, src_objs,
+						 tag);
+			if (ret)
+				return ret;
+		}
+
+		trailer_rev_xrefs_release(&rxrefs);
+	}
+
+	commit_notes(&tree, "Notes updated by 'git reverse-trailer-xrefs'");
+	return 0;
+}
+
+int cmd_reverse_trailer_xrefs(int argc, const char **argv, const char *prefix)
+{
+	struct rev_info revs;
+	struct setup_revision_opt s_r_opt = {
+		.def = "HEAD",
+		.revarg_opt = REVARG_CANNOT_BE_FILENAME
+	};
+	int cherry = 0, clear = 0;
+	const char *trailer_prefix = NULL, *notes_ref = NULL, *tag = NULL;
+	struct option options[] = {
+		OPT_BOOL(0, "xref-cherry-picks", &cherry, N_("use preset for xref-cherry-picks notes")),
+		OPT_STRING(0, "trailer-prefix", &trailer_prefix, N_("prefix"), N_("process trailers starting with <prefix>")),
+		OPT_STRING(0, "notes", &notes_ref, N_("notes-ref"), N_("update notes in <notes-ref>")),
+		OPT_STRING(0, "tag", &tag, N_("tag"), N_("tag xref notes with <tag>")),
+		OPT_BOOL(0, "clear", &clear, N_("clear trailer xref notes from the specified commits")),
+		OPT__VERBOSE(&verbose, N_("verbose")),
+		OPT_END()
+	};
+
+	git_config(git_default_config, NULL);
+
+	argc = parse_options(argc, argv, prefix, options,
+			     reverse_trailer_xrefs_usage,
+			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
+
+	init_revisions(&revs, prefix);
+	argc = setup_revisions(argc, argv, &revs, &s_r_opt);
+
+	if (cherry) {
+		trailer_prefix = CHERRY_PICKED_PREFIX;
+		notes_ref = CHERRY_PICKS_REF;
+		tag = CHERRY_PICKED_TO_TAG;
+	}
+
+	if (!notes_ref || (!clear && (!trailer_prefix || !tag)))
+		die(_("insufficient arguments"));
+
+	if (argc > 1)
+		die(_("unrecognized argument: %s"), argv[1]);
+
+	return reverse_trailer_xrefs(&revs, clear,
+				     trailer_prefix, notes_ref, tag);
+}
diff --git a/git.c b/git.c
index 2f604a41e..4948c8e01 100644
--- a/git.c
+++ b/git.c
@@ -515,6 +515,7 @@ static struct cmd_struct commands[] = {
 	{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
 	{ "name-rev", cmd_name_rev, RUN_SETUP },
 	{ "notes", cmd_notes, RUN_SETUP },
+	{ "reverse-trailer-xrefs", cmd_reverse_trailer_xrefs, RUN_SETUP },
 	{ "pack-objects", cmd_pack_objects, RUN_SETUP },
 	{ "pack-redundant", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT },
 	{ "pack-refs", cmd_pack_refs, RUN_SETUP },
-- 
2.17.1

[PATCH 4/5] githooks: Add post-cherry-pick and post-fetch hooks

[Tejun Heo]

From: Tejun Heo <htejun@fb.com>

* post-cherry-pick: Called after a cherry-pick and given parameters so
  that it can tell which are the new cherry-picks.

* post-fetch: Called after a fetch.  Each updated ref and sha1 are fed
  on stdin.

These two hooks will be used to keep refs/notes/xref-cherry-picks
up-to-date.

Signed-off-by: Tejun Heo <htejun@fb.com>
---
 Documentation/git-cherry-pick.txt |  5 +++
 Documentation/git-fetch.txt       |  5 +++
 Documentation/githooks.txt        | 23 ++++++++++
 builtin/fetch.c                   | 72 ++++++++++++++++++++++++++++---
 builtin/revert.c                  | 14 ++++++
 5 files changed, 114 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index d35d771fc..527cb9fea 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -224,6 +224,11 @@ the working tree.
 spending extra time to avoid mistakes based on incorrectly matching
 context lines.
 
+HOOKS
+-----
+This command can run `post-cherry-pick` hook. See linkgit:githooks[5]
+for more information.
+
 SEE ALSO
 --------
 linkgit:git-revert[1]
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index e31993559..a04c6079a 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -290,6 +290,11 @@ fetched, making it impossible to check out that submodule later without
 having to do a fetch again. This is expected to be fixed in a future Git
 version.
 
+HOOKS
+-----
+This command can run `post-fetch` hook. See linkgit:githooks[5]
+for more information.
+
 SEE ALSO
 --------
 linkgit:git-pull[1]
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 959044347..24c122343 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -149,6 +149,14 @@ invoked after a commit is made.
 This hook is meant primarily for notification, and cannot affect
 the outcome of `git commit`.
 
+post-cherry-pick
+~~~~~~~~~~~~~~~~
+
+This hook is invoked by linkgit:git-cherry-pick[1]. This hook is
+called with two parameters. The first is `<old sha1>` and the second
+`<new sha1>`, where `<old sha1>..<new sha1>` describes all new
+cherry-picked commits.
+
 pre-rebase
 ~~~~~~~~~~
 
@@ -191,6 +199,21 @@ save and restore any form of metadata associated with the working tree
 (e.g.: permissions/ownership, ACLS, etc).  See contrib/hooks/setgitperms.perl
 for an example of how to do this.
 
+post-fetch
+~~~~~~~~~~
+This hook is called by linkgit:git-fetch[1] and can be used to process
+newly fetched commits and tags.
+
+Information about what was fetched is provided on the hook's standard
+input with lines of the form:
+
+  <local ref> SP <old sha1> SP <remote ref> SP <new sha1> LF
+
+where `<local ref>` got updated from `<old sha1>` to `<new sha1>` as a
+result of fetching `<remote ref>`. If a branch or tag was created,
+`<old_sha1>` will be 40 `0`. If a tag was pruned, `<remote_ref>` will
+be `(delete)` and <new sha1> will be 40 `0`.
+
 pre-push
 ~~~~~~~~
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e0140327a..eac792a33 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -66,6 +66,7 @@ static struct refspec refmap = REFSPEC_INIT_FETCH;
 static struct list_objects_filter_options filter_options;
 static struct string_list server_options = STRING_LIST_INIT_DUP;
 static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
+static struct strbuf post_fetch_sb = STRBUF_INIT;
 
 static int git_fetch_config(const char *k, const char *v, void *cb)
 {
@@ -510,10 +511,24 @@ static struct ref *get_ref_map(struct remote *remote,
 	return ref_map;
 }
 
+static void record_post_fetch(const char *name,
+			      const struct object_id *old_oid,
+			      const char *remote,
+			      const struct object_id *new_oid)
+{
+	char old_hex[GIT_MAX_HEXSZ + 1], new_hex[GIT_MAX_HEXSZ + 1];
+
+	oid_to_hex_r(old_hex, old_oid);
+	oid_to_hex_r(new_hex, new_oid);
+	strbuf_addf(&post_fetch_sb, "%s %s %s %s\n",
+		    name, old_hex, remote ?: "(delete)", new_hex);
+}
+
 #define STORE_REF_ERROR_OTHER 1
 #define STORE_REF_ERROR_DF_CONFLICT 2
 
 static int s_update_ref(const char *action,
+			const char *remote,
 			struct ref *ref,
 			int check_old)
 {
@@ -546,6 +561,7 @@ static int s_update_ref(const char *action,
 	ref_transaction_free(transaction);
 	strbuf_release(&err);
 	free(msg);
+	record_post_fetch(ref->name, &ref->old_oid, remote, &ref->new_oid);
 	return 0;
 fail:
 	ref_transaction_free(transaction);
@@ -726,7 +742,7 @@ static int update_local_ref(struct ref *ref,
 	    starts_with(ref->name, "refs/tags/")) {
 		if (force || ref->force) {
 			int r;
-			r = s_update_ref("updating tag", ref, 0);
+			r = s_update_ref("updating tag", remote, ref, 0);
 			format_display(display, r ? '!' : 't', _("[tag update]"),
 				       r ? _("unable to update local ref") : NULL,
 				       remote, pretty_ref, summary_width);
@@ -766,7 +782,7 @@ static int update_local_ref(struct ref *ref,
 		if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(&ref->new_oid);
-		r = s_update_ref(msg, ref, 0);
+		r = s_update_ref(msg, remote, ref, 0);
 		format_display(display, r ? '!' : '*', what,
 			       r ? _("unable to update local ref") : NULL,
 			       remote, pretty_ref, summary_width);
@@ -782,7 +798,7 @@ static int update_local_ref(struct ref *ref,
 		if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(&ref->new_oid);
-		r = s_update_ref("fast-forward", ref, 1);
+		r = s_update_ref("fast-forward", remote, ref, 1);
 		format_display(display, r ? '!' : ' ', quickref.buf,
 			       r ? _("unable to update local ref") : NULL,
 			       remote, pretty_ref, summary_width);
@@ -797,7 +813,7 @@ static int update_local_ref(struct ref *ref,
 		if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
 			check_for_new_submodule_commits(&ref->new_oid);
-		r = s_update_ref("forced-update", ref, 1);
+		r = s_update_ref("forced-update", remote, ref, 1);
 		format_display(display, r ? '!' : '+', quickref.buf,
 			       r ? _("unable to update local ref") : _("forced update"),
 			       remote, pretty_ref, summary_width);
@@ -1071,8 +1087,11 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,
 	if (!dry_run) {
 		struct string_list refnames = STRING_LIST_INIT_NODUP;
 
-		for (ref = stale_refs; ref; ref = ref->next)
+		for (ref = stale_refs; ref; ref = ref->next) {
 			string_list_append(&refnames, ref->name);
+			record_post_fetch(ref->name, &ref->old_oid,
+					  NULL, &null_oid);
+		}
 
 		result = delete_refs("fetch: prune", &refnames, 0);
 		string_list_clear(&refnames, 0);
@@ -1561,6 +1580,47 @@ static int fetch_one(struct remote *remote, int argc, const char **argv, int pru
 	return exit_code;
 }
 
+static int run_post_fetch_hook(void)
+{
+	int ret = 0, x;
+	struct child_process proc = CHILD_PROCESS_INIT;
+	const char *argv[2];
+
+	if (!(argv[0] = find_hook("post-fetch")))
+		return 0;
+	argv[1] = NULL;
+
+	proc.argv = argv;
+	proc.in = -1;
+
+	if (start_command(&proc)) {
+		finish_command(&proc);
+		return -1;
+	}
+
+	sigchain_push(SIGPIPE, SIG_IGN);
+
+	if (write_in_full(proc.in, post_fetch_sb.buf, post_fetch_sb.len) < 0) {
+		/* We do not mind if a hook does not read all refs. */
+		if (errno != EPIPE)
+			ret = -1;
+	}
+
+	strbuf_release(&post_fetch_sb);
+
+	x = close(proc.in);
+	if (!ret)
+		ret = x;
+
+	sigchain_pop(SIGPIPE);
+
+	x = finish_command(&proc);
+	if (!ret)
+		ret = x;
+
+	return ret;
+}
+
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -1669,6 +1729,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 
 	close_all_packs(the_repository->objects);
 
+	run_post_fetch_hook();
+
 	argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
 	if (verbosity < 0)
 		argv_array_push(&argv_gc_auto, "--quiet");
diff --git a/builtin/revert.c b/builtin/revert.c
index c93393c89..0b7e578cc 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -8,6 +8,8 @@
 #include "dir.h"
 #include "sequencer.h"
 #include "branch.h"
+#include "refs.h"
+#include "run-command.h"
 
 /*
  * This implements the builtins revert and cherry-pick.
@@ -223,12 +225,24 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
 	struct replay_opts opts = REPLAY_OPTS_INIT;
+	struct object_id old_oid, new_oid;
+	char old_hex[GIT_MAX_HEXSZ + 1], new_hex[GIT_MAX_HEXSZ + 1];
 	int res;
 
+	if (read_ref("HEAD", &old_oid))
+		die(_("failed to read HEAD, cherry-pick failed"));
+
 	opts.action = REPLAY_PICK;
 	sequencer_init_config(&opts);
 	res = run_sequencer(argc, argv, &opts);
 	if (res < 0)
 		die(_("cherry-pick failed"));
+
+	if (read_ref("HEAD", &new_oid))
+		die(_("failed to read HEAD after cherry-pick"));
+
+	oid_to_hex_r(old_hex, &old_oid);
+	oid_to_hex_r(new_hex, &new_oid);
+	run_hook_le(0, "post-cherry-pick", old_hex, new_hex, NULL);
 	return res;
 }
-- 
2.17.1

[PATCH 5/5] notes: Implement xref-cherry-picks hooks and tests

[Tejun Heo]

From: Tejun Heo <htejun@fb.com>

Add post-cherry-pick.sample and post-fetch.sample which, when enabled,
will keep refs/notes/xref-cherry-picks up-to-date as new cherry-picks
are created and fetched.  Also, add tests to verify xref-cherry-picks.

Signed-off-by: Tejun Heo <htejun@fb.com>
---
 Documentation/git-reverse-trailer-xrefs.txt |   9 ++
 t/t3321-notes-xref-cherry-picks.sh          | 124 ++++++++++++++++++++
 templates/hooks--post-cherry-pick.sample    |   8 ++
 templates/hooks--post-fetch.sample          |  30 +++++
 4 files changed, 171 insertions(+)
 create mode 100644 t/t3321-notes-xref-cherry-picks.sh
 create mode 100644 templates/hooks--post-cherry-pick.sample
 create mode 100644 templates/hooks--post-fetch.sample

diff --git a/Documentation/git-reverse-trailer-xrefs.txt b/Documentation/git-reverse-trailer-xrefs.txt
index 20d260486..651ecdce1 100644
--- a/Documentation/git-reverse-trailer-xrefs.txt
+++ b/Documentation/git-reverse-trailer-xrefs.txt
@@ -131,6 +131,15 @@ ddd1bf2 commit A
 ------------
 
 
+Keeping xref-cherry-picks up-to-date
+------------------------------------
+
+Reverse-maps can be kept-up incrementally with hooks. For example, to
+keep xref-cherry-picks up-to-date, `git-reverse-trailer-xrefs` should
+be invoked on new cherry-picks and fetched commits. See
+`hooks/post-cherry-pick.sample` and `hooks/post-fetch.sample`.
+
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/t/t3321-notes-xref-cherry-picks.sh b/t/t3321-notes-xref-cherry-picks.sh
new file mode 100644
index 000000000..96b6731c9
--- /dev/null
+++ b/t/t3321-notes-xref-cherry-picks.sh
@@ -0,0 +1,124 @@
+#!/bin/sh
+
+test_description='Verify xref-cherry-picks handling
+
+Assume the following git repository.
+
+	  D*---E** release-B
+	 /
+	C*      E* release-D
+       /       /
+  A---B---C---D---E master
+
+which contains the following cherry-picks.
+
+  C -> C*
+  D -> D*
+  E -> E* -> E**
+
+1. Build the above repository using `git-cherry-pick -x` with the
+   sample post-cherry-pick hook enabled.  Verify that the
+   xref-cherry-picks notes are populated correctly.
+
+2. Clear the notes and rebuild them by directly running
+   git-reverse-xref-trailers and verify the output.
+
+3. Run it again and check the output still agrees to verify duplicate
+   handling.
+
+4. Build a cloned repository using per-branch fetches with the sample
+   post-fetch hook enabled. Verify that the xref-cherry-picks notes
+   are populatec correctly.
+'
+
+TEST_NO_CREATE_REPO=1
+
+. ./test-lib.sh
+
+GIT_AUTHOR_EMAIL=bogus_email_address
+export GIT_AUTHOR_EMAIL
+
+test_expect_success \
+    'Build repo with cherry-picks and verify xref-cherry-picks' \
+    'test_create_repo main &&
+     cd main &&
+     mkdir -p .git/hooks &&
+     mv .git/hooks-disabled/post-cherry-pick.sample .git/hooks/post-cherry-pick &&
+
+     test_tick &&
+     echo A >> testfile &&
+     git update-index --add testfile &&
+     git commit -am "A" &&
+     echo B >> testfile &&
+     git commit -am "B" &&
+     echo C >> testfile &&
+     git commit -am "C" &&
+     echo D >> testfile &&
+     git commit -am "D" &&
+     echo E >> testfile &&
+     git commit -am "E" &&
+
+     test_tick &&
+     git checkout -b release-D master^ &&
+     git cherry-pick -x master &&
+
+     test_tick &&
+     git checkout -b release-B master^^^ &&
+     git cherry-pick -x release-D^^ &&
+     git cherry-pick -x release-D^ &&
+     git cherry-pick -x release-D &&
+
+     cat > expect <<-EOF &&
+master E
+Notes (xref-cherry-picks):
+    Cherry-picked-to: release-D
+    Cherry-picked-to:   release-B
+
+master~1 D
+Notes (xref-cherry-picks):
+    Cherry-picked-to: release-B~1
+
+master~2 C
+Notes (xref-cherry-picks):
+    Cherry-picked-to: release-B~2
+
+master~3 B
+master~4 A
+EOF
+
+     git log --pretty=oneline --notes=xref-cherry-picks master | git name-rev --name-only --stdin > actual &&
+     test_cmp expect actual
+'
+
+test_expect_success \
+    'Clear, rebuild and verify xref-cherry-picks' \
+    'git reverse-trailer-xrefs --xref-cherry-picks --all --clear &&
+     git reverse-trailer-xrefs --xref-cherry-picks --all &&
+     git log --pretty=oneline --notes=xref-cherry-picks master | git name-rev --name-only --stdin > actual &&
+    test_cmp expect actual
+'
+
+test_expect_success \
+    'Build it again to verify duplicate handling' \
+    'git reverse-trailer-xrefs --xref-cherry-picks --all &&
+     git log --pretty=oneline --notes=xref-cherry-picks master | git name-rev --name-only --stdin > actual &&
+    test_cmp expect actual
+'
+
+test_expect_success \
+    'Build a clone through per-branch fetches and verify xref-cherry-picks' \
+    'cd .. &&
+     test_create_repo clone &&
+     cd clone &&
+     mkdir -p .git/hooks &&
+     mv .git/hooks-disabled/post-fetch.sample .git/hooks/post-fetch &&
+
+     git fetch -fu ../main master:master &&
+     git fetch -fu ../main release-D:release-D &&
+     git fetch -fu ../main release-B:release-B &&
+
+     git log --pretty=oneline --notes=xref-cherry-picks master | git name-rev --name-only --stdin > actual &&
+    test_cmp ../main/expect actual
+'
+
+test_done
diff --git a/templates/hooks--post-cherry-pick.sample b/templates/hooks--post-cherry-pick.sample
new file mode 100644
index 000000000..3af8b5d23
--- /dev/null
+++ b/templates/hooks--post-cherry-pick.sample
@@ -0,0 +1,8 @@
+#!/bin/sh
+#
+# An example hook script to reverse map new cherry-picks. See
+# git-reverse-trailer-xrefs(1) for details.
+#
+# To enable this hook, rename this file to "post-cherry-pick".
+
+git reverse-trailer-xrefs --xref-cherry-picks $1..$2
diff --git a/templates/hooks--post-fetch.sample b/templates/hooks--post-fetch.sample
new file mode 100644
index 000000000..6b98a5c10
--- /dev/null
+++ b/templates/hooks--post-fetch.sample
@@ -0,0 +1,30 @@
+#!/bin/sh
+#
+# An example hook script to reverse map cherry-picks in newly fetched
+# commits. See git-reverse-trailer-xrefs(1) for details.
+#
+# To enable this hook, rename this file to "post-fetch".
+
+z40=0000000000000000000000000000000000000000
+
+while read ref old_sha remote new_sha
+do
+    case $ref in
+    refs/heads/*)
+	;;
+    *)
+	continue
+    esac
+
+    if [ $new_sha == $z40 ]
+    then
+	continue
+    fi
+
+    if [ $old_sha != $z40 ]
+    then
+	git reverse-trailer-xrefs --xref-cherry-picks $old_sha..$new_sha
+    else
+	git reverse-trailer-xrefs --xref-cherry-picks $new_sha
+    fi
+done
-- 
2.17.1

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Junio C Hamano]

Tejun Heo <tj@kernel.org> writes:

> Some trailers refer to other commits.  Let's call them xrefs
> (cross-references).  For example, a cherry pick trailer points to the
> source commit.  It is sometimes useful to build a reverse map of these
> xrefs - ie. source -> cherry-pick instead of cherry-pick -> source.
>
> This, e.g, can answer which releases a commit ended up in on
> repositories which cherry-picks fixes back from the development
> branch.  Let's say the repository looks like the following.
>
> 	    D'---E'' release-B
> 	   /
> 	  C'      E' release-D
>        /       /
>   A---B---C---D---E master
>
> where the followings cherry-picks took place.
>
>   C -> C'
>   D -> D'
>   E -> E' -> E''
>
> Using the reverse-mapping in this patchset, we can get the following
> annotated log which succinctly shows which commit ended up where.
> ...

Thanks.  A few comments, after skimming the patches enough to get
the general feel of the design but before deeply reading them to see
how complete the implementation is.

The topic introduces two new hooks: one to run immediately after
cherry-picking so that the reverse mapping for a single src->dst
pair can be added, and another to run immediately after fetching so
that all new commits that have appeared over the wire can be scanned
to see if any of them is a cherry-pick of other commits.

They are good starting points, but for a complete solution I'd
imagine that you'd need to hook to many other places in the
workflow.  For example, you'd need to deal with the case where a
commit created by cherry-picking an original is further rewritten
with "commit --amend" or "rebase".  They may already trigger the
post rewrite hook, so you may not have to add a new hook, but in
[3/5], you'd need to describe in the documentaiton for the new
reverse-trailer command full set of hooks you expect the command to
be used to maintain the reverse mapping, as the hooks you give them
as examples in [5/5] are merely a part of a complete solution.

It also is not immediately obvious to me what your general strategy
to maintain this reverse mapping is, when new ways and codepaths to
cause new commits with "cherry-picked-from" trailer appear.  Do we
keep piling on new hooks?  Or is the reverse mapping information
allowed to be a bit stale and be completed immediately before it
gets used?  A totally different approach could be to record list of
commits, all commits behind which have been scanned for reverse
mapping, in the tip of the notes history, perhaps in the commit log
message (which is machine generated anyway).  Then, before you need
the up-to-date-to-the-last-second reverse mapping, you could run

	git rev-list --all --not $these_tips_recorded

and scan the commits, just like you scan what you fetched.  And when
you update the reverse mapping notes tree, the commit to record that
notes update can record the tip of the above traverseal.

I also wonder how this topic and the topic Stefan Xenos has been
working on (cf. <20181201005240.113210-1-sxenos@google.com>) can
benefit from each other by cross pollination.  Stefan's topic also
needs to answer, given a commit, what other commits were created out
of it by cherry-picking and then further amending the results, so
that when the original commit itself gets amended, the cherry-picked
ones that were created from the original can be easily identified
and get updated in the same way as necessary.  

The motivating workflow your topic's cover letter gives us is to
maintain the release branch that goes only forward, so in that
context, how a commit on the release branch that was created by
cherry-picking an original gets updated when the original commit
gets amended would be different (I am assuming that you cannot
affored to rebase the release branch to update a deeply buried
commit that was an earlier cherry-pick), but I would imagine that
both topics share the need for a good data structure to keep track
of (1) the relationship between the original and copy of the
cherry-pick and (2) the fact that the original of such a cherry-pick
is now stale and the copy might want to get updated.

Thanks.

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Tejun Heo]

Hello, Junio.

On Wed, Dec 12, 2018 at 04:26:57PM +0900, Junio C Hamano wrote:
> It also is not immediately obvious to me what your general strategy
> to maintain this reverse mapping is, when new ways and codepaths to
> cause new commits with "cherry-picked-from" trailer appear.  Do we
> keep piling on new hooks?  Or is the reverse mapping information

So, what I actually wanted was "this repo now has these new commits"
hook, but that didn't seem to be in line with other hooks as most were
bound to specific directly user-visible commands and actions.

> allowed to be a bit stale and be completed immediately before it
> gets used?  A totally different approach could be to record list of
> commits, all commits behind which have been scanned for reverse
> mapping, in the tip of the notes history, perhaps in the commit log
> message (which is machine generated anyway).  Then, before you need
> the up-to-date-to-the-last-second reverse mapping, you could run
> 
> 	git rev-list --all --not $these_tips_recorded

Wouldn't it be more useful to have repo-updated-with-these-commits
hook instead rather than putting more logic on note handling?

> and scan the commits, just like you scan what you fetched.  And when
> you update the reverse mapping notes tree, the commit to record that
> notes update can record the tip of the above traverseal.
> 
> I also wonder how this topic and the topic Stefan Xenos has been
> working on (cf. <20181201005240.113210-1-sxenos@google.com>) can
> benefit from each other by cross pollination.  Stefan's topic also
> needs to answer, given a commit, what other commits were created out
> of it by cherry-picking and then further amending the results, so
> that when the original commit itself gets amended, the cherry-picked
> ones that were created from the original can be easily identified
> and get updated in the same way as necessary.  

Ah, I see, forward-propagating amends to cherry-picks.

> The motivating workflow your topic's cover letter gives us is to
> maintain the release branch that goes only forward, so in that
> context, how a commit on the release branch that was created by
> cherry-picking an original gets updated when the original commit
> gets amended would be different (I am assuming that you cannot

At least we don't do this with our trees and most trees that I've
worked with don't either as that would make it really confusing for
people to work together.

> affored to rebase the release branch to update a deeply buried
> commit that was an earlier cherry-pick), but I would imagine that
> both topics share the need for a good data structure to keep track
> of (1) the relationship between the original and copy of the
> cherry-pick and (2) the fact that the original of such a cherry-pick
> is now stale and the copy might want to get updated.

As long as we can keep the reverse rference notes consistent, wouldn't
amend propagation just consume them?

Thanks.

-- 
tejun

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Junio C Hamano]

Tejun Heo <tj@kernel.org> writes:

>> allowed to be a bit stale and be completed immediately before it
>> gets used?  A totally different approach could be to record list of
>> commits, all commits behind which have been scanned for reverse
>> mapping, in the tip of the notes history, perhaps in the commit log
>> message (which is machine generated anyway).  Then, before you need
>> the up-to-date-to-the-last-second reverse mapping, you could run
>> 
>> 	git rev-list --all --not $these_tips_recorded
>
> Wouldn't it be more useful to have repo-updated-with-these-commits
> hook instead rather than putting more logic on note handling?
>
>> and scan the commits, just like you scan what you fetched.  And when
>> you update the reverse mapping notes tree, the commit to record that
>> notes update can record the tip of the above traversal.

I do not consider what you do in notes/xref-* "more logic on note
handling" in the sense that the logic is part of "notes" API.

The moment you decided to reserve one hierarchy in refs/notes/ and
designed what the mapping recorded there means, you designed a new
trailer-xrefs API.  It is a part of that API how blobs stored in
your refs/notes/xref-cherry-picks are formatted and what they mean.
It's the same thing---it is also part of your API how the log
message for recording commits in that hierarchy is formatted and
what it means.

> As long as we can keep the reverse rference notes consistent, wouldn't
> amend propagation just consume them?

Yes.  Would that mean you do not need the notes/xref-* series we are
seeing here, and instead (re)use what Stefan's series, which already
needs to have access to and record the information anyway, records?

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> As long as we can keep the reverse rference notes consistent, wouldn't
>> amend propagation just consume them?
>
> Yes.  Would that mean you do not need the notes/xref-* series we are
> seeing here, and instead (re)use what Stefan's series, which already
> needs to have access to and record the information anyway, records?

Just to avoid confusion or unnecessary guessing.

Please do not take the above as "don't do notes/xref-; instead read
from the 'meta commits'".  I do not have a preference between the
two proposed implementations.  The important thing is that we won't
end up with having to maintain two separate mechanisms that want to
keep track of essentially the same class of information.  FWIW I'd
be perfectly fine if the unification goes the other way, as long as
goals of both parties are met, and for that, I'd like to see you two
work together, or at least be aware of what each other is doing and
see if cross-pollination would result in a mutually better solution.

Thanks

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Tejun Heo]

Hello, Junio, Stefan.

On Thu, Dec 13, 2018 at 12:09:39PM +0900, Junio C Hamano wrote:
> Please do not take the above as "don't do notes/xref-; instead read
> from the 'meta commits'".  I do not have a preference between the
> two proposed implementations.  The important thing is that we won't
> end up with having to maintain two separate mechanisms that want to
> keep track of essentially the same class of information.  FWIW I'd
> be perfectly fine if the unification goes the other way, as long as
> goals of both parties are met, and for that, I'd like to see you two
> work together, or at least be aware of what each other is doing and
> see if cross-pollination would result in a mutually better solution.

So, from my POV, the only thing I want is being able to easily tell
which commit got cherry picked where.  I really don't care how that's
achieved.  Stefan, if there's anything I can do to push it forward,
let me know and if you see anything useful in my patchset, please feel
free to incorporate them in any way.

Thanks.

-- 
tejun

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Stefan Xenos]

Heya, Tejun!

Thanks for getting in touch. I agree with Junio that we probably
shouldn't be tracking the same information in two ways if we can think
of something that does both... so let's see if we can think of
something! The evolve proposal suggests having a metas/ namespace to
track ongoing changes is intended as a way to track the changes a user
is actively working on. If we were to use it to retroactively store
every historical cherry-pick in a large repository, I'm concerned it
may get too cluttered to use for its originally-intended purpose. I'm
also not sure how well things would perform with a huge number of
refs. The remainder of the proposal (using metacommits to store the
relationships) would work for the xref use-case, but we'd probably
want to tweak the way we store the root refs in order to do a good job
with the large number of inactive cherry-picks he probably wants. Any
code that is looking for cross-reference metadata could look in both
namespaces.

Conversely, if we were to tweak the xrefs proposal by adding the
obsolescence information that evolve needs, we'd be missing a place to
store the user's ongoing changes, a way to push and pull specific
changes, a way to describe alternate histories for a commit, and a
mechanism for preventing the commits needed by evolve from getting
garbage collected.

All the problems with both approaches are solve-able, though.

I spent a few hours discussing this with Stefan Beller last week and I
think we've come up with a variation on the evolve proposal that would
cover both our use-cases. Let me know what you think. To address the
cluttering of the metas namespace, we could introduce a new
"hiddenmetas" namespace. It would contain the same type of data
recorded in the metas namespace, but it would normally be hidden from
the user when they list their changes, etc. It would also be immune
from the automatic deletions that are applied to the "metas"
namespace. From your point of view, the critical bit is that it would
contain information about cherry-picks. That would address the
"user-visible clutter" problem. Utility methods that want to search
for cherry-picks would iterate over both namespaces.

For the performance problem, I think we could just live with it
temporarily and wait for the currently-ongoing reftable work since the
reftable proposal would address exactly this performance issue (by
allowing us to store and manipulate large numbers of refs
efficiently).

A nice characteristic of this approach is that it would also address a
problem with the evolve proposal that had concerned me: how to deal
with the filter-branch command, which would have created pretty much
the same problems regarding the large number of metadata refs for
changes the user isn't actively working on.

It might be nice to also consider and some alternatives, but I haven't
had enough time to think up more of them (and I haven't digested the
xrefs proposal sufficiently to suggest an enhanced version of it yet).
If anyone else has ideas for combining the xrefs and evolve use-cases,
having more alternatives to choose from is always better!

If you're okay with the "hiddenmetas" approach in principle, I'll
update the evolve proposal doc to include it. Once we work out how the
storage format will work, we can coordinate our implementation work.

  - Stefan




On Wed, Dec 12, 2018 at 7:46 PM Tejun Heo <tj@kernel.org> wrote:
>
> Hello, Junio, Stefan.
>
> On Thu, Dec 13, 2018 at 12:09:39PM +0900, Junio C Hamano wrote:
> > Please do not take the above as "don't do notes/xref-; instead read
> > from the 'meta commits'".  I do not have a preference between the
> > two proposed implementations.  The important thing is that we won't
> > end up with having to maintain two separate mechanisms that want to
> > keep track of essentially the same class of information.  FWIW I'd
> > be perfectly fine if the unification goes the other way, as long as
> > goals of both parties are met, and for that, I'd like to see you two
> > work together, or at least be aware of what each other is doing and
> > see if cross-pollination would result in a mutually better solution.
>
> So, from my POV, the only thing I want is being able to easily tell
> which commit got cherry picked where.  I really don't care how that's
> achieved.  Stefan, if there's anything I can do to push it forward,
> let me know and if you see anything useful in my patchset, please feel
> free to incorporate them in any way.
>
> Thanks.
>
> --
> tejun

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Stefan Xenos]

I've just uploaded a new evolve proposal that includes a spec for the
"hiddenmetas" namespace, where we can store historical cherry-pick
information.

On Tue, Dec 18, 2018 at 6:40 AM Stefan Xenos <sxenos@google.com> wrote:
>
> Heya, Tejun!
>
> Thanks for getting in touch. I agree with Junio that we probably
> shouldn't be tracking the same information in two ways if we can think
> of something that does both... so let's see if we can think of
> something! The evolve proposal suggests having a metas/ namespace to
> track ongoing changes is intended as a way to track the changes a user
> is actively working on. If we were to use it to retroactively store
> every historical cherry-pick in a large repository, I'm concerned it
> may get too cluttered to use for its originally-intended purpose. I'm
> also not sure how well things would perform with a huge number of
> refs. The remainder of the proposal (using metacommits to store the
> relationships) would work for the xref use-case, but we'd probably
> want to tweak the way we store the root refs in order to do a good job
> with the large number of inactive cherry-picks he probably wants. Any
> code that is looking for cross-reference metadata could look in both
> namespaces.
>
> Conversely, if we were to tweak the xrefs proposal by adding the
> obsolescence information that evolve needs, we'd be missing a place to
> store the user's ongoing changes, a way to push and pull specific
> changes, a way to describe alternate histories for a commit, and a
> mechanism for preventing the commits needed by evolve from getting
> garbage collected.
>
> All the problems with both approaches are solve-able, though.
>
> I spent a few hours discussing this with Stefan Beller last week and I
> think we've come up with a variation on the evolve proposal that would
> cover both our use-cases. Let me know what you think. To address the
> cluttering of the metas namespace, we could introduce a new
> "hiddenmetas" namespace. It would contain the same type of data
> recorded in the metas namespace, but it would normally be hidden from
> the user when they list their changes, etc. It would also be immune
> from the automatic deletions that are applied to the "metas"
> namespace. From your point of view, the critical bit is that it would
> contain information about cherry-picks. That would address the
> "user-visible clutter" problem. Utility methods that want to search
> for cherry-picks would iterate over both namespaces.
>
> For the performance problem, I think we could just live with it
> temporarily and wait for the currently-ongoing reftable work since the
> reftable proposal would address exactly this performance issue (by
> allowing us to store and manipulate large numbers of refs
> efficiently).
>
> A nice characteristic of this approach is that it would also address a
> problem with the evolve proposal that had concerned me: how to deal
> with the filter-branch command, which would have created pretty much
> the same problems regarding the large number of metadata refs for
> changes the user isn't actively working on.
>
> It might be nice to also consider and some alternatives, but I haven't
> had enough time to think up more of them (and I haven't digested the
> xrefs proposal sufficiently to suggest an enhanced version of it yet).
> If anyone else has ideas for combining the xrefs and evolve use-cases,
> having more alternatives to choose from is always better!
>
> If you're okay with the "hiddenmetas" approach in principle, I'll
> update the evolve proposal doc to include it. Once we work out how the
> storage format will work, we can coordinate our implementation work.
>
>   - Stefan
>
>
>
>
> On Wed, Dec 12, 2018 at 7:46 PM Tejun Heo <tj@kernel.org> wrote:
> >
> > Hello, Junio, Stefan.
> >
> > On Thu, Dec 13, 2018 at 12:09:39PM +0900, Junio C Hamano wrote:
> > > Please do not take the above as "don't do notes/xref-; instead read
> > > from the 'meta commits'".  I do not have a preference between the
> > > two proposed implementations.  The important thing is that we won't
> > > end up with having to maintain two separate mechanisms that want to
> > > keep track of essentially the same class of information.  FWIW I'd
> > > be perfectly fine if the unification goes the other way, as long as
> > > goals of both parties are met, and for that, I'd like to see you two
> > > work together, or at least be aware of what each other is doing and
> > > see if cross-pollination would result in a mutually better solution.
> >
> > So, from my POV, the only thing I want is being able to easily tell
> > which commit got cherry picked where.  I really don't care how that's
> > achieved.  Stefan, if there's anything I can do to push it forward,
> > let me know and if you see anything useful in my patchset, please feel
> > free to incorporate them in any way.
> >
> > Thanks.
> >
> > --
> > tejun

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Tejun Heo]

Hey, guys.

On Tue, Dec 18, 2018 at 08:48:35AM -0800, Stefan Xenos wrote:
> I've just uploaded a new evolve proposal that includes a spec for the
> "hiddenmetas" namespace, where we can store historical cherry-pick
> information.

Total noob question - where can I read that?  Also, as long as I can
have the back references and follow to the existing cherry-picks, I
don't have any further requirements, so any working mechanism is great
by me.

Thanks.

-- 
tejun

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Tejun Heo]

Hello, Junio.

On Thu, Dec 13, 2018 at 12:01:25PM +0900, Junio C Hamano wrote:
> > Wouldn't it be more useful to have repo-updated-with-these-commits
> > hook instead rather than putting more logic on note handling?
> >
> >> and scan the commits, just like you scan what you fetched.  And when
> >> you update the reverse mapping notes tree, the commit to record that
> >> notes update can record the tip of the above traversal.
> 
> I do not consider what you do in notes/xref-* "more logic on note
> handling" in the sense that the logic is part of "notes" API.
> 
> The moment you decided to reserve one hierarchy in refs/notes/ and
> designed what the mapping recorded there means, you designed a new
> trailer-xrefs API.  It is a part of that API how blobs stored in
> your refs/notes/xref-cherry-picks are formatted and what they mean.
> It's the same thing---it is also part of your API how the log
> message for recording commits in that hierarchy is formatted and
> what it means.

Hmmm... I see.  I still have a bit of trouble seeing why doing it that
way is better tho.  Wouldn't new-object-hook be simpler?  They'll
achieve about the same thing but one would need to keep the states in
two places.

> > As long as we can keep the reverse rference notes consistent, wouldn't
> > amend propagation just consume them?
> 
> Yes.  Would that mean you do not need the notes/xref-* series we are
> seeing here, and instead (re)use what Stefan's series, which already
> needs to have access to and record the information anyway, records?

Oh yeah, for sure.  Didn't know Stefan was doing something similar.
Will continue on the other reply.

Thanks.

-- 
tejun

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Junio C Hamano]

Tejun Heo <tj@kernel.org> writes:

> Hmmm... I see.  I still have a bit of trouble seeing why doing it that
> way is better tho.  Wouldn't new-object-hook be simpler?  They'll
> achieve about the same thing but one would need to keep the states in
> two places.

The new-object-hook thing will not have keep the states.  Whenever
Git realizes that it created a new object, it must call that hook,
and at that point in time, without keeping any state, it knows which
objects are what it has just created.  So "in two places" is not a
problem at all.  There is only one place (i.e. the place the sweeper
would record what it just did to communicate with its future
invocation).

A new-object-hook that will always be called any time a new object
enters the picture would be a nightmare to maintain up-to-date.  One
missed codepath and your coverage will have holes.  Having a back-up
mechanism to sweep for new objects will give you a better chance of
staying correct as the system evolves, I'd think.

Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references

[Tejun Heo]

Hello, Junio.

On Thu, Dec 13, 2018 at 02:47:36PM +0900, Junio C Hamano wrote:
> Tejun Heo <tj@kernel.org> writes:
> 
> > Hmmm... I see.  I still have a bit of trouble seeing why doing it that
> > way is better tho.  Wouldn't new-object-hook be simpler?  They'll
> > achieve about the same thing but one would need to keep the states in
> > two places.
> 
> The new-object-hook thing will not have keep the states.  Whenever
> Git realizes that it created a new object, it must call that hook,
> and at that point in time, without keeping any state, it knows which
> objects are what it has just created.  So "in two places" is not a

Yeap, that's what I was trying to say.

> problem at all.  There is only one place (i.e. the place the sweeper
> would record what it just did to communicate with its future
> invocation).
> 
> A new-object-hook that will always be called any time a new object
> enters the picture would be a nightmare to maintain up-to-date.  One

But I didn't know this.  I probably am too naive in thinking so but
it's a bit surprising that there's no single / few chokepoints for
repo updates that we can attach to.

> missed codepath and your coverage will have holes.  Having a back-up
> mechanism to sweep for new objects will give you a better chance of
> staying correct as the system evolves, I'd think.

The duplicate state tracking still bothers me a bit (duplicate in the
sense that the end results are the same) especially as this would mean
any similar mechanism would need to track their own states too, but I
really don't have the full (not even close) picture here and it can
easily be me missing something.

Anyways, I'll wait for Stefan to chime in.

Thanks.

-- 
tejun