"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
forbid overlapping repo + work trees
To:
gameoftrees@openbsd.org
Date:
Mon, 28 Aug 2023 16:40:22 +0200

Download raw body.

Thread
Some people seem eager to want to use Got like they use Git and check out
a work tree into a repository or vice versa, and then report errors to me
in private then something doesn't work.

The initial checkout currently works, but subsequent commands can fail in
various ways due to surprising side effects of unveil configurations.
We have always assumed that these directories will be kept separate while
writing our unveil rules.

I see little point in supporting such use cases and adding test coverage
for them to ensure that they keep working. It's just extra busy work for us,
for no good reason. Instead, 'got checkout' should reject such attempts very
early on, to prevent people from getting the idea that Got could be used in
this way.

Tests are still passing with this diff.

ok?

-----------------------------------------------
 prevent overlapping repo and work tree in 'got checkout'
 
 Some people are eager to try to make Got work just like Git by overlaying
 the repository and work tree. This causes problems with unveil conflicts
 at run-time.
 Fail as early as possible during 'got checkout' when users attempt this.
 
diff 9c6f408e227e19f58acfc34e96b12d9de29581fd ff0a4bbe58b81c63940a51dfccc919139be64600
commit - 9c6f408e227e19f58acfc34e96b12d9de29581fd
commit + ff0a4bbe58b81c63940a51dfccc919139be64600
blob - 29feb8935ab716a9191034d2ebede3da7cea1f73
blob + 6b0ac0a793a06fb02e013282d02b3e9541e02971
--- got/got.c
+++ got/got.c
@@ -3077,6 +3077,16 @@ cmd_checkout(int argc, char *argv[])
 	got_path_strip_trailing_slashes(repo_path);
 	got_path_strip_trailing_slashes(worktree_path);
 
+	if (got_path_is_child(worktree_path, repo_path, strlen(repo_path)) ||
+	    got_path_is_child(repo_path, worktree_path,
+	        strlen(worktree_path))) {
+		error = got_error_fmt(GOT_ERR_BAD_PATH,
+		    "work tree and repository paths may not overlap: %s",
+		    worktree_path);
+		goto done;
+	}
+
+
 	error = got_repo_pack_fds_open(&pack_fds);
 	if (error != NULL)
 		goto done;
blob - 993c64afcbd5f4598770774738e513c4afb9387c
blob + 54fdac57ec03cd5202c851099383637b93d6d7df
--- regress/cmdline/checkout.sh
+++ regress/cmdline/checkout.sh
@@ -146,6 +146,90 @@ test_checkout_sets_xbit() {
 
 }
 
+test_checkout_into_repo() {
+	local testroot=`test_init checkout_into_repo`
+	local commit_id=`git_show_head $testroot/repo`
+
+	echo "A  $testroot/wt/alpha" > $testroot/stdout.expected
+	echo "A  $testroot/wt/beta" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/epsilon/zeta" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/gamma/delta" >> $testroot/stdout.expected
+	echo "Checked out refs/heads/master: $commit_id" \
+		>> $testroot/stdout.expected
+	echo "Now shut up and hack" >> $testroot/stdout.expected
+
+	got checkout $testroot/repo $testroot/repo/wt \
+		> $testroot/stdout 2> $testroot/stderr
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "checkout succeeded unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	echo -n > $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo -n "got: work tree and repository paths may not overlap: " \
+		> $testroot/stderr.expected
+	echo "$testroot/repo/wt: bad path" >> $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+	fi
+	test_done "$testroot" "$ret"
+}
+
+test_checkout_overlap_repo() {
+	local testroot=`test_init checkout_into_repo`
+	local commit_id=`git_show_head $testroot/repo`
+
+	echo "A  $testroot/wt/alpha" > $testroot/stdout.expected
+	echo "A  $testroot/wt/beta" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/epsilon/zeta" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/gamma/delta" >> $testroot/stdout.expected
+	echo "Checked out refs/heads/master: $commit_id" \
+		>> $testroot/stdout.expected
+	echo "Now shut up and hack" >> $testroot/stdout.expected
+
+	got checkout $testroot/repo $testroot \
+		> $testroot/stdout 2> $testroot/stderr
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "checkout succeeded unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	echo -n > $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo -n "got: work tree and repository paths may not overlap: " \
+		> $testroot/stderr.expected
+	echo "$testroot: bad path" >> $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+	fi
+	test_done "$testroot" "$ret"
+}
+
 test_checkout_sets_xbit() {
 	local testroot=`test_init checkout_sets_xbit 1`
 
@@ -1010,6 +1094,8 @@ run_test test_checkout_sets_xbit
 run_test test_checkout_basic
 run_test test_checkout_dir_exists
 run_test test_checkout_dir_not_empty
+run_test test_checkout_into_repo
+run_test test_checkout_overlap_repo
 run_test test_checkout_sets_xbit
 run_test test_checkout_commit_from_wrong_branch
 run_test test_checkout_tag