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

2015-11-12 Thread David A. Greene
Rob Mayoff  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] contrib/subtree: unwrap tag refs

2015-11-12 Thread Rob Mayoff
From: Rob Mayoff 

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 .

Signed-off-by: Rob Mayoff 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-11-12 Thread 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html