From 6abb3655efe364cf0375b5cdae2729af7562ed45 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Tue, 9 Nov 2010 22:49:52 +0100 Subject: [PATCH] git notes merge: Manual conflict resolution, part 2/2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the notes merge conflicts in .git/NOTES_MERGE_WORKTREE have been resolved, we need to record a new notes commit on the appropriate notes ref with the resolved notes. This patch implements 'git notes merge --commit' which the user should run after resolving conflicts in the notes merge worktree. This command finalizes the notes merge by recombining the partial notes tree from part 1 with the now-resolved conflicts in the notes merge worktree in a merge commit, and updating the appropriate ref to this merge commit. In order to correctly finalize the merge, we need to keep track of three things: - The partial merge result from part 1, containing the auto-merged notes. This is now stored into a ref called .git/NOTES_MERGE_PARTIAL. - The unmerged notes. These are already stored in .git/NOTES_MERGE_WORKTREE, thanks to part 1. - The notes ref to be updated by the finalized merge result. This is now stored in a symref called .git/NOTES_MERGE_REF. In addition to "git notes merge --commit", which uses the above details to create the finalized notes merge commit, this patch also implements "git notes merge --reset", which aborts the ongoing notes merge by simply removing the files/directory described above. FTR, "git notes merge --commit" reuses "git notes merge --reset" to remove the information described above (.git/NOTES_MERGE_*) after the notes merge have been successfully finalized. The patch also contains documentation and testcases for the two new options. This patch has been improved by the following contributions: - Ævar Arnfjörð Bjarmason: Fix nonsense sentence in --commit description - Sverre Rabbelier: Rename --reset to --abort Thanks-to: Ævar Arnfjörð Bjarmason Thanks-to: Sverre Rabbelier Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- Documentation/git-notes.txt | 22 ++++ builtin/notes.c | 106 +++++++++++++++- notes-merge.c | 71 ++++++++++- notes-merge.h | 23 ++++ t/t3310-notes-merge-manual-resolve.sh | 176 ++++++++++++++++++++++++++ 5 files changed, 394 insertions(+), 4 deletions(-) diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt index f003b7ca1c..a82255119a 100644 --- a/Documentation/git-notes.txt +++ b/Documentation/git-notes.txt @@ -15,6 +15,8 @@ SYNOPSIS 'git notes' edit [] 'git notes' show [] 'git notes' merge [-v | -q] [-s ] +'git notes' merge --commit [-v | -q] +'git notes' merge --abort [-v | -q] 'git notes' remove [] 'git notes' prune [-n | -v] @@ -95,6 +97,9 @@ conflicting notes (see the -s/--strategy option) is not given, the "manual" resolver is used. This resolver checks out the conflicting notes in a special worktree (`.git/NOTES_MERGE_WORKTREE`), and instructs the user to manually resolve the conflicts there. +When done, the user can either finalize the merge with +'git notes merge --commit', or abort the merge with +'git notes merge --abort'. remove:: Remove the notes for a given object (defaults to HEAD). @@ -154,6 +159,20 @@ OPTIONS See the "NOTES MERGE STRATEGIES" section below for more information on each notes merge strategy. +--commit:: + Finalize an in-progress 'git notes merge'. Use this option + when you have resolved the conflicts that 'git notes merge' + stored in .git/NOTES_MERGE_WORKTREE. This amends the partial + merge commit created by 'git notes merge' (stored in + .git/NOTES_MERGE_PARTIAL) by adding the notes in + .git/NOTES_MERGE_WORKTREE. The notes ref stored in the + .git/NOTES_MERGE_REF symref is updated to the resulting commit. + +--abort:: + Abort/reset a in-progress 'git notes merge', i.e. a notes merge + with conflicts. This simply removes all files related to the + notes merge. + -q:: --quiet:: When merging notes, operate quietly. @@ -197,6 +216,9 @@ The default notes merge strategy is "manual", which checks out conflicting notes in a special work tree for resolving notes conflicts (`.git/NOTES_MERGE_WORKTREE`), and instructs the user to resolve the conflicts in that work tree. +When done, the user can either finalize the merge with +'git notes merge --commit', or abort the merge with +'git notes merge --abort'. "ours" automatically resolves conflicting notes in favor of the local version (i.e. the current notes ref). diff --git a/builtin/notes.c b/builtin/notes.c index 309edc741e..b5385238ea 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -27,6 +27,8 @@ static const char * const git_notes_usage[] = { "git notes [--ref ] edit []", "git notes [--ref ] show []", "git notes [--ref ] merge [-v | -q] [-s ] ", + "git notes merge --commit [-v | -q]", + "git notes merge --abort [-v | -q]", "git notes [--ref ] remove []", "git notes [--ref ] prune [-n | -v]", NULL @@ -65,6 +67,8 @@ static const char * const git_notes_show_usage[] = { static const char * const git_notes_merge_usage[] = { "git notes merge [] ", + "git notes merge --commit []", + "git notes merge --abort []", NULL }; @@ -761,33 +765,119 @@ static int show(int argc, const char **argv, const char *prefix) return retval; } +static int merge_abort(struct notes_merge_options *o) +{ + int ret = 0; + + /* + * Remove .git/NOTES_MERGE_PARTIAL and .git/NOTES_MERGE_REF, and call + * notes_merge_abort() to remove .git/NOTES_MERGE_WORKTREE. + */ + + if (delete_ref("NOTES_MERGE_PARTIAL", NULL, 0)) + ret += error("Failed to delete ref NOTES_MERGE_PARTIAL"); + if (delete_ref("NOTES_MERGE_REF", NULL, REF_NODEREF)) + ret += error("Failed to delete ref NOTES_MERGE_REF"); + if (notes_merge_abort(o)) + ret += error("Failed to remove 'git notes merge' worktree"); + return ret; +} + +static int merge_commit(struct notes_merge_options *o) +{ + struct strbuf msg = STRBUF_INIT; + unsigned char sha1[20]; + struct notes_tree *t; + struct commit *partial; + struct pretty_print_context pretty_ctx; + + /* + * Read partial merge result from .git/NOTES_MERGE_PARTIAL, + * and target notes ref from .git/NOTES_MERGE_REF. + */ + + if (get_sha1("NOTES_MERGE_PARTIAL", sha1)) + die("Failed to read ref NOTES_MERGE_PARTIAL"); + else if (!(partial = lookup_commit_reference(sha1))) + die("Could not find commit from NOTES_MERGE_PARTIAL."); + else if (parse_commit(partial)) + die("Could not parse commit from NOTES_MERGE_PARTIAL."); + + t = xcalloc(1, sizeof(struct notes_tree)); + init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0); + + o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, 0); + if (!o->local_ref) + die("Failed to resolve NOTES_MERGE_REF"); + + if (notes_merge_commit(o, t, partial, sha1)) + die("Failed to finalize notes merge"); + + /* Reuse existing commit message in reflog message */ + memset(&pretty_ctx, 0, sizeof(pretty_ctx)); + format_commit_message(partial, "%s", &msg, &pretty_ctx); + strbuf_trim(&msg); + strbuf_insert(&msg, 0, "notes: ", 7); + update_ref(msg.buf, o->local_ref, sha1, NULL, 0, DIE_ON_ERR); + + free_notes(t); + strbuf_release(&msg); + return merge_abort(o); +} + 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_tree *t; struct notes_merge_options o; + int do_merge = 0, do_commit = 0, do_abort = 0; int verbosity = 0, result; const char *strategy = NULL; struct option options[] = { + OPT_GROUP("General options"), OPT__VERBOSITY(&verbosity), + OPT_GROUP("Merge options"), OPT_STRING('s', "strategy", &strategy, "strategy", "resolve notes conflicts using the given " "strategy (manual/ours/theirs/union)"), + OPT_GROUP("Committing unmerged notes"), + { OPTION_BOOLEAN, 0, "commit", &do_commit, NULL, + "finalize notes merge by committing unmerged notes", + PARSE_OPT_NOARG | PARSE_OPT_NONEG }, + OPT_GROUP("Aborting notes merge resolution"), + { OPTION_BOOLEAN, 0, "abort", &do_abort, NULL, + "abort notes merge", + PARSE_OPT_NOARG | PARSE_OPT_NONEG }, OPT_END() }; argc = parse_options(argc, argv, prefix, options, git_notes_merge_usage, 0); - if (argc != 1) { + if (strategy || do_commit + do_abort == 0) + do_merge = 1; + if (do_merge + do_commit + do_abort != 1) { + error("cannot mix --commit, --abort or -s/--strategy"); + usage_with_options(git_notes_merge_usage, options); + } + + if (do_merge && argc != 1) { error("Must specify a notes ref to merge"); usage_with_options(git_notes_merge_usage, options); + } else if (!do_merge && argc) { + error("too many parameters"); + usage_with_options(git_notes_merge_usage, options); } init_notes_merge_options(&o); o.verbosity = verbosity + NOTES_MERGE_VERBOSITY_DEFAULT; + if (do_abort) + return merge_abort(&o); + if (do_commit) + return merge_commit(&o); + o.local_ref = default_notes_ref(); strbuf_addstr(&remote_ref, argv[0]); expand_notes_ref(&remote_ref); @@ -820,9 +910,19 @@ static int merge(int argc, const char **argv, const char *prefix) /* Update default notes ref with new commit */ update_ref(msg.buf, default_notes_ref(), result_sha1, NULL, 0, DIE_ON_ERR); - else /* Merge has unresolved conflicts */ - printf("Automatic notes merge failed. Fix conflicts in %s.\n", + else { /* Merge has unresolved conflicts */ + /* Update .git/NOTES_MERGE_PARTIAL with partial merge result */ + update_ref(msg.buf, "NOTES_MERGE_PARTIAL", result_sha1, NULL, + 0, DIE_ON_ERR); + /* Store ref-to-be-updated into .git/NOTES_MERGE_REF */ + if (create_symref("NOTES_MERGE_REF", default_notes_ref(), NULL)) + die("Failed to store link to current notes ref (%s)", + default_notes_ref()); + printf("Automatic notes merge failed. Fix conflicts in %s and " + "commit the result with 'git notes merge --commit', or " + "abort the merge with 'git notes merge --abort'.\n", git_path(NOTES_MERGE_WORKTREE)); + } free_notes(t); strbuf_release(&remote_ref); diff --git a/notes-merge.c b/notes-merge.c index ada29067bd..a2feab6d1b 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -278,7 +278,7 @@ static void check_notes_merge_worktree(struct notes_merge_options *o) die("You have not concluded your previous " "notes merge (%s exists).\nPlease, use " "'git notes merge --commit' or 'git notes " - "merge --reset' to commit/abort the " + "merge --abort' to commit/abort the " "previous merge before you start a new " "notes merge.", git_path("NOTES_MERGE_*")); else @@ -650,3 +650,72 @@ found_result: result, sha1_to_hex(result_sha1)); return result; } + +int notes_merge_commit(struct notes_merge_options *o, + struct notes_tree *partial_tree, + struct commit *partial_commit, + unsigned char *result_sha1) +{ + /* + * Iterate through files in .git/NOTES_MERGE_WORKTREE and add all + * found notes to 'partial_tree'. Write the updates notes tree to + * the DB, and commit the resulting tree object while reusing the + * commit message and parents from 'partial_commit'. + * Finally store the new commit object SHA1 into 'result_sha1'. + */ + struct dir_struct dir; + const char *path = git_path(NOTES_MERGE_WORKTREE "/"); + int path_len = strlen(path), i; + const char *msg = strstr(partial_commit->buffer, "\n\n"); + + OUTPUT(o, 3, "Committing notes in notes merge worktree at %.*s", + path_len - 1, path); + + if (!msg || msg[2] == '\0') + die("partial notes commit has empty message"); + msg += 2; + + memset(&dir, 0, sizeof(dir)); + read_directory(&dir, path, path_len, NULL); + for (i = 0; i < dir.nr; i++) { + struct dir_entry *ent = dir.entries[i]; + struct stat st; + const char *relpath = ent->name + path_len; + unsigned char obj_sha1[20], blob_sha1[20]; + + if (ent->len - path_len != 40 || get_sha1_hex(relpath, obj_sha1)) { + OUTPUT(o, 3, "Skipping non-SHA1 entry '%s'", ent->name); + continue; + } + + /* write file as blob, and add to partial_tree */ + if (stat(ent->name, &st)) + die_errno("Failed to stat '%s'", ent->name); + if (index_path(blob_sha1, ent->name, &st, 1)) + die("Failed to write blob object from '%s'", ent->name); + if (add_note(partial_tree, obj_sha1, blob_sha1, NULL)) + die("Failed to add resolved note '%s' to notes tree", + ent->name); + OUTPUT(o, 4, "Added resolved note for object %s: %s", + sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1)); + } + + create_notes_commit(partial_tree, partial_commit->parents, msg, + result_sha1); + OUTPUT(o, 4, "Finalized notes merge commit: %s", + sha1_to_hex(result_sha1)); + return 0; +} + +int notes_merge_abort(struct notes_merge_options *o) +{ + /* Remove .git/NOTES_MERGE_WORKTREE directory and all files within */ + struct strbuf buf = STRBUF_INIT; + int ret; + + strbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE)); + OUTPUT(o, 3, "Removing notes merge worktree at %s", buf.buf); + ret = remove_dir_recursively(&buf, 0); + strbuf_release(&buf); + return ret; +} diff --git a/notes-merge.h b/notes-merge.h index 55ef3d957f..aa756931a5 100644 --- a/notes-merge.h +++ b/notes-merge.h @@ -71,4 +71,27 @@ int notes_merge(struct notes_merge_options *o, struct notes_tree *local_tree, unsigned char *result_sha1); +/* + * Finalize conflict resolution from an earlier notes_merge() + * + * The given notes tree 'partial_tree' must be the notes_tree corresponding to + * the given 'partial_commit', the partial result commit created by a previous + * call to notes_merge(). + * + * This function will add the (now resolved) notes in .git/NOTES_MERGE_WORKTREE + * to 'partial_tree', and create a final notes merge commit, the SHA1 of which + * will be stored in 'result_sha1'. + */ +int notes_merge_commit(struct notes_merge_options *o, + struct notes_tree *partial_tree, + struct commit *partial_commit, + unsigned char *result_sha1); + +/* + * Abort conflict resolution from an earlier notes_merge() + * + * Removes the notes merge worktree in .git/NOTES_MERGE_WORKTREE. + */ +int notes_merge_abort(struct notes_merge_options *o); + #endif diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh index eadb6b7df6..0ec315aa9b 100755 --- a/t/t3310-notes-merge-manual-resolve.sh +++ b/t/t3310-notes-merge-manual-resolve.sh @@ -171,6 +171,7 @@ cp expect_notes_y expect_notes_m cp expect_log_y expect_log_m git rev-parse refs/notes/y > pre_merge_y +git rev-parse refs/notes/z > pre_merge_z test_expect_success 'merge z into m (== y) with default ("manual") resolver => Conflicting 3-way merge' ' git update-ref refs/notes/m refs/notes/y && @@ -289,4 +290,179 @@ test_expect_success 'can do merge without conflicts even if previous merge is un verify_notes y ' +cat <expect_notes_m +021faa20e931fb48986ffc6282b4bb05553ac946 $commit_sha4 +5772f42408c0dd6f097a7ca2d24de0e78d1c46b1 $commit_sha3 +283b48219aee9a4105f6cab337e789065c82c2b9 $commit_sha2 +0a59e787e6d688aa6309e56e8c1b89431a0fc1c1 $commit_sha1 +EOF + +cat >expect_log_m < m)' ' + # Resolve conflicts and finalize merge + cat >.git/NOTES_MERGE_WORKTREE/$commit_sha1 <.git/NOTES_MERGE_WORKTREE/$commit_sha4 <output 2>/dev/null && + test_cmp /dev/null output && + # Merge commit has pre-merge y and pre-merge z as parents + test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" && + test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" && + # Merge commit mentions the notes refs merged + git log -1 --format=%B refs/notes/m > merge_commit_msg && + grep -q refs/notes/m merge_commit_msg && + grep -q refs/notes/z merge_commit_msg && + # Verify contents of merge result + verify_notes m && + # Verify that other notes refs has not changed (w, x, y and z) + verify_notes w && + verify_notes x && + verify_notes y && + verify_notes z +' + +cat >expect_conflict_$commit_sha4 <>>>>>> refs/notes/z +EOF + +cp expect_notes_y expect_notes_m +cp expect_log_y expect_log_m + +git rev-parse refs/notes/y > pre_merge_y +git rev-parse refs/notes/z > pre_merge_z + +test_expect_success 'redo merge of z into m (== y) with default ("manual") resolver => Conflicting 3-way merge' ' + git update-ref refs/notes/m refs/notes/y && + git config core.notesRef refs/notes/m && + test_must_fail git notes merge z >output && + # Output should point to where to resolve conflicts + grep -q "\\.git/NOTES_MERGE_WORKTREE" output && + # Inspect merge conflicts + ls .git/NOTES_MERGE_WORKTREE >output_conflicts && + test_cmp expect_conflicts output_conflicts && + ( for f in $(cat expect_conflicts); do + test_cmp "expect_conflict_$f" ".git/NOTES_MERGE_WORKTREE/$f" || + exit 1 + done ) && + # Verify that current notes tree (pre-merge) has not changed (m == y) + verify_notes y && + verify_notes m && + test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" +' + +test_expect_success 'abort notes merge' ' + git notes merge --abort && + # No .git/NOTES_MERGE_* files left + test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && + test_cmp /dev/null output && + # m has not moved (still == y) + test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" + # Verify that other notes refs has not changed (w, x, y and z) + verify_notes w && + verify_notes x && + verify_notes y && + verify_notes z +' + +git rev-parse refs/notes/y > pre_merge_y +git rev-parse refs/notes/z > pre_merge_z + +test_expect_success 'redo merge of z into m (== y) with default ("manual") resolver => Conflicting 3-way merge' ' + test_must_fail git notes merge z >output && + # Output should point to where to resolve conflicts + grep -q "\\.git/NOTES_MERGE_WORKTREE" output && + # Inspect merge conflicts + ls .git/NOTES_MERGE_WORKTREE >output_conflicts && + test_cmp expect_conflicts output_conflicts && + ( for f in $(cat expect_conflicts); do + test_cmp "expect_conflict_$f" ".git/NOTES_MERGE_WORKTREE/$f" || + exit 1 + done ) && + # Verify that current notes tree (pre-merge) has not changed (m == y) + verify_notes y && + verify_notes m && + test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" +' + +cat <expect_notes_m +304dfb4325cf243025b9957486eb605a9b51c199 $commit_sha5 +283b48219aee9a4105f6cab337e789065c82c2b9 $commit_sha2 +0a59e787e6d688aa6309e56e8c1b89431a0fc1c1 $commit_sha1 +EOF + +cat >expect_log_m < m)' ' + # Resolve one conflict + cat >.git/NOTES_MERGE_WORKTREE/$commit_sha1 < .git/NOTES_MERGE_WORKTREE/$commit_sha5 && + # Finalize merge + git notes merge --commit && + # No .git/NOTES_MERGE_* files left + test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && + test_cmp /dev/null output && + # Merge commit has pre-merge y and pre-merge z as parents + test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" && + test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" && + # Merge commit mentions the notes refs merged + git log -1 --format=%B refs/notes/m > merge_commit_msg && + grep -q refs/notes/m merge_commit_msg && + grep -q refs/notes/z merge_commit_msg && + # Verify contents of merge result + verify_notes m && + # Verify that other notes refs has not changed (w, x, y and z) + verify_notes w && + verify_notes x && + verify_notes y && + verify_notes z +' + test_done