On Wed, Nov 03, 2021 at 08:16:37PM +0100, Stefan Sperling wrote:
> On Wed, Nov 03, 2021 at 01:16:45PM -0400, Josh Rickmar wrote:
> > When 'got rebase' is successful, the worktree is switched to the
> > rebased branch.  However, when there are no commits on the current
> > branch to rebase, got will error and worktree is left unmodified.
> > More than once I have not paid attention to the error message (instead
> > treating it more as information), and assumed that the branch was
> > switched, only to end up committing later changes to origin/main
> > instead of main.
> > 
> > Since a noop rebase could also be seen as successful rebase operation,
> > we could switch the branch as well, and exit with code 0 instead of 1.
> > If my memory serves me right, git rebase also behaves similarly when
> > there is no commits to rebase.
> 
> This does not yet work properly.
> 
> To test this patch I have committed the patch to a branch named
> noop-rebase. Then I added another commit with a new line in CHANGES
> on this branch.
> 
> This is the behaviour I see, starting on branch 'main':
> 
> $ got up -b noop-rebase
> Switching work tree from refs/heads/main to refs/heads/noop-rebase
> U  CHANGES
> U  got/got.c
> U  regress/cmdline/rebase.sh
> Updated to refs/heads/noop-rebase: b3c428a82c8f8666e6c7bc3034b5988b8467d852
> $ got up -b main
> Switching work tree from refs/heads/noop-rebase to refs/heads/main
> U  CHANGES
> U  got/got.c
> U  regress/cmdline/rebase.sh
> Updated to refs/heads/main: a9e66726ec8e6d4997a024f572ee53719f7b9cdb
> $ got rb noop-rebase
> refs/heads/noop-rebase is already based on refs/heads/main
> Switching work tree from refs/heads/main to refs/heads/noop-rebase
> $
> 
> And the files in my work tree are still those from the main branch.
> 
> Comparing your patch to what 'got branch' does, you are missing the
> call to got_worktree_set_base_commit_id(). In addition to changing
> the branch referencfe, The work tree's base commit needs to be set
> to the ID of the tip commit of the branch we are switching to (already
> available in branch_head_commit_id).

Thanks for testing.  Also discovered that an input path list is
necessary with a single empty string.  After these changes, it is
updating as expected.

$ got info 
work tree: /home/jrick/src/got
work tree base commit: 5c8e1ce7ae24b3ec36e093959b6b9afad428d5a2
work tree path prefix: /
work tree branch reference: refs/heads/noop-rebase
work tree UUID: 0d58b06a-c169-44cb-b2dd-69ca213419f1
repository: /home/jrick/src/got.git
$ got up -b main
Switching work tree from refs/heads/noop-rebase to refs/heads/main
U  got/got.c
U  regress/cmdline/rebase.sh
Updated to refs/heads/main: a9e66726ec8e6d4997a024f572ee53719f7b9cdb
$ got rebase noop-rebase
refs/heads/noop-rebase is already based on refs/heads/main
Switching work tree from refs/heads/main to refs/heads/noop-rebase
U  got/got.c
U  regress/cmdline/rebase.sh
Updated to refs/heads/noop-rebase: 5c8e1ce7ae24b3ec36e093959b6b9afad428d5a2

> Perhaps we should add a dedicated test case to rebase.sh as well to
> cover this new behaviour?

The modifications necessary to test_rebase_ancestry_check should cover
this, right?

diff refs/heads/main refs/heads/noop-rebase
blob - 488cd9f35722bed936af6e7d9e2c6aa77ea2a3d8
blob + f21c34a4f3f2dd8d6fc850ebc957f8683898e50b
--- got/got.c
+++ got/got.c
@@ -9147,12 +9147,41 @@ cmd_rebase(int argc, char *argv[])
 				goto done;
 			error = NULL;
 		} else {
-			static char msg[128];
-			snprintf(msg, sizeof(msg),
-			    "%s is already based on %s",
+			struct got_pathlist_head paths;
+			char *id_str;
+
+			printf("%s is already based on %s\n",
 			    got_ref_get_name(branch),
 			    got_worktree_get_head_ref_name(worktree));
-			error = got_error_msg(GOT_ERR_SAME_BRANCH, msg);
+			error = got_object_id_str(&id_str,
+			    branch_head_commit_id);
+			if (error)
+				goto done;
+			error = switch_head_ref(branch, branch_head_commit_id,
+			    worktree, repo);
+			if (error)
+				goto done;
+			error = got_worktree_set_base_commit_id(worktree, repo,
+			    branch_head_commit_id);
+			if (error)
+				goto done;
+			error = got_pathlist_append(&merged_paths, "", NULL);
+			if (error)
+				goto done;
+			TAILQ_INIT(&paths);
+			error = got_worktree_checkout_files(worktree,
+			    &merged_paths, repo, update_progress, &upa,
+			    check_cancelled, NULL);
+			got_pathlist_free(&paths);
+			if (error)
+				goto done;
+			if (upa.did_something) {
+				printf("Updated to %s: %s\n",
+				    got_worktree_get_head_ref_name(worktree),
+				    id_str);
+			} else
+				printf("Already up-to-date\n");
+			print_update_progress_stats(&upa);
 			goto done;
 		}
 		error = got_worktree_rebase_prepare(&new_base_branch,
blob - b17666eaf0ae899543adfa66236bdfd3eb49e92d
blob + c629d068ddf4ae78f0a2184ea064823c4c1926de
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -243,11 +243,18 @@ test_rebase_ancestry_check() {
 	(cd $testroot/repo && git checkout -q -b newbranch)
 	echo "modified delta on branch" > $testroot/repo/gamma/delta
 	git_commit $testroot/repo -m "committing to delta on newbranch"
+	local newbranch_id=`git_show_head $testroot/repo`
 
 	(cd $testroot/wt && got rebase newbranch > $testroot/stdout \
 		2> $testroot/stderr)
 
-	echo -n > $testroot/stdout.expected
+	echo "refs/heads/newbranch is already based on refs/heads/master" \
+		> $testroot/stdout.expected
+	echo "Switching work tree from refs/heads/master to refs/heads/newbranch" \
+		>> $testroot/stdout.expected
+	echo "U  gamma/delta" >> $testroot/stdout.expected
+	echo "Updated to refs/heads/newbranch: ${newbranch_id}" \
+		>> $testroot/stdout.expected
 	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret="$?"
 	if [ "$ret" != "0" ]; then
@@ -256,8 +263,7 @@ test_rebase_ancestry_check() {
 		return 1
 	fi
 
-	echo "got: refs/heads/newbranch is already based on refs/heads/master" \
-		> $testroot/stderr.expected
+	echo -n > $testroot/stderr.expected
 	cmp -s $testroot/stderr.expected $testroot/stderr
 	ret="$?"
 	if [ "$ret" != "0" ]; then