git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [RFC] cherry-pick notes to find out cherry-picks from the origin
@ 2018-10-17 14:39 Tejun Heo
  2018-10-24 12:24 ` Tejun Heo
  2018-11-15 14:40 ` Jeff King
  0 siblings, 2 replies; 6+ messages in thread
From: Tejun Heo @ 2018-10-17 14:39 UTC (permalink / raw)
  To: git, Junio C Hamano, Jeff King; +Cc: kernel-team

Hello, Junio, Jeff.

A while ago, I proposed changes to name-rev and describe so that they
can identify the commits cherry-picked from the one which is being
shown.

  https://public-inbox.org/git/20180726153714.GX1934745@devbig577.frc2.facebook.com/T/

While the use-cases - e.g. tracking down which release / stable
branches a given commit ended up in - weren't controversial, it was
suggested that it'd make more sense to use notes to link cherry-picks
instead of building the feature into name-rev.

The patch appended to this message implements most of it (sans tests
and documentation).  It's composed of the following two parts.

* A new built-in command note-cherry-picks, which walks the specified
  commits and if they're marked with the cherry-pick trailer, adds the
  backlink to the origin commit using Cherry-picked-to tag in a
  cherry-picks note.

* When formatting a cherry-picks note for display, nested cherry-picks
  are followed from each Cherry-picked-to tag and printed out with
  matching indentations.

Combined with name-rev --stdin, it can produce outputs like the following.

    commit 82cddd79f962de0bb1e7cdd95d48b48633335816 (branch2)
    Author: Tejun Heo <tj@kernel.org>
    Date:   Wed Jul 25 21:31:35 2018 -0700

	commit 4

    (cherry picked from commit 10f7ce0a0e524279f022b48460c088a108b45d54 (master~1))
    (cherry picked from commit d433e3b4d5a19b3d29e2c8349fe88ceade5f6190 (branch1))

    commit d433e3b4d5a19b3d29e2c8349fe88ceade5f6190 (branch1)
    Author: Tejun Heo <tj@kernel.org>
    Date:   Wed Jul 25 21:31:35 2018 -0700

	commit 4

	(cherry picked from commit 10f7ce0a0e524279f022b48460c088a108b45d54 (master~1))

    Notes (cherry-picks):
	Cherry-picked-to: 82cddd79f962de0bb1e7cdd95d48b48633335816 (branch2)

    commit 10f7ce0a0e524279f022b48460c088a108b45d54 (master~1)
    Author: Tejun Heo <tj@kernel.org>
    Date:   Wed Jul 25 21:31:35 2018 -0700

	commit 4

    Notes (cherry-picks):
	Cherry-picked-to: d433e3b4d5a19b3d29e2c8349fe88ceade5f6190 (branch1)
	Cherry-picked-to:   82cddd79f962de0bb1e7cdd95d48b48633335816 (branch2)
	Cherry-picked-to: 2dd08fe869986c26bc1152a0bcec8c2fa48c50f7 (branch5)
	Cherry-picked-to: fa8b79edc5dfff21753c2ccfc1a1828336c4c070 (branch4~5)
	Cherry-picked-to:   a1fb6024e3bda5549de1d15d8fa37e8c3a7eecbe (branch4~2)
	Cherry-picked-to:     58964342321a65e316ff47db33f7063743bc0de8 (branch4)
	Cherry-picked-to:   45e0d5f31c869dcc89b9737853e64489e2ad80b0 (branch4~1)
	Cherry-picked-to: 58a8d36b2532feb0a14b4fc2a50d587e64f38324 (branch3)

Locally, the notes can be kept up-to-date with a trivial post-commit
hook which invokes note-cherry-picks on the new commit; however, I'm
having a bit of trouble figuring out a way to keep it up-to-date when
multiple trees are involved.  AFAICS, there are two options.

1. Ensuring that the notes are always generated on local commits and
   whenever new commits are received through fetch/pulls.

2. Ensuring that the notes are always generated on local commits and
   transported with push/pulls.

3. A hybrid approach - also generate notes on the receiving end and
   ensure that fetch/pulls receives the notes together (ie. similar to
   --tags option to git-fetch).

#1 seems simpler and more robust to me.  Unfortunately, I can't see a
way to implement any of the three options with the existing hooks.
For #1, there's no post-fetch hook.  For #2 and #3, there doesn't seem
to be a fool-proof way to make sure that the notes are transported
together.  Any suggestions would be greatly appreciated.

Please let me know what you think.

Thanks.

---
 Makefile                    |    1 
 builtin.h                   |    1 
 builtin/note-cherry-picks.c |  197 ++++++++++++++++++++++++++++++++++++++++++++
 builtin/notes.c             |   17 ++-
 git.c                       |    1 
 notes.c                     |   95 +++++++++++++++++++++
 notes.h                     |    7 +
 object.c                    |    4 
 object.h                    |    6 +
 9 files changed, 320 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 5c8307b7c..fb0ff3ce9 100644
--- a/Makefile
+++ b/Makefile
@@ -1073,6 +1073,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/note-cherry-picks.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 962f0489a..d9d019abc 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_note_cherry_picks(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/note-cherry-picks.c b/builtin/note-cherry-picks.c
new file mode 100644
index 000000000..343e22c0d
--- /dev/null
+++ b/builtin/note-cherry-picks.c
@@ -0,0 +1,197 @@
+#include "builtin.h"
+#include "cache.h"
+#include "strbuf.h"
+#include "repository.h"
+#include "config.h"
+#include "commit.h"
+#include "notes.h"
+#include "trailer.h"
+#include "revision.h"
+#include "argv-array.h"
+#include "commit-slab.h"
+#include "list-objects.h"
+#include "object-store.h"
+#include "parse-options.h"
+
+define_commit_slab(commit_cherry_picks, struct object_array *);
+
+static const char * const note_cherry_picks_usage[] = {
+	N_("git note-cherry-picks [<options>] [<commit-ish>...]"),
+	NULL
+};
+
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
+static const char cherry_picked_to_tag[] = "Cherry-picked-to: ";
+static int verbose, clear;
+static struct object_array cherry_picked = OBJECT_ARRAY_INIT;
+static struct commit_cherry_picks cherry_picks;
+
+static struct object_array *get_commit_cherry_picks(struct commit *commit)
+{
+	struct object_array **slot =
+		commit_cherry_picks_peek(&cherry_picks, commit);
+
+	return slot ? *slot : NULL;
+}
+
+static struct object_array *get_create_commit_cherry_picks(struct commit *commit)
+{
+	struct object_array **slot =
+		commit_cherry_picks_at(&cherry_picks, commit);
+	struct object_array *cps = *slot;
+	int i;
+
+	if (cps)
+		return cps;
+
+	add_object_array(&commit->object, oid_to_hex(&commit->object.oid),
+			 &cherry_picked);
+	*slot = cps = xmalloc(sizeof(struct object_array));
+	*cps = (struct object_array)OBJECT_ARRAY_INIT;
+
+	read_cherry_picks_note(&commit->object.oid, cps);
+	if (verbose) {
+		for (i = 0; i < cps->nr; i++)
+			fprintf(stderr, "Read  note %s -> %s\n",
+				oid_to_hex(&commit->object.oid),
+				cps->objects[i].name);
+	}
+	return cps;
+}
+
+static void record_cherry_pick(struct commit *commit, void *unused)
+{
+	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--) {
+		const int prefix_len = sizeof(cherry_picked_prefix) - 1;
+		char *line = info.trailers[i];
+
+		if (!strncmp(line, cherry_picked_prefix, prefix_len)) {
+			struct object_id from_oid;
+			struct object *from_object;
+			struct commit *from_commit;
+			struct object_array *from_cps;
+			char cherry_hex[GIT_MAX_HEXSZ + 1];
+
+			if (get_oid_hex(line + prefix_len, &from_oid))
+				continue;
+
+			from_object = parse_object(the_repository, &from_oid);
+			if (!from_object || from_object->type != OBJ_COMMIT)
+				continue;
+
+			from_commit = (struct commit *)from_object;
+			from_cps = get_create_commit_cherry_picks(from_commit);
+
+			oid_to_hex_r(cherry_hex, &commit->object.oid);
+
+			if (!object_array_contains_name(from_cps, cherry_hex))
+				add_object_array(&commit->object, cherry_hex,
+						 from_cps);
+			break;
+		}
+	}
+
+	free(buffer);
+}
+
+static void clear_cherry_pick_note(struct commit *commit, void *prefix)
+{
+	struct argv_array args;
+
+	argv_array_init(&args);
+	argv_array_pushl(&args, "notes", "--ref", "cherry-picks", "remove",
+			 "--ignore-missing",
+			 oid_to_hex(&commit->object.oid), NULL);
+	cmd_notes(args.argc, args.argv, prefix);
+}
+
+static int note_cherry_picks(struct commit *commit, const char *prefix)
+{
+	char from_hex[GIT_MAX_HEXSZ + 1];
+	struct strbuf note = STRBUF_INIT;
+	struct argv_array args;
+	struct object_array *cps;
+	int i, ret;
+
+	cps = get_commit_cherry_picks(commit);
+	if (!cps)
+		return 0;
+
+	oid_to_hex_r(from_hex, &commit->object.oid);
+
+	for (i = 0; i < cps->nr; i++) {
+		const char *cherry_hex = cps->objects[i].name;
+
+		strbuf_addf(&note, "%s%s\n", NOTES_CHERRY_PICKED_TO, cherry_hex);
+		if (verbose)
+			fprintf(stderr, "Write note %s -> %s\n",
+				from_hex, cherry_hex);
+	}
+
+	argv_array_init(&args);
+	argv_array_pushl(&args, "notes", "--ref", "cherry-picks", "add",
+			 "--force", "--message", note.buf, from_hex, NULL);
+	if (!verbose)
+		argv_array_push(&args, "--quiet");
+	ret = cmd_notes(args.argc, args.argv, prefix);
+	strbuf_release(&note);
+	return ret;
+}
+
+int cmd_note_cherry_picks(int argc, const char **argv, const char *prefix)
+{
+	struct rev_info revs;
+	int i, ret;
+	struct setup_revision_opt s_r_opt = {
+		.def = "HEAD",
+		.revarg_opt = REVARG_CANNOT_BE_FILENAME
+	};
+	struct option options[] = {
+		OPT_BOOL(0, "clear", &clear, N_("clear cherry-pick notes from the specified commits")),
+		OPT__VERBOSE(&verbose, N_("verbose")),
+		OPT_END()
+	};
+
+	git_config(git_default_config, NULL);
+
+	init_revisions(&revs, prefix);
+	argc = setup_revisions(argc, argv, &revs, &s_r_opt);
+	argc = parse_options(argc, argv, prefix, options,
+			     note_cherry_picks_usage, 0);
+	if (argc > 1)
+		die(_("unrecognized argument: %s"), argv[1]);
+
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
+
+	if (clear) {
+		traverse_commit_list(&revs, clear_cherry_pick_note, NULL,
+				     (void *)prefix);
+		return 0;
+	}
+
+	init_commit_cherry_picks(&cherry_picks);
+	traverse_commit_list(&revs, record_cherry_pick, NULL, NULL);
+
+	object_array_remove_duplicates(&cherry_picked);
+
+	for (i = 0; i < cherry_picked.nr; i++) {
+		ret = note_cherry_picks((void *)cherry_picked.objects[i].item,
+					prefix);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
diff --git a/builtin/notes.c b/builtin/notes.c
index c05cd004a..6b623b25c 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -26,7 +26,7 @@
 
 static const char * const git_notes_usage[] = {
 	N_("git notes [--ref <notes-ref>] [list [<object>]]"),
-	N_("git notes [--ref <notes-ref>] add [-f] [--allow-empty] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
+	N_("git notes [--ref <notes-ref>] add [-f] [-q] [--allow-empty] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
 	N_("git notes [--ref <notes-ref>] copy [-f] <from-object> <to-object>"),
 	N_("git notes [--ref <notes-ref>] append [--allow-empty] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
 	N_("git notes [--ref <notes-ref>] edit [--allow-empty] [<object>]"),
@@ -394,7 +394,7 @@ static int append_edit(int argc, const char **argv, const char *prefix);
 
 static int add(int argc, const char **argv, const char *prefix)
 {
-	int force = 0, allow_empty = 0;
+	int force = 0, quiet = 0, allow_empty = 0;
 	const char *object_ref;
 	struct notes_tree *t;
 	struct object_id object, new_note;
@@ -416,6 +416,7 @@ static int add(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "allow-empty", &allow_empty,
 			N_("allow storing empty note")),
 		OPT__FORCE(&force, N_("replace existing notes"), PARSE_OPT_NOCOMPLETE),
+		OPT__QUIET(&quiet, N_("suppress informational messages")),
 		OPT_END()
 	};
 
@@ -455,8 +456,9 @@ static int add(int argc, const char **argv, const char *prefix)
 			argv[0] = "edit";
 			return append_edit(argc, argv, prefix);
 		}
-		fprintf(stderr, _("Overwriting existing notes for object %s\n"),
-			oid_to_hex(&object));
+		if (!quiet)
+			fprintf(stderr, _("Overwriting existing notes for object %s\n"),
+				oid_to_hex(&object));
 	}
 
 	prepare_note_data(&object, &d, note);
@@ -466,8 +468,9 @@ static int add(int argc, const char **argv, const char *prefix)
 			BUG("combine_notes_overwrite failed");
 		commit_notes(t, "Notes added by 'git notes add'");
 	} else {
-		fprintf(stderr, _("Removing note for object %s\n"),
-			oid_to_hex(&object));
+		if (!quiet)
+			fprintf(stderr, _("Removing note for object %s\n"),
+				oid_to_hex(&object));
 		remove_note(t, object.hash);
 		commit_notes(t, "Notes removed by 'git notes add'");
 	}
@@ -898,7 +901,7 @@ static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag
 static int remove_cmd(int argc, const char **argv, const char *prefix)
 {
 	unsigned flag = 0;
-	int from_stdin = 0;
+	int from_stdin = 0, quiet = 0;
 	struct option options[] = {
 		OPT_BIT(0, "ignore-missing", &flag,
 			N_("attempt to remove non-existent note is not an error"),
diff --git a/git.c b/git.c
index a6f4b44af..aedafff02 100644
--- a/git.c
+++ b/git.c
@@ -512,6 +512,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 },
+	{ "note-cherry-picks", cmd_note_cherry_picks, 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 },
diff --git a/notes.c b/notes.c
index 25cdce28b..19fa3451d 100644
--- a/notes.c
+++ b/notes.c
@@ -1189,6 +1189,26 @@ void free_notes(struct notes_tree *t)
 	memset(t, 0, sizeof(struct notes_tree));
 }
 
+static void walk_cherry_picks(struct object_id *from_oid, struct strbuf *sb,
+			      int level)
+{
+	struct object_array cps = OBJECT_ARRAY_INIT;
+	int i;
+
+	read_cherry_picks_note(from_oid, &cps);
+
+	for (i = 0; i < cps.nr; i++) {
+		strbuf_addf(sb, "    %s%*s%s\n",
+			    NOTES_CHERRY_PICKED_TO, 2 * level, "",
+			    cps.objects[i].name);
+		if (cps.objects[i].item)
+			walk_cherry_picks(&cps.objects[i].item->oid, sb,
+					  level + 1);
+	}
+
+	object_array_clear(&cps);
+}
+
 /*
  * Fill the given strbuf with the notes associated with the given object.
  *
@@ -1208,6 +1228,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_cherry_picks;
 
 	if (!t)
 		t = &default_notes_tree;
@@ -1250,6 +1271,8 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
 		}
 	}
 
+	format_cherry_picks = !raw && !strcmp(t->ref, "refs/notes/cherry-picks");
+
 	for (msg_p = msg; msg_p < msg + msglen; msg_p += linelen + 1) {
 		linelen = strchrnul(msg_p, '\n') - msg_p;
 
@@ -1257,6 +1280,17 @@ 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_cherry_picks &&
+		    starts_with(msg_p, NOTES_CHERRY_PICKED_TO)) {
+			struct object_id oid;
+
+			if (get_oid_hex(msg_p + strlen(NOTES_CHERRY_PICKED_TO),
+					&oid))
+				continue;
+
+			walk_cherry_picks(&oid, sb, 1);
+		}
 	}
 
 	free(msg);
@@ -1309,3 +1343,64 @@ void expand_loose_notes_ref(struct strbuf *sb)
 		expand_notes_ref(sb);
 	}
 }
+
+void read_cherry_picks_note(const struct object_id *commit_oid,
+			    struct object_array *result)
+{
+	static struct notes_tree notes_tree;
+	const struct object_id *note_oid;
+	unsigned long size;
+	enum object_type type;
+	char *note;
+	struct strbuf **lines, **pline;
+
+	if (!notes_tree.initialized)
+		init_notes(&notes_tree, NOTES_CHERRY_PICKS_REF, NULL, 0);
+
+	note_oid = get_note(&notes_tree, commit_oid);
+	if (!note_oid)
+		return;
+
+	note = read_object_file(note_oid, &type, &size);
+	if (!size) {
+		free(note);
+		return;
+	}
+
+	lines = strbuf_split_buf(note, size, '\n', 0);
+
+	for (pline = lines; *pline; pline++) {
+		struct strbuf *line = *pline;
+		const char *cherry_hex;
+		struct object_id cherry_oid;
+		struct object *cherry_obj;
+
+		strbuf_trim(line);
+
+		if (!starts_with(line->buf, NOTES_CHERRY_PICKED_TO)) {
+			warning("read invalid cherry-pick note on %s: %s",
+				oid_to_hex(commit_oid), line->buf);
+			continue;
+		}
+
+		cherry_hex = line->buf + strlen(NOTES_CHERRY_PICKED_TO);
+
+		if (get_oid_hex(cherry_hex, &cherry_oid)) {
+			warning("read invalid cherry-pick sha1 on %s: %s",
+				oid_to_hex(commit_oid), line->buf);
+			continue;
+		}
+
+		cherry_obj = parse_object(the_repository, &cherry_oid);
+		if (!cherry_obj || cherry_obj->type != OBJ_COMMIT) {
+			warning("read invalid cherry-pick commit on %s: %s",
+				oid_to_hex(commit_oid), line->buf);
+			continue;
+		}
+
+		add_object_array(cherry_obj, cherry_hex, result);
+	}
+
+	strbuf_list_free(lines);
+	free(note);
+}
diff --git a/notes.h b/notes.h
index 414bc6855..b58899c74 100644
--- a/notes.h
+++ b/notes.h
@@ -2,10 +2,14 @@
 #define NOTES_H
 
 #include "string-list.h"
+#include "object.h"
 
 struct object_id;
 struct strbuf;
 
+#define NOTES_CHERRY_PICKS_REF		"refs/notes/cherry-picks"
+#define NOTES_CHERRY_PICKED_TO		"Cherry-picked-to: "
+
 /*
  * Function type for combining two notes annotating the same object.
  *
@@ -317,4 +321,7 @@ void expand_notes_ref(struct strbuf *sb);
  */
 void expand_loose_notes_ref(struct strbuf *sb);
 
+void read_cherry_picks_note(const struct object_id *commit_oid,
+			    struct object_array *result);
+
 #endif
diff --git a/object.c b/object.c
index e54160550..f79652a34 100644
--- a/object.c
+++ b/object.c
@@ -404,7 +404,7 @@ void object_array_clear(struct object_array *array)
 /*
  * Return true iff array already contains an entry with name.
  */
-static int contains_name(struct object_array *array, const char *name)
+int object_array_contains_name(struct object_array *array, const char *name)
 {
 	unsigned nr = array->nr, i;
 	struct object_array_entry *object = array->objects;
@@ -422,7 +422,7 @@ void object_array_remove_duplicates(struct object_array *array)
 
 	array->nr = 0;
 	for (src = 0; src < nr; src++) {
-		if (!contains_name(array, objects[src].name)) {
+		if (!object_array_contains_name(array, objects[src].name)) {
 			if (src != array->nr)
 				objects[array->nr] = objects[src];
 			array->nr++;
diff --git a/object.h b/object.h
index 0feb90ae6..ee14ce595 100644
--- a/object.h
+++ b/object.h
@@ -172,6 +172,12 @@ typedef int (*object_array_each_func_t)(struct object_array_entry *, void *);
 void object_array_filter(struct object_array *array,
 			 object_array_each_func_t want, void *cb_data);
 
+/*
+ * Returns 1 if array already contains an entry with the specified name.
+ * Otherwise, 0.
+ */
+int object_array_contains_name(struct object_array *array, const char *name);
+
 /*
  * Remove from array all but the first entry with a given name.
  * Warning: this function uses an O(N^2) algorithm.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] cherry-pick notes to find out cherry-picks from the origin
  2018-10-17 14:39 [RFC] cherry-pick notes to find out cherry-picks from the origin Tejun Heo
@ 2018-10-24 12:24 ` Tejun Heo
  2018-11-13 18:00   ` Tejun Heo
  2018-11-15 14:40 ` Jeff King
  1 sibling, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2018-10-24 12:24 UTC (permalink / raw)
  To: git, Junio C Hamano, Jeff King; +Cc: kernel-team

Ping, thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] cherry-pick notes to find out cherry-picks from the origin
  2018-10-24 12:24 ` Tejun Heo
@ 2018-11-13 18:00   ` Tejun Heo
  0 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2018-11-13 18:00 UTC (permalink / raw)
  To: git, Junio C Hamano, Jeff King; +Cc: kernel-team

On Wed, Oct 24, 2018 at 05:24:01AM -0700, Tejun Heo wrote:
> Ping, thanks.

Ping again.  Any comments?  Wasn't this the direction you guys were
suggesting?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] cherry-pick notes to find out cherry-picks from the origin
  2018-10-17 14:39 [RFC] cherry-pick notes to find out cherry-picks from the origin Tejun Heo
  2018-10-24 12:24 ` Tejun Heo
@ 2018-11-15 14:40 ` Jeff King
  2018-12-05 16:20   ` Tejun Heo
  1 sibling, 1 reply; 6+ messages in thread
From: Jeff King @ 2018-11-15 14:40 UTC (permalink / raw)
  To: Tejun Heo; +Cc: git, Junio C Hamano, kernel-team

On Wed, Oct 17, 2018 at 07:39:21AM -0700, Tejun Heo wrote:

> A while ago, I proposed changes to name-rev and describe so that they
> can identify the commits cherry-picked from the one which is being
> shown.
> 
>   https://public-inbox.org/git/20180726153714.GX1934745@devbig577.frc2.facebook.com/T/
> 
> While the use-cases - e.g. tracking down which release / stable
> branches a given commit ended up in - weren't controversial, it was
> suggested that it'd make more sense to use notes to link cherry-picks
> instead of building the feature into name-rev.

Sorry for the slow reply. This was on my to-look-at pile, but for
some reason I accidentally put in my done pile.

> The patch appended to this message implements most of it (sans tests
> and documentation).  It's composed of the following two parts.
> 
> * A new built-in command note-cherry-picks, which walks the specified
>   commits and if they're marked with the cherry-pick trailer, adds the
>   backlink to the origin commit using Cherry-picked-to tag in a
>   cherry-picks note.

That makes sense. I think this could also be an option to cherry-pick,
to instruct it to create the note when the cherry-pick is made.

But you may still want a command to backfill older cherry-picks, or
those done by other people who do not care themselves about maintaining
the notes tree.

It _feels_ like this is something that should be do-able by plugging a
few commands together, rather than writing a new C program. I.e.,
something like:

  git rev-list --format='%(trailers)' HEAD |
  perl -lne '
	/^commit ([0-9]+)/ and $commit = $1;
	/^\(cherry picked from commit ([0-9]+)/
		and print "$commit $1";
  ' |
  while read from to; do
	# One process per note isn't very efficient. Ideally there would
	# be an "append --stdin" mode. Double points if it understands
	# how to avoid adding existing lines.
	git notes append -m "Cherry-picked-to: $to" $from
  done

which is roughly what your program is doing.  Not that I'm entirely
opposed to doing something in C (we've been moving away from shell
scripts anyway). But mostly I am wondering if we can leverage existing
tools, and fill in their gaps in a way that lets people easily do
similar things.

And on that note...

> * When formatting a cherry-picks note for display, nested cherry-picks
>   are followed from each Cherry-picked-to tag and printed out with
>   matching indentations.

That makes sense to me, but does this have to be strictly related to
cherry-picks? I.e., in the more generic form, could we have a way of
marking a note as "transitive" for display, and the notes-display code
would automatically recognize and walk hashes?

That would serve your purpose, but would also allow similar things to
easily be done in the future.

> Combined with name-rev --stdin, it can produce outputs like the following.
> [...]

Yeah, that looks pretty good.

> Locally, the notes can be kept up-to-date with a trivial post-commit
> hook which invokes note-cherry-picks on the new commit; however, I'm
> having a bit of trouble figuring out a way to keep it up-to-date when
> multiple trees are involved.  AFAICS, there are two options.
> 
> 1. Ensuring that the notes are always generated on local commits and
>    whenever new commits are received through fetch/pulls.
> 
> 2. Ensuring that the notes are always generated on local commits and
>    transported with push/pulls.
> 
> 3. A hybrid approach - also generate notes on the receiving end and
>    ensure that fetch/pulls receives the notes together (ie. similar to
>    --tags option to git-fetch).
> 
> #1 seems simpler and more robust to me.  Unfortunately, I can't see a
> way to implement any of the three options with the existing hooks.
> For #1, there's no post-fetch hook.  For #2 and #3, there doesn't seem
> to be a fool-proof way to make sure that the notes are transported
> together.  Any suggestions would be greatly appreciated.

Yeah, I think (1) is the simplest: it becomes a purely local thing that
you've generated these annotations. Unfortunately, no, I don't think
there's anything like a post-fetch hook. This might be a good reason to
have one. One can always do "git fetch && update-notes" of course, but
having fetch feed the script the set of updated ref tips would be very
helpful (so you know you can traverse from $old..$new looking for
cherry-picks).

For (2) and (3), you can push/pull a notes tree, but setting up the
refspecs and config is fairly manual. Using the cherry-pick option I
suggested above would most locally-made picks, but you'd probably still
need to backfill sometimes anyway.

I don't think you'd need to be (nor am I sure you _could_ be) as fancy
as fetching notes and their respective commits together. The notes are
bound together in a single tree, so you cannot say "I don't have commit
1234abcd, I don't need the note for it". You get the whole tree. And
that is not such a bad thing. The big reason _not_ to fetch tags that
point to commits we don't have is that the tag fetch implies
reachability, which may mean sucking in a big chunk of history. Whereas
you can happily have a note pointing to a commit you don't have (though
I suppose you should avoid running "git notes prune" in that case).

> ---
>  Makefile                    |    1 
>  builtin.h                   |    1 
>  builtin/note-cherry-picks.c |  197 ++++++++++++++++++++++++++++++++++++++++++++
>  builtin/notes.c             |   17 ++-
>  git.c                       |    1 
>  notes.c                     |   95 +++++++++++++++++++++
>  notes.h                     |    7 +
>  object.c                    |    4 
>  object.h                    |    6 +
>  9 files changed, 320 insertions(+), 9 deletions(-)

I only looked briefly over your implementation, but didn't see anything
obviously wrong. I do think it would be nice to make it more generic, as
much as possible. I think the most generic form is really:

  traverse-and-show-trailers | invert-trailers | add-notes

In theory I should be able to do the same inversion step on any trailer
which mentions another commit.

If it is going to stay in C and be cherry-pick-specific, one obvious
improvement would be to use the notes API directly, rather than spawning
subprocesses. That should be much more efficient if you have a lot of
notes to write.

-Peff

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] cherry-pick notes to find out cherry-picks from the origin
  2018-11-15 14:40 ` Jeff King
@ 2018-12-05 16:20   ` Tejun Heo
  2018-12-06 22:15     ` Tejun Heo
  0 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2018-12-05 16:20 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, kernel-team

Hello, Jeff.

On Thu, Nov 15, 2018 at 09:40:44AM -0500, Jeff King wrote:
> Sorry for the slow reply. This was on my to-look-at pile, but for
> some reason I accidentally put in my done pile.

No worries and sorry about my late reply too.  Things were a bit
hectic.

> > * A new built-in command note-cherry-picks, which walks the specified
> >   commits and if they're marked with the cherry-pick trailer, adds the
> >   backlink to the origin commit using Cherry-picked-to tag in a
> >   cherry-picks note.
> 
> That makes sense. I think this could also be an option to cherry-pick,
> to instruct it to create the note when the cherry-pick is made.
> 
> But you may still want a command to backfill older cherry-picks, or
> those done by other people who do not care themselves about maintaining
> the notes tree.

So, I wanted to do both with the same command.  git-cherry-pick knows
which commits are new, so it can just pass those commits to
note-cherry-picks.  When backfilling the whole tree or newly pulled
commits, the appropriate command can invoke note-cherry-picks with the
new commits which should be super cheap.

> It _feels_ like this is something that should be do-able by plugging a
> few commands together, rather than writing a new C program. I.e.,
> something like:
> 
>   git rev-list --format='%(trailers)' HEAD |
>   perl -lne '
> 	/^commit ([0-9]+)/ and $commit = $1;
> 	/^\(cherry picked from commit ([0-9]+)/
> 		and print "$commit $1";
>   ' |
>   while read from to; do
> 	# One process per note isn't very efficient. Ideally there would
> 	# be an "append --stdin" mode. Double points if it understands
> 	# how to avoid adding existing lines.
> 	git notes append -m "Cherry-picked-to: $to" $from
>   done
> 
> which is roughly what your program is doing.  Not that I'm entirely
> opposed to doing something in C (we've been moving away from shell
> scripts anyway). But mostly I am wondering if we can leverage existing

But the above wouldn't clean up stale commits, which could happen with
e.g. abandoned releases, and would be prone to creating duplicates.
We sure can add all those to shell / perl scripts but it's difficult
for me to see the upsides of doing it that way.

> tools, and fill in their gaps in a way that lets people easily do
> similar things.

I'll respond to this together below.

> And on that note...
> 
> > * When formatting a cherry-picks note for display, nested cherry-picks
> >   are followed from each Cherry-picked-to tag and printed out with
> >   matching indentations.
> 
> That makes sense to me, but does this have to be strictly related to
> cherry-picks? I.e., in the more generic form, could we have a way of
> marking a note as "transitive" for display, and the notes-display code
> would automatically recognize and walk hashes?
> 
> That would serve your purpose, but would also allow similar things to
> easily be done in the future.

Below.

> > Combined with name-rev --stdin, it can produce outputs like the following.
> > [...]
> 
> Yeah, that looks pretty good.
> 
> > Locally, the notes can be kept up-to-date with a trivial post-commit
> > hook which invokes note-cherry-picks on the new commit; however, I'm
> > having a bit of trouble figuring out a way to keep it up-to-date when
> > multiple trees are involved.  AFAICS, there are two options.
> > 
> > 1. Ensuring that the notes are always generated on local commits and
> >    whenever new commits are received through fetch/pulls.
> > 
> > 2. Ensuring that the notes are always generated on local commits and
> >    transported with push/pulls.
> > 
> > 3. A hybrid approach - also generate notes on the receiving end and
> >    ensure that fetch/pulls receives the notes together (ie. similar to
> >    --tags option to git-fetch).
> > 
> > #1 seems simpler and more robust to me.  Unfortunately, I can't see a
> > way to implement any of the three options with the existing hooks.
> > For #1, there's no post-fetch hook.  For #2 and #3, there doesn't seem
> > to be a fool-proof way to make sure that the notes are transported
> > together.  Any suggestions would be greatly appreciated.
> 
> Yeah, I think (1) is the simplest: it becomes a purely local thing that
> you've generated these annotations. Unfortunately, no, I don't think
> there's anything like a post-fetch hook. This might be a good reason to
> have one. One can always do "git fetch && update-notes" of course, but
> having fetch feed the script the set of updated ref tips would be very
> helpful (so you know you can traverse from $old..$new looking for
> cherry-picks).

This sounds great to me.

> I only looked briefly over your implementation, but didn't see anything
> obviously wrong. I do think it would be nice to make it more generic, as
> much as possible. I think the most generic form is really:
> 
>   traverse-and-show-trailers | invert-trailers | add-notes
> 
> In theory I should be able to do the same inversion step on any trailer
> which mentions another commit.

Hmm... yeah, I can definitely separate out the first part into a
separate function, move dedup of notes to add-notes, and then glue
them together.

I'm a bit hesitant to expose the first part as a separate command
without knowing what other use cases would actually look like.  I'll
make it so that it's easy to do so when a new use case arises.

> If it is going to stay in C and be cherry-pick-specific, one obvious
> improvement would be to use the notes API directly, rather than spawning
> subprocesses. That should be much more efficient if you have a lot of
> notes to write.

I'll try to make the components more generic and then glue them
together in C and make it use the APIs directly.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] cherry-pick notes to find out cherry-picks from the origin
  2018-12-05 16:20   ` Tejun Heo
@ 2018-12-06 22:15     ` Tejun Heo
  0 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2018-12-06 22:15 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, kernel-team

Hello, Jeff.

So, this is what I currently have.  It still does the same thing but a
lot more generic in terms of both interface and implementation.

* All core logics are implemented as core helpers / features.

  * Trailer parsing and reverse-mapping in trailer_rev_xrefs_*().

  * Note refs which start with xref- (cross-reference) are recognized
    by notes core.  When notes are added, a dedicated combine_notes
    function is used to remove duplicates and curl unreachable
    commits.  When xref- notes are formatted for printing, it
    automatically follows and prints nested xrefs.

* note-cherry-picks is replaced with reverse-trailer-xrefs which can
  use other trailers, note refs and tags.  --xref-cherry-picks option
  makes it use the cherry-pick presets.

Please note that the patch is still a bit rough.  I'm polishing and
documenting.  Please let me know what you think.

Thanks.

---
 Makefile                        |    1 
 builtin.h                       |    1 
 builtin/reverse-trailer-xrefs.c |  148 ++++++++++++++++++++++++
 git.c                           |    1 
 notes.c                         |  245 +++++++++++++++++++++++++++++++++++++++-
 notes.h                         |   10 +
 object.c                        |    4 
 object.h                        |    6 
 trailer.c                       |  102 ++++++++++++++++
 trailer.h                       |   26 ++++
 10 files changed, 540 insertions(+), 4 deletions(-)

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..b2879be6c
--- /dev/null
+++ b/builtin/reverse-trailer-xrefs.c
@@ -0,0 +1,148 @@
+#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 [<options>] [<commit-ish>...]"),
+	NULL
+};
+
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
+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 *from_commit, struct object_array *to_objs,
+			      const char *tag)
+{
+	char from_hex[GIT_MAX_HEXSZ + 1];
+	struct strbuf note = STRBUF_INIT;
+	struct object_id note_oid;
+	int i, ret;
+
+	oid_to_hex_r(from_hex, &from_commit->object.oid);
+
+	for (i = 0; i < to_objs->nr; i++) {
+		const char *hex = to_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", from_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, &from_commit->object.oid, &note_oid, NULL);
+	return ret;
+}
+
+int cmd_reverse_trailer_xrefs(int argc, const char **argv, const char *prefix)
+{
+	static struct notes_tree tree;
+	struct rev_info revs;
+	int i, ret;
+	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 options for xref-cherry-picks notes")),
+		OPT_STRING(0, "trailer-prefix", &trailer_prefix, N_("prefix"), N_("process trailers starting with <prefix>")),
+		OPT_STRING(0, "ref", &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);
+
+	init_revisions(&revs, prefix);
+	argc = setup_revisions(argc, argv, &revs, &s_r_opt);
+	argc = parse_options(argc, argv, prefix, options,
+			     reverse_trailer_xrefs_usage, 0);
+
+	/* allow inidividual options to override parts of --cherry */
+	if (cherry) {
+		if (!trailer_prefix)
+			trailer_prefix = cherry_picked_prefix;
+		if (!notes_ref)
+			notes_ref = NOTES_CHERRY_PICKS_REF;
+		if (!tag)
+			tag = NOTES_CHERRY_PICKED_TO_TAG;
+	}
+
+	if (!notes_ref || (!clear && (!trailer_prefix || !tag)))
+		die(_("insufficient arguments"));
+
+	if (argc > 1)
+		die(_("unrecognized argument: %s"), argv[1]);
+
+	if (!tree.initialized)
+		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 *from_commit;
+		struct object_array *to_objs;
+
+		trailer_rev_xrefs_init(&rxrefs, trailer_prefix);
+		traverse_commit_list(&revs, record_trailer_xrefs, NULL, &rxrefs);
+
+		trailer_rev_xrefs_for_each(&rxrefs, i, from_commit, to_objs) {
+			ret = note_trailer_xrefs(&tree, from_commit, to_objs,
+						 tag);
+			if (ret)
+				return ret;
+		}
+	}
+
+	commit_notes(&tree, "Notes updated by 'git reverse-trailer-xrefs'");
+
+	return 0;
+}
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 },
diff --git a/notes.c b/notes.c
index 25cdce28b..c32064bfe 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
@@ -79,6 +80,10 @@ static struct notes_tree **display_notes_trees;
 
 static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 		struct int_node *node, unsigned int n);
+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);
 
 /*
  * Search the tree until the appropriate location for the given key is found:
@@ -914,6 +919,60 @@ int combine_notes_cat_sort_uniq(struct object_id *cur_oid,
 	return ret;
 }
 
+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;
+}
+
 static int string_list_add_one_ref(const char *refname, const struct object_id *oid,
 				   int flag, void *cb)
 {
@@ -996,8 +1055,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 +1252,67 @@ void free_notes(struct notes_tree *t)
 	memset(t, 0, sizeof(struct notes_tree));
 }
 
+/*
+ * Parse a "[TAG:]HEX" line.  @xref is trimmed.  If @tag_p is not NULL and
+ * TAG exists, the string is split.  Returns the pointer to the OID and
+ * *@tag_p is updated to the TAG if requested.
+ */
+static char *parse_xref(char *xref, char **tag_p)
+{
+	char *p, *hex;
+
+	while (isspace(*xref))
+		xref++;
+
+	p = strchr(xref, ':');
+	if (p) {
+		if (tag_p) {
+			*tag_p = xref;
+			*p = '\0';
+		}
+		p++;
+		while (isspace(*p))
+			p++;
+		hex = p;
+	} else {
+		if (tag_p)
+			*tag_p = NULL;
+		hex = xref;
+	}
+
+	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)
+			walk_xrefs(tree_ref, &xrefs.objects[i].item->oid, sb,
+				   level + 1);
+	}
+
+	object_array_clear(&xrefs);
+	string_list_clear(&lines, 0);
+}
+
 /*
  * Fill the given strbuf with the notes associated with the given object.
  *
@@ -1208,6 +1332,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 +1375,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 +1384,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 +1444,109 @@ void expand_loose_notes_ref(struct strbuf *sb)
 		expand_notes_ref(sb);
 	}
 }
+
+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);
+}
+
+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);
+}
+
+/*
+ * Read a cross-referencing note.
+ *
+ * Notes in @notes_ref contains lines of "[TAG:]HEX" pointing to other
+ * commits.  Read the target commits and add the objects to @result.  If
+ * @result_lines is non-NULL, it should point to a strdup'ing string_list.
+ * The verbatim note lines matching the target commits are appened to the
+ * list.
+ */
+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);
+}
diff --git a/notes.h b/notes.h
index 414bc6855..fb8153334 100644
--- a/notes.h
+++ b/notes.h
@@ -2,10 +2,14 @@
 #define NOTES_H
 
 #include "string-list.h"
