Dave Ware <davidw@xxxxxxxxxxxxxxxxxxxx> writes:
> A bug occurs in 'git-subtree split' where a merge is skipped even when
> both parents act on the subtree, provided the merge results in a tree
> identical to one of the parents. Fix by copying the merge if at least
> one parent is non-identical, and the non-identical parent is not an
> ancestor of the identical parent.
>
> Also, add a test case which checks that a descendant can be pushed to
> its ancestor in this case.
>
> Signed-off-by: Dave Ware <davidw@xxxxxxxxxxxxxxxxxxxx>
> ---
The first sentence may be made clearer if you rephrased the early
part of the sentence this way:
'git subtree split' can incorrectly skip a merge even when
both parents ...
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 9f06571..b837531 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -479,8 +479,16 @@ copy_or_skip()
> p="$p -p $parent"
> fi
> done
> -
> - if [ -n "$identical" ]; then
> +
> + copycommit=
> + if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
> + extras=$(git rev-list --boundary $identical..$nonidentical)
> + if [ -n "$extras" ]; then
> + # we need to preserve history along the other branch
> + copycommit=1
> + fi
What is the significance of "--boundary" here? I think for the
purpose of "is the identical one part of the nonidentical one?" you
do not need it, but there may be something subtle I missed. I am
asking this because use of "rev-list --boundary" in scripts is
almost always a bug.
Also, depending on how huge the output from the rev-list could be,
you might want to use "rev-list --count $i..$n" and compare it with
0 instead--that way, you would not have to be worried about having
to carry around a huge string that you would otherwise not use, only
to see if that string is empty.
Thanks.
> + fi
> + if [ -n "$identical" ] && [ -z "$copycommit" ]; then
> echo $identical
> else
> copy_commit $rev $tree "$p" || exit $?
> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
> index 9051982..710278c 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' '
> ))
> '
>
> +test_expect_success 'subtree descendent check' '
> + mkdir git_subtree_split_check &&
> + (
> + cd git_subtree_split_check &&
> + git init &&
> +
> + mkdir folder &&
> +
> + echo a >folder/a &&
> + git add . &&
> + git commit -m "first commit" &&
> +
> + git branch branch &&
> +
> + echo 0 >folder/0 &&
> + git add . &&
> + git commit -m "adding 0 to folder" &&
> +
> + echo b >folder/b &&
> + git add . &&
> + git commit -m "adding b to folder" &&
> + cherry=$(git rev-parse HEAD) &&
> +
> + git checkout branch &&
> + echo text >textBranch.txt &&
> + git add . &&
> + git commit -m "commit to fiddle with branch: branch" &&
> +
> + git cherry-pick $cherry &&
> + git checkout master &&
> + git merge -m "merge" branch &&
> +
> + git branch noop_branch &&
> +
> + echo d >folder/d &&
> + git add . &&
> + git commit -m "adding d to folder" &&
> +
> + git checkout noop_branch &&
> + echo moreText >anotherText.txt &&
> + git add . &&
> + git commit -m "irrelevant" &&
> +
> + git checkout master &&
> + git merge -m "second merge" noop_branch &&
> +
> + git subtree split --prefix folder/ --branch subtree_tip master &&
> + git subtree split --prefix folder/ --branch subtree_branch branch &&
> + git push . subtree_tip:subtree_branch
> + )
> + '
> +
> test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html