@matheusbsbs / matheus-bagliano / commits / dabba590aa

notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd

When a manual notes merge is committed or aborted, we need to remove the temporary worktree at .git/NOTES_MERGE_WORKTREE. However, removing the entire directory is not good if the user ran the 'git notes merge --commit/--abort' from within that directory. On Windows, the directory removal would simply fail, while on POSIX systems, users would suddenly find themselves in an invalid current directory. Therefore, instead of deleting the entire directory, we delete everything _within_ the directory, and leave the (empty) directory in place. This would cause a subsequent notes merge to abort, complaining about a previous - unfinished - notes merge (due to the presence of .git/NOTES_MERGE_WORKTREE), so we also need to adjust this check to only trigger when .git/NOTES_MERGE_WORKTREE is non-empty. Finally, adjust the t3310 manual notes merge testcases to correctly handle the existence of an empty .git/NOTES_MERGE_WORKTREE directory. Inspired-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johan Herland committed Mar 15, 2012 at 15:58 UTC dabba590aa0e94a8deb6f0e827144697895bcfe8
2 files changed +13 -8
notes-merge.c
+9 -4
@@ -267,7 +267,8 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
267 * Must establish NOTES_MERGE_WORKTREE.
268 * Abort if NOTES_MERGE_WORKTREE already exists
269 */
270 - if (file_exists(git_path(NOTES_MERGE_WORKTREE))) {
270 + if (file_exists(git_path(NOTES_MERGE_WORKTREE)) &&
271 + !is_empty_dir(git_path(NOTES_MERGE_WORKTREE))) {
272 if (advice_resolve_conflict)
273 die("You have not concluded your previous "
274 "notes merge (%s exists).\nPlease, use "
@@ -756,14 +757,18 @@ int notes_merge_commit(struct notes_merge_options *o,
757
758 int notes_merge_abort(struct notes_merge_options *o)
759 {
759 - /* Remove .git/NOTES_MERGE_WORKTREE directory and all files within */
760 + /*
761 + * Remove all files within .git/NOTES_MERGE_WORKTREE. We do not remove
762 + * the .git/NOTES_MERGE_WORKTREE directory itself, since it might be
763 + * the current working directory of the user.
764 + */
765 struct strbuf buf = STRBUF_INIT;
766 int ret;
767
768 strbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE));
769 if (o->verbosity >= 3)
765 - printf("Removing notes merge worktree at %s\n", buf.buf);
766 - ret = remove_dir_recursively(&buf, 0);
770 + printf("Removing notes merge worktree at %s/*\n", buf.buf);
771 + ret = remove_dir_recursively(&buf, REMOVE_DIR_KEEP_TOPLEVEL);
772 strbuf_release(&buf);
773 return ret;
774 }
t/t3310-notes-merge-manual-resolve.sh
+4 -4
@@ -324,7 +324,7 @@ y and z notes on 4th commit
324 EOF
325 git notes merge --commit &&
326 # No .git/NOTES_MERGE_* files left
327 - test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
327 + test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
328 test_cmp /dev/null output &&
329 # Merge commit has pre-merge y and pre-merge z as parents
330 test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
@@ -386,7 +386,7 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
386 test_expect_success 'abort notes merge' '
387 git notes merge --abort &&
388 # No .git/NOTES_MERGE_* files left
389 - test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
389 + test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
390 test_cmp /dev/null output &&
391 # m has not moved (still == y)
392 test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" &&
@@ -453,7 +453,7 @@ EOF
453 # Finalize merge
454 git notes merge --commit &&
455 # No .git/NOTES_MERGE_* files left
456 - test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
456 + test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
457 test_cmp /dev/null output &&
458 # Merge commit has pre-merge y and pre-merge z as parents
459 test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
@@ -542,7 +542,7 @@ EOF
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 &&
545 + test_might_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)" &&