Merge branch 'jh/notes-merge-in-git-dir-worktree'

Running "notes merge --commit" failed to perform correctly when run
from any directory inside $GIT_DIR/.  When "notes merge" stops with
conflicts, $GIT_DIR/NOTES_MERGE_WORKTREE is the place a user edits
to resolve it.

By Johan Herland (3) and Junio C Hamano (1)
* jh/notes-merge-in-git-dir-worktree:
  notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd
  notes-merge: use opendir/readdir instead of using read_directory()
  t3310: illustrate failure to "notes merge --commit" inside $GIT_DIR/
  remove_dir_recursively(): Add flag for skipping removal of toplevel dir
This commit is contained in:
Junio C Hamano 2012-04-15 22:51:44 -07:00
commit 4e520081b5
4 changed files with 73 additions and 32 deletions

14
dir.c
View File

@ -1178,6 +1178,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)
struct dirent *e; struct dirent *e;
int ret = 0, original_len = path->len, len; int ret = 0, original_len = path->len, len;
int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY); int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
unsigned char submodule_head[20]; unsigned char submodule_head[20];
if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) && if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
@ -1185,9 +1186,14 @@ int remove_dir_recursively(struct strbuf *path, int flag)
/* Do not descend and nuke a nested git work tree. */ /* Do not descend and nuke a nested git work tree. */
return 0; return 0;
flag &= ~(REMOVE_DIR_KEEP_TOPLEVEL|REMOVE_DIR_KEEP_NESTED_GIT);
dir = opendir(path->buf); dir = opendir(path->buf);
if (!dir) if (!dir) {
return rmdir(path->buf); if (!keep_toplevel)
return rmdir(path->buf);
else
return -1;
}
if (path->buf[original_len - 1] != '/') if (path->buf[original_len - 1] != '/')
strbuf_addch(path, '/'); strbuf_addch(path, '/');
@ -1202,7 +1208,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)
if (lstat(path->buf, &st)) if (lstat(path->buf, &st))
; /* fall thru */ ; /* fall thru */
else if (S_ISDIR(st.st_mode)) { else if (S_ISDIR(st.st_mode)) {
if (!remove_dir_recursively(path, only_empty)) if (!remove_dir_recursively(path, flag))
continue; /* happy */ continue; /* happy */
} else if (!only_empty && !unlink(path->buf)) } else if (!only_empty && !unlink(path->buf))
continue; /* happy, too */ continue; /* happy, too */
@ -1214,7 +1220,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)
closedir(dir); closedir(dir);
strbuf_setlen(path, original_len); strbuf_setlen(path, original_len);
if (!ret) if (!ret && !keep_toplevel)
ret = rmdir(path->buf); ret = rmdir(path->buf);
return ret; return ret;
} }

1
dir.h
View File

@ -102,6 +102,7 @@ extern void setup_standard_excludes(struct dir_struct *dir);
#define REMOVE_DIR_EMPTY_ONLY 01 #define REMOVE_DIR_EMPTY_ONLY 01
#define REMOVE_DIR_KEEP_NESTED_GIT 02 #define REMOVE_DIR_KEEP_NESTED_GIT 02
#define REMOVE_DIR_KEEP_TOPLEVEL 04
extern int remove_dir_recursively(struct strbuf *path, int flag); extern int remove_dir_recursively(struct strbuf *path, int flag);
/* tries to remove the path with empty directories along it, ignores ENOENT */ /* tries to remove the path with empty directories along it, ignores ENOENT */

View File

