From: Stefan Sperling <stsp@stsp.name>
Subject: Re: allow vendor branch merges with 'got merge'
To: gameoftrees@openbsd.org
Date: Sun, 26 Sep 2021 15:16:08 +0200

On Sat, Sep 25, 2021 at 06:33:10PM +0200, Stefan Sperling wrote:
> This makes it possible to merge vendor branches with 'got merge'.
> The idea is that when 'got import' is used to create a vendor branch,
> as an entirely separate line of history with its own root commit, then
> we should be able to use 'got merge' in order to merge vendor code
> changes into the 'main' branch.
> 
> The initial 'got merge' implementation required that twe two branches
> share a common ancestry. This makes it impossible to merge branches
> which do not share the same root commit.
> 
> With this patch, this requirement is lifted. When merging multiple times,
> we detect the last common merge commit between the branches and use it
> to drive the tree diff such that only new changes will be merged.
> To achieve this, we make use of the commit graph's existing ability to
> traverse history of multiple branches in parallel to identify a common
> ancestor which is the most recent merge commit between the two otherwise
> unrelated branches.
> 
> While here, also fix the behaviour of 'got merge' if no changes are
> available for merging. We required the user to abort such a merge
> manually with 'got merge -a'. This now happens automatically and
> the user will simply see the message 'Already up-to-date'.
>  
> ok?
> 
> (this patch sits on top of my "bad-symlinks-merge" patch which has
> not been committed for lack of OKs)

I have committed the symlinks-merge fix mentioned above, after Thomas
provided feedback on it.

So this patch should now apply cleanly to the main branch.

