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 <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68