+#include "object.h"
 
 struct object_id;
 struct strbuf;
 
+#define NOTES_CHERRY_PICKS_REF		"refs/notes/xref-cherry-picks"
+#define NOTES_CHERRY_PICKED_TO_TAG	"Cherry-picked-to"
+
 /*
  * Function type for combining two notes annotating the same object.
  *
@@ -38,6 +42,8 @@ int combine_notes_ignore(struct object_id *cur_oid,
 			 const struct object_id *new_oid);
 int combine_notes_cat_sort_uniq(struct object_id *cur_oid,
 				const struct object_id *new_oid);
+int combine_notes_cat_xrefs(struct object_id *cur_oid,
+			    const struct object_id *new_oid);
 
 /*
  * Notes tree object
@@ -317,4 +323,8 @@ void expand_notes_ref(struct strbuf *sb);
  */
 void expand_loose_notes_ref(struct strbuf *sb);
 
+void read_xref_note(const char *notes_ref, const struct object_id *commit_oid,
+		    struct object_array *result,
+		    struct string_list *result_lines);
+
 #endif
diff --git a/object.c b/object.c
index e54160550..f79652a34 100644
--- a/object.c
+++ b/object.c
@@ -404,7 +404,7 @@ void object_array_clear(struct object_array *array)
 /*
  * Return true iff array already contains an entry with name.
  */
