From: Stefan Sperling <stsp@stsp.name>
Subject: Re: Switch branches during rebase even if there are no commits to rebase
To: Josh Rickmar <openbsd+lists@zettaport.com>
Cc: gameoftrees@openbsd.org
Date: Wed, 3 Nov 2021 20:16:37 +0100

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).

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

> diff a9e66726ec8e6d4997a024f572ee53719f7b9cdb /home/jrick/src/got
> blob - 488cd9f35722bed936af6e7d9e2c6aa77ea2a3d8
> file + got/got.c
> --- got/got.c
> +++ got/got.c
> @@ -9147,12 +9147,16 @@ 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",
> +			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 = switch_head_ref(branch, branch_head_commit_id,
> +			    worktree, repo);
> +			if (error)
> +				goto done;
> +			error = got_worktree_checkout_files(worktree,
> +			    &merged_paths, repo, update_progress, &upa,
> +			    check_cancelled, NULL);
>  			goto done;
>  		}
>  		error = got_worktree_rebase_prepare(&new_base_branch,
> blob - b17666eaf0ae899543adfa66236bdfd3eb49e92d
> file + regress/cmdline/rebase.sh
> --- regress/cmdline/rebase.sh
> +++ regress/cmdline/rebase.sh
> @@ -247,7 +247,10 @@ test_rebase_ancestry_check() {
>  	(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
>  	cmp -s $testroot/stdout.expected $testroot/stdout
>  	ret="$?"
>  	if [ "$ret" != "0" ]; then
> @@ -256,8 +259,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
> 
>