Re: [PATCH v7] contrib/subtree: fix "subtree split" skipped-merge bug

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 15, 2016, 01:06 UTC
Message-ID: <CAPig+cS3hCjkb7bc6Z8CMFBU81Mp1Nn=xUUu4ZveVzOWAM5Ybw@mail.gmail.com>
In-Reply-To: <1452818503-21079-1-git-send-email-davidw@realtimegenomics.com>

On Thu, Jan 14, 2016 at 7:41 PM, Dave Ware <davidw@realtimegenomics.com> wrote:

Show 18 quoted lines

> 'git subtree split' can incorrectly skip a merge 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 remains a
> descendent on the subtree in this case.
>
> Signed-off-by: Dave Ware <davidw@realtimegenomics.com>
> ---
> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
> @@ -1014,4 +1014,64 @@ test_expect_success 'push split to subproj' '
> +# This test covers 2 cases in subtree split copy_or_skip code
> +# 1) Merges where one parent is a superset of the changes of the other
> +#    parent regarding changes to the subtree, in this case the merge
> +#    commit should be copied
> +# 2) Merges where only one parent operate on the subtree, and the merge

s/operate/operates/

Show 9 quoted lines

> +#    commit should be skipped
> +#
> +next_test
> +test_expect_success 'subtree descendant check' '
> +       subtree_test_create_repo "$subtree_test_count" &&
> +       test_create_commit "$subtree_test_count" folder_subtree/a &&
> +       (
> +               cd "$subtree_test_count" &&
> +               git branch branch

Not worth a re-roll (and probably not worthwhile anyhow since it would be inconsistent with the rest of the script), but for these really simple cases, you can use -C and avoid the subshell altogether:

    git -C "$subtree_test_count" branch branch

Show 34 quoted lines

> +       ) &&
> +       test_create_commit "$subtree_test_count" folder_subtree/0 &&
> +       test_create_commit "$subtree_test_count" folder_subtree/b &&
> +       cherry=$(cd "$subtree_test_count"; git rev-parse HEAD) &&
> +       (
> +               cd "$subtree_test_count" &&
> +               git checkout branch
> +       ) &&
> +       test_create_commit "$subtree_test_count" commit_on_branch &&
> +       (
> +               cd "$subtree_test_count" &&
> +               git cherry-pick $cherry &&
> +               git checkout master &&
> +               git merge -m "merge should be kept on subtree" branch &&
> +               git branch no_subtree_work_branch
> +       ) &&
> +       test_create_commit "$subtree_test_count" folder_subtree/d &&
> +       (
> +               cd "$subtree_test_count" &&
> +               git checkout no_subtree_work_branch
> +       ) &&
> +       test_create_commit "$subtree_test_count" not_a_subtree_change &&
> +       (
> +               cd "$subtree_test_count" &&
> +               git checkout master &&
> +               git merge -m "merge should be skipped on subtree" no_subtree_work_branch &&
> +
> +               git subtree split --prefix folder_subtree/ --branch subtree_tip master &&
> +               git subtree split --prefix folder_subtree/ --branch subtree_branch branch &&
> +               check_equal $(git rev-list --count subtree_tip..subtree_branch) 0
> +       )
> +'
> +
>  test_done