git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/2] notes-merge: use opendir/readdir instead of using read_directory()

From
Johan Herland <johan@herland.net>
Date
Mar 12, 2012, 14:57 UTC
Message-ID
<1331564233-1969-2-git-send-email-johan@herland.net>
In-Reply-To
<1331564233-1969-1-git-send-email-johan@herland.net>

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 <pclouds@gmail.com>
Updated-by:  Johan Herland <johan@herland.net>
Signed-off-by: Johan Herland <johan@herland.net>
---
(sending again in the correct thread. Sorry for the screwup.)

This is a resurrection of pclouds' patch 2/11 in a patch series sent last October for rewriting read_directory(). This patch doesn't actually touch read_directory(), but instead rewrites notes_merge_commit() to use opendir()/readdir() instead of read_directory(). Since the usage of read_directory() is what caused the bug that David found (in the previous patch), this rewrite happens to fix that bug as well.

Have fun! :)
...Johan
 notes-merge.c                         |   50 ++++++++++++++++++++-------------
 t/t3310-notes-merge-manual-resolve.sh |    2 +-
 2 files changed, 31 insertions(+), 21 deletions(-)
diff --git a/notes-merge.c b/notes-merge.c
index fb0832f..3a16af2 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;
 }

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index 0c531c3..d6d6ac6 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -558,7 +558,7 @@ foo
 bar
 EOF

-test_expect_failure 'switch cwd before committing notes merge' '
+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 &&
--
1.7.9.2
Previous: Johan HerlandNext: Junio C Hamano
Message 4 of 6 in “problem with merging notes.”
  1. David BremnerMar 11, 2012
  2. Michael SchubertMar 11, 2012
  3. 1/2 t3310: Add testcase demonstrating failure to --commit from within another dirJohan Herland, Mar 12, 2012
  4. 2/2 notes-merge: use opendir/readdir instead of using read_directory()Johan Herland, Mar 12, 2012
  5. Junio C HamanoMar 12, 2012
  6. Johan HerlandMar 12, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.