git notes merge: --commit should fail if underlying notes ref has moved
When manually resolving a notes merge, if the merging ref has moved since the merge started, we should fail to complete the merge, and alert the user to what's going on. This situation may arise if you start a 'git notes merge' which results in conflicts, and you then update the current notes ref (using for example 'git notes add/copy/amend/edit/remove/prune', 'git update-ref', etc.), before you get around to resolving the notes conflicts and calling 'git notes merge --commit'. We detect this situation by comparing the first parent of the partial merge commit (which was created when the merge started) to the current value of the merging notes ref (pointed to by the .git/NOTES_MERGE_REF symref). If we don't fail in this situation, the notes merge commit would overwrite the updated notes ref, thus losing the changes that happened in the meantime. The patch includes a testcase verifying that we fail correctly in this situation. Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johan Herland committed
Nov 9, 2010 at 22:49 UTC
6cfd6a9dea889707fa207ee2003010c3b56b2131
2 files changed
+85
-2
builtin/notes.c
+9
-2
@@ -786,7 +786,7 @@ static int merge_abort(struct notes_merge_options *o)
786
static int merge_commit(struct notes_merge_options *o)
787
{
788
struct strbuf msg = STRBUF_INIT;
789
- unsigned char sha1[20];
789
+ unsigned char sha1[20], parent_sha1[20];
790
struct notes_tree *t;
791
struct commit *partial;
792
struct pretty_print_context pretty_ctx;
@@ -803,6 +803,11 @@ static int merge_commit(struct notes_merge_options *o)
803
else if (parse_commit(partial))
804
die("Could not parse commit from NOTES_MERGE_PARTIAL.");
805
806
+ if (partial->parents)
807
+ hashcpy(parent_sha1, partial->parents->item->object.sha1);
808
+ else
809
+ hashclr(parent_sha1);
810
+
811
t = xcalloc(1, sizeof(struct notes_tree));
812
init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
813
@@ -818,7 +823,9 @@ static int merge_commit(struct notes_merge_options *o)
823
format_commit_message(partial, "%s", &msg, &pretty_ctx);
824
strbuf_trim(&msg);
825
strbuf_insert(&msg, 0, "notes: ", 7);
821
- update_ref(msg.buf, o->local_ref, sha1, NULL, 0, DIE_ON_ERR);
826
+ update_ref(msg.buf, o->local_ref, sha1,
827
+ is_null_sha1(parent_sha1) ? NULL : parent_sha1,
828
+ 0, DIE_ON_ERR);
829
830
free_notes(t);
831
strbuf_release(&msg);
t/t3310-notes-merge-manual-resolve.sh
+76
@@ -477,4 +477,80 @@ EOF
477
verify_notes z
478
'
479
480
+cp expect_notes_y expect_notes_m
481
+cp expect_log_y expect_log_m
482
+
483
+test_expect_success 'redo merge of z into m (== y) with default ("manual") resolver => Conflicting 3-way merge' '
484
+ git update-ref refs/notes/m refs/notes/y &&
485
+ test_must_fail git notes merge z >output &&
486
+ # Output should point to where to resolve conflicts
487
+ grep -q "\\.git/NOTES_MERGE_WORKTREE" output &&
488
+ # Inspect merge conflicts
489
+ ls .git/NOTES_MERGE_WORKTREE >output_conflicts &&
490
+ test_cmp expect_conflicts output_conflicts &&
491
+ ( for f in $(cat expect_conflicts); do
492
+ test_cmp "expect_conflict_$f" ".git/NOTES_MERGE_WORKTREE/$f" ||
493
+ exit 1
494
+ done ) &&
495
+ # Verify that current notes tree (pre-merge) has not changed (m == y)
496
+ verify_notes y &&
497
+ verify_notes m &&
498
+ test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
499
+'
500
+
501
+cp expect_notes_w expect_notes_m
502
+cp expect_log_w expect_log_m
503
+
504
+test_expect_success 'reset notes ref m to somewhere else (w)' '
505
+ git update-ref refs/notes/m refs/notes/w &&
506
+ verify_notes m &&
507
+ test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)"
508
+'
509
+
510
+test_expect_success 'fail to finalize conflicting merge if underlying ref has moved in the meantime (m != NOTES_MERGE_PARTIAL^1)' '
511
+ # Resolve conflicts
512
+ cat >.git/NOTES_MERGE_WORKTREE/$commit_sha1 <<EOF &&
513
+y and z notes on 1st commit
514
+EOF
515
+ cat >.git/NOTES_MERGE_WORKTREE/$commit_sha4 <<EOF &&
516
+y and z notes on 4th commit
517
+EOF
518
+ # Fail to finalize merge
519
+ test_must_fail git notes merge --commit >output 2>&1 &&
520
+ # .git/NOTES_MERGE_* must remain
521
+ test -f .git/NOTES_MERGE_PARTIAL &&
522
+ test -f .git/NOTES_MERGE_REF &&
523
+ test -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&
524
+ test -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&
525
+ test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
526
+ test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
527
+ # Refs are unchanged
528
+ test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)"
529
+ test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)"
530
+ test "$(git rev-parse refs/notes/m)" != "$(git rev-parse NOTES_MERGE_PARTIAL^1)"
531
+ # Mention refs/notes/m, and its current and expected value in output
532
+ grep -q "refs/notes/m" output &&
533
+ grep -q "$(git rev-parse refs/notes/m)" output &&
534
+ grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output &&
535
+ # Verify that other notes refs has not changed (w, x, y and z)
536
+ verify_notes w &&
537
+ verify_notes x &&
538
+ verify_notes y &&
539
+ verify_notes z
540
+'
541
+
542
+test_expect_success 'resolve situation by aborting the notes merge' '
543
+ git notes merge --abort &&
544
+ # No .git/NOTES_MERGE_* files left
545
+ test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
546
+ test_cmp /dev/null output &&
547
+ # m has not moved (still == w)
548
+ test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)"
549
+ # Verify that other notes refs has not changed (w, x, y and z)
550
+ verify_notes w &&
551
+ verify_notes x &&
552
+ verify_notes y &&
553
+ verify_notes z
554
+'
555
+
556
test_done