From: Stefan Sperling Subject: Re: "no commits to rebase" when rebasing on the initial commit To: Omar Polo Cc: gameoftrees@openbsd.org Date: Tue, 31 Jan 2023 13:27:21 +0100 On Tue, Jan 31, 2023 at 12:01:00PM +0100, Omar Polo wrote: > apparently 'got rebase' fails when rebasing directly on top of the > initial repository. Test below simulates it, uncomment the > un-indented lines to make it pass. Thanks, this is indeed a bug. Fix below, with the test you provided and several adjustments. The existing test case which collided with your test new case has been tweaked. It now triggers an appropriate error which had no test coverage yet, and was not actually reachable :-/ With this fix, got rebase will always fast-forward the specified branch when history has not diverged, even if the specified branch's tip has no parent commits. make 'got rebase' work when the to-be-rebased branch has no parent commit found by op@, who also provided the test case diff 0d98195bc8c97f44f5635b97b8020dd934f677f0 fd5a170f62829f82b63fdcf4c7ad4c5363483b1a commit - 0d98195bc8c97f44f5635b97b8020dd934f677f0 commit + fd5a170f62829f82b63fdcf4c7ad4c5363483b1a blob - 5d94c4ade9d0584ccb4b04bf99cb1c7f8a4d8745 blob + 06e066fceda7062b02b365d0bb490cd359852ca7 --- got/got.c +++ got/got.c @@ -11165,13 +11165,13 @@ cmd_rebase(int argc, char *argv[]) error = got_commit_graph_find_youngest_common_ancestor(&yca_id, base_commit_id, branch_head_commit_id, 1, repo, check_cancelled, NULL); - if (error) + if (error) { + if (error->code == GOT_ERR_ANCESTRY) { + error = got_error_msg(GOT_ERR_ANCESTRY, + "specified branch shares no common " + "ancestry with work tree's branch"); + } goto done; - if (yca_id == NULL) { - error = got_error_msg(GOT_ERR_ANCESTRY, - "specified branch shares no common ancestry " - "with work tree's branch"); - goto done; } error = check_same_branch(base_commit_id, branch, yca_id, repo); @@ -11226,17 +11226,16 @@ cmd_rebase(int argc, char *argv[]) parent_ids = got_object_commit_get_parent_ids(commit); pid = STAILQ_FIRST(parent_ids); - if (pid == NULL) { - error = got_error(GOT_ERR_EMPTY_REBASE); - goto done; + if (pid) { + error = collect_commits(&commits, commit_id, &pid->id, + yca_id, got_worktree_get_path_prefix(worktree), + GOT_ERR_REBASE_PATH, repo); + if (error) + goto done; } - error = collect_commits(&commits, commit_id, &pid->id, - yca_id, got_worktree_get_path_prefix(worktree), - GOT_ERR_REBASE_PATH, repo); + got_object_commit_close(commit); commit = NULL; - if (error) - goto done; if (!continue_rebase) { error = got_worktree_rebase_prepare(&new_base_branch, blob - c951c76bc67d396a591c05f06bd362c98aa7f678 blob + 3fd06efcdadb789fef14e683086013ed87d5bfd9 --- include/got_error.h +++ include/got_error.h @@ -100,7 +100,7 @@ #define GOT_ERR_BRANCH_EXISTS 83 #define GOT_ERR_MODIFIED 84 #define GOT_ERR_NOT_REBASING 85 -#define GOT_ERR_EMPTY_REBASE 86 +/* 86 is currently unused */ #define GOT_ERR_REBASE_COMMITID 87 #define GOT_ERR_REBASING 88 #define GOT_ERR_REBASE_PATH 89 blob - 7e698531e6e75bd34bc4b2f0c280aded67f43de0 blob + 3c84ba7eeb9c88175a7652495f934263b6fd2328 --- lib/error.c +++ lib/error.c @@ -129,7 +129,6 @@ static const struct got_error got_errors[] = { { GOT_ERR_MODIFIED, "work tree contains local changes; these " "changes must be committed or reverted first" }, { GOT_ERR_NOT_REBASING, "rebase operation not in progress" }, - { GOT_ERR_EMPTY_REBASE, "no commits to rebase" }, { GOT_ERR_REBASE_COMMITID,"rebase commit ID mismatch" }, { GOT_ERR_REBASING, "a rebase operation is in progress in this " "work tree and must be continued or aborted first" }, blob - f87f1d9e51fc63c6ebf7cc957f15ec52ddd15420 blob + fceaf181cd74c8b1810cfe97234af037bb77561c --- regress/cmdline/rebase.sh +++ regress/cmdline/rebase.sh @@ -912,7 +912,11 @@ test_rebase_no_commits_to_rebase() { return 1 fi - (cd $testroot/wt && got branch -n newbranch) + # Create an unrelated branch with 'got import'. + mkdir -p $testroot/newtree + echo "new file" > $testroot/newtree/newfile + got import -m new -b newbranch -r $testroot/repo \ + $testroot/newtree > /dev/null echo "modified alpha on master" > $testroot/wt/alpha (cd $testroot/wt && got commit -m 'test rebase_no_commits_to_rebase' \ @@ -922,7 +926,9 @@ test_rebase_no_commits_to_rebase() { (cd $testroot/wt && got rebase newbranch > $testroot/stdout \ 2> $testroot/stderr) - echo "got: no commits to rebase" > $testroot/stderr.expected + echo -n "got: specified branch shares no common ancestry " \ + > $testroot/stderr.expected + echo "with work tree's branch" >> $testroot/stderr.expected cmp -s $testroot/stderr.expected $testroot/stderr ret=$? if [ $ret -ne 0 ]; then @@ -1885,6 +1891,47 @@ test_parseargs "$@" test_done "$testroot" "$ret" } +test_rebase_one_commit() { + local testroot=`test_init rebase_one_commit` + + if ! got checkout $testroot/repo $testroot/wt >/dev/null; then + test_done "$testroot" 1 + return 1 + fi + + (cd $testroot/wt && got branch newbranch) >/dev/null + + echo "modified alpha on newbranch" >$testroot/wt/alpha + (cd $testroot/wt && got commit -m 'edit alpha') >/dev/null + (cd $testroot/wt && got update) >/dev/null + local commit=`git_show_branch_head $testroot/repo newbranch` + + echo -n '' > $testroot/stderr.expected + + (cd $testroot/wt && got rebase master >$testroot/stdout \ + 2> $testroot/stderr) + ret=$? + if [ $ret -ne 0 ]; then + echo "rebase comand failed unexpectedly" >&2 + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "1" + return 1 + fi + + echo "Forwarding refs/heads/master to commit $commit" \ + >$testroot/stdout.expected + echo "Switching work tree to refs/heads/master" \ + >> $testroot/stdout.expected + + if ! cmp -s $testroot/stdout.expected $testroot/stdout; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" 1 + return 1 + fi + + test_done "$testroot" 0 +} + test_parseargs "$@" run_test test_rebase_basic run_test test_rebase_ancestry_check @@ -1906,3 +1953,4 @@ run_test test_rebase_out_of_date2 run_test test_rebase_nonbranch run_test test_rebase_umask run_test test_rebase_out_of_date2 +run_test test_rebase_one_commit