[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)

12 messages, 3 authors, 2016-06-15 · open the first message on its own page

[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)

From: David Barr <hidden>
Date: 2016-06-15 22:53:14

On Thu, Mar 8, 2012 at 11:46 AM, David Barr [off-list ref] wrote:
Hi Andrew,

On Thu, Mar 8, 2012 at 10:13 AM, Andrew Sayers
[off-list ref] wrote:
quoted
Here's a bug with svn-fe that I stumbled over while snorkelling through
repo madness.  I've tested it with the version of svn-fe in git.git's
master branch.

Copying the root directory to a sub-directory (e.g. doing `svn cp .
trunk` to standardise your layout) doesn't correctly initialise the new
directory.
This issue sounds very familiar, I wonder if there's an existing test
or pending patch for it? Maybe Dmitry or Jonathan can recall.
I've stepped through the reproduction and the bug seems to arise when
the following command is sent to git-fast-import:

  'ls' SP ':1' SP LF

The expected output in this example is:

  '400000' SP 'tree' SP 'dd59323fe27c5647cb7ef15ce4637faae199c5f0' HT LF

The actual output is:

  'missing' SP LF

--
David Barr

[PATCH] fast-import: fix ls command with empty path

From: David Barr <hidden>
Date: 2016-06-15 22:53:14

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 and force the mode to S_IFDIR.

Reported-by: Andrew Sayers <redacted>
Signed-off-by: David Barr <redacted>
---
 fast-import.c          |    7 ++++++-
 t/t9300-fast-import.sh |   31 +++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/fast-import.c b/fast-import.c
index c1486ca..8dbfd4c 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3019,7 +3019,12 @@ 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 {
+		leaf = *root;
+		leaf.versions[1].mode = S_IFDIR;
+	}
 	/*
 	 * 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 438aaf6..2558a2e 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1400,6 +1400,37 @@ test_expect_success \
 	 test_cmp expect.qux actual.qux &&
 	 test_cmp expect.qux actual.quux'
 
+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.3

Re: [PATCH] fast-import: fix ls command with empty path

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:14

(+cc: Sverre)
David Barr wrote:
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 and force the mode to S_IFDIR.

Reported-by: Andrew Sayers <redacted>
Signed-off-by: David Barr <redacted>
For what it's worth,
Acked-by: Jonathan Nieder <redacted>

Thanks very much for taking care of it.
[Subject: fast-import: fix ls command with empty path]
I would s/fix/accept/ to be more precise about the nature of the
breakage.  (In other words: rather than mishandling ls with an empty
path, fast-import was not handling it at all.)

[...]
quoted hunk
--- a/fast-import.c
+++ b/fast-import.c
@@ -3019,7 +3019,12 @@ 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 {
+		leaf = *root;
+		leaf.versions[1].mode = S_IFDIR;
+	}
If this special case were implemented in tree_content_get(), we would
get support for paths with a trailing '/' (allows useless
incompatibility, bad) and support for requests like

	C "" some/subdir

(good).  What do you think?

-- >8 --
Subject: fast-import: allow filecopy to copy from root

Some subversion users apparently use "svn copy $SVN_ROOT
$SVN_ROOT/subdirectory" from time to time.  svn-fe handles this fine
already since it translates the subversion copy instruction to

	ls ""
	M 040000 <returned tree name> subdirectory

We can easily imagine an alternate importer that would write

	C "" subdirectory

instead, so handle that, too.

A naive implementation would also mean gaining support for copies
where the source has a trailing '/', as in

	C onedir/ anotherdir

but in the spirit of 34215783 (fast-import: tighten M 040000 syntax,
2010-10-17), this patch is careful to reject that syntax to avoid
making it too easy for frontends to introduce unnecessary
incompatibilities with git fast-import 1.7.8 and older and other
fast-import consumers.

Signed-off-by: Jonathan Nieder <redacted>
---
 fast-import.c          |   32 ++++++++++++++-----------
 t/t9300-fast-import.sh |   62 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 15 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 8dbfd4cc..5ce61bca 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1636,6 +1636,10 @@ static int tree_content_get(
 	unsigned int i, n;
 	struct tree_entry *e;
 
+	if (!*p) {
+		e = root;
+		goto last_component;
+	}
 	slash1 = strchr(p, '/');
 	if (slash1)
 		n = slash1 - p;
@@ -1648,14 +1652,10 @@ static int tree_content_get(
 	for (i = 0; i < t->entry_count; i++) {
 		e = t->entries[i];
 		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
-			if (!slash1) {
-				memcpy(leaf, e, sizeof(*leaf));
-				if (e->tree && is_null_sha1(e->versions[1].sha1))
-					leaf->tree = dup_tree_content(e->tree);
-				else
-					leaf->tree = NULL;
-				return 1;
-			}
+			if (!slash1)
+				goto last_component;
+			if (!slash1[1])	/* paths with trailing '/' do not match */
+				return 0;
 			if (!S_ISDIR(e->versions[1].mode))
 				return 0;
 			if (!e->tree)