@ -267,7 +267,8 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
* Must establish NOTES_MERGE_WORKTREE. * Must establish NOTES_MERGE_WORKTREE.
* Abort if NOTES_MERGE_WORKTREE already exists * Abort if NOTES_MERGE_WORKTREE already exists
*/ */
if (file_exists(git_path(NOTES_MERGE_WORKTREE))) { if (file_exists(git_path(NOTES_MERGE_WORKTREE)) &&
!is_empty_dir(git_path(NOTES_MERGE_WORKTREE))) {
if (advice_resolve_conflict) if (advice_resolve_conflict)
die("You have not concluded your previous " die("You have not concluded your previous "
"notes merge (%s exists).\nPlease, use " "notes merge (%s exists).\nPlease, use "
@ -687,51 +688,60 @@ int notes_merge_commit(struct notes_merge_options *o,
{ {
/* /*
* Iterate through files in .git/NOTES_MERGE_WORKTREE and add all * Iterate through files in .git/NOTES_MERGE_WORKTREE and add all
* found notes to 'partial_tree'. Write the updates notes tree to * found notes to 'partial_tree'. Write the updated notes tree to
* the DB, and commit the resulting tree object while reusing the * the DB, and commit the resulting tree object while reusing the
* commit message and parents from 'partial_commit'. * commit message and parents from 'partial_commit'.
* Finally store the new commit object SHA1 into 'result_sha1'. * Finally store the new commit object SHA1 into 'result_sha1'.
*/ */
struct dir_struct dir; DIR *dir;
char *path = xstrdup(git_path(NOTES_MERGE_WORKTREE "/")); struct dirent *e;
int path_len = strlen(path), i; struct strbuf path = STRBUF_INIT;
char *msg = strstr(partial_commit->buffer, "\n\n"); char *msg = strstr(partial_commit->buffer, "\n\n");
struct strbuf sb_msg = STRBUF_INIT; struct strbuf sb_msg = STRBUF_INIT;
int baselen;
strbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE));
if (o->verbosity >= 3) if (o->verbosity >= 3)
printf("Committing notes in notes merge worktree at %.*s\n", printf("Committing notes in notes merge worktree at %s\n",
path_len - 1, path); path.buf);
if (!msg || msg[2] == '\0') if (!msg || msg[2] == '\0')
die("partial notes commit has empty message"); die("partial notes commit has empty message");
msg += 2; msg += 2;
memset(&dir, 0, sizeof(dir)); dir = opendir(path.buf);
read_directory(&dir, path, path_len, NULL); if (!dir)
for (i = 0; i < dir.nr; i++) { die_errno("could not open %s", path.buf);
struct dir_entry *ent = dir.entries[i];
strbuf_addch(&path, '/');
baselen = path.len;
while ((e = readdir(dir)) != NULL) {
struct stat st; struct stat st;
const char *relpath = ent->name + path_len;
unsigned char obj_sha1[20], blob_sha1[20]; unsigned char obj_sha1[20], blob_sha1[20];
if (ent->len - path_len != 40 || get_sha1_hex(relpath, obj_sha1)) { if (is_dot_or_dotdot(e->d_name))
continue;
if (strlen(e->d_name) != 40 || get_sha1_hex(e->d_name, obj_sha1)) {
if (o->verbosity >= 3) if (o->verbosity >= 3)
printf("Skipping non-SHA1 entry '%s'\n", printf("Skipping non-SHA1 entry '%s%s'\n",
ent->name); path.buf, e->d_name);
continue; continue;
} }
strbuf_addstr(&path, e->d_name);
/* write file as blob, and add to partial_tree */ /* write file as blob, and add to partial_tree */
if (stat(ent->name, &st)) if (stat(path.buf, &st))
die_errno("Failed to stat '%s'", ent->name); die_errno("Failed to stat '%s'", path.buf);
if (index_path(blob_sha1, ent->name, &st, HASH_WRITE_OBJECT)) if (index_path(blob_sha1, path.buf, &st, HASH_WRITE_OBJECT))
die("Failed to write blob object from '%s'", ent->name); die("Failed to write blob object from '%s'", path.buf);
if (add_note(partial_tree, obj_sha1, blob_sha1, NULL)) if (add_note(partial_tree, obj_sha1, blob_sha1, NULL))
die("Failed to add resolved note '%s' to notes tree", die("Failed to add resolved note '%s' to notes tree",
ent->name); path.buf);
if (o->verbosity >= 4) if (o->verbosity >= 4)
printf("Added resolved note for object %s: %s\n", printf("Added resolved note for object %s: %s\n",
sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1)); sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1));
strbuf_setlen(&path, baselen);
} }
strbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1); strbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1);
@ -740,20 +750,25 @@ int notes_merge_commit(struct notes_merge_options *o,
if (o->verbosity >= 4) if (o->verbosity >= 4)
printf("Finalized notes merge commit: %s\n", printf("Finalized notes merge commit: %s\n",
sha1_to_hex(result_sha1)); sha1_to_hex(result_sha1));
free(path); strbuf_release(&path);
closedir(dir);
return 0; return 0;
} }
int notes_merge_abort(struct notes_merge_options *o) int notes_merge_abort(struct notes_merge_options *o)
{ {
/* Remove .git/NOTES_MERGE_WORKTREE directory and all files within */ /*
* Remove all files within .git/NOTES_MERGE_WORKTREE. We do not remove
* the .git/NOTES_MERGE_WORKTREE directory itself, since it might be
* the current working directory of the user.
*/
struct strbuf buf = STRBUF_INIT; struct strbuf buf = STRBUF_INIT;
int ret; int ret;
strbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE)); strbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE));
if (o->verbosity >= 3) if (o->verbosity >= 3)
printf("Removing notes merge worktree at %s\n", buf.buf); printf("Removing notes merge worktree at %s/*\n", buf.buf);
ret = remove_dir_recursively(&buf, 0); ret = remove_dir_recursively(&buf, REMOVE_DIR_KEEP_TOPLEVEL);
strbuf_release(&buf); strbuf_release(&buf);
return ret; return ret;
} }

