On Wed, Nov 03, 2021 at 05:19:41PM -0400, Josh Rickmar wrote:
> On Wed, Nov 03, 2021 at 10:07:26PM +0100, Stefan Sperling wrote:
> > On Wed, Nov 03, 2021 at 04:36:16PM -0400, Josh Rickmar wrote:
> > > The modifications necessary to test_rebase_ancestry_check should cover
> > > this, right?
> > 
> > Yes, that should be good enough.
> > 
> > A few comments inline:
> 
> doh. certainly intended to actually use that declared variable.
> 
> moved the id_str declaration and assignment closer to usage too.
> 

Great! This version is ok by me. Thanks.

I have one more small suggestion if you want to take it:

> diff refs/heads/main refs/heads/noop-rebase
> blob - 488cd9f35722bed936af6e7d9e2c6aa77ea2a3d8
> blob + 7c817edb02fa861df31ee4804f30af110be3c191
> --- got/got.c
> +++ got/got.c
> @@ -9147,12 +9147,43 @@ 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;
> +			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_set_base_commit_id(worktree, repo,
> +			    branch_head_commit_id);
> +			if (error)
> +				goto done;
> +			TAILQ_INIT(&paths);
> +			error = got_pathlist_append(&paths, "", NULL);
> +			if (error) {
> +				got_pathlist_free(&paths);

No need to free the path list here since we know the list is still empty.

> +				goto done;
> +			}
> +			error = got_worktree_checkout_files(worktree,
> +			    &paths, repo, update_progress, &upa,
> +			    check_cancelled, NULL);
> +			got_pathlist_free(&paths);
> +			if (error)
> +				goto done;
> +			if (upa.did_something) {
> +				char *id_str;
> +				error = got_object_id_str(&id_str,
> +				    branch_head_commit_id);
> +				if (error)
> +					goto done;
> +				printf("Updated to %s: %s\n",
> +				    got_worktree_get_head_ref_name(worktree),
> +				    id_str);
> +				free(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
> 
>