@@ -1664,6 +1664,14 @@ static int tree_content_get(
 		}
 	}
 	return 0;
+
+last_component:
+	memcpy(leaf, e, sizeof(*leaf));
+	if (e->tree && is_null_sha1(e->versions[1].sha1))
+		leaf->tree = dup_tree_content(e->tree);
+	else
+		leaf->tree = NULL;
+	return 1;
 }
 
 static int update_branch(struct branch *b)
@@ -3005,6 +3013,7 @@ static void parse_ls(struct branch *b)
 		struct object_entry *e = parse_treeish_dataref(&p);
 		root = new_tree_entry();
 		hashcpy(root->versions[1].sha1, e->idx.sha1);
+		root->versions[1].mode = S_IFDIR;
 		load_tree(root);
 		if (*p++ != ' ')
 			die("Missing space after tree-ish: %s", command_buf.buf);
@@ -3019,12 +3028,7 @@ static void parse_ls(struct branch *b)
 			die("Garbage after path in: %s", command_buf.buf);
 		p = uq.buf;
 	}
-	if (*p) {
-		tree_content_get(root, p, &leaf);
-	} else {
-		leaf = *root;
-		leaf.versions[1].mode = S_IFDIR;
-	}
+	tree_content_get(root, p, &leaf);
 	/*
 	 * 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 2558a2ed..5316b73c 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1047,6 +1047,66 @@ test_expect_success \
 	 git diff-tree -C --find-copies-harder -r N1^ N1 >actual &&
 	 compare_diff_raw expect actual'
 
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/N-root-to-subdir
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+copy to subdir
+COMMIT
+
+from refs/heads/branch^0
+C "" subdir
+
+INPUT_END
+
+cat >expect <<\EOF
+:100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc f1fb5da718392694d0076d677d6d0e364c79b0bc C100	file2/newf	subdir/file2/newf
+:100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1 7123f7f44e39be127c5eb701e5968176ee9d78b1 C100	file2/oldf	subdir/file2/oldf
+:100755 100755 85df50785d62d3b05ab03d9cbf7e4a0b49449730 85df50785d62d3b05ab03d9cbf7e4a0b49449730 C100	file4	subdir/file4
+:100755 100755 e74b7d465e52746be2b4bae983670711e6e66657 e74b7d465e52746be2b4bae983670711e6e66657 C100	newdir/exec.sh	subdir/newdir/exec.sh
+:100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 C100	newdir/interesting	subdir/newdir/interesting
+EOF
+test_expect_success \
+	'N: copy with empty source path' \
+	'git fast-import <input &&
+	 git diff-tree -C -C -r --no-commit-id N-root-to-subdir >actual &&
+	 compare_diff_raw expect actual'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/N-unquoted-root-to-subdir
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+increase nesting
+COMMIT
+
+from refs/heads/branch^0
+C  subdir
+
+INPUT_END
+test_expect_success \
+	'N: copy with unquoted empty source path' \
+	'git fast-import <input &&
+	 git diff --exit-code N-root-to-subdir N-unquoted-root-to-subdir'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/N-trailing-slash-in-src
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+copy newdir to newerdir
+COMMIT
+
+from refs/heads/branch^0
+C newdir/ newerdir
+
+INPUT_END
+test_expect_success \
+	'N: reject foo/ as source path' \
+	'# fatal: path not in branch
+	 test_must_fail git fast-import <input'
+
 cat >input <<INPUT_END
 commit refs/heads/N2
 committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
@@ -1293,7 +1353,7 @@ test_expect_success \
 	 compare_diff_raw expect actual'
 
 test_expect_success \
-	'N: reject foo/ syntax' \
+	'N: filemodify: reject foo/ syntax' \
 	'subdir=$(git rev-parse refs/heads/branch^0:file2) &&
 	 test_must_fail git fast-import <<-INPUT_END
 	commit refs/heads/N5B
-- 
1.7.9.2

Re: [PATCH] fast-import: fix ls command with empty path

From: Sverre Rabbelier <hidden>
Date: 2016-06-15 22:53:15

On Thu, Mar 8, 2012 at 01:09, Jonathan Nieder [off-list ref] wrote:
quoted
[Subject: fast-import: fix ls command with empty path]
I would s/fix/accept/ to be more precise about the nature of the
breakage.  (In other words: rather than mishandling ls with an empty
path, fast-import was not handling it at all.)
Makes sense to me :)

-- 
Cheers,

Sverre Rabbelier

[PATCH v2 0/2] Re: fast-import: fix ls command with empty path

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:15

David Barr wrote:
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 and force the mode to S_IFDIR.
Thanks again for your help.  I've pushed the following changes to

  git://repo.or.cz/git/jrn.git fast-import-pu

Testing, review, and improvements welcome.

David Barr (1):
  fast-import: teach ls command to accept empty path

Jonathan Nieder (1):
  fast-import: plug leak of dirty trees in 'ls' command

 fast-import.c          |   19 +++++++++-
 t/t9300-fast-import.sh |   95 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 1 deletion(-)

[PATCH 1/2] fast-import: plug leak of dirty trees in 'ls' command

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:15

When the named directory has changed since it was last written to
pack, "tree_content_get" makes a deep copy of the list of tree entries
which we forgot to free.

This memory leak has been present since the "ls" command was
introduced in v1.7.5-rc0~3^2~33 (fast-import: add 'ls' command,
2010-12-02).

Signed-off-by: Jonathan Nieder <redacted>
---
After rediscovering this, I found [1] which mentions the same bug.

[1] http://thread.gmane.org/gmane.comp.version-control.git/178007/focus=178044

 fast-import.c |    2 ++
 1 file changed, 2 insertions(+)
diff --git a/fast-import.c b/fast-import.c
index c1486cab..1758da94 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3028,6 +3028,8 @@ static void parse_ls(struct branch *b)
 		store_tree(&leaf);
 
 	print_ls(leaf.versions[1].mode, leaf.versions[1].sha1, p);
+	if (leaf.tree)
+		release_tree_content_recursive(leaf.tree);
 	if (!b || root != &b->branch_tree)
 		release_tree_entry(root);
 }
-- 
1.7.9.2

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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:15

From: David Barr <redacted>

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 <redacted>
Signed-off-by: David Barr <redacted>
Signed-off-by: Dmitry Ivankov <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
 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

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

From: David Barr <hidden>
Date: 2016-06-15 22:53:15

On Fri, Mar 9, 2012 at 7:33 AM, Jonathan Nieder [off-list ref] wrote:
From: David Barr <redacted>

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 <redacted>
Signed-off-by: David Barr <redacted>
Signed-off-by: Dmitry Ivankov <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
+               /*
+                * 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);
Is it ok to call store_tree(root)? If so, could we not introduce a
pointer rather than a deep copy?
diff --git a/fast-import.c b/fast-import.c
index 94d7037..eab24f3 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2993,7 +2993,8 @@ static void parse_ls(struct branch *b)
 {
 	const char *p;
 	struct tree_entry *root = NULL;
-	struct tree_entry leaf = {NULL};
+	struct tree_entry tmp_tree = {NULL};
+	struct tree_entry *leaf = &tmp_tree;

 	/* ls SP (<treeish> SP)? <path> */
 	p = command_buf.buf + strlen("ls ");
