[PATCH] contrib/subtree: unwrap tag refs

[PATCH] contrib/subtree: unwrap tag refs

[Rob Mayoff]

From: Rob Mayoff <mayoff@dqd.com>

If a subtree was added using a tag ref, the tag ref is stored in
the subtree commit message instead of the underlying commit's ref.
To split or push subsequent changes to the subtree, the subtree
command needs to unwrap the tag ref.  This patch makes it do so.

The problem was described in a message to the mailing list from
Junio C Hamano dated 29 Apr 2014, with the subject "Re: git subtree
issue in more recent versions". The archived message can be found
at <http://comments.gmane.org/gmane.comp.version-control.git/247503>.

Signed-off-by: Rob Mayoff <mayoff@dqd.com>
---
 contrib/subtree/git-subtree.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 9f06571..b051600 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -245,7 +245,10 @@ find_latest_squash()
 		case "$a" in
 			START) sq="$b" ;;
 			git-subtree-mainline:) main="$b" ;;
-			git-subtree-split:) sub="$b" ;;
+			git-subtree-split:)
+				sub="$b"
+				sub="$(git rev-parse "$b^0")" || die "could not rev-parse split hash $b from commit $sq"
+				;;
 			END)
 				if [ -n "$sub" ]; then
 					if [ -n "$main" ]; then
@@ -278,7 +281,10 @@ find_existing_splits()
 		case "$a" in
 			START) sq="$b" ;;
 			git-subtree-mainline:) main="$b" ;;
-			git-subtree-split:) sub="$b" ;;
+			git-subtree-split:)
+				sub="$b"
+				sub="$(git rev-parse "$b^0")" || die "could not rev-parse split hash $b from commit $sq"
+				;;
 			END)
 				debug "  Main is: '$main'"
 				if [ -z "$main" -a -n "$sub" ]; then
-- 
2.4.3

Re: [PATCH] contrib/subtree: unwrap tag refs

[David A. Greene]

Rob Mayoff <mayoff@dqd.com> writes:

> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 9f06571..b051600 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -245,7 +245,10 @@ find_latest_squash()
>  		case "$a" in
>  			START) sq="$b" ;;
>  			git-subtree-mainline:) main="$b" ;;
> -			git-subtree-split:) sub="$b" ;;
> +			git-subtree-split:)
> +				sub="$b"

Why include the above line?

> +				sub="$(git rev-parse "$b^0")" || die "could not rev-parse split hash $b from commit $sq"

This seems like odd quoting.  Would not this do the same?

				sub="$(git rev-parse $b^0)" || die "could not rev-parse split hash $b from commit $sq"

Perhaps I am missing something.

> +				;;
>  			END)
>  				if [ -n "$sub" ]; then
>  					if [ -n "$main" ]; then
> @@ -278,7 +281,10 @@ find_existing_splits()
>  		case "$a" in
>  			START) sq="$b" ;;
>  			git-subtree-mainline:) main="$b" ;;
> -			git-subtree-split:) sub="$b" ;;
> +			git-subtree-split:)
> +				sub="$b"

And here too.

> +				sub="$(git rev-parse "$b^0")" || die "could not rev-parse split hash $b from commit $sq"

Same as above.

                        -David

Re: [PATCH] contrib/subtree: unwrap tag refs

[Jeff King]

On Thu, Nov 12, 2015 at 10:36:16PM -0600, David A. Greene wrote:

> > +				sub="$(git rev-parse "$b^0")" || die "could not rev-parse split hash $b from commit $sq"
> 
> This seems like odd quoting.  Would not this do the same?
> 
> 				sub="$(git rev-parse $b^0)" || die "could not rev-parse split hash $b from commit $sq"
> 
> Perhaps I am missing something.

The former is quoting "$b" against whitespace splitting in the
sub-command. Given that the value just came from a "read" call, I think
by definition it cannot contains IFS. Still, quoting here is a good
habit.

It is actually the _outer_ quotes that are unnecessary, as variable
assignment does not do extra splitting. So:

  foo=$(echo one two)

will put the full "one two" into $foo. But the quotes do not hurt
anything, and it is a reasonable style to use them to avoid this
discussion. :)

It also matches style-wise with nearby assignments, like:

  main="$b"

-Peff

Re: [PATCH] contrib/subtree: unwrap tag refs

[David A. Greene]

I am sorry I neglected to follow-up on this.

greened@obbligato.org (David A. Greene) writes:

> Rob Mayoff <mayoff@dqd.com> writes:
>
>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
>> index 9f06571..b051600 100755
>> --- a/contrib/subtree/git-subtree.sh
>> +++ b/contrib/subtree/git-subtree.sh
>> @@ -245,7 +245,10 @@ find_latest_squash()
>>  		case "$a" in
>>  			START) sq="$b" ;;
>>  			git-subtree-mainline:) main="$b" ;;
>> -			git-subtree-split:) sub="$b" ;;
>> +			git-subtree-split:)
>> +				sub="$b"
>
> Why include the above line?

My bad.  Missed the diff markers.  This is fine.

>> +				sub="$(git rev-parse "$b^0")" || die "could not rev-parse split hash $b from commit $sq"
>
> This seems like odd quoting.  Would not this do the same?
>
> 				sub="$(git rev-parse $b^0)" || die "could not rev-parse split hash $b from commit $sq"
>
> Perhaps I am missing something.

Jeff explained it, so this is ok.

This all looks good to me.  Junio, please apply.

                            -David

Re: [PATCH] contrib/subtree: unwrap tag refs

[Junio C Hamano]

greened@obbligato.org (David A. Greene) writes:

> I am sorry I neglected to follow-up on this.
>
> greened@obbligato.org (David A. Greene) writes:
>
>> Rob Mayoff <mayoff@dqd.com> writes:
>>
>>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
>>> index 9f06571..b051600 100755
>>> --- a/contrib/subtree/git-subtree.sh
>>> +++ b/contrib/subtree/git-subtree.sh
>>> @@ -245,7 +245,10 @@ find_latest_squash()
>>>  		case "$a" in
>>>  			START) sq="$b" ;;
>>>  			git-subtree-mainline:) main="$b" ;;
>>> -			git-subtree-split:) sub="$b" ;;
>>> +			git-subtree-split:)
>>> +				sub="$b"
>>
>> Why include the above line?
>
> My bad.  Missed the diff markers.  This is fine.
>
>>> +				sub="$(git rev-parse "$b^0")" || die "could not rev-parse split hash $b from commit $sq"
>>
>> This seems like odd quoting.  Would not this do the same?
>>
>> 				sub="$(git rev-parse $b^0)" || die "could not rev-parse split hash $b from commit $sq"
>>
>> Perhaps I am missing something.
>
> Jeff explained it, so this is ok.
>
> This all looks good to me.  Junio, please apply.
>
>                             -David

Thanks, all.  Let's mark rm/subtree-unwrap-tags topic to be merged
to 'next', then.