commit 1b093d84c1f8f17f66aec3a337a121edcc6f77d9
from: Stefan Sperling <stsp@stsp.name>
date: Wed Apr 12 19:12:37 2023 UTC

fix rebase/histedit -a leaving some files on the temporary branch

The commands 'got rebase -a' and 'got histedit -a' were checking out
files from the wrong commit. Make them check files out from the commit
we are switching the work tree to, as intended.

Avoids spurious merge conflicts when the work tree is later used for
another rebase operation. It also makes 'got update' right after
'rebase -a' a no-op, as it should be.

Problem found by naddy@ while rebasing jca's llvm15 branch

ok op, jamsek earlier version

commit - fd785a9ac70d88714b1d0533768076bcf5c9c2d2
commit + 1b093d84c1f8f17f66aec3a337a121edcc6f77d9
blob - cf84e01c6343a70a2374f95e81c6952c1ce51ad6
blob + 7baaf97d3128b44b6690ee4e4a24bbbbd2d7c1c8
--- lib/worktree.c
+++ lib/worktree.c
@@ -7253,11 +7253,6 @@ got_worktree_rebase_abort(struct got_worktree *worktre
 	err = lock_worktree(worktree, LOCK_EX);
 	if (err)
 		return err;
-
-	err = got_object_open_as_commit(&commit, repo,
-	    worktree->base_commit_id);
-	if (err)
-		goto done;
 
 	err = got_ref_open(&resolved, repo,
 	    got_ref_get_symref_target(new_base_branch), 0);
@@ -7278,6 +7273,11 @@ got_worktree_rebase_abort(struct got_worktree *worktre
 		goto done;
 
 	err = got_worktree_set_base_commit_id(worktree, repo, commit_id);
+	if (err)
+		goto done;
+
+	err = got_object_open_as_commit(&commit, repo,
+	    worktree->base_commit_id);
 	if (err)
 		goto done;
 
@@ -7624,11 +7624,6 @@ got_worktree_histedit_abort(struct got_worktree *workt
 	err = lock_worktree(worktree, LOCK_EX);
 	if (err)
 		return err;
-
-	err = got_object_open_as_commit(&commit, repo,
-	    worktree->base_commit_id);
-	if (err)
-		goto done;
 
 	err = got_ref_open(&resolved, repo,
 	    got_ref_get_symref_target(branch), 0);
@@ -7643,6 +7638,11 @@ got_worktree_histedit_abort(struct got_worktree *workt
 	if (err)
 		goto done;
 
+	err = got_object_open_as_commit(&commit, repo,
+	    worktree->base_commit_id);
+	if (err)
+		goto done;
+
 	err = got_object_id_by_path(&tree_id, repo, commit,
 	    worktree->path_prefix);
 	if (err)
blob - 6a4b676aff1b3de6b3384590c038eb5b5045981b
blob + bb762cd05b066923e9538c4bdf31982707dba134
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -413,11 +413,16 @@ test_rebase_abort() {
 	local init_commit=`git_show_head $testroot/repo`
 
 	(cd $testroot/repo && git checkout -q -b newbranch)
-	echo "modified alpha on branch" > $testroot/repo/alpha
-	git_commit $testroot/repo -m "committing to alpha on newbranch"
+	echo "modified beta on branch" > $testroot/repo/beta
+	git_commit $testroot/repo -m "committing to beta on newbranch"
 	local orig_commit1=`git_show_head $testroot/repo`
 	local short_orig_commit1=`trim_obj_id 28 $orig_commit1`
 
+	echo "modified alpha on branch" > $testroot/repo/alpha
+	git_commit $testroot/repo -m "committing to alpha on newbranch"
+	local orig_commit2=`git_show_head $testroot/repo`
+	local short_orig_commit2=`trim_obj_id 28 $orig_commit2`
+
 	(cd $testroot/repo && git checkout -q master)
 	echo "modified alpha on master" > $testroot/repo/alpha
 	git_commit $testroot/repo -m "committing to alpha on master"
@@ -436,9 +441,17 @@ test_rebase_abort() {
 	(cd $testroot/wt && got rebase newbranch > $testroot/stdout \
 		2> $testroot/stderr)
 
-	echo "C  alpha" > $testroot/stdout.expected
+	new_commit1=$(cd $testroot/wt && got info beta | \
+		grep '^based on commit:' | cut -d' ' -f4)
+	local short_new_commit1=`trim_obj_id 28 $new_commit1`
+	
+	echo "G  beta" > $testroot/stdout.expected
+	echo -n "$short_orig_commit1 -> $short_new_commit1" \
+		>> $testroot/stdout.expected
+	echo ": committing to beta on newbranch" >> $testroot/stdout.expected
+	echo "C  alpha" >> $testroot/stdout.expected
 	echo "Files with new merge conflicts: 1" >> $testroot/stdout.expected
-	echo -n "$short_orig_commit1 -> merge conflict" \
+	echo -n "$short_orig_commit2 -> merge conflict" \
 		>> $testroot/stdout.expected
 	echo ": committing to alpha on newbranch" >> $testroot/stdout.expected
 	cmp -s $testroot/stdout.expected $testroot/stdout
@@ -461,12 +474,12 @@ test_rebase_abort() {
 
 	echo '<<<<<<<' > $testroot/content.expected
 	echo "modified alpha on master" >> $testroot/content.expected
-	echo "||||||| 3-way merge base: commit $init_commit" \
+	echo "||||||| 3-way merge base: commit $orig_commit1" \
 		>> $testroot/content.expected
 	echo "alpha" >> $testroot/content.expected
 	echo "=======" >> $testroot/content.expected
 	echo "modified alpha on branch" >> $testroot/content.expected
-	echo ">>>>>>> merged change: commit $orig_commit1" \
+	echo ">>>>>>> merged change: commit $orig_commit2" \
 		>> $testroot/content.expected
 	cat $testroot/wt/alpha > $testroot/content
 	cmp -s $testroot/content.expected $testroot/content
@@ -496,6 +509,7 @@ test_rebase_abort() {
 	echo "Switching work tree to refs/heads/master" \
 		> $testroot/stdout.expected
 	echo 'R  alpha' >> $testroot/stdout.expected
+	echo 'U  beta' >> $testroot/stdout.expected
 	echo "Rebase of refs/heads/newbranch aborted" \
 		>> $testroot/stdout.expected
 
@@ -519,13 +533,56 @@ test_rebase_abort() {
 
 	(cd $testroot/wt && got log -l3 -c newbranch \
 		| grep ^commit > $testroot/stdout)
-	echo "commit $orig_commit1 (newbranch)" > $testroot/stdout.expected
+	echo "commit $orig_commit2 (newbranch)" > $testroot/stdout.expected
+	echo "commit $orig_commit1" >> $testroot/stdout.expected
 	echo "commit $init_commit" >> $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
+
+	(cd $testroot/wt && got info .| \
+		grep '^based on commit:' | sort | uniq > $testroot/stdout)
+	echo "based on commit: $master_commit" > $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
+
+	(cd $testroot/wt && got status > $testroot/stdout)
+	echo "?  unversioned-file" > $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
+
+	cat $testroot/wt/beta > $testroot/content
+	echo 'beta' > $testroot/content.expected
+	cmp -s $testroot/content.expected $testroot/content
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# A subsequent update should be a no-op.
+	(cd $testroot/wt && got update > $testroot/stdout)
+	echo 'Already up-to-date' > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
 	test_done "$testroot" "$ret"
 }