From: Mark Jamsek Subject: Re: add missing rebase out-of-date check To: gameoftrees@openbsd.org Date: Sun, 8 Jan 2023 22:28:57 +1100 On 23-01-08 11:52AM, Stefan Sperling wrote: > I found a situation where 'got rebase' will proceed even though the > work tree is out-of-date. This happens because a seqence of youngest > common-ancestor and same-branch checks will trivially succeed, leading > 'got rebase' to assume that this rebase is a fast-forward. However, > it only looks like a fast-forward because we miss the fact that the > work-tree's base commit is out of date, ignoring newer commits on the > work-tree's branch. > > This situation is not covered by the existing rebase_out_of_date test. > To reproduce the problem, run the new test added with this patch, with > the got/got.c part of this patch backed out: > > test_rebase_out_of_date2 rebase succeeded unexpectedly > test failed; leaving test data in /tmp/got-test-rebase_out_of_date2-N3qmgXLx > $ cat /tmp/got-test-rebase_out_of_date2-N3qmgXLx/stdout > refs/heads/newbranch is already based on refs/heads/master > Switching work tree from refs/heads/master to refs/heads/newbranch > U epsilon/zeta > U gamma/delta > Updated to refs/heads/newbranch: f5fd50fc48b880a976504bb432c60f1372c5dc9c > $ > > ok? Nice find! ok > fix 'got rebase' not detecting an out-of-date work tree in some cases > > diff 1a99e0b4097b26cac736de07239a3be7589a48f7 cb22360ec8580f5213a60a01295a4121ac013556 > commit - 1a99e0b4097b26cac736de07239a3be7589a48f7 > commit + cb22360ec8580f5213a60a01295a4121ac013556 > blob - 4e1c7e548dd68228986b5949d2af99e7e032a6ba > blob + 9a89d5b62f1635fb4b385534abdd217ee4a06611 > --- got/got.c > +++ got/got.c > @@ -10154,6 +10154,8 @@ cmd_rebase(int argc, char *argv[]) > struct got_object_id *commit_id = NULL, *parent_id = NULL; > struct got_object_id *resume_commit_id = NULL; > struct got_object_id *branch_head_commit_id = NULL, *yca_id = NULL; > + struct got_object_id *head_commit_id = NULL; > + struct got_reference *head_ref = NULL; > struct got_commit_object *commit = NULL; > int ch, rebase_in_progress = 0, abort_rebase = 0, continue_rebase = 0; > int histedit_in_progress = 0, merge_in_progress = 0; > @@ -10356,7 +10358,19 @@ cmd_rebase(int argc, char *argv[]) > if (!continue_rebase) { > struct got_object_id *base_commit_id; > > + error = got_ref_open(&head_ref, repo, > + got_worktree_get_head_ref_name(worktree), 0); > + if (error) > + goto done; > + error = got_ref_resolve(&head_commit_id, repo, head_ref); > + if (error) > + goto done; > base_commit_id = got_worktree_get_base_commit_id(worktree); > + if (got_object_id_cmp(base_commit_id, head_commit_id) != 0) { > + error = got_error(GOT_ERR_REBASE_OUT_OF_DATE); > + goto done; > + } > + > error = got_commit_graph_find_youngest_common_ancestor(&yca_id, > base_commit_id, branch_head_commit_id, 1, repo, > check_cancelled, NULL); > @@ -10537,6 +10551,7 @@ done: > got_object_id_queue_free(&commits); > free(branch_head_commit_id); > free(resume_commit_id); > + free(head_commit_id); > free(yca_id); > if (commit) > got_object_commit_close(commit); > @@ -10546,6 +10561,8 @@ done: > got_ref_close(new_base_branch); > if (tmp_branch) > got_ref_close(tmp_branch); > + if (head_ref) > + got_ref_close(head_ref); > if (worktree) > got_worktree_close(worktree); > if (repo) { > blob - f809879812d293f5b44a4ad0e1a299f1384dc027 > blob + f87f1d9e51fc63c6ebf7cc957f15ec52ddd15420 > --- regress/cmdline/rebase.sh > +++ regress/cmdline/rebase.sh > @@ -1834,6 +1834,57 @@ test_parseargs "$@" > test_done "$testroot" 0 > } > > +test_rebase_out_of_date2() { > + local testroot=`test_init rebase_out_of_date2` > + local commit0=`git_show_head $testroot/repo` > + local commit0_author_time=`git_show_author_time $testroot/repo` > + > + (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 orig_commit1=`git_show_parent_commit $testroot/repo` > + local orig_commit2=`git_show_head $testroot/repo` > + local orig_author_time2=`git_show_author_time $testroot/repo` > + > + (cd $testroot/repo && git checkout -q master) > + echo "modified zeta on master" > $testroot/repo/epsilon/zeta > + git_commit $testroot/repo -m "committing to zeta on master" > + local master_commit=`git_show_head $testroot/repo` > + > + got checkout $testroot/repo $testroot/wt > /dev/null > + ret=$? > + if [ $ret -ne 0 ]; then > + test_done "$testroot" "$ret" > + return 1 > + fi > + > + # Backdate the file alpha to an earlier version. > + # This sets the work tree's base commit ID back to $commit0, > + # which is out-of-date with respect to the master branch. > + (cd $testroot/wt && got update -c $commit0 alpha > /dev/null) > + > + # Rebase into an out-of-date work tree should be refused. > + (cd $testroot/wt && got rebase newbranch > $testroot/stdout \ > + 2> $testroot/stderr) > + ret=$? > + if [ $ret -eq 0 ]; then > + echo "rebase succeeded unexpectedly" >&2 > + test_done "$testroot" "1" > + return 1 > + fi > + echo -n > $testroot/stdout.expected > + echo -n "got: work tree must be updated before it can be used to " \ > + > $testroot/stderr.expected > + echo "rebase a branch" >> $testroot/stderr.expected > + cmp -s $testroot/stderr.expected $testroot/stderr > + ret=$? > + if [ $ret -ne 0 ]; then > + diff -u $testroot/stderr.expected $testroot/stderr > + fi > + test_done "$testroot" "$ret" > +} > + > test_parseargs "$@" > run_test test_rebase_basic > run_test test_rebase_ancestry_check > @@ -1854,3 +1905,4 @@ run_test test_rebase_umask > run_test test_rebase_no_author_info > run_test test_rebase_nonbranch > run_test test_rebase_umask > +run_test test_rebase_out_of_date2 > -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68