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

[PATCH 2/2] fast-import: teach ls command to accept empty path

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Mar 8, 2012, 20:33 UTC
Message-ID
<20120308203330.GC8992@burratino>
In-Reply-To
<20120308202721.GA8992@burratino>
From: David Barr <davidbarr@google.com>

There is a pathological Subversion operation that svn-fe handles incorrectly due to an unexpected response from fast-import:

  svn cp $SVN_ROOT $SVN_ROOT/subdirectory
When the following command is sent to fast-import:
 'ls' SP ':1' SP LF
The expected output is:
 '040000' SP 'tree' SP <dataref> HT LF
The actual output is:
 'missing' SP LF

This is because tree_content_get() is called but expects a non-empty path. Instead, copy the root entry.

[jn: using a deep copy; w/ more tests]
[jn: with a fix from Dmitry to fully initialize root->versions[0]
 and versions[1] now that root can be passed to store_tree]
Reported-by: Andrew Sayers <andrew-git@pileofstuff.org>
Signed-off-by: David Barr <davidbarr@google.com>
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 fast-import.c          |   17 ++++++++-
 t/t9300-fast-import.sh |   95 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+), 1 deletion(-)
diff --git a/fast-import.c b/fast-import.c
index 1758da94..31857e95 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3004,7 +3004,10 @@ static void parse_ls(struct branch *b)
 	} else {
 		struct object_entry *e = parse_treeish_dataref(&p);
 		root = new_tree_entry();
+		hashclr(root->versions[0].sha1);
 		hashcpy(root->versions[1].sha1, e->idx.sha1);
+		root->versions[0].mode = 0;
+		root->versions[1].mode = S_IFDIR;
 		load_tree(root);
 		if (*p++ != ' ')
 			die("Missing space after tree-ish: %s", command_buf.buf);
@@ -3019,7 +3022,19 @@ static void parse_ls(struct branch *b)
 			die("Garbage after path in: %s", command_buf.buf);
 		p = uq.buf;
 	}