View File

@ -324,7 +324,7 @@ y and z notes on 4th commit
EOF EOF
git notes merge --commit && git notes merge --commit &&
# No .git/NOTES_MERGE_* files left # No .git/NOTES_MERGE_* files left
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
test_cmp /dev/null output && test_cmp /dev/null output &&
# Merge commit has pre-merge y and pre-merge z as parents # 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^1)" = "$(cat pre_merge_y)" &&
@ -386,7 +386,7 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
test_expect_success 'abort notes merge' ' test_expect_success 'abort notes merge' '
git notes merge --abort && git notes merge --abort &&
# No .git/NOTES_MERGE_* files left # No .git/NOTES_MERGE_* files left
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
test_cmp /dev/null output && test_cmp /dev/null output &&
# m has not moved (still == y) # m has not moved (still == y)
test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" && test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" &&
@ -453,7 +453,7 @@ EOF
# Finalize merge # Finalize merge
git notes merge --commit && git notes merge --commit &&
# No .git/NOTES_MERGE_* files left # No .git/NOTES_MERGE_* files left
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
test_cmp /dev/null output && test_cmp /dev/null output &&
# Merge commit has pre-merge y and pre-merge z as parents # 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^1)" = "$(cat pre_merge_y)" &&
@ -542,7 +542,7 @@ EOF
test_expect_success 'resolve situation by aborting the notes merge' ' test_expect_success 'resolve situation by aborting the notes merge' '
git notes merge --abort && git notes merge --abort &&
# No .git/NOTES_MERGE_* files left # No .git/NOTES_MERGE_* files left
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
test_cmp /dev/null output && test_cmp /dev/null output &&
# m has not moved (still == w) # m has not moved (still == w)
test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" && test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
@ -553,4 +553,23 @@ test_expect_success 'resolve situation by aborting the notes merge' '
verify_notes z verify_notes z
' '
cat >expect_notes <<EOF
foo
bar
EOF
test_expect_success 'switch cwd before committing notes merge' '
git notes add -m foo HEAD &&
git notes --ref=other add -m bar HEAD &&
test_must_fail git notes merge refs/notes/other &&
(
cd .git/NOTES_MERGE_WORKTREE &&
echo "foo" > $(git rev-parse HEAD) &&
echo "bar" >> $(git rev-parse HEAD) &&
git notes merge --commit
) &&
git notes show HEAD > actual_notes &&
test_cmp expect_notes actual_notes
'
test_done test_done