git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCHv4 10/21] git notes merge: Initial implementation handling trivial merges only

From
Johan Herland <johan@herland.net>
Date
Oct 21, 2010, 02:08 UTC
Message-ID
<1287626936-32232-11-git-send-email-johan@herland.net>
In-Reply-To
<1287626936-32232-1-git-send-email-johan@herland.net>

This initial implementation of 'git notes merge' only handles the trivial merge cases (i.e. where the merge is either a no-op, or a fast-forward).

The patch includes testcases for these trivial merge cases.
Future patches will extend the functionality of 'git notes merge'.
This patch has been improved by the following contributions:
- Stephen Boyd: Simplify argc logic
- Stephen Boyd: Use test_commit
- Ævar Arnfjörð Bjarmason: Don't use C99 comments.
- Jonathan Nieder: Add constants for common verbosity values
- Jonathan Nieder: Use trace_printf(...) instead of OUTPUT(o, 5, ...)
- Jonathan Nieder: Remove extraneous show() function
- Jonathan Nieder: Clarify handling of empty/missing notes ref in notes_merge()
Thanks-to: Stephen Boyd <bebarino@gmail.com>
Thanks-to: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Thanks-to: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Makefile               |    2 +
 builtin/notes.c        |   54 ++++++++++++++
 notes-merge.c          |  120 ++++++++++++++++++++++++++++++++
 notes-merge.h          |   36 ++++++++++
 t/t3308-notes-merge.sh |  180 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 392 insertions(+), 0 deletions(-)
 create mode 100644 notes-merge.c
 create mode 100644 notes-merge.h
 create mode 100755 t/t3308-notes-merge.sh
diff --git a/Makefile b/Makefile
index f33648d..14c0ff1 100644
--- a/Makefile
+++ b/Makefile
@@ -503,6 +503,7 @@ LIB_H += mailmap.h
 LIB_H += merge-recursive.h
 LIB_H += notes.h
 LIB_H += notes-cache.h
+LIB_H += notes-merge.h
 LIB_H += object.h
 LIB_H += pack.h
 LIB_H += pack-refs.h
@@ -593,6 +594,7 @@ LIB_OBJS += merge-recursive.o
 LIB_OBJS += name-hash.o
 LIB_OBJS += notes.o
 LIB_OBJS += notes-cache.o
+LIB_OBJS += notes-merge.o
 LIB_OBJS += object.o
 LIB_OBJS += pack-check.o
 LIB_OBJS += pack-refs.o
diff --git a/builtin/notes.c b/builtin/notes.c
index 9c91c59..fbabdc7 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -17,6 +17,7 @@
 #include "run-command.h"
 #include "parse-options.h"
 #include "string-list.h"
