diff options
| -rw-r--r-- | Documentation/git-notes.txt | 10 | ||||
| -rw-r--r-- | builtin/notes.c | 8 | ||||
| -rw-r--r-- | notes-merge.c | 166 | ||||
| -rw-r--r-- | notes-merge.h | 11 | ||||
| -rwxr-xr-x | t/t3310-notes-merge-manual-resolve.sh | 292 |
5 files changed, 474 insertions, 13 deletions
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt index 07a5042..f003b7c 100644 --- a/Documentation/git-notes.txt +++ b/Documentation/git-notes.txt @@ -92,7 +92,9 @@ merge:: + If conflicts arise and a strategy for automatically resolving conflicting notes (see the -s/--strategy option) is not given, -the merge fails (TODO). +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. remove:: Remove the notes for a given object (defaults to HEAD). @@ -191,8 +193,10 @@ object, in which case the history of the notes can be read with NOTES MERGE STRATEGIES ---------------------- -The default notes merge strategy is "manual", which is not yet -implemented (TODO). +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. "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 64b2be9..309edc7 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -820,14 +820,14 @@ 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 - /* TODO: */ - die("'git notes merge' cannot yet handle conflicts!"); + else /* Merge has unresolved conflicts */ + printf("Automatic notes merge failed. Fix conflicts in %s.\n", + git_path(NOTES_MERGE_WORKTREE)); free_notes(t); strbuf_release(&remote_ref); strbuf_release(&msg); - return 0; + return result < 0; /* return non-zero on conflicts */ } static int remove_cmd(int argc, const char **argv, const char *prefix) diff --git a/notes-merge.c b/notes-merge.c index 2c0b25b..ada2906 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -3,6 +3,9 @@ #include "refs.h" #include "diff.h" #include "diffcore.h" +#include "xdiff-interface.h" +#include "ll-merge.h" +#include "dir.h" #include "notes.h" #include "notes-merge.h" @@ -263,16 +266,169 @@ static void diff_tree_local(struct notes_merge_options *o, diff_tree_release_paths(&opt); } +static void check_notes_merge_worktree(struct notes_merge_options *o) +{ + if (!o->has_worktree) { + /* + * Must establish NOTES_MERGE_WORKTREE. + * Abort if NOTES_MERGE_WORKTREE already exists + */ + if (file_exists(git_path(NOTES_MERGE_WORKTREE))) { + if (advice_resolve_conflict) + 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 " + "previous merge before you start a new " + "notes merge.", git_path("NOTES_MERGE_*")); + else + die("You have not concluded your notes merge " + "(%s exists).", git_path("NOTES_MERGE_*")); + } + + if (safe_create_leading_directories(git_path( + NOTES_MERGE_WORKTREE "/.test"))) + die_errno("unable to create directory %s", + git_path(NOTES_MERGE_WORKTREE)); + o->has_worktree = 1; + } else if (!file_exists(git_path(NOTES_MERGE_WORKTREE))) + /* NOTES_MERGE_WORKTREE should already be established */ + die("missing '%s'. This should not happen", + git_path(NOTES_MERGE_WORKTREE)); +} + +static void write_buf_to_worktree(const unsigned char *obj, + const char *buf, unsigned long size) +{ + int fd; + char *path = git_path(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj)); + if (safe_create_leading_directories(path)) + die_errno("unable to create directory for '%s'", path); + if (file_exists(path)) + die("found existing file at '%s'", path); + + fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, 0666); + if (fd < 0) + die_errno("failed to open '%s'", path); + + while (size > 0) { + long ret = write_in_full(fd, buf, size); + if (ret < 0) { + /* Ignore epipe */ + if (errno == EPIPE) + break; + die_errno("notes-merge"); + } else if (!ret) { + die("notes-merge: disk full?"); + } + size -= ret; + buf += ret; + } + + close(fd); +} + +static void write_note_to_worktree(const unsigned char *obj, + const unsigned char *note) +{ + enum object_type type; + unsigned long size; + void *buf = read_sha1_file(note, &type, &size); + + if (!buf) + die("cannot read note %s for object %s", + sha1_to_hex(note), sha1_to_hex(obj)); + if (type != OBJ_BLOB) + die("blob expected in note %s for object %s", + sha1_to_hex(note), sha1_to_hex(obj)); + write_buf_to_worktree(obj, buf, size); + free(buf); +} + +static int ll_merge_in_worktree(struct notes_merge_options *o, + struct notes_merge_pair *p) +{ + mmbuffer_t result_buf; + mmfile_t base, local, remote; + int status; + + read_mmblob(&base, p->base); + read_mmblob(&local, p->local); + read_mmblob(&remote, p->remote); + + status = ll_merge(&result_buf, sha1_to_hex(p->obj), &base, NULL, + &local, o->local_ref, &remote, o->remote_ref, 0); + + free(base.ptr); + free(local.ptr); + free(remote.ptr); + + if ((status < 0) || !result_buf.ptr) + die("Failed to execute internal merge"); + + write_buf_to_worktree(p->obj, result_buf.ptr, result_buf.size); + free(result_buf.ptr); + + return status; +} + +static int merge_one_change_manual(struct notes_merge_options *o, + struct notes_merge_pair *p, + struct notes_tree *t) +{ + const char *lref = o->local_ref ? o->local_ref : "local version"; + const char *rref = o->remote_ref ? o->remote_ref : "remote version"; + + trace_printf("\t\t\tmerge_one_change_manual(obj = %.7s, base = %.7s, " + "local = %.7s, remote = %.7s)\n", + sha1_to_hex(p->obj), sha1_to_hex(p->base), + sha1_to_hex(p->local), sha1_to_hex(p->remote)); + + OUTPUT(o, 2, "Auto-merging notes for %s", sha1_to_hex(p->obj)); + check_notes_merge_worktree(o); + if (is_null_sha1(p->local)) { + /* D/F conflict, checkout p->remote */ + assert(!is_null_sha1(p->remote)); + OUTPUT(o, 1, "CONFLICT (delete/modify): Notes for object %s " + "deleted in %s and modified in %s. Version from %s " + "left in tree.", sha1_to_hex(p->obj), lref, rref, rref); + write_note_to_worktree(p->obj, p->remote); + } else if (is_null_sha1(p->remote)) { + /* D/F conflict, checkout p->local */ + assert(!is_null_sha1(p->local)); + OUTPUT(o, 1, "CONFLICT (delete/modify): Notes for object %s " + "deleted in %s and modified in %s. Version from %s " + "left in tree.", sha1_to_hex(p->obj), rref, lref, lref); + write_note_to_worktree(p->obj, p->local); + } else { + /* "regular" conflict, checkout result of ll_merge() */ + const char *reason = "content"; + if (is_null_sha1(p->base)) + reason = "add/add"; + assert(!is_null_sha1(p->local)); + assert(!is_null_sha1(p->remote)); + OUTPUT(o, 1, "CONFLICT (%s): Merge conflict in notes for " + "object %s", reason, sha1_to_hex(p->obj)); + ll_merge_in_worktree(o, p); + } + + trace_printf("\t\t\tremoving from partial merge result\n"); + remove_note(t, p->obj); + + return 1; +} + static int merge_one_change(struct notes_merge_options *o, struct notes_merge_pair *p, struct notes_tree *t) { /* - * Return 0 if change was resolved (and added to notes_tree), - * 1 if conflict + * Return 0 if change is successfully resolved (stored in notes_tree). + * Return 1 is change results in a conflict (NOT stored in notes_tree, + * but instead written to NOTES_MERGE_WORKTREE with conflict markers). */ switch (o->strategy) { case NOTES_MERGE_RESOLVE_MANUAL: - return 1; + return merge_one_change_manual(o, p, t); case NOTES_MERGE_RESOLVE_OURS: OUTPUT(o, 2, "Using local notes for %s", sha1_to_hex(p->obj)); /* nothing to do */ @@ -479,8 +635,8 @@ int notes_merge(struct notes_merge_options *o, result = merge_from_diffs(o, base_tree_sha1, local->tree->object.sha1, remote->tree->object.sha1, local_tree); - if (result > 0) { /* successful non-trivial merge */ - /* Commit result */ + if (result != 0) { /* non-trivial merge (with or without conflicts) */ + /* Commit (partial) result */ struct commit_list *parents = NULL; commit_list_insert(remote, &parents); /* LIFO order */ commit_list_insert(local, &parents); diff --git a/notes-merge.h b/notes-merge.h index ed356ae..55ef3d9 100644 --- a/notes-merge.h +++ b/notes-merge.h @@ -1,6 +1,8 @@ #ifndef NOTES_MERGE_H #define NOTES_MERGE_H +#define NOTES_MERGE_WORKTREE "NOTES_MERGE_WORKTREE" + enum notes_merge_verbosity { NOTES_MERGE_VERBOSITY_DEFAULT = 2, NOTES_MERGE_VERBOSITY_MAX = 5 @@ -17,6 +19,7 @@ struct notes_merge_options { NOTES_MERGE_RESOLVE_THEIRS, NOTES_MERGE_RESOLVE_UNION } strategy; + unsigned has_worktree:1; }; void init_notes_merge_options(struct notes_merge_options *o); @@ -51,7 +54,13 @@ void create_notes_commit(struct notes_tree *t, struct commit_list *parents, * 2. The merge successfully completes, producing a merge commit. local_tree * contains the updated notes tree, the SHA1 of the resulting commit is * written into 'result_sha1', and 1 is returned. - * 3. The merge fails. result_sha1 is set to null_sha1, and -1 is returned. + * 3. The merge results in conflicts. This is similar to #2 in that the + * partial merge result (i.e. merge result minus the unmerged entries) + * are stored in 'local_tree', and the SHA1 or the resulting commit + * (to be amended when the conflicts have been resolved) is written into + * 'result_sha1'. The unmerged entries are written into the + * .git/NOTES_MERGE_WORKTREE directory with conflict markers. + * -1 is 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 diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh new file mode 100755 index 0000000..eadb6b7 --- /dev/null +++ b/t/t3310-notes-merge-manual-resolve.sh @@ -0,0 +1,292 @@ +#!/bin/sh +# +# Copyright (c) 2010 Johan Herland +# + +test_description='Test notes merging with manual conflict resolution' + +. ./test-lib.sh + +# Set up a notes merge scenario with different kinds of conflicts +test_expect_success 'setup commits' ' + test_commit 1st && + test_commit 2nd && + test_commit 3rd && + test_commit 4th && + test_commit 5th +' + +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 +6e8e3febca3c2bb896704335cc4d0c34cb2f8715 $commit_sha4 +e5388c10860456ee60673025345fe2e153eb8cf8 $commit_sha3 +ceefa674873670e7ecd131814d909723cce2b669 $commit_sha2 +EOF + +cat >expect_log_x <<EOF +$commit_sha5 5th + +$commit_sha4 4th +x notes on 4th commit + +$commit_sha3 3rd +x notes on 3rd commit + +$commit_sha2 2nd +x notes on 2nd commit + +$commit_sha1 1st + +EOF + +test_expect_success 'setup merge base (x)' ' + git config core.notesRef refs/notes/x && + git notes add -m "x notes on 2nd commit" 2nd && + git notes add -m "x notes on 3rd commit" 3rd && + git notes add -m "x notes on 4th commit" 4th && + verify_notes x +' + +cat <<EOF | sort >expect_notes_y +e2bfd06a37dd2031684a59a6e2b033e212239c78 $commit_sha4 +5772f42408c0dd6f097a7ca2d24de0e78d1c46b1 $commit_sha3 +b0a6021ec006d07e80e9b20ec9b444cbd9d560d3 $commit_sha1 +EOF + +cat >expect_log_y <<EOF +$commit_sha5 5th + +$commit_sha4 4th +y notes on 4th commit + +$commit_sha3 3rd +y notes on 3rd commit + +$commit_sha2 2nd + +$commit_sha1 1st +y notes on 1st commit + +EOF + +test_expect_success 'setup local branch (y)' ' + git update-ref refs/notes/y refs/notes/x && + git config core.notesRef refs/notes/y && + git notes add -f -m "y notes on 1st commit" 1st && + git notes remove 2nd && + git notes add -f -m "y notes on 3rd commit" 3rd && + git notes add -f -m "y notes on 4th commit" 4th && + verify_notes y +' + +cat <<EOF | sort >expect_notes_z +cff59c793c20bb49a4e01bc06fb06bad642e0d54 $commit_sha4 +283b48219aee9a4105f6cab337e789065c82c2b9 $commit_sha2 +0a81da8956346e19bcb27a906f04af327e03e31b $commit_sha1 +EOF + +cat >expect_log_z <<EOF +$commit_sha5 5th + +$commit_sha4 4th +z notes on 4th commit + +$commit_sha3 3rd + +$commit_sha2 2nd +z notes on 2nd commit + +$commit_sha1 1st +z notes on 1st commit + +EOF + +test_expect_success 'setup remote branch (z)' ' + git update-ref refs/notes/z refs/notes/x && + git config core.notesRef refs/notes/z && + git notes add -f -m "z notes on 1st commit" 1st && + git notes add -f -m "z notes on 2nd commit" 2nd && + git notes remove 3rd && + git notes add -f -m "z notes on 4th commit" 4th && + verify_notes z +' + +# At this point, before merging z into y, we have the following status: +# +# commit | base/x | local/y | remote/z | diff from x to y/z +# -------|---------|---------|----------|--------------------------- +# 1st | [none] | b0a6021 | 0a81da8 | added / added (diff) +# 2nd | ceefa67 | [none] | 283b482 | removed / changed +# 3rd | e5388c1 | 5772f42 | [none] | changed / removed +# 4th | 6e8e3fe | e2bfd06 | cff59c7 | changed / changed (diff) +# 5th | [none] | [none] | [none] | [none] + +cat <<EOF | sort >expect_conflicts +$commit_sha1 +$commit_sha2 +$commit_sha3 +$commit_sha4 +EOF + +cat >expect_conflict_$commit_sha1 <<EOF +<<<<<<< refs/notes/m +y notes on 1st commit +======= +z notes on 1st commit +>>>>>>> refs/notes/z +EOF + +cat >expect_conflict_$commit_sha2 <<EOF +z notes on 2nd commit +EOF + +cat >expect_conflict_$commit_sha3 <<EOF +y notes on 3rd commit +EOF + +cat >expect_conflict_$commit_sha4 <<EOF +<<<<<<< refs/notes/m +y notes on 4th commit +======= +z notes on 4th commit +>>>>>>> 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 + +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 && + 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)" +' + +cat <<EOF | sort >expect_notes_z +00494adecf2d9635a02fa431308d67993f853968 $commit_sha4 +283b48219aee9a4105f6cab337e789065c82c2b9 $commit_sha2 +0a81da8956346e19bcb27a906f04af327e03e31b $commit_sha1 +EOF + +cat >expect_log_z <<EOF +$commit_sha5 5th + +$commit_sha4 4th +z notes on 4th commit + +More z notes on 4th commit + +$commit_sha3 3rd + +$commit_sha2 2nd +z notes on 2nd commit + +$commit_sha1 1st +z notes on 1st commit + +EOF + +test_expect_success 'change notes in z' ' + git notes --ref z append -m "More z notes on 4th commit" 4th && + verify_notes z +' + +test_expect_success 'cannot do merge w/conflicts when previous merge is unfinished' ' + test -d .git/NOTES_MERGE_WORKTREE && + test_must_fail git notes merge z >output 2>&1 && + # Output should indicate what is wrong + grep -q "\\.git/NOTES_MERGE_\\* exists" output +' + +# Setup non-conflicting merge between x and new notes ref w + +cat <<EOF | sort >expect_notes_w +ceefa674873670e7ecd131814d909723cce2b669 $commit_sha2 +f75d1df88cbfe4258d49852f26cfc83f2ad4494b $commit_sha1 +EOF + +cat >expect_log_w <<EOF +$commit_sha5 5th + +$commit_sha4 4th + +$commit_sha3 3rd + +$commit_sha2 2nd +x notes on 2nd commit + +$commit_sha1 1st +w notes on 1st commit + +EOF + +test_expect_success 'setup unrelated notes ref (w)' ' + git config core.notesRef refs/notes/w && + git notes add -m "w notes on 1st commit" 1st && + git notes add -m "x notes on 2nd commit" 2nd && + verify_notes w +' + +cat <<EOF | sort >expect_notes_w +6e8e3febca3c2bb896704335cc4d0c34cb2f8715 $commit_sha4 +e5388c10860456ee60673025345fe2e153eb8cf8 $commit_sha3 +ceefa674873670e7ecd131814d909723cce2b669 $commit_sha2 +f75d1df88cbfe4258d49852f26cfc83f2ad4494b $commit_sha1 +EOF + +cat >expect_log_w <<EOF +$commit_sha5 5th + +$commit_sha4 4th +x notes on 4th commit + +$commit_sha3 3rd +x notes on 3rd commit + +$commit_sha2 2nd +x notes on 2nd commit + +$commit_sha1 1st +w notes on 1st commit + +EOF + +test_expect_success 'can do merge without conflicts even if previous merge is unfinished (x => w)' ' + test -d .git/NOTES_MERGE_WORKTREE && + git notes merge x && + verify_notes w && + # Verify that other notes refs has not changed (x and y) + verify_notes x && + verify_notes y +' + +test_done |