@@ -3022,27 +3023,18 @@ static void parse_ls(struct branch *b)
 			die("Garbage after path in: %s", command_buf.buf);
 		p = uq.buf;
 	}
-	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;
-	}
+	if (*p)
+		tree_content_get(root, p, leaf);
+	else
+		leaf = root;
 	/*
 	 * A directory in preparation would have a sha1 of zero
 	 * until it is saved.  Save, for simplicity.
 	 */
-	if (S_ISDIR(leaf.versions[1].mode))
-		store_tree(&leaf);
+	if (S_ISDIR(leaf->versions[1].mode))
+		store_tree(leaf);

-	print_ls(leaf.versions[1].mode, leaf.versions[1].sha1, p);
+	print_ls(leaf->versions[1].mode, leaf->versions[1].sha1, p);
 	if (!b || root != &b->branch_tree)
 		release_tree_entry(root);
 }

--
David Barr

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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:15

David Barr wrote:
On Fri, Mar 9, 2012 at 7:33 AM, Jonathan Nieder [off-list ref] wrote:
quoted
+               /*
+                * 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);
Is it ok to call store_tree(root)?
Yes.
                                    If so, could we not introduce a
pointer rather than a deep copy?
If using 'ls' with an empty path after dirtying the root tree is
common, then that would work as an optimization.  The fussy bit is
making sure the call to

	release_tree_content_recursive(leaf.tree);

is skipped in this case and not skipped when tree_content_get() made a
copy.  That is, something like this (patch against fast-import-pu on
repo.or.cz/git/jrn.git):

-- >8 --
From: David Barr <redacted>
Subject: fast-import: optimize 'ls' command with empty path to avoid a copy

fast-import's "ls" command normally copies a tree (implicitly, by
calling tree_content_get) before passing it to store_tree.  Otherwise:

 - after versions[0] is overwritten by versions[1] in child
   directories, it would be impossible to rebuild the tree object for
   version 0 of the current tree, so parse_ls would need to

	hashcpy(leaf.versions[0].sha1, leaf.versions[1].sha1)

   so version 0 points to a tree that can be rebuilt.

 - in turn, that would make it impossible to rebuild the tree object
   for version 0 of the parent tree.  And so on.

The above considerations do not apply when the tree we are examining
with 'ls' has no parent.  Avoid a copy in that case.

Signed-off-by: Jonathan Nieder <redacted>
---
 fast-import.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 1e5d59b4..28fe4c35 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3002,7 +3002,8 @@ static void parse_ls(struct branch *b)
 {
 	const char *p;
 	struct tree_entry *root = NULL;
-	struct tree_entry leaf = {NULL};
+	struct tree_entry leaf_storage = {NULL};
+	struct tree_entry *leaf = &leaf_storage;
 
 	/* ls SP (<treeish> SP)? <path> */
 	p = command_buf.buf + strlen("ls ");
@@ -3029,17 +3030,24 @@ 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
+		leaf = root;
+
 	/*
 	 * A directory in preparation would have a sha1 of zero
 	 * until it is saved.  Save, for simplicity.
 	 */
-	if (S_ISDIR(leaf.versions[1].mode))
-		store_tree(&leaf);
+	if (S_ISDIR(leaf->versions[1].mode)
+	    && is_null_sha1(leaf->versions[1].sha1)) {
+		store_tree(leaf);
+		hashcpy(leaf->versions[0].sha1, leaf->versions[1].sha1);
+	}
 
-	print_ls(leaf.versions[1].mode, leaf.versions[1].sha1, p);
-	if (leaf.tree)
-		release_tree_content_recursive(leaf.tree);
+	print_ls(leaf->versions[1].mode, leaf->versions[1].sha1, p);
+	if (*p && leaf->tree)
+		release_tree_content_recursive(leaf->tree);
 	if (!b || root != &b->branch_tree)
 		release_tree_entry(root);
 }
-- 
1.7.9.2

[PATCH v3] fast-import: allow 'ls' and filecopy to read the root

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:16

In the same spirit as v1.7.4-rc0~177 (fast-import: Allow filemodify to
set the root, 2010-10-10), teach the 'ls' and 'C' commands the
following syntax:

	ls ""
	ls <dataref> ""
	C "" <path>

All three are requests to read from the directory at the top of the
hierarchy.

The potential usefulness of this extension was discovered by using
svn-fe to import from a repository whose history included a
pathological Subversion operation:

  svn cp $SVN_ROOT $SVN_ROOT/subdirectory

Since v1.7.10-rc0~118^2~4^2~5^2~4 (vcs-svn: eliminate repo_tree
structure, 2010-12-10) svn-fe handles this by sending the command
'ls :1 ' to fast-import, expecting output in the form

  '040000' SP 'tree' SP <dataref> HT LF

describing the toplevel directory so it can be copied.  After this
patch, the import works, with no modification to svn-fe needed.

Subtleties:

The 'ls <dataref> ""' command involves printing the makeshift "root"
tree that represents <dataref>, so we need to initialize its mode.

The 'C "" <path>' command needs to be careful not to copy an empty
tree to a subdirectory, as explained in v1.7.4~2^2~2^2 (fast-import:
treat filemodify with empty tree as delete, 2011-01-27).

Based on a patch by David Barr that made the same change at the
parse_ls level.  David's tests were carried over and some new ones
added.

Reported-by: Andrew Sayers <redacted>
Signed-off-by: David Barr <redacted>
Signed-off-by: Jonathan Nieder <redacted>
Improved-by: Dmitry Ivankov [off-list ref]
---
Ok, here's a patch for the svn-fe bug that I could live with.

It has a semantic conflict with the fast-import-ls-fixes series that
I sent separately, which is fixed by adding

			if (!slash1[1])
				die("Empty path component found in input");

after

			if (!slash1)
				goto last_component;

and removing the now-useless

	if (!n)
		die("Empty path component found in input");

I'll send a fixup patch as a reply, for squashing into the merge or
this patch, whichever is the first commit that contains both topics.

 fast-import.c          |   43 ++++++++----
 t/t9010-svn-fe.sh      |   69 +++++++++++++++++++
 t/t9300-fast-import.sh |  174 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 272 insertions(+), 14 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index c1486cab..75da2954 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1473,6 +1473,9 @@ static void tree_content_replace(
 	root->tree = newtree;
 }
 
+static int tree_content_remove(struct tree_entry *,
+					const char *, struct tree_entry *);
+
 static int tree_content_set(
 	struct tree_entry *root,
 	const char *p,
@@ -1495,6 +1498,15 @@ static int tree_content_set(
 	if (!slash1 && !S_ISDIR(mode) && subtree)
 		die("Non-directories cannot have subtrees");
 
+	/* Git does not track empty directories. */
+	if (S_ISDIR(mode)) {
+		if ((is_null_sha1(sha1) && !subtree->entry_count)
+		    || !memcmp(sha1, EMPTY_TREE_SHA1_BIN, 20)) {
+			tree_content_remove(root, p, NULL);
+			return 1;
+		}
+	}
+
 	if (!root->tree)
 		load_tree(root);
 	t = root->tree;
@@ -1636,6 +1648,11 @@ static int tree_content_get(
 	unsigned int i, n;
 	struct tree_entry *e;
 
+	if (!*p) {
+		e = root;
+		goto last_component;
+	}
+
 	slash1 = strchr(p, '/');
 	if (slash1)
 		n = slash1 - p;
@@ -1648,14 +1665,8 @@ static int tree_content_get(
 	for (i = 0; i < t->entry_count; i++) {
 		e = t->entries[i];
 		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
-			if (!slash1) {
-				memcpy(leaf, e, sizeof(*leaf));
-				if (e->tree && is_null_sha1(e->versions[1].sha1))
-					leaf->tree = dup_tree_content(e->tree);
-				else
-					leaf->tree = NULL;
-				return 1;
-			}
+			if (!slash1)
+				goto last_component;
 			if (!S_ISDIR(e->versions[1].mode))
 				return 0;
 			if (!e->tree)
@@ -1664,6 +1675,14 @@ static int tree_content_get(
 		}
 	}
 	return 0;
+
+last_component:
+	memcpy(leaf, e, sizeof(*leaf));
+	if (e->tree && is_null_sha1(e->versions[1].sha1))
+		leaf->tree = dup_tree_content(e->tree);
+	else
+		leaf->tree = NULL;
+	return 1;
 }
 
 static int update_branch(struct branch *b)
@@ -2256,12 +2275,6 @@ static void file_change_m(struct branch *b)
 		p = uq.buf;
 	}
 
-	/* Git does not track empty, non-toplevel directories. */
-	if (S_ISDIR(mode) && !memcmp(sha1, EMPTY_TREE_SHA1_BIN, 20) && *p) {
-		tree_content_remove(&b->branch_tree, p, NULL);
-		return;
-	}
-
 	if (S_ISGITLINK(mode)) {
 		if (inline_data)
 			die("Git links cannot be specified 'inline': %s",
@@ -2369,6 +2382,7 @@ static void file_change_cr(struct branch *b, int rename)
 			leaf.tree);
 		return;
 	}
+
 	tree_content_set(&b->branch_tree, d,
 		leaf.versions[1].sha1,
 		leaf.versions[1].mode,
@@ -3005,6 +3019,7 @@ static void parse_ls(struct branch *b)
 		struct object_entry *e = parse_treeish_dataref(&p);
 		root = new_tree_entry();
 		hashcpy(root->versions[1].sha1, e->idx.sha1);
+		root->versions[1].mode = S_IFDIR;
 		load_tree(root);
 		if (*p++ != ' ')
 			die("Missing space after tree-ish: %s", command_buf.buf);
diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index b7eed248..45706bde 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -271,6 +271,75 @@ test_expect_success PIPE 'directory with files' '
 	test_cmp hi directory/file2
 '
 
+test_expect_success PIPE 'copy from root to directory' '
+	reinit_git &&
+	echo hello >hello &&
+	hello_blob=$(git hash-object -w -t blob hello) &&
+	subtree=$(
+		echo "100644 blob $hello_blob	README.txt" |
+		git mktree
+	) &&
+	expect=$(
+		git mktree <<-EOF
+			100644 blob $hello_blob	README.txt
+			040000 tree $subtree	trunk
+		EOF
+	) &&
+
+	{
+		properties \
+			svn:author author@example.com \
+			svn:date "2012-10-10T00:01:003.000000Z" \
+			svn:log "created README.txt" &&
+		echo PROPS-END
+	} >r1.props &&
+	{
+		properties \
+			svn:author author@example.com \
+			svn:date "2012-10-10T00:02:005.000000Z" \
+			svn:log "created trunk" &&
+		echo PROPS-END
+	} >r2.props &&
+	{
+		cat <<-EOF &&
+		SVN-fs-dump-format-version: 3
+
+		Revision-number: 1
+		EOF
+		echo Prop-content-length: $(wc -c <r1.props) &&
+		echo Content-length: $(wc -c <r1.props) &&
+		echo &&
+		cat r1.props &&
+		cat <<-\EOF &&
+
+		Node-path: README.txt
+		Node-kind: file
+		Node-action: add
+		EOF
+		text_no_props hello &&
+		echo Revision-number: 2
+		echo Prop-content-length: $(wc -c <r2.props) &&
+		echo Content-length: $(wc -c <r2.props) &&
+		echo &&
+		cat r2.props &&
+		sed -e "s/X\$//" <<-\EOF
+
+		Node-path: trunk
+		Node-kind: dir
+		Node-action: add
+		Node-copyfrom-rev: 1
+		Node-copyfrom-path: X
+		Prop-content-length: 10
+		Content-length: 10
+
+		PROPS-END
+		EOF
+	} >copy-root.dump &&
+	try_dump copy-root.dump &&
+
+	git diff-tree --exit-code $expect HEAD
+'
+
 test_expect_success PIPE 'branch name with backslash' '
 	reinit_git &&
 	sort <<-\EOF >expect.branch-files &&
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 438aaf6b..e5460994 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1047,6 +1047,49 @@ test_expect_success \
 	 git diff-tree -C --find-copies-harder -r N1^ N1 >actual &&
 	 compare_diff_raw expect actual'
 
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/N-root-to-subdir
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+copy to subdir
+COMMIT
+
+from refs/heads/branch^0
+C "" subdir
+
+INPUT_END
+
+cat >expect <<\EOF
+:100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc f1fb5da718392694d0076d677d6d0e364c79b0bc C100	file2/newf	subdir/file2/newf
+:100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1 7123f7f44e39be127c5eb701e5968176ee9d78b1 C100	file2/oldf	subdir/file2/oldf
+:100755 100755 85df50785d62d3b05ab03d9cbf7e4a0b49449730 85df50785d62d3b05ab03d9cbf7e4a0b49449730 C100	file4	subdir/file4
+:100755 100755 e74b7d465e52746be2b4bae983670711e6e66657 e74b7d465e52746be2b4bae983670711e6e66657 C100	newdir/exec.sh	subdir/newdir/exec.sh
+:100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 C100	newdir/interesting	subdir/newdir/interesting
+EOF
+test_expect_success \
+	'N: copy with empty source path' \
+	'git fast-import <input &&
+	 git diff-tree -C -C -r --no-commit-id N-root-to-subdir >actual &&
+	 compare_diff_raw expect actual'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/N-unquoted-root-to-subdir
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+increase nesting
+COMMIT
+
+from refs/heads/branch^0
+C  subdir
+
+INPUT_END
+test_expect_success \
+	'N: copy with unquoted empty source path' \
+	'git fast-import <input &&
+	 git diff --exit-code N-root-to-subdir N-unquoted-root-to-subdir'
+
 cat >input <<INPUT_END
 commit refs/heads/N2
 committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
@@ -1400,6 +1443,137 @@ 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: copying unborn branch root has no effect' '
+	empty_tree=$(git mktree </dev/null) &&
+	echo tree $empty_tree >expect &&
+	git fast-import <<-EOF &&
+	commit refs/heads/N-copy-unborn
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	copy empty root directory
+	COMMIT
+
+	C "" subdir
+	EOF
+	git cat-file commit N-copy-unborn >cmit &&
+	head -n1 cmit >actual &&
+	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: copying empty root has no effect' '
+	empty_tree=$(git mktree </dev/null) &&
+	echo tree $empty_tree >expect &&
+	echo empty >msg &&
+	cmit=$(git commit-tree "$empty_tree" -p refs/heads/branch^0 <msg) &&
+	git fast-import <<-EOF &&
+	commit refs/heads/N-copy-empty
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	copy empty root directory
+	COMMIT
+
+	C "" subdir
+	EOF
+	git cat-file commit N-copy-empty >cmit &&
+	head -n1 cmit >actual &&
+	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

Re: [PATCH v3] fast-import: allow 'ls' and filecopy to read the root

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:16

Jonathan Nieder wrote:
It has a semantic conflict with the fast-import-ls-fixes series that
I sent separately
[...]
I'll send a fixup patch as a reply, for squashing into the merge or
this patch, whichever is the first commit that contains both topics.
With this tweak, the merge passes the merged set of tests.
diff --git i/fast-import.c c/fast-import.c
index 51cdda29..fe1c8643 100644
--- i/fast-import.c
+++ c/fast-import.c
@@ -1658,8 +1658,6 @@ static int tree_content_get(
 		n = slash1 - p;
 	else
 		n = strlen(p);
-	if (!n)
-		die("Empty path component found in input");
 
 	if (!root->tree)
 		load_tree(root);
@@ -1669,6 +1667,8 @@ static int tree_content_get(
 		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
 			if (!slash1)
 				goto last_component;
+			if (!slash1[1])
+				die("Empty path component found in input");
 			if (!S_ISDIR(e->versions[1].mode))
 				return 0;
 			if (!e->tree)

[PATCH 2/1] fixup! fast-import: allow 'ls' and filecopy to read the root

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:16

Jonathan Nieder wrote:
quoted hunk
--- a/fast-import.c
+++ b/fast-import.c
@@ -2369,6 +2382,7 @@ static void file_change_cr(struct branch *b, int rename)
 			leaf.tree);
 		return;
 	}
+
 	tree_content_set(&b->branch_tree, d,
 		leaf.versions[1].sha1,
 		leaf.versions[1].mode,
Maybe next time I will send the patch to myself and bounce it to the
list.  Sorry for the noise.
diff --git i/fast-import.c w/fast-import.c
index 51cdda29..013cbd5e 100644
--- i/fast-import.c
+++ w/fast-import.c
@@ -2384,7 +2384,6 @@ static void file_change_cr(struct branch *b, int rename)
 			leaf.tree);
 		return;
 	}
-
 	tree_content_set(&b->branch_tree, d,
 		leaf.versions[1].sha1,
 		leaf.versions[1].mode,
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help