+#include "notes-merge.h"
 
 static const char * const git_notes_usage[] = {
 	"git notes [--ref <notes_ref>] [list [<object>]]",
@@ -25,6 +26,7 @@ static const char * const git_notes_usage[] = {
 	"git notes [--ref <notes_ref>] append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]",
 	"git notes [--ref <notes_ref>] edit [<object>]",
 	"git notes [--ref <notes_ref>] show [<object>]",
+	"git notes [--ref <notes_ref>] merge [-v | -q] <notes_ref>",
 	"git notes [--ref <notes_ref>] remove [<object>]",
 	"git notes [--ref <notes_ref>] prune [-n | -v]",
 	NULL
@@ -61,6 +63,11 @@ static const char * const git_notes_show_usage[] = {
 	NULL
 };
 
+static const char * const git_notes_merge_usage[] = {
+	"git notes merge [<options>] <notes_ref>",
+	NULL
+};
+
 static const char * const git_notes_remove_usage[] = {
 	"git notes remove [<object>]",
 	NULL
@@ -772,6 +779,51 @@ static int show(int argc, const char **argv, const char *prefix)
 	return retval;
 }
 
+static int merge(int argc, const char **argv, const char *prefix)
+{
+	struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;
+	unsigned char result_sha1[20];
+	struct notes_merge_options o;
+	int verbosity = 0, result;
+	struct option options[] = {
+		OPT__VERBOSITY(&verbosity),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_notes_merge_usage, 0);
+
+	if (argc != 1) {
+		error("Must specify a notes ref to merge");
+		usage_with_options(git_notes_merge_usage, options);
+	}
+
+	init_notes_merge_options(&o);
+	o.verbosity = verbosity + NOTES_MERGE_VERBOSITY_DEFAULT;
+
+	o.local_ref = default_notes_ref();
+	strbuf_addstr(&remote_ref, argv[0]);
+	expand_notes_ref(&remote_ref);
+	o.remote_ref = remote_ref.buf;
+
+	result = notes_merge(&o, result_sha1);
+
+	strbuf_addf(&msg, "notes: Merged notes from %s into %s",
+		    remote_ref.buf, default_notes_ref());
+	if (result == 0) { /* Merge resulted (trivially) in result_sha1 */
+		/* Update default notes ref with new commit */
+		update_ref(msg.buf, default_notes_ref(), result_sha1, NULL,
+			   0, DIE_ON_ERR);
+	} else {
+		/* TODO: */
+		die("'git notes merge' cannot yet handle non-trivial merges!");
+	}
+
+	strbuf_release(&remote_ref);
+	strbuf_release(&msg);
+	return 0;
+}
+
 static int remove_cmd(int argc, const char **argv, const char *prefix)
 {
 	struct option options[] = {
@@ -865,6 +917,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		result = append_edit(argc, argv, prefix);
 	else if (!strcmp(argv[0], "show"))
 		result = show(argc, argv, prefix);
+	else if (!strcmp(argv[0], "merge"))
+		result = merge(argc, argv, prefix);
 	else if (!strcmp(argv[0], "remove"))
 		result = remove_cmd(argc, argv, prefix);
 	else if (!strcmp(argv[0], "prune"))
diff --git a/notes-merge.c b/notes-merge.c
new file mode 100644
index 0000000..cd917f9
--- /dev/null
+++ b/notes-merge.c
@@ -0,0 +1,120 @@
+#include "cache.h"
+#include "commit.h"
+#include "refs.h"
+#include "notes-merge.h"
+
+void init_notes_merge_options(struct notes_merge_options *o)
+{
+	memset(o, 0, sizeof(struct notes_merge_options));
+	o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;
+}
+
+#define OUTPUT(o, v, ...) \
+	do { \
+		if ((o)->verbosity >= (v)) { \
+			printf(__VA_ARGS__); \
+			puts(""); \
+		} \
+	} while (0)
+
+int notes_merge(struct notes_merge_options *o,
+		unsigned char *result_sha1)
+{
+	unsigned char local_sha1[20], remote_sha1[20];
+	struct commit *local, *remote;
+	struct commit_list *bases = NULL;
+	const unsigned char *base_sha1;
+	int result = 0;
+
+	assert(o->local_ref && o->remote_ref);
+	hashclr(result_sha1);
+
+	trace_printf("notes_merge(o->local_ref = %s, o->remote_ref = %s)\n",
+	       o->local_ref, o->remote_ref);
+
+	/* Dereference o->local_ref into local_sha1 */
+	if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
+		die("Failed to resolve local notes ref '%s'", o->local_ref);
+	else if (!check_ref_format(o->local_ref) && is_null_sha1(local_sha1))
+		local = NULL; /* local_sha1 == null_sha1 indicates unborn ref */
+	else if (!(local = lookup_commit_reference(local_sha1)))
+		die("Could not parse local commit %s (%s)",
+		    sha1_to_hex(local_sha1), o->local_ref);
+	trace_printf("\tlocal commit: %.7s\n", sha1_to_hex(local_sha1));
+
+	/* Dereference o->remote_ref into remote_sha1 */
+	if (get_sha1(o->remote_ref, remote_sha1)) {
+		/*
+		 * Failed to get remote_sha1. If o->remote_ref looks like an
+		 * unborn ref, perform the merge using an empty notes tree.
+		 */
+		if (!check_ref_format(o->remote_ref)) {
+			hashclr(remote_sha1);
+			remote = NULL;
+		}
+		else
+			die("Failed to resolve remote notes ref '%s'",
+			    o->remote_ref);
+	}
+	else if (!(remote = lookup_commit_reference(remote_sha1)))
+		die("Could not parse remote commit %s (%s)",
+		    sha1_to_hex(remote_sha1), o->remote_ref);
+	trace_printf("\tremote commit: %.7s\n", sha1_to_hex(remote_sha1));
+
+	if (!local && !remote)
+		die("Cannot merge empty notes ref (%s) into empty notes ref "
+		    "(%s)", o->remote_ref, o->local_ref);
+	if (!local) {
+		/* result == remote commit */
+		hashcpy(result_sha1, remote_sha1);
+		goto found_result;
+	}
+	if (!remote) {
+		/* result == local commit */
+		hashcpy(result_sha1, local_sha1);
+		goto found_result;
+	}
+	assert(local && remote);
+
+	/* Find merge bases */
+	bases = get_merge_bases(local, remote, 1);
+	if (!bases) {
+		base_sha1 = null_sha1;
+		OUTPUT(o, 4, "No merge base found; doing history-less merge");
+	} else if (!bases->next) {
+		base_sha1 = bases->item->object.sha1;
+		OUTPUT(o, 4, "One merge base found (%.7s)",
+		       sha1_to_hex(base_sha1));
+	} else {
+		/* TODO: How to handle multiple merge-bases? */
+		base_sha1 = bases->item->object.sha1;
+		OUTPUT(o, 3, "Multiple merge bases found. Using the first "
+		       "(%.7s)", sha1_to_hex(base_sha1));
+	}
+
+	OUTPUT(o, 4, "Merging remote commit %.7s into local commit %.7s with "
+	       "merge-base %.7s", sha1_to_hex(remote->object.sha1),
+	       sha1_to_hex(local->object.sha1), sha1_to_hex(base_sha1));
+
+	if (!hashcmp(remote->object.sha1, base_sha1)) {
+		/* Already merged; result == local commit */
+		OUTPUT(o, 2, "Already up-to-date!");
+		hashcpy(result_sha1, local->object.sha1);
+		goto found_result;
+	}
+	if (!hashcmp(local->object.sha1, base_sha1)) {
+		/* Fast-forward; result == remote commit */
+		OUTPUT(o, 2, "Fast-forward");
+		hashcpy(result_sha1, remote->object.sha1);
+		goto found_result;
+	}
+
+	/* TODO: */
+	result = error("notes_merge() cannot yet handle real merges.");
+
+found_result:
+	free_commit_list(bases);
+	trace_printf("notes_merge(): result = %i, result_sha1 = %.7s\n",
+	       result, sha1_to_hex(result_sha1));
+	return result;
+}
diff --git a/notes-merge.h b/notes-merge.h
new file mode 100644
index 0000000..fd572ac
--- /dev/null
+++ b/notes-merge.h
@@ -0,0 +1,36 @@
+#ifndef NOTES_MERGE_H
+#define NOTES_MERGE_H
+
+enum notes_merge_verbosity {
+	NOTES_MERGE_VERBOSITY_DEFAULT = 2,
+	NOTES_MERGE_VERBOSITY_MAX = 5
+};
+
+struct notes_merge_options {
+	const char *local_ref;
+	const char *remote_ref;
+	int verbosity;
+};
+
+void init_notes_merge_options(struct notes_merge_options *o);
+
+/*
+ * Merge notes from o->remote_ref into o->local_ref
+ *
+ * The commits given by the two refs are merged, producing one of the following
+ * outcomes:
+ *
+ * 1. The merge trivially results in an existing commit (e.g. fast-forward or
+ *    already-up-to-date). The SHA1 of the result is written into 'result_sha1'
+ *    and 0 is returned.
+ * 2. The merge fails. result_sha1 is set to null_sha1, and non-zero returned.
+ *
+ * Both o->local_ref and o->remote_ref must be given (non-NULL), but either ref
+ * (although not both) may refer to a non-existing notes ref, in which case
+ * that notes ref is interpreted as an empty notes tree, and the merge
+ * trivially results in what the other ref points to.
+ */
+int notes_merge(struct notes_merge_options *o,
+		unsigned char *result_sha1);
+
+#endif
diff --git a/t/t3308-notes-merge.sh b/t/t3308-notes-merge.sh
new file mode 100755
index 0000000..9acb684
--- /dev/null
+++ b/t/t3308-notes-merge.sh
@@ -0,0 +1,180 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Johan Herland
+#
+
+test_description='Test merging of notes trees'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	test_commit 1st &&
+	test_commit 2nd &&
+	test_commit 3rd &&
+	test_commit 4th &&
+	test_commit 5th &&
+	# Create notes on 4 first commits
+	git config core.notesRef refs/notes/x &&
+	git notes add -m "Notes on 1st commit" 1st &&
+	git notes add -m "Notes on 2nd commit" 2nd &&
+	git notes add -m "Notes on 3rd commit" 3rd &&
+	git notes add -m "Notes on 4th commit" 4th
+'
+
+commit_sha1=$(git rev-parse 1st^{commit})
+commit_sha2=$(git rev-parse 2nd^{commit})
+commit_sha3=$(git rev-parse 3rd^{commit})
+commit_sha4=$(git rev-parse 4th^{commit})
+commit_sha5=$(git rev-parse 5th^{commit})
+
+verify_notes () {
+	notes_ref="$1"
+	git -c core.notesRef="refs/notes/$notes_ref" notes |
+		sort >"output_notes_$notes_ref" &&
+	test_cmp "expect_notes_$notes_ref" "output_notes_$notes_ref" &&
+	git -c core.notesRef="refs/notes/$notes_ref" log --format="%H %s%n%N" \
+		>"output_log_$notes_ref" &&
+	test_cmp "expect_log_$notes_ref" "output_log_$notes_ref"
+}
+
+cat <<EOF | sort >expect_notes_x
+5e93d24084d32e1cb61f7070505b9d2530cca987 $commit_sha4
+8366731eeee53787d2bdf8fc1eff7d94757e8da0 $commit_sha3
+eede89064cd42441590d6afec6c37b321ada3389 $commit_sha2
+daa55ffad6cb99bf64226532147ffcaf5ce8bdd1 $commit_sha1
+EOF
+
+cat >expect_log_x <<EOF
+$commit_sha5 5th
+
+$commit_sha4 4th
+Notes on 4th commit
+
+$commit_sha3 3rd
+Notes on 3rd commit
+
+$commit_sha2 2nd
+Notes on 2nd commit
+
+$commit_sha1 1st
+Notes on 1st commit
+
+EOF
+
+test_expect_success 'verify initial notes (x)' '
+	verify_notes x
+'
+
+cp expect_notes_x expect_notes_y
+cp expect_log_x expect_log_y
+
+test_expect_success 'fail to merge empty notes ref into empty notes ref (z => y)' '
+	test_must_fail git -c "core.notesRef=refs/notes/y" notes merge z
+'
+
+test_expect_success 'fail to merge into various non-notes refs' '
+	test_must_fail git -c "core.notesRef=refs/notes" notes merge x &&
+	test_must_fail git -c "core.notesRef=refs/notes/" notes merge x &&
+	mkdir -p .git/refs/notes/dir &&
+	test_must_fail git -c "core.notesRef=refs/notes/dir" notes merge x &&
+	test_must_fail git -c "core.notesRef=refs/notes/dir/" notes merge x &&
+	test_must_fail git -c "core.notesRef=refs/heads/master" notes merge x &&
+	test_must_fail git -c "core.notesRef=refs/notes/y:" notes merge x &&
+	test_must_fail git -c "core.notesRef=refs/notes/y:foo" notes merge x &&
+	test_must_fail git -c "core.notesRef=refs/notes/foo^{bar" notes merge x
+'
+
+test_expect_success 'fail to merge various non-note-trees' '
+	git config core.notesRef refs/notes/y &&
+	test_must_fail git notes merge refs/notes &&
+	test_must_fail git notes merge refs/notes/ &&
+	test_must_fail git notes merge refs/notes/dir &&
+	test_must_fail git notes merge refs/notes/dir/ &&
+	test_must_fail git notes merge refs/heads/master &&
+	test_must_fail git notes merge x: &&
+	test_must_fail git notes merge x:foo &&
+	test_must_fail git notes merge foo^{bar
+'
+
+test_expect_success 'merge notes into empty notes ref (x => y)' '
+	git config core.notesRef refs/notes/y &&
+	git notes merge x &&
+	verify_notes y &&
+	# x and y should point to the same notes commit
+	test "$(git rev-parse refs/notes/x)" = "$(git rev-parse refs/notes/y)"
+'
+
+test_expect_success 'merge empty notes ref (z => y)' '
+	git notes merge z &&
+	# y should not change (still == x)
+	test "$(git rev-parse refs/notes/x)" = "$(git rev-parse refs/notes/y)"
+'
+
+test_expect_success 'change notes on other notes ref (y)' '
+	# Not touching notes to 1st commit
+	git notes remove 2nd &&
+	git notes append -m "More notes on 3rd commit" 3rd &&
+	git notes add -f -m "New notes on 4th commit" 4th &&
+	git notes add -m "Notes on 5th commit" 5th
+'
+
+test_expect_success 'merge previous notes commit (y^ => y) => No-op' '
+	pre_state="$(git rev-parse refs/notes/y)" &&
+	git notes merge y^ &&
+	# y should not move
+	test "$pre_state" = "$(git rev-parse refs/notes/y)"
+'
+
+cat <<EOF | sort >expect_notes_y
+0f2efbd00262f2fd41dfae33df8765618eeacd99 $commit_sha5
+dec2502dac3ea161543f71930044deff93fa945c $commit_sha4
+4069cdb399fd45463ec6eef8e051a16a03592d91 $commit_sha3
+daa55ffad6cb99bf64226532147ffcaf5ce8bdd1 $commit_sha1
+EOF
+
+cat >expect_log_y <<EOF
+$commit_sha5 5th
+Notes on 5th commit
+
+$commit_sha4 4th
+New notes on 4th commit
+
+$commit_sha3 3rd
+Notes on 3rd commit
+
+More notes on 3rd commit
+
+$commit_sha2 2nd
+
+$commit_sha1 1st
+Notes on 1st commit
+
+EOF
+
+test_expect_success 'verify changed notes on other notes ref (y)' '
+	verify_notes y
+'
+
+test_expect_success 'verify unchanged notes on original notes ref (x)' '
+	verify_notes x
+'
+
+test_expect_success 'merge original notes (x) into changed notes (y) => No-op' '
+	git notes merge -vvv x &&
+	verify_notes y &&
+	verify_notes x
+'
+
+cp expect_notes_y expect_notes_x
+cp expect_log_y expect_log_x
+
+test_expect_success 'merge changed (y) into original (x) => Fast-forward' '
+	git config core.notesRef refs/notes/x &&
+	git notes merge y &&
+	verify_notes x &&
+	verify_notes y &&
+	# x and y should point to same the notes commit
+	test "$(git rev-parse refs/notes/x)" = "$(git rev-parse refs/notes/y)"
+'
+
+test_done
-- 
1.7.3.98.g5ad7d9
Previous: Johan HerlandNext: Johan Herland
Message 17 of 43 in “[PATCHv4 00/21] git notes merge”
  1. Johan HerlandOct 21, 2010
  2. 01/21 notes.c: Hexify SHA1 in die() message from init_notes()Johan Herland, Oct 21, 2010
  3. 02/21 (trivial) notes.h: Minor documentation fixes to copy_notes()Johan Herland, Oct 21, 2010
  4. 03/21 notes.h: Make default_notes_ref() available in notes APIJohan Herland, Oct 21, 2010
  5. 04/21 notes.c: Reorder functions in preparation for next commitJohan Herland, Oct 21, 2010
  6. 05/21 notes.h/c: Clarify the handling of notes objects that are == null_sha1Johan Herland, Oct 21, 2010
  7. Jonathan NiederOct 21, 2010
  8. Johan HerlandOct 21, 2010
  9. Jonathan NiederOct 21, 2010
  10. Johan HerlandOct 22, 2010
  11. 06/21 notes.h/c: Propagate combine_notes_fn return value to add_note() and beyondJohan Herland, Oct 21, 2010
  12. Jonathan NiederOct 21, 2010
  13. Johan HerlandOct 21, 2010
  14. 07/21 (trivial) t3303: Indent with tabs instead of spaces for consistencyJohan Herland, Oct 21, 2010
  15. 08/21 notes.c: Use two newlines (instead of one) when concatenating notesJohan Herland, Oct 21, 2010
  16. 09/21 builtin/notes.c: Split notes ref DWIMmery into a separate functionJohan Herland, Oct 21, 2010
  17. 10/21 git notes merge: Initial implementation handling trivial merges onlyJohan Herland, Oct 21, 2010
  18. 11/21 builtin/notes.c: Refactor creation of notes commits.Johan Herland, Oct 21, 2010
  19. 12/21 git notes merge: Handle real, non-conflicting notes mergesJohan Herland, Oct 21, 2010
  20. 13/21 git notes merge: Add automatic conflict resolvers (ours, theirs, union)Johan Herland, Oct 21, 2010
  21. 14/21 Documentation: Preliminary docs on 'git notes merge'Johan Herland, Oct 21, 2010
  22. 15/21 git notes merge: Manual conflict resolution, part 1/2Johan Herland, Oct 21, 2010
  23. 16/21 git notes merge: Manual conflict resolution, part 2/2Johan Herland, Oct 21, 2010
  24. 17/21 git notes merge: List conflicting notes in notes merge commit messageJohan Herland, Oct 21, 2010
  25. 18/21 git notes merge: --commit should fail if underlying notes ref has movedJohan Herland, Oct 21, 2010
  26. 19/21 git notes merge: Add another auto-resolving strategy: "cat_sort_uniq"Johan Herland, Oct 21, 2010
  27. 20/21 git notes merge: Add testcases for merging notes trees at different fanoutsJohan Herland, Oct 21, 2010
  28. 21/21 Provide 'git notes get-ref' to easily retrieve current notes refJohan Herland, Oct 21, 2010
  29. Sverre RabbelierOct 21, 2010
  30. Junio C HamanoOct 21, 2010
  31. Jonathan NiederOct 21, 2010
  32. git merge --abort? [was: Re: [PATCHv4 00/21] git notes merge]Johan Herland, Oct 22, 2010
  33. Jonathan NiederOct 22, 2010
  34. Johan HerlandOct 22, 2010
  35. Sverre RabbelierOct 22, 2010
  36. Jonathan NiederOct 22, 2010
  37. Sverre RabbelierOct 22, 2010
  38. Johan HerlandOct 22, 2010
  39. Sverre RabbelierOct 22, 2010
  40. Johan HerlandOct 23, 2010
  41. Sverre RabbelierOct 23, 2010
  42. Johan HerlandOct 22, 2010
  43. Sverre RabbelierOct 23, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.