From: Junio C Hamano <gitster@pobox•com>
To: Dave Ware <davidw@realtimegenomics•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH v3] contrib/subtree: fix "subtree split" skipped-merge bug
Date: Tue, 08 Dec 2015 13:23:21 -0800 [thread overview]
Message-ID: <xmqqk2ook52u.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1449607160-20608-1-git-send-email-davidw@realtimegenomics.com> (Dave Ware's message of "Wed, 9 Dec 2015 09:39:20 +1300")
Dave Ware <davidw@realtimegenomics•com> 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@realtimegenomics•com>
> ---
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
next prev parent reply other threads:[~2015-12-08 21:23 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-06 22:09 git subtree bug produces divergent descendants David Ware
2015-12-07 4:53 ` Eric Sunshine
2015-12-07 20:50 ` [PATCH] contrib/subtree: fix "subtree split" skipped-merge bug Dave Ware
2015-12-08 6:49 ` Eric Sunshine
2015-12-08 20:39 ` [PATCH v3] " Dave Ware
2015-12-08 21:23 ` Junio C Hamano [this message]
2015-12-09 0:16 ` David Ware
2015-12-09 0:19 ` [PATCH v4] " Dave Ware
2015-12-09 7:52 ` Eric Sunshine
2015-12-09 21:17 ` [PATCH v5] " Dave Ware
2016-01-13 3:27 ` David A. Greene
2016-01-13 19:33 ` David Ware
2016-01-14 3:12 ` David A. Greene
2016-01-14 20:45 ` David Ware
2016-01-17 22:40 ` David A. Greene
2016-01-14 21:26 ` [PATCH v6] " Dave Ware
2016-01-15 0:41 ` [PATCH v7] " Dave Ware
2016-01-15 1:06 ` Eric Sunshine
2016-01-15 18:58 ` Junio C Hamano
2016-01-15 23:24 ` Eric Sunshine
2016-01-17 22:41 ` David A. Greene
2015-12-07 21:01 ` git subtree bug produces divergent descendants David Ware
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqk2ook52u.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=davidw@realtimegenomics$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox