From: Stefan Sperling <stsp@stsp.name>
Subject: Re: submodules: got: bad file type
To: ori@eigenstate.org
Cc: semarie@online.fr, gameoftrees@openbsd.org
Date: Fri, 15 May 2020 12:21:20 +0200

On Tue, May 12, 2020 at 08:04:44AM -0700, ori@eigenstate.org wrote:
> What I'm suggesting is that the submodules are *not* created,
> modified, or deleted. They're merely preserved. The only issue
> that this approach causes, I think, is pressure to one day
> support submodules fully.

Yes. It should be clear that I will keep resisting such pressure ;)

Since submodules never actually appear in a Got work tree it should
be obvious that users cannot expect built-in support for submodules.
Of course, anyone is free to do whatever they want with external wrapper
tooling, which I believe is where such features belong.
 
> I think this covers the majority of uses we'd need in got, where
> we ended up stuck with a repository that uses submodules, and
> now we need to make some change in it.

You both have convinced me that just saying "no" to this won't help.
And with the test coverage we have now I am much less afraid that
allowing submodules to be present would cause unexpected problems.

So this patch allows creation of commits which carry submodules along.

ok?


diff f4d0e3fbae5165979285f3c67e9cdfceea1c7ad5 /home/stsp/src/got
blob - 515477b2d9c25ebe5208ca3566e02cd34c00219d
file + lib/object_create.c
--- lib/object_create.c
+++ lib/object_create.c
@@ -202,7 +202,7 @@ done:
 }
 
 static const struct got_error *
-te_mode2str(char *buf, size_t len, mode_t te_mode)
+te_mode2str(char *buf, size_t len, struct got_tree_entry *te)
 {
 	int ret;
 	mode_t mode;
@@ -210,13 +210,15 @@ te_mode2str(char *buf, size_t len, mode_t te_mode)
 	/*
 	 * Some Git implementations are picky about modes seen in tree entries.
 	 * For best compatibility we normalize the file/directory mode here.
-	 * Note that we do not support committing symlinks or submodules.
+	 * Note that we do not support committing symlinks.
 	 */
-	if (S_ISREG(te_mode)) {
+	if (S_ISREG(te->mode)) {
 		mode = GOT_DEFAULT_FILE_MODE;
-		if (te_mode & (S_IXUSR | S_IXGRP | S_IXOTH))
+		if (te->mode & (S_IXUSR | S_IXGRP | S_IXOTH))
 			mode |= S_IXUSR | S_IXGRP | S_IXOTH;
-	} else if (S_ISDIR(te_mode))
+	} else if (got_object_tree_entry_is_submodule(te))
+		mode = S_IFDIR | S_IFLNK;
+	else if (S_ISDIR(te->mode))
 		mode = S_IFDIR; /* Git leaves all the other bits unset. */
 	else
 		return got_error(GOT_ERR_BAD_FILETYPE);
@@ -281,7 +283,7 @@ got_object_tree_create(struct got_object_id **id,
 
 	for (i = 0; i < nentries; i++) {
 		te = sorted_entries[i];
-		err = te_mode2str(modebuf, sizeof(modebuf), te->mode);
+		err = te_mode2str(modebuf, sizeof(modebuf), te);
 		if (err)
 			goto done;
 		len += strlen(modebuf) + strlen(te->name) + 1 +
@@ -309,7 +311,7 @@ got_object_tree_create(struct got_object_id **id,
 
 	for (i = 0; i < nentries; i++) {
 		te = sorted_entries[i];
-		err = te_mode2str(modebuf, sizeof(modebuf), te->mode);
+		err = te_mode2str(modebuf, sizeof(modebuf), te);
 		if (err)
 			goto done;
 		len = strlen(modebuf);
blob - 3c8e833ec7ad8718957fd61f1be12dd5eceabd1f
file + regress/cmdline/commit.sh
--- regress/cmdline/commit.sh
+++ regress/cmdline/commit.sh
@@ -873,28 +873,31 @@ function test_commit_with_unrelated_submodule {
 	got checkout $testroot/repo $testroot/wt > /dev/null
 	ret="$?"
 	if [ "$ret" != "0" ]; then
+		echo "checkout failed unexpectedly" >&2
 		test_done "$testroot" "$ret"
 		return 1
 	fi
 
 	echo "modified alpha" > $testroot/wt/alpha
 
-	# Currently fails with "bad file type" error
-	(cd $testroot/wt && got commit -m 'modify alpha' \
-		> $testroot/stdout 2> $testroot/stderr)
+	echo "" > $testroot/stdout.expected
+
+	cd $testroot/wt && got commit -m 'modify alpha' > $testroot/stdout
 	ret="$?"
-	if [ "$ret" == "0" ]; then
-		echo "commit succeeded unexpectedly" >&2
-		test_done "$testroot" "1"
+	if [ "$ret" != "0" ]; then
+		echo "commit failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
 		return 1
 	fi
-	echo "got: bad file type" > $testroot/stderr.expected
 
-	cmp -s $testroot/stderr.expected $testroot/stderr
+	local head_rev=`git_show_head $testroot/repo`
+	echo "M  alpha" > $testroot/stdout.expected
+	echo "Created commit $head_rev" >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret="$?"
 	if [ "$ret" != "0" ]; then
-		diff -u $testroot/stderr.expected $testroot/stderr
-		return 1
+		diff -u $testroot/stdout.expected $testroot/stdout
 	fi
 	test_done "$testroot" "$ret"
 }