> 
> diff b380729c04bdb5ee1ee63cf542b892a03486b82a 3900e2cd23b57a17655f6b84cdf02e7eaa6753ba
> blob - 06422c812cb6a86575f656e0a5293c72ed775769
> blob + c408a296cf7c404394c1001c724ca9a0355c7e6a
> --- got/got.1
> +++ got/got.1
> @@ -2141,8 +2141,9 @@ If a linear project history is desired, then use of
>  should be preferred over
>  .Cm got merge .
>  However, even strictly linear projects may require merge commits in order
> -to merge in new versions of code imported from third-party projects on
> -vendor branches.
> +to merge in new versions of third-party code stored on vendor branches
> +created with
> +.Cm got import .
>  .Pp
>  Merge commits are commits based on multiple parent commits.
>  The tip commit of the work tree's current branch, which must be set with
> @@ -2154,14 +2155,14 @@ The tip commit of the specified
>  .Ar branch
>  will be used as the second parent.
>  .Pp
> +No ancestral relationship between the two branches is required.
> +If the two branches have already been merged previously, only new changes
> +will be merged.
> +.Pp
>  It is not possible to create merge commits with more than two parents.
>  If more than one branch needs to be merged, then multiple merge commits
>  with two parents each can be created in sequence.
>  .Pp
> -The
> -.Ar branch
> -must share common ancestry with the work tree's current branch.
> -.Pp
>  While merging changes found on the
>  .Ar branch
>  into the work tree, show the status of each affected file,
> blob - 66b864fee620072787f5c6149732f3c409f4f0cf
> blob + 39c5172e4d8e1b72ff03f0dbd81fc4ae8935b1b9
> --- got/got.c
> +++ got/got.c
> @@ -2701,7 +2701,7 @@ check_linear_ancestry(struct got_object_id *commit_id,
>  	struct got_object_id *yca_id;
>  
>  	err = got_commit_graph_find_youngest_common_ancestor(&yca_id,
> -	    commit_id, base_commit_id, repo, check_cancelled, NULL);
> +	    commit_id, base_commit_id, 1, repo, check_cancelled, NULL);
>  	if (err)
>  		return err;
>  
> @@ -8597,7 +8597,7 @@ print_backup_ref(const char *branch_name, const char *
>  		goto done;
>  
>  	err = got_commit_graph_find_youngest_common_ancestor(&yca_id,
> -	    old_commit_id, new_commit_id, repo, check_cancelled, NULL);
> +	    old_commit_id, new_commit_id, 1, repo, check_cancelled, NULL);
>  	if (err)
>  		goto done;
>  
> @@ -8954,7 +8954,7 @@ cmd_rebase(int argc, char *argv[])
>  
>  		base_commit_id = got_worktree_get_base_commit_id(worktree);
>  		error = got_commit_graph_find_youngest_common_ancestor(&yca_id,
> -		    base_commit_id, branch_head_commit_id, repo,
> +		    base_commit_id, branch_head_commit_id, 1, repo,
>  		    check_cancelled, NULL);
>  		if (error)
>  			goto done;
> @@ -10729,16 +10729,10 @@ cmd_merge(int argc, char *argv[])
>  	if (error)
>  		goto done;
>  	error = got_commit_graph_find_youngest_common_ancestor(&yca_id,
> -	    wt_branch_tip, branch_tip, repo,
> +	    wt_branch_tip, branch_tip, 0, repo,
>  	    check_cancelled, NULL);
> -	if (error)
> +	if (error && error->code != GOT_ERR_ANCESTRY)
>  		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;
> -	}
>  
>  	if (!continue_merge) {
>  		error = check_path_prefix(wt_branch_tip, branch_tip,
> @@ -10746,22 +10740,24 @@ cmd_merge(int argc, char *argv[])
>  		    GOT_ERR_MERGE_PATH, repo);
>  		if (error)
>  			goto done;
> -		error = check_same_branch(wt_branch_tip, branch,
> -		    yca_id, repo);
> -		if (error) {
> -			if (error->code != GOT_ERR_ANCESTRY)
> +		if (yca_id) {
> +			error = check_same_branch(wt_branch_tip, branch,
> +			    yca_id, repo);
> +			if (error) {
> +				if (error->code != GOT_ERR_ANCESTRY)
> +					goto done;
> +				error = NULL;
> +			} else {
> +				static char msg[512];
> +				snprintf(msg, sizeof(msg),
> +				    "cannot create a merge commit because "
> +				    "%s is based on %s; %s can be integrated "
> +				    "with 'got integrate' instead", branch_name,
> +				    got_worktree_get_head_ref_name(worktree),
> +				    branch_name);
> +				error = got_error_msg(GOT_ERR_SAME_BRANCH, msg);
>  				goto done;
> -			error = NULL;
> -		} else {
> -			static char msg[512];
> -			snprintf(msg, sizeof(msg),
> -			    "cannot create a merge commit because "
> -			    "%s is based on %s; %s can be integrated "
> -			    "with 'got integrate' instead", branch_name,
> -			    got_worktree_get_head_ref_name(worktree),
> -			    branch_name);
> -			error = got_error_msg(GOT_ERR_SAME_BRANCH, msg);
> -			goto done;
> +			}
>  		}
>  		error = got_worktree_merge_prepare(&fileindex, worktree,
>  		    branch, repo);
> @@ -10774,6 +10770,14 @@ cmd_merge(int argc, char *argv[])
>  		if (error)
>  			goto done;
>  		print_update_progress_stats(&upa);
> +		if (!upa.did_something) {
> +			error = got_worktree_merge_abort(worktree, fileindex,
> +			    repo, update_progress, &upa);
> +			if (error)
> +				goto done;
> +			printf("Already up-to-date\n");
> +			goto done;
> +		}
>  	}
>  
>  	if (upa.conflicts > 0 || upa.missing > 0) {
> blob - 1538d549835d34b1e9f1400e8da7166397acd4c0
> blob + 617697ddc049b408054512990ba787719cfad80a
> --- include/got_commit_graph.h
> +++ include/got_commit_graph.h
> @@ -32,4 +32,4 @@ const struct got_error *got_commit_graph_intersect(str
>  /* Find the youngest common ancestor of two commits. */
>  const struct got_error *got_commit_graph_find_youngest_common_ancestor(
>      struct got_object_id **, struct got_object_id *, struct got_object_id *,
> -    struct got_repository *, got_cancel_cb, void *);
> +    int, struct got_repository *, got_cancel_cb, void *);
> blob - 02b48fae94b68b2e73ce447027d7d94d3be3b6c5
> blob + 55b4da29492815a14efed2a3680b55d68dddbfd0
> --- lib/commit_graph.c
> +++ lib/commit_graph.c
> @@ -600,6 +600,7 @@ got_commit_graph_iter_next(struct got_object_id **id,
>  const struct got_error *
>  got_commit_graph_find_youngest_common_ancestor(struct got_object_id **yca_id,
>      struct got_object_id *commit_id, struct got_object_id *commit_id2,
> +    int first_parent_traversal,
>      struct got_repository *repo, got_cancel_cb cancel_cb, void *cancel_arg)
>  {
>  	const struct got_error *err = NULL;
> @@ -613,11 +614,11 @@ got_commit_graph_find_youngest_common_ancestor(struct 
>  	if (commit_ids == NULL)
>  		return got_error_from_errno("got_object_idset_alloc");
>  
> -	err = got_commit_graph_open(&graph, "/", 1);
> +	err = got_commit_graph_open(&graph, "/", first_parent_traversal);
>  	if (err)
>  		goto done;
>  
> -	err = got_commit_graph_open(&graph2, "/", 1);
> +	err = got_commit_graph_open(&graph2, "/", first_parent_traversal);
>  	if (err)
>  		goto done;
>  
> blob - 439105b666bbf6eb9e40bf4c2cfa6b3c25e52eeb
> blob + 6a928efef76409f746a25050dabef6f6c6001c95
> --- lib/send.c
> +++ lib/send.c
> @@ -167,7 +167,7 @@ check_linear_ancestry(const char *refname, struct got_
>  		    "bad object type on server for %s", refname);
>  
>  	err = got_commit_graph_find_youngest_common_ancestor(&yca_id,
> -	    my_id, their_id, repo, cancel_cb, cancel_arg);
> +	    my_id, their_id, 1, repo, cancel_cb, cancel_arg);
>  	if (err)
>  		return err;
>  	if (yca_id == NULL)
> blob - 1c7ea8430ba3f5a85df6f00f7a3587641ae683ff
> blob + 1d534c5958a2e7da9413b01d8fcff42599c6f25b
> --- regress/cmdline/merge.sh
> +++ regress/cmdline/merge.sh
> @@ -1131,6 +1131,111 @@ test_merge_no_op() {
>  	test_done "$testroot" "$ret"
>  }
>  
> +test_merge_imported_branch() {
> +	local testroot=`test_init merge_import`
> +	local commit0=`git_show_head $testroot/repo`
> +	local commit0_author_time=`git_show_author_time $testroot/repo`
> +
> +	# import a new sub-tree to the 'files' branch such that
> +	# none of the files added here collide with existing ones
> +	mkdir -p $testroot/tree/there
> +	mkdir -p $testroot/tree/be/lots
> +	mkdir -p $testroot/tree/files
> +	echo "there should" > $testroot/tree/there/should
> +	echo "be lots of" > $testroot/tree/be/lots/of
> +	echo "files here" > $testroot/tree/files/here
> +	got import -r $testroot/repo -b files -m 'import files' \
> +		$testroot/tree > /dev/null
> +
> +	got checkout -b master $testroot/repo $testroot/wt > /dev/null
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		echo "got checkout failed unexpectedly" >&2
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	(cd $testroot/wt && got merge files > $testroot/stdout)
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		echo "got merge failed unexpectedly" >&2
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	local merge_commit0=`git_show_head $testroot/repo`
> +	cat > $testroot/stdout.expected <<EOF
> +A  be/lots/of
> +A  files/here
> +A  there/should
> +Merged refs/heads/files into refs/heads/master: $merge_commit0
> +EOF
> +	cmp -s $testroot/stdout.expected $testroot/stdout
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		diff -u $testroot/stdout.expected $testroot/stdout
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	# try to merge again while no new changes are available
> +	(cd $testroot/wt && got merge files > $testroot/stdout)
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		echo "got merge failed unexpectedly" >&2
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +	echo "Already up-to-date" > $testroot/stdout.expected
> +	cmp -s $testroot/stdout.expected $testroot/stdout
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		diff -u $testroot/stdout.expected $testroot/stdout
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	# update the 'files' branch
> +	(cd $testroot/repo && git reset -q --hard master)
> +	(cd $testroot/repo && git checkout -q files)
> +	echo "indeed" > $testroot/repo/indeed
> +	(cd $testroot/repo && git add indeed)
> +	git_commit $testroot/repo -m "adding another file indeed"
> +	echo "be lots and lots of" > $testroot/repo/be/lots/of
> +	git_commit $testroot/repo -m "lots of changes"
> +
> +	(cd $testroot/wt && got update > /dev/null)
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		echo "got update failed unexpectedly" >&2
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	# we should now be able to merge more changes from files branch
> +	(cd $testroot/wt && got merge files > $testroot/stdout)
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		echo "got merge failed unexpectedly" >&2
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	local merge_commit1=`git_show_branch_head $testroot/repo master`
> +	cat > $testroot/stdout.expected <<EOF
> +G  be/lots/of
> +A  indeed
> +Merged refs/heads/files into refs/heads/master: $merge_commit1
> +EOF
> +
> +	cmp -s $testroot/stdout.expected $testroot/stdout
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		diff -u $testroot/stdout.expected $testroot/stdout
> +	fi
> +	test_done "$testroot" "$ret"
> +}
> +
>  test_parseargs "$@"
>  run_test test_merge_basic
>  run_test test_merge_continue
> @@ -1139,3 +1244,4 @@ run_test test_merge_in_progress
>  run_test test_merge_path_prefix
>  run_test test_merge_missing_file
>  run_test test_merge_no_op
> +run_test test_merge_imported_branch
> 
>