commit f7b97ccb29b3e414e360ff635f9bc114f8db7c2f
from: Stefan Sperling <stsp@stsp.name>
date: Tue Apr 14 11:37:39 2020 UTC

normalize tree entry modes to 0100644 or 0100755 when writing tree objects

semarie@ reported an error from go-git used by Cirrus CI:
"57243613255d758e15b0f5ae1c960b970f0435f8: malformed mode (0100640)!"

'git fsck' has no problem with mode 0100640. But writing just the two
most commonly used file modes should provide the best compatibility.

commit - d2ad595c2cf140e23c761ef62916dbc49b40f66b
commit + f7b97ccb29b3e414e360ff635f9bc114f8db7c2f
blob - 8438cb9700eb8696249c22aec54bfa6249f98bc5
blob + d17ff2d8455425992dbcd98fe8e0d53acfd2c208
--- include/got_error.h
+++ include/got_error.h
@@ -19,7 +19,7 @@
 #define GOT_ERR_OK		0
 #define GOT_ERR_ERRNO		1
 #define GOT_ERR_NOT_GIT_REPO	2
-/* 3 is currently unused */
+#define GOT_ERR_BAD_FILETYPE	3
 #define GOT_ERR_BAD_PATH	4
 #define GOT_ERR_NOT_REF		5
 #define GOT_ERR_IO		6
@@ -148,7 +148,7 @@ static const struct got_error {
 	{ GOT_ERR_OK,		"no error occured?!?" },
 	{ GOT_ERR_ERRNO,	"see errno" },
 	{ GOT_ERR_NOT_GIT_REPO, "no git repository found" },
-	{ 3,			"unused error code" },
+	{ GOT_ERR_BAD_FILETYPE,	"bad file type" },
 	{ GOT_ERR_BAD_PATH,	"bad path" },
 	{ GOT_ERR_NOT_REF,	"no such reference found" },
 	{ GOT_ERR_IO,		"input/output error" },
blob - b9728b4ffb0613650c0438ce082ae6194bd635cc
blob + de279d7ac5f87f20be6173c3f7932fb898e2254d
--- lib/object_create.c
+++ lib/object_create.c
@@ -202,9 +202,25 @@ done:
 }
 
 static const struct got_error *
-mode2str(char *buf, size_t len, mode_t mode)
+te_mode2str(char *buf, size_t len, mode_t te_mode)
 {
 	int ret;
+	mode_t 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.
+	 */
+	if (S_ISREG(te_mode))
+		mode = GOT_DEFAULT_FILE_MODE;
+	else if (S_ISDIR(te_mode))
+		mode = GOT_DEFAULT_DIR_MODE;
+	else
+		return got_error(GOT_ERR_BAD_FILETYPE);
+	if (te_mode & (S_IXUSR | S_IXGRP | S_IXOTH))
+		mode |= S_IXUSR | S_IXGRP | S_IXOTH;
+
 	ret = snprintf(buf, len, "%o ", mode);
 	if (ret == -1 || ret >= len)
 		return got_error(GOT_ERR_NO_SPACE);
@@ -265,7 +281,7 @@ got_object_tree_create(struct got_object_id **id,
 
 	for (i = 0; i < nentries; i++) {
 		te = sorted_entries[i];
-		err = mode2str(modebuf, sizeof(modebuf), te->mode);
+		err = te_mode2str(modebuf, sizeof(modebuf), te->mode);
 		if (err)
 			goto done;
 		len += strlen(modebuf) + strlen(te->name) + 1 +
@@ -293,7 +309,7 @@ got_object_tree_create(struct got_object_id **id,
 
 	for (i = 0; i < nentries; i++) {
 		te = sorted_entries[i];
-		err = mode2str(modebuf, sizeof(modebuf), te->mode);
+		err = te_mode2str(modebuf, sizeof(modebuf), te->mode);
 		if (err)
 			goto done;
 		len = strlen(modebuf);
blob - 270ee1fe177e6657aa51f55d0f13dae24a800402
blob + 0c80d8ca1cb82c44fdb5ac37032d91b93b191c92
--- regress/cmdline/commit.sh
+++ regress/cmdline/commit.sh
@@ -775,12 +775,87 @@ function test_commit_xbit_change {
 
 	echo 'm  alpha' > $testroot/stdout.expected
 	(cd $testroot/wt && got status > $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"
+}
+
+function commit_check_mode {
+	local mode="$1"
+	local expected_mode="$2"
+
+	chmod 644 $testroot/wt/alpha
+	echo a >> $testroot/wt/alpha
+	chmod $mode $testroot/wt/alpha
+
+	(cd $testroot/wt && got commit -mm > $testroot/stdout)
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		echo "got commit failed unexpectedly"
+		test_done "$testroot" "$ret"
+		return 1
+	fi
 
+	local commit_id=`git_show_head $testroot/repo`
+	echo 'M  alpha' > $testroot/stdout.expected
+	echo "Created commit $commit_id" >> $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"
+	fi
+
+	local tree_id=$(got cat -r $testroot/repo $commit_id | \
+		grep ^tree | cut -d' ' -f2)
+	local alpha_id=$(got cat -r $testroot/repo $tree_id | \
+		grep 'alpha$' | cut -d' ' -f1)
+	echo "$alpha_id $expected_mode alpha" > $testroot/stdout.expected
+	got cat -r $testroot/repo $tree_id | grep 'alpha$' > $testroot/stdout
 	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret="$?"
 	if [ "$ret" != "0" ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
 	fi
+	return $ret
+}
+
+function test_commit_normalizes_filemodes {
+	local testroot=`test_init commit_normalizes_filemodes`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	modes="600 400 460 640 440 660 444 666"
+	for m in $modes; do
+		commit_check_mode "$m" "0100644"
+		if [ "$ret" != "0" ]; then
+			break
+		fi
+	done
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+	modes="700 500 570 750 550 770 555 777"
+	for m in $modes; do
+		commit_check_mode "$m" "0100755"
+		if [ "$ret" != "0" ]; then
+			break
+		fi
+	done
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
 	test_done "$testroot" "$ret"
 }
 
@@ -802,3 +877,4 @@ run_test test_commit_no_email
 run_test test_commit_tree_entry_sorting
 run_test test_commit_gitconfig_author
 run_test test_commit_xbit_change
+run_test test_commit_normalizes_filemodes