From: Josh Rickmar Subject: Re: Switch branches during rebase even if there are no commits to rebase To: gameoftrees@openbsd.org Date: Wed, 3 Nov 2021 16:36:16 -0400 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