From a0be62c100897573ef1575ec0d5e8b215e9dcafe Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Mon, 12 Mar 2012 15:57:13 +0100 Subject: notes-merge: use opendir/readdir instead of using read_directory() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit notes_merge_commit() only needs to list all entries (non-recursively) under a directory, which can be easily accomplished with opendir/readdir and would be more lightweight than read_directory(). read_directory() is designed to list paths inside a working directory. Using it outside of its scope may lead to undesired effects. Apparently, one of the undesired effects of read_directory() is that it doesn't deal with being given absolute paths. This creates problems for notes_merge_commit() when git_path() returns an absolute path, which happens when the current working directory is in a subdirectory of the .git directory. Originally-by: Nguyễn Thái Ngọc Duy Updated-by: Johan Herland Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- notes-merge.c | 50 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 20 deletions(-) (limited to 'notes-merge.c') diff --git a/notes-merge.c b/notes-merge.c index fb0832f97..3a16af281 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -687,51 +687,60 @@ int notes_merge_commit(struct notes_merge_options *o, { /* * 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 * commit message and parents from 'partial_commit'. * Finally store the new commit object SHA1 into 'result_sha1'. */ - struct dir_struct dir; - char *path = xstrdup(git_path(NOTES_MERGE_WORKTREE "/")); - int path_len = strlen(path), i; + DIR *dir; + struct dirent *e; + struct strbuf path = STRBUF_INIT; char *msg = strstr(partial_commit->buffer, "\n\n"); struct strbuf sb_msg = STRBUF_INIT; + int baselen; + strbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE)); if (o->verbosity >= 3) - printf("Committing notes in notes merge worktree at %.*s\n", - path_len - 1, path); + printf("Committing notes in notes merge worktree at %s\n", + path.buf); 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]; + dir = opendir(path.buf); + if (!dir) + die_errno("could not open %s", path.buf); + + strbuf_addch(&path, '/'); + baselen = path.len; + while ((e = readdir(dir)) != NULL) { 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)) { + 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) - printf("Skipping non-SHA1 entry '%s'\n", - ent->name); + printf("Skipping non-SHA1 entry '%s%s'\n", + path.buf, e->d_name); continue; } + strbuf_addstr(&path, e->d_name); /* 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, HASH_WRITE_OBJECT)) - die("Failed to write blob object from '%s'", ent->name); + if (stat(path.buf, &st)) + die_errno("Failed to stat '%s'", path.buf); + if (index_path(blob_sha1, path.buf, &st, HASH_WRITE_OBJECT)) + die("Failed to write blob object from '%s'", path.buf); if (add_note(partial_tree, obj_sha1, blob_sha1, NULL)) die("Failed to add resolved note '%s' to notes tree", - ent->name); + path.buf); if (o->verbosity >= 4) printf("Added resolved note for object %s: %s\n", sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1)); + strbuf_setlen(&path, baselen); } strbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1); @@ -740,7 +749,8 @@ int notes_merge_commit(struct notes_merge_options *o, if (o->verbosity >= 4) printf("Finalized notes merge commit: %s\n", sha1_to_hex(result_sha1)); - free(path); + strbuf_release(&path); + closedir(dir); return 0; } -- cgit v1.2.1 From dabba590aa0e94a8deb6f0e827144697895bcfe8 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Thu, 15 Mar 2012 15:58:56 +0100 Subject: 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 Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- notes-merge.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'notes-merge.c') diff --git a/notes-merge.c b/notes-merge.c index 3a16af281..74aa77ce4 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -267,7 +267,8 @@ static void check_notes_merge_worktree(struct notes_merge_options *o) * Must establish NOTES_MERGE_WORKTREE. * 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) die("You have not concluded your previous " "notes merge (%s exists).\nPlease, use " @@ -756,14 +757,18 @@ int notes_merge_commit(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; int ret; strbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE)); if (o->verbosity >= 3) - printf("Removing notes merge worktree at %s\n", buf.buf); - ret = remove_dir_recursively(&buf, 0); + printf("Removing notes merge worktree at %s/*\n", buf.buf); + ret = remove_dir_recursively(&buf, REMOVE_DIR_KEEP_TOPLEVEL); strbuf_release(&buf); return ret; } -- cgit v1.2.1