threads / patch / 40788

patchcontrib/subtree: unwrap tag refs

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

## tl;dr

5 messages between Nov 13, 2015 and Jan 13, 2016. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Rob Mayoff· Nov 13, 2015, 04:15 UTC · lore
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(-)
Show changes to contrib/subtree/git-subtree.sh +8 −2
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
David A. Greene· Nov 13, 2015, 04:36 UTC · re: Rob Mayoff · lore

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

Rob Mayoff <mayoff@dqd.com> writes:
Show 11 quoted lines
> 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.
Show 11 quoted lines
> +				;;
>  			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
Jeff King· Nov 13, 2015, 05:01 UTC · re: David A. Greene · lore

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

On Thu, Nov 12, 2015 at 10:36:16PM -0600, David A. Greene wrote:
Show 7 quoted lines
> > +				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
David A. Greene· Jan 13, 2016, 02:35 UTC · re: David A. Greene · lore

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

I am sorry I neglected to follow-up on this.
greened@obbligato.org (David A. Greene) writes:
Show 15 quoted lines
> 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.
Show 7 quoted lines
>> +				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
Junio C Hamano· Jan 13, 2016, 17:40 UTC · re: David A. Greene · lore

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

greened@obbligato.org (David A. Greene) writes:
Show 35 quoted lines
> 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.

← back to recent threads