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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: "no commits to rebase" when rebasing on the initial commit
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Tue, 31 Jan 2023 13:27:21 +0100

Download raw body.

Thread
On Tue, Jan 31, 2023 at 12:01:00PM +0100, Omar Polo wrote:
> apparently 'got rebase' fails when rebasing directly on top of the
> initial repository.  Test below simulates it, uncomment the
> un-indented lines to make it pass.

Thanks, this is indeed a bug.

Fix below, with the test you provided and several adjustments.
The existing test case which collided with your test new case has
been tweaked. It now triggers an appropriate error which had no
test coverage yet, and was not actually reachable :-/

With this fix, got rebase will always fast-forward the specified
branch when history has not diverged, even if the specified branch's
tip has no parent commits.
 
 make 'got rebase' work when the to-be-rebased branch has no parent commit
 
 found by op@, who also provided the test case
 
diff 0d98195bc8c97f44f5635b97b8020dd934f677f0 fd5a170f62829f82b63fdcf4c7ad4c5363483b1a
commit - 0d98195bc8c97f44f5635b97b8020dd934f677f0
commit + fd5a170f62829f82b63fdcf4c7ad4c5363483b1a
blob - 5d94c4ade9d0584ccb4b04bf99cb1c7f8a4d8745
blob + 06e066fceda7062b02b365d0bb490cd359852ca7
--- got/got.c
+++ got/got.c
@@ -11165,13 +11165,13 @@ cmd_rebase(int argc, char *argv[])
 		error = got_commit_graph_find_youngest_common_ancestor(&yca_id,
 		    base_commit_id, branch_head_commit_id, 1, repo,
 		    check_cancelled, NULL);
-		if (error)
+		if (error) {
+			if (error->code == GOT_ERR_ANCESTRY) {
+				error = got_error_msg(GOT_ERR_ANCESTRY,
+				    "specified branch shares no common "
+				    "ancestry with work tree's branch");
+			}
 			goto done;
-		if (yca_id == NULL) {
-			error = got_error_msg(GOT_ERR_ANCESTRY,
-			    "specified branch shares no common ancestry "
-			    "with work tree's branch");
-			goto done;
 		}
 
 		error = check_same_branch(base_commit_id, branch, yca_id, repo);
@@ -11226,17 +11226,16 @@ cmd_rebase(int argc, char *argv[])
 
 	parent_ids = got_object_commit_get_parent_ids(commit);
 	pid = STAILQ_FIRST(parent_ids);
-	if (pid == NULL) {
-		error = got_error(GOT_ERR_EMPTY_REBASE);
-		goto done;
+	if (pid) {
+		error = collect_commits(&commits, commit_id, &pid->id,
+		    yca_id, got_worktree_get_path_prefix(worktree),
+		    GOT_ERR_REBASE_PATH, repo);
+		if (error)
+			goto done;
 	}
-	error = collect_commits(&commits, commit_id, &pid->id,
-	    yca_id, got_worktree_get_path_prefix(worktree),
-	    GOT_ERR_REBASE_PATH, repo);
+
 	got_object_commit_close(commit);
 	commit = NULL;
-	if (error)
-		goto done;
 
 	if (!continue_rebase) {
 		error = got_worktree_rebase_prepare(&new_base_branch,
blob - c951c76bc67d396a591c05f06bd362c98aa7f678
blob + 3fd06efcdadb789fef14e683086013ed87d5bfd9
--- include/got_error.h
+++ include/got_error.h
@@ -100,7 +100,7 @@
 #define GOT_ERR_BRANCH_EXISTS	83
 #define GOT_ERR_MODIFIED	84
 #define GOT_ERR_NOT_REBASING	85
-#define GOT_ERR_EMPTY_REBASE	86
+/* 86 is currently unused */
 #define GOT_ERR_REBASE_COMMITID	87
 #define GOT_ERR_REBASING	88
 #define GOT_ERR_REBASE_PATH	89
blob - 7e698531e6e75bd34bc4b2f0c280aded67f43de0
blob + 3c84ba7eeb9c88175a7652495f934263b6fd2328
--- lib/error.c
+++ lib/error.c
@@ -129,7 +129,6 @@ static const struct got_error got_errors[] = {
 	{ GOT_ERR_MODIFIED,	"work tree contains local changes; these "
 	    "changes must be committed or reverted first" },
 	{ GOT_ERR_NOT_REBASING,	"rebase operation not in progress" },
-	{ GOT_ERR_EMPTY_REBASE,	"no commits to rebase" },
 	{ GOT_ERR_REBASE_COMMITID,"rebase commit ID mismatch" },
 	{ GOT_ERR_REBASING,	"a rebase operation is in progress in this "
 	    "work tree and must be continued or aborted first" },
blob - f87f1d9e51fc63c6ebf7cc957f15ec52ddd15420
blob + fceaf181cd74c8b1810cfe97234af037bb77561c
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -912,7 +912,11 @@ test_rebase_no_commits_to_rebase() {
 		return 1
 	fi
 
-	(cd $testroot/wt && got branch -n newbranch)
+	# Create an unrelated branch with 'got import'.
+	mkdir -p $testroot/newtree
+	echo "new file" > $testroot/newtree/newfile
+	got import -m new -b newbranch -r $testroot/repo \
+		$testroot/newtree > /dev/null
 
 	echo "modified alpha on master" > $testroot/wt/alpha
 	(cd $testroot/wt && got commit -m 'test rebase_no_commits_to_rebase' \
@@ -922,7 +926,9 @@ test_rebase_no_commits_to_rebase() {
 	(cd $testroot/wt && got rebase newbranch > $testroot/stdout \
 		2> $testroot/stderr)
 
-	echo "got: no commits to rebase" > $testroot/stderr.expected
+	echo -n "got: specified branch shares no common ancestry " \
+		> $testroot/stderr.expected
+	echo "with work tree's branch" >> $testroot/stderr.expected
 	cmp -s $testroot/stderr.expected $testroot/stderr
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -1885,6 +1891,47 @@ test_parseargs "$@"
 	test_done "$testroot" "$ret"
 }
 
+test_rebase_one_commit() {
+	local testroot=`test_init rebase_one_commit`
+
+	if ! got checkout $testroot/repo $testroot/wt >/dev/null; then
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	(cd $testroot/wt && got branch newbranch) >/dev/null
+
+	echo "modified alpha on newbranch" >$testroot/wt/alpha
+	(cd $testroot/wt && got commit -m 'edit alpha') >/dev/null
+	(cd $testroot/wt && got update) >/dev/null
+	local commit=`git_show_branch_head $testroot/repo newbranch`
+
+	echo -n '' > $testroot/stderr.expected
+
+	(cd $testroot/wt && got rebase master >$testroot/stdout \
+		2> $testroot/stderr)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "rebase comand failed unexpectedly" >&2
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	echo "Forwarding refs/heads/master to commit $commit" \
+		>$testroot/stdout.expected
+	echo "Switching work tree to refs/heads/master" \
+		>> $testroot/stdout.expected
+
+	if ! cmp -s $testroot/stdout.expected $testroot/stdout; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	test_done "$testroot" 0
+}
+
 test_parseargs "$@"
 run_test test_rebase_basic
 run_test test_rebase_ancestry_check
@@ -1906,3 +1953,4 @@ run_test test_rebase_out_of_date2
 run_test test_rebase_nonbranch
 run_test test_rebase_umask
 run_test test_rebase_out_of_date2
+run_test test_rebase_one_commit