git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* problem with merging notes.
@ 2012-03-11 17:17 David Bremner
  2012-03-11 18:37 ` Michael Schubert
  2012-03-12 14:57 ` [PATCH 1/2] t3310: Add testcase demonstrating failure to --commit from within another dir Johan Herland
  0 siblings, 2 replies; 8+ messages in thread
From: David Bremner @ 2012-03-11 17:17 UTC (permalink / raw)
  To: Git Mailing List


Hi;

It seems to me there is a bug (or at least some missing documentation)
for git notes merge --commit. If the working directory is
.git/NOTES_MERGE_WORKTREE, then all notes for the conflicting commit are
silently dropped.

A test script follows. Have I missed something?  

David

P.S. Please CC me, I'm not subscribed.


tmpdir=$(mktemp -d)
cd $tmpdir
git init
git commit --allow-empty -m'empty commit'
git notes add -m 'foo' HEAD
git notes --ref=other add -m 'bar' HEAD 
git notes merge refs/notes/other
cd .git/NOTES_MERGE_WORKTREE
echo "foo\nbar\n"> $(git rev-parse HEAD)
git notes merge --commit
cd ../..
git notes list | wc -l

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()
@ 2011-10-24  6:36 Nguyễn Thái Ngọc Duy
  2012-03-12 14:47 ` [PATCH 1/2] t3310: Add testcase demonstrating failure to --commit from within another dir Johan Herland
  0 siblings, 1 reply; 8+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2011-10-24  6:36 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

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.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 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;
 }
 
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-03-12 18:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-11 17:17 problem with merging notes David Bremner
2012-03-11 18:37 ` Michael Schubert
2012-03-12 14:57 ` [PATCH 1/2] t3310: Add testcase demonstrating failure to --commit from within another dir Johan Herland
2012-03-12 14:57   ` [PATCH 2/2] notes-merge: use opendir/readdir instead of using read_directory() Johan Herland
2012-03-12 18:21   ` [PATCH 1/2] t3310: Add testcase demonstrating failure to --commit from within another dir Junio C Hamano
2012-03-12 18:54     ` Johan Herland
  -- strict thread matches above, loose matches on Subject: below --
2011-10-24  6:36 [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory() Nguyễn Thái Ngọc Duy
2012-03-12 14:47 ` [PATCH 1/2] t3310: Add testcase demonstrating failure to --commit from within another dir Johan Herland
2012-03-12 14:47   ` [PATCH 2/2] notes-merge: use opendir/readdir instead of using read_directory() Johan Herland
2012-03-12 14:53     ` Nguyen Thai Ngoc Duy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).