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

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

From: Tejun Heo <[email protected]>

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

---
 trailer.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 trailer.h |  38 ++++++++++++++++++++
 2 files changed, 143 insertions(+)

index 0796f326b..15744600b 100644
@@ -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)++;
+}

index b99773964..07090a11f 100644
@@ -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

From: Tejun Heo <[email protected]>

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.

---
 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(-)

index df2b64dbb..872919ad4 100644
@@ -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
 --------

index bd05d50b0..3fe2389a1 100644
@@ -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);
 }

index 14ea03178..db6363c39 100644
@@ -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;
 

index 540830652..b604f9b51 100644
@@ -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 {

index 414bc6855..09ca92f36 100644
@@ -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

From: Tejun Heo <[email protected]>

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.

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

new file mode 100644
index 000000000..20d260486
@@ -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

index 1a44c811a..3c23ecf9d 100644
@@ -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

index 6538932e9..51089e258 100644
@@ -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);

index 2f604a41e..4948c8e01 100644
@@ -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

From: Tejun Heo <[email protected]>

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

---
 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(-)

index d35d771fc..527cb9fea 100644
@@ -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]

index e31993559..a04c6079a 100644
@@ -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]

index 959044347..24c122343 100644
@@ -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
 ~~~~~~~~
 

index c93393c89..0b7e578cc 100644
@@ -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

From: Tejun Heo <[email protected]>

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.

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

index 20d260486..651ecdce1 100644
@@ -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

new file mode 100644
index 000000000..3af8b5d23
@@ -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

new file mode 100644
index 000000000..6b98a5c10
@@ -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

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. <[email protected]>) 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.

Hello, Junio.

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.

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

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

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.

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

Thanks.

tejun

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.

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

Hello, Junio, Stefan.

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

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

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

Hey, guys.

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

Hello, Junio.

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.

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

Thanks.

tejun

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.

Hello, Junio.

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

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.

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