-	tree_content_get(root, p, &leaf);
+	if (*p) {
+		tree_content_get(root, p, &leaf);
+	} else {
+		memcpy(&leaf, root, sizeof(leaf));
+		/*
+		 * store_tree scribbles over version[0] in leaf.tree's
+		 * entries, so we need a deep copy.
+		 */
+		if (root->tree && is_null_sha1(root->versions[1].sha1))
+			leaf.tree = dup_tree_content(root->tree);
+		else
+			leaf.tree = NULL;
+	}
 	/*
 	 * A directory in preparation would have a sha1 of zero
 	 * until it is saved.  Save, for simplicity.
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 438aaf6b..635bfadb 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1400,6 +1400,101 @@ test_expect_success \
 	 test_cmp expect.qux actual.qux &&
 	 test_cmp expect.qux actual.quux'
 
+test_expect_success 'N: root of unborn branch reads as present and empty' '
+	empty_tree=$(git mktree </dev/null) &&
+	echo "040000 tree $empty_tree	" >expect &&
+	git fast-import --cat-blob-fd=3 3>actual <<-EOF &&
+	commit refs/heads/N-empty
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	read empty root directory via ls
+	COMMIT
+
+	ls ""
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'N: empty root reads as present and empty' '
+	empty_tree=$(git mktree </dev/null) &&
+	echo "040000 tree $empty_tree	" >expect &&
+	echo empty >msg &&
+	cmit=$(git commit-tree "$empty_tree" -p refs/heads/branch^0 <msg) &&
+	git fast-import --cat-blob-fd=3 3>actual <<-EOF &&
+	commit refs/heads/N-empty-existing
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	read empty root directory via ls
+	COMMIT
+
+	ls ""
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'N: "ls" command can read subdir of named tree' '
+	branch_cmit=$(git rev-parse --verify refs/heads/branch^0) &&
+	subdir_tree=$(git rev-parse $branch_cmit:newdir) &&
+	echo "040000 tree $subdir_tree	newdir" >expect &&
+	git fast-import --cat-blob-fd=3 3>actual <<-EOF &&
+	commit refs/heads/N-subdir-of-named-tree
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	read from commit with ls
+	COMMIT
+
+	ls $branch_cmit "newdir"
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'N: "ls" command can read root of named commit' '
+	branch_cmit=$(git rev-parse --verify refs/heads/branch^0) &&
+	branch_tree=$(git rev-parse --verify $branch_cmit^{tree}) &&
+	echo "040000 tree $branch_tree	" >expect &&
+	git fast-import --cat-blob-fd=3 3>actual <<-EOF &&
+	commit refs/heads/N-root-of-named-tree
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	read root directory of commit with ls
+	COMMIT
+
+	ls $branch_cmit ""
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success PIPE 'N: read and copy root' '
+	cat >expect <<-\EOF &&
+	:100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc f1fb5da718392694d0076d677d6d0e364c79b0bc C100	file2/newf	file3/file2/newf
+	:100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1 7123f7f44e39be127c5eb701e5968176ee9d78b1 C100	file2/oldf	file3/file2/oldf
+	:100755 100755 85df50785d62d3b05ab03d9cbf7e4a0b49449730 85df50785d62d3b05ab03d9cbf7e4a0b49449730 C100	file4	file3/file4
+	:100755 100755 e74b7d465e52746be2b4bae983670711e6e66657 e74b7d465e52746be2b4bae983670711e6e66657 C100	newdir/exec.sh	file3/newdir/exec.sh
+	:100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 C100	newdir/interesting	file3/newdir/interesting
+	EOF
+	git update-ref -d refs/heads/N12 &&
+	rm -f backflow &&
+	mkfifo backflow &&
+	(
+		exec <backflow &&
+		cat <<-EOF &&
+		commit refs/heads/N12
+		committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+		data <<COMMIT
+		copy root directory by tree hash read via ls
+		COMMIT
+
+		from refs/heads/branch^0
+		ls ""
+		EOF
+		read mode type tree filename &&
+		echo "M 040000 $tree file3"
+	) |
+	git fast-import --cat-blob-fd=3 3>backflow &&
+	git diff-tree -C --find-copies-harder -r N12^ N12 >actual &&
+	compare_diff_raw expect actual
+'
+
 ###
 ### series O
 ###

-- 
1.7.9.2
Previous: Jonathan NiederNext: David Barr
Message 18 of 23 in “[BUG] fast-import: ls command on commit root returns missing (was: Bug in svn-fe: copying the root directory acts as if it's an empty directory)”
  1. David BarrMar 8, 2012
  2. fast-import: fix ls command with empty pathDavid Barr, Mar 8, 2012
  3. Jonathan NiederMar 8, 2012
  4. Sverre RabbelierMar 8, 2012
  5. Junio C HamanoMar 8, 2012
  6. Dmitry IvankovMar 8, 2012
  7. Jonathan NiederMar 8, 2012
  8. Jonathan NiederMar 10, 2012
  9. Jonathan NiederMar 8, 2012
  10. Jonathan NiederMar 10, 2012
  11. fast-import: leakfix for 'ls' of dirty treesJonathan Nieder, Mar 10, 2012
  12. fast-import: don't allow 'ls' of path with empty componentsJonathan Nieder, Mar 10, 2012
  13. [PULL maint] two fast-import "ls" fixesJonathan Nieder, Mar 10, 2012
  14. vcs-svn: avoid 'ls' and filedelete with empty pathJonathan Nieder, Mar 10, 2012
  15. Dave AbrahamsJun 21, 2013
  16. 0/2 Re: fast-import: fix ls command with empty pathJonathan Nieder, Mar 8, 2012
  17. 1/2 fast-import: plug leak of dirty trees in 'ls' commandJonathan Nieder, Mar 8, 2012
  18. 2/2 fast-import: teach ls command to accept empty pathJonathan Nieder, Mar 8, 2012
  19. David BarrMar 9, 2012
  20. Jonathan NiederMar 9, 2012
  21. fast-import: allow 'ls' and filecopy to read the rootJonathan Nieder, Mar 10, 2012
  22. Jonathan NiederMar 10, 2012
  23. 2/1 fixup! fast-import: allow 'ls' and filecopy to read the rootJonathan Nieder, Mar 10, 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.