-static int contains_name(struct object_array *array, const char *name)
+int object_array_contains_name(struct object_array *array, const char *name)
 {
 	unsigned nr = array->nr, i;
 	struct object_array_entry *object = array->objects;
@@ -422,7 +422,7 @@ void object_array_remove_duplicates(struct object_array *array)
 
 	array->nr = 0;
 	for (src = 0; src < nr; src++) {
-		if (!contains_name(array, objects[src].name)) {
+		if (!object_array_contains_name(array, objects[src].name)) {
 			if (src != array->nr)
 				objects[array->nr] = objects[src];
 			array->nr++;
diff --git a/object.h b/object.h
index 796792cb3..a0b3dd312 100644
--- a/object.h
+++ b/object.h
@@ -172,6 +172,12 @@ typedef int (*object_array_each_func_t)(struct object_array_entry *, void *);
 void object_array_filter(struct object_array *array,
 			 object_array_each_func_t want, void *cb_data);
 
+/*
+ * Returns 1 if array already contains an entry with the specified name.
+ * Otherwise, 0.
+ */
+int object_array_contains_name(struct object_array *array, const char *name);
+
 /*
  * Remove from array all but the first entry with a given name.
  * Warning: this function uses an O(N^2) algorithm.
diff --git a/trailer.c b/trailer.c
index 0796f326b..3afa38d25 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,104 @@ 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->from_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 *tag)
+{
+	rxrefs->tag = xstrdup(tag);
+	rxrefs->tag_len = strlen(tag);
+	init_trailer_rxrefs_slab(&rxrefs->slab);
+	rxrefs->from_commits = (struct object_array)OBJECT_ARRAY_INIT;
+}
+
+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->tag)) {
+			struct object_id from_oid;
+			struct object *from_object;
+			struct commit *from_commit;
+			struct object_array *to_objs;
+			char cherry_hex[GIT_MAX_HEXSZ + 1];
+
+			if (get_oid_hex(line + rxrefs->tag_len, &from_oid))
+				continue;
+
+			from_object = parse_object(the_repository, &from_oid);
+			if (!from_object || from_object->type != OBJ_COMMIT)
+				continue;
+
+			from_commit = (struct commit *)from_object;
+			to_objs = get_create_trailer_rxrefs(rxrefs, from_commit);
+
+			oid_to_hex_r(cherry_hex, &commit->object.oid);
+			add_object_array(&commit->object, cherry_hex, to_objs);
+			break;
+		}
+	}
+
+	free(buffer);
+}
+
+void trailer_rev_xrefs_release(struct trailer_rev_xrefs *rxrefs)
+{
+	clear_trailer_rxrefs_slab(&rxrefs->slab);
+	object_array_clear(&rxrefs->from_commits);
+	free(rxrefs->tag);
+}
+
+void trailer_rev_xrefs_next(struct trailer_rev_xrefs *rxrefs, int *idx_p,
+			    struct commit **from_commit_p,
+			    struct object_array **to_objs_p)
+{
+	if (*idx_p >= rxrefs->from_commits.nr) {
+		*from_commit_p = NULL;
+		*to_objs_p = NULL;
+		return;
+	}
+
+	*from_commit_p = (struct commit *)
+		rxrefs->from_commits.objects[*idx_p].item;
+	*to_objs_p = get_trailer_rxrefs(rxrefs, *from_commit_p);
+	(*idx_p)++;
+}
diff --git a/trailer.h b/trailer.h
index b99773964..5a9704e19 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,28 @@ 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);
 
+declare_commit_slab(trailer_rxrefs_slab, struct object_array *);
+
+struct trailer_rev_xrefs {
+	char *tag;
+	int tag_len;
+	struct trailer_rxrefs_slab slab;
+	struct object_array from_commits;
+};
+
+void trailer_rev_xrefs_init(struct trailer_rev_xrefs *rxrefs, const char *tag);
+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 **from_commit_p,
+			    struct object_array **to_objs_p);
+
+#define trailer_rev_xrefs_for_each(rxrefs, idx, from_commit, to_objs)		\
+	for ((idx) = 0,								\
+	     trailer_rev_xrefs_next(rxrefs, &(idx), &(from_commit), &(to_objs));\
+	     (from_commit);							\
+	     trailer_rev_xrefs_next(rxrefs, &(idx), &(from_commit), &(to_objs)))
+
 #endif /* TRAILER_H */

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-17 14:39 [RFC] cherry-pick notes to find out cherry-picks from the origin Tejun Heo
2018-10-24 12:24 ` Tejun Heo
2018-11-13 18:00   ` Tejun Heo
2018-11-15 14:40 ` Jeff King
2018-12-05 16:20   ` Tejun Heo
2018-12-06 22:15     ` Tejun Heo

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox