On Wed, Oct 02, 2019 at 09:28:35AM +0100, Stuart Henderson wrote:
> There's a problem doing a checkout when a tree contains an empty dir.

Thanks for the report, Stuart.

I would pin-point problem in 'got import' rather than 'got checkout'.

Git does not allow empty directories to be added to version control.
This is because Git tracks files only. Directories appear as a mere
side-effect of the existence of a versioned file at a particular path.

We could in fact allow 'got import' to import empty directories and have
'got checkout' check them out. This would work. However, Git will ignore
empty directories, and Got users cannot add additional empty directories with
'got add' later. Making 'got import' special in this regard seems like a bad
idea so I'd prefer to prevent 'got import' from importing empty directories.

> Sorry I don't have time to figure out the regress infrastructure to add a
> proper test for this and wanted to send a mail before I forget, but here's
> a simple reproducer:

Below is a fix and a regress test based on your reproduction recipe.
After reading the test I hope you will agree that the regress
"infrastructure" is not all that complicated :)

Does this look good?

diff --git a/lib/object_parse.c b/lib/object_parse.c
index 4066dbb..a037ceb 100644
--- a/lib/object_parse.c
+++ b/lib/object_parse.c
@@ -728,15 +728,15 @@ got_object_parse_tree(struct got_tree_object **tree, uint8_t *buf, size_t len)
 
 	TAILQ_INIT(&pathlist);
 
-	if (remain == 0)
-		return got_error(GOT_ERR_BAD_OBJ_DATA);
-
 	*tree = calloc(1, sizeof(**tree));
 	if (*tree == NULL)
 		return got_error_from_errno("calloc");
 
 	SIMPLEQ_INIT(&(*tree)->entries.head);
 
+	if (remain == 0)
+		return NULL; /* tree is empty */
+
 	while (remain > 0) {
 		struct got_tree_entry *te;
 		struct got_pathlist_entry *new = NULL;
diff --git a/lib/repository.c b/lib/repository.c
index b32361c..22355c1 100644
--- a/lib/repository.c
+++ b/lib/repository.c
@@ -1492,8 +1492,12 @@ write_tree(struct got_object_id **new_tree_id, const char *path_dir,
 		if (de->d_type == DT_DIR) {
 			err = import_subdir(&new_te, de, path_dir,
 			    ignores, repo, progress_cb, progress_arg);
-			if (err)
-				goto done;
+			if (err) {
+				if (err->code != GOT_ERR_NO_TREE_ENTRY)
+					goto done;
+				err = NULL;
+				continue;
+			}
 		} else if (de->d_type == DT_REG) {
 			err = import_file(&new_te, de, path_dir, repo);
 			if (err)
@@ -1506,6 +1510,11 @@ write_tree(struct got_object_id **new_tree_id, const char *path_dir,
 			goto done;
 	}
 
+	if (TAILQ_EMPTY(&paths)) {
+		err = got_error(GOT_ERR_NO_TREE_ENTRY);
+		goto done;
+	}
+
 	TAILQ_FOREACH(pe, &paths, entry) {
 		struct got_tree_entry *te = pe->data;
 		char *path;
diff --git a/regress/cmdline/import.sh b/regress/cmdline/import.sh
index ce3cb34..42ff367 100755
--- a/regress/cmdline/import.sh
+++ b/regress/cmdline/import.sh
@@ -196,10 +196,52 @@ function test_import_ignores {
 		diff -u $testroot/stdout.expected $testroot/stdout
 	fi
 	test_done "$testroot" "$ret"
+}
+
+function test_import_empty_dir {
+	local testname=import_empty_dir
+	local testroot=`mktemp -p /tmp -d got-test-$testname-XXXXXXXX`
+
+	got init $testroot/repo
+
+	mkdir $testroot/tree
+	mkdir -p $testroot/tree/empty $testroot/tree/notempty
+	echo "alpha" > $testroot/tree/notempty/alpha
+
+	got import -m 'init' -r $testroot/repo $testroot/tree > $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
 
+	local head_commit=`git_show_head $testroot/repo`
+	echo "A  $testroot/tree/notempty/alpha" >> $testroot/stdout.expected
+	echo "Created branch refs/heads/master with commit $head_commit" \
+		>> $testroot/stdout.expected
 
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# Verify that Got did not import the empty directory
+	echo "notempty/" > $testroot/stdout.expected
+	echo "notempty/alpha" >> $testroot/stdout.expected
+
+	got tree -r $testroot/repo -R > $testroot/stdout
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
 }
 
 run_test test_import_basic
 run_test test_import_requires_new_branch
 run_test test_import_ignores
+run_test test_import_empty_dir