Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> 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.
Technically isn't the directory structure this codepath looks at a working
tree that has extract of a notes tree commit?
Looking at the result of the patch I do not have strong opinions either
way, though. It isn't like we care about gitignore or attributes rules in
the notes tree, so using read_directory() does feel like an overkill.
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> notes-merge.c | 45 +++++++++++++++++++++++++++------------------
> 1 files changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/notes-merge.c b/notes-merge.c
> index e9e4199..80d64a2 100644
> --- a/notes-merge.c
> +++ b/notes-merge.c
> @@ -680,48 +680,57 @@ int notes_merge_commit(struct notes_merge_options *o,
> * 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;
> const char *msg = strstr(partial_commit->buffer, "\n\n");
> + int baselen;
>
> - OUTPUT(o, 3, "Committing notes in notes merge worktree at %.*s",
> - path_len - 1, path);
> + strbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE));
> + OUTPUT(o, 3, "Committing notes in notes merge worktree at %s", 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)) {
> - OUTPUT(o, 3, "Skipping non-SHA1 entry '%s'", ent->name);
> + if (is_dot_or_dotdot(e->d_name))
> + continue;
> +
> + if (strlen(e->d_name) != 40 || get_sha1_hex(e->d_name, obj_sha1)) {
> + OUTPUT(o, 3, "Skipping non-SHA1 entry '%s%s'", 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);
> OUTPUT(o, 4, "Added resolved note for object %s: %s",
> sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1));
> + strbuf_setlen(&path, baselen);
> }
>
> create_notes_commit(partial_tree, partial_commit->parents, msg,
> result_sha1);
> OUTPUT(o, 4, "Finalized notes merge commit: %s",
> sha1_to_hex(result_sha1));
> - free(path);
> + strbuf_release(&path);
> + closedir(dir);
> return 0;
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html