Git development
 help / color / mirror / Atom feed
* [PATCH 13/13] contrib/subtree: Remove --annotate
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: "David A. Greene" <greened@obbligato.org>

Remove --annotate.  This obviates the need for an --unannotate
command.  We really want a more generalized commit message rewrite
mechanism.

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/git-subtree.sh     |    6 +----
 contrib/subtree/t/t7900-subtree.sh |   50 ++++++++++++++++++------------------
 2 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 0493e47..888a191 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -21,7 +21,6 @@ d             show debug messages
 P,prefix=     the name of the subdir to split out
 m,message=    use the given message as the commit message for the merge commit
  options for 'split'
-annotate=     add a prefix to commit message of new commits
 b,branch=     create a new branch from the split subtree
 ignore-joins  ignore prior --rejoin commits
 onto=         try connecting new tree to an existing one
@@ -43,7 +42,6 @@ command=
 onto=
 rejoin=
 ignore_joins=
-annotate=
 squash=
 message=
 
@@ -79,8 +77,6 @@ while [ $# -gt 0 ]; do
 	case "$opt" in
 		-q) quiet=1 ;;
 		-d) debug=1 ;;
-		--annotate) annotate="$1"; shift ;;
-		--no-annotate) annotate= ;;
 		-b) branch="$1"; shift ;;
 		-P) prefix="${1%/}"; shift ;;
 		-m) message="$1"; shift ;;
@@ -311,7 +307,7 @@ copy_commit()
 			GIT_COMMITTER_NAME \
 			GIT_COMMITTER_EMAIL \
 			GIT_COMMITTER_DATE
-		(echo -n "$annotate"; cat ) |
+		(echo -n ""; cat ) |
 		git commit-tree "$2" $3  # reads the rest of stdin
 	) || die "Can't copy commit $1"
 }
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 1afd544..59889f1 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -318,8 +318,8 @@ test_expect_success 'split subdir/ with --rejoin' '
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		split_hash=$(git subtree split --prefix=subdir --annotate="*") &&
-		git subtree split --prefix=subdir --annotate="*" --rejoin &&
+		split_hash=$(git subtree split --prefix=subdir) &&
+		git subtree split --prefix=subdir --rejoin &&
 		test_equal "$(last_commit_message)" "Split '\''subdir/'\'' into commit '\''$split_hash'\''"
 	)
  '
@@ -342,7 +342,7 @@ test_expect_success 'split subdir/ with --rejoin and --message' '
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		git subtree split --prefix=subdir --message="Split & rejoin" --annotate="*" --rejoin &&
+		git subtree split --prefix=subdir --message="Split & rejoin" --rejoin &&
 		test_equal "$(last_commit_message)" "Split & rejoin"
 	)
 '
@@ -365,8 +365,8 @@ test_expect_success 'split subdir/ with --branch' '
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		split_hash=$(git subtree split --prefix=subdir --annotate="*") &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br &&
+		split_hash=$(git subtree split --prefix=subdir) &&
+		git subtree split --prefix=subdir --branch subproj-br &&
 		test_equal "$(git rev-parse subproj-br)" "$split_hash"
 	)
  '
@@ -390,8 +390,8 @@ test_expect_success 'split subdir/ with --branch for an existing branch' '
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		split_hash=$(git subtree split --prefix=subdir --annotate="*") &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br &&
+		split_hash=$(git subtree split --prefix=subdir) &&
+		git subtree split --prefix=subdir --branch subproj-br &&
 		test_equal "$(git rev-parse subproj-br)" "$split_hash"
 	)
 '
@@ -441,7 +441,7 @@ test_expect_success 'make sure exactly the right set of files ends up in the sub
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count/subproj" sub3 &&
 	test_create_commit "$test_count" subdir/main-sub3 &&
@@ -452,12 +452,12 @@ test_expect_success 'make sure exactly the right set of files ends up in the sub
 	test_create_commit "$test_count/subproj" sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count" subdir/main-sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	(
 		cd "$test_count/subproj" &&
@@ -491,7 +491,7 @@ test_expect_success 'make sure the subproj *only* contains commits that affect t
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count/subproj" sub3 &&
 	test_create_commit "$test_count" subdir/main-sub3 &&
@@ -502,12 +502,12 @@ test_expect_success 'make sure the subproj *only* contains commits that affect t
 	test_create_commit "$test_count/subproj" sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count" subdir/main-sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	(
 		cd "$test_count/subproj" &&
@@ -541,7 +541,7 @@ test_expect_success 'make sure exactly the right set of files ends up in the mai
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count/subproj" sub3 &&
 	test_create_commit "$test_count" subdir/main-sub3 &&
@@ -552,12 +552,12 @@ test_expect_success 'make sure exactly the right set of files ends up in the mai
 	test_create_commit "$test_count/subproj" sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count" subdir/main-sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	(
 		cd "$test_count/subproj" &&
@@ -595,7 +595,7 @@ test_expect_success 'make sure each filename changed exactly once in the entire
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count/subproj" sub3 &&
 	test_create_commit "$test_count" subdir/main-sub3 &&
@@ -606,12 +606,12 @@ test_expect_success 'make sure each filename changed exactly once in the entire
 	test_create_commit "$test_count/subproj" sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count" subdir/main-sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	(
 		cd "$test_count/subproj" &&
@@ -653,7 +653,7 @@ test_expect_success 'make sure the --rejoin commits never make it into subproj'
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count/subproj" sub3 &&
 	test_create_commit "$test_count" subdir/main-sub3 &&
@@ -664,12 +664,12 @@ test_expect_success 'make sure the --rejoin commits never make it into subproj'
 	test_create_commit "$test_count/subproj" sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count" subdir/main-sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	(
 		cd "$test_count/subproj" &&
@@ -701,7 +701,7 @@ test_expect_success 'make sure no "git subtree" tagged commits make it into subp
 		cd "$test_count" &&
 		git fetch ./subproj master &&
 		git subtree merge --prefix=subdir FETCH_HEAD &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count/subproj" sub3 &&
 	test_create_commit "$test_count" subdir/main-sub3 &&
@@ -712,12 +712,12 @@ test_expect_success 'make sure no "git subtree" tagged commits make it into subp
 	test_create_commit "$test_count/subproj" sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	test_create_commit "$test_count" subdir/main-sub4 &&
 	(
 		cd "$test_count" &&
-		git subtree split --prefix=subdir --annotate="*" --branch subproj-br --rejoin
+		git subtree split --prefix=subdir --branch subproj-br --rejoin
 	) &&
 	(
 		cd "$test_count/subproj" &&
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 12/13] contrib/subtree: Handle '--prefix' argument with a slash appended
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: Techlive Zheng <techlivezheng@gmail.com>

'git subtree merge' will fail if the argument of '--prefix' has a slash
appended.

Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/git-subtree.sh     |    2 +-
 contrib/subtree/t/t7900-subtree.sh |   19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index c72af95..0493e47 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -82,7 +82,7 @@ while [ $# -gt 0 ]; do
 		--annotate) annotate="$1"; shift ;;
 		--no-annotate) annotate= ;;
 		-b) branch="$1"; shift ;;
-		-P) prefix="$1"; shift ;;
+		-P) prefix="${1%/}"; shift ;;
 		-m) message="$1"; shift ;;
 		--no-prefix) prefix= ;;
 		--onto) onto="$1"; shift ;;
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 769b116..1afd544 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -239,6 +239,25 @@ test_expect_success 'merge new subproj history into subdir/ with --squash and --
 	)
 '
 
+test_expect_success 'merge new subproj history into subdir/ with a slash appended to the argument of --prefix' '
+	test_create_repo "$test_count" &&
+	test_create_repo "$test_count/subproj" &&
+	test_create_commit "$test_count" main1 &&
+	test_create_commit "$test_count/subproj" sub1 &&
+	(
+		cd "$test_count" &&
+		git fetch ./subproj master &&
+		git subtree add --prefix=subdir/ FETCH_HEAD
+	) &&
+	test_create_commit "$test_count/subproj" sub2 &&
+	(
+		cd "$test_count" &&
+		git fetch ./subproj master &&
+		git subtree merge --prefix=subdir/ FETCH_HEAD &&
+		test_equal "$(last_commit_message)" "Merge commit '\''$(git rev-parse FETCH_HEAD)'\''"
+	)
+'
+
 #
 # Tests for 'git subtree split'
 #
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH 09/13] contrib/subtree: Ignore testing directory
From: Junio C Hamano @ 2013-02-05  4:36 UTC (permalink / raw)
  To: David A. Greene
  Cc: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani,
	"David A. Greene <greened
In-Reply-To: <1360037173-23291-10-git-send-email-greened@obbligato.org>

"David A. Greene" <greened@obbligato.org> writes:

> From: Techlive Zheng <techlivezheng@gmail.com>
>

Justification is missing here.

contrib/subtree/Makefile seems to do "rm -rf" on 'mainline' and
'subproj' in its "clean" target, which makes them look like
build/test artifacts that need to be ignored.

If they are no longer build/test artifacts, perhaps their removal
also need to be dropped from the Makefile?

> Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
>  contrib/subtree/.gitignore |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/subtree/.gitignore b/contrib/subtree/.gitignore
> index 91360a3..59aeeb4 100644
> --- a/contrib/subtree/.gitignore
> +++ b/contrib/subtree/.gitignore
> @@ -1,6 +1,5 @@
>  *~
>  git-subtree
> -git-subtree.xml
>  git-subtree.1
> -mainline
> -subproj
> +git-subtree.xml
> +t/trash\ directory.*

^ permalink raw reply

* Re: [PATCH 08/13] contrib/subtree: Add vim modeline
From: Junio C Hamano @ 2013-02-05  4:32 UTC (permalink / raw)
  To: David A. Greene
  Cc: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani,
	"David A. Greene <greened
In-Reply-To: <1360037173-23291-9-git-send-email-greened@obbligato.org>

"David A. Greene" <greened@obbligato.org> writes:

> From: Techlive Zheng <techlivezheng@gmail.com>
>
> Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
>  contrib/subtree/git-subtree.sh     |    2 ++
>  contrib/subtree/t/t7900-subtree.sh |    2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 6c3929b..c72af95 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -722,3 +722,5 @@ cmd_push()
>  }
>  
>  "cmd_$command" "$@"
> +
> +# vim: set ts=4 sw=4 noet
> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
> index e6a3702..e6bcd50 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -466,3 +466,5 @@ test_expect_success 'verify one file change per commit' '
>  '
>  
>  test_done
> +
> +# vim: set ts=4 sw=4 noet

Please don't.

^ permalink raw reply

* [PATCH 08/13] contrib/subtree: Add vim modeline
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: Techlive Zheng <techlivezheng@gmail.com>

Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/git-subtree.sh     |    2 ++
 contrib/subtree/t/t7900-subtree.sh |    2 ++
 2 files changed, 4 insertions(+)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 6c3929b..c72af95 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -722,3 +722,5 @@ cmd_push()
 }
 
 "cmd_$command" "$@"
+
+# vim: set ts=4 sw=4 noet
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index e6a3702..e6bcd50 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -466,3 +466,5 @@ test_expect_success 'verify one file change per commit' '
 '
 
 test_done
+
+# vim: set ts=4 sw=4 noet
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 09/13] contrib/subtree: Ignore testing directory
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: Techlive Zheng <techlivezheng@gmail.com>

Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/.gitignore |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/contrib/subtree/.gitignore b/contrib/subtree/.gitignore
index 91360a3..59aeeb4 100644
--- a/contrib/subtree/.gitignore
+++ b/contrib/subtree/.gitignore
@@ -1,6 +1,5 @@
 *~
 git-subtree
-git-subtree.xml
 git-subtree.1
-mainline
-subproj
+git-subtree.xml
+t/trash\ directory.*
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 07/13] contrib/subtree: Fix whitespaces
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: Techlive Zheng <techlivezheng@gmail.com>

Previous code does not fulfill Git's whitespace policy.

Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/git-subtree.sh     |   68 ++++----
 contrib/subtree/git-subtree.txt    |   42 ++---
 contrib/subtree/t/t7900-subtree.sh |  314 ++++++++++++++++++------------------
 3 files changed, 212 insertions(+), 212 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 8a23f58..6c3929b 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -5,7 +5,7 @@
 # Copyright (C) 2009 Avery Pennarun <apenwarr@gmail.com>
 #
 if [ $# -eq 0 ]; then
-    set -- -h
+	set -- -h
 fi
 OPTS_SPEC="\
 git subtree add   --prefix=<prefix> <commit>
@@ -111,9 +111,9 @@ if [ -z "$prefix" ]; then
 fi
 
 case "$command" in
-	add) [ -e "$prefix" ] && 
+	add) [ -e "$prefix" ] &&
 		die "prefix '$prefix' already exists." ;;
-	*)   [ -e "$prefix" ] || 
+	*)   [ -e "$prefix" ] ||
 		die "'$prefix' does not exist; use 'git subtree add'" ;;
 esac
 
@@ -182,8 +182,8 @@ cache_set()
 	oldrev="$1"
 	newrev="$2"
 	if [ "$oldrev" != "latest_old" \
-	     -a "$oldrev" != "latest_new" \
-	     -a -e "$cachedir/$oldrev" ]; then
+		-a "$oldrev" != "latest_new" \
+		-a -e "$cachedir/$oldrev" ]; then
 		die "cache for $oldrev already exists!"
 	fi
 	echo "$newrev" >"$cachedir/$oldrev"
@@ -305,7 +305,7 @@ copy_commit()
 		read GIT_COMMITTER_NAME
 		read GIT_COMMITTER_EMAIL
 		read GIT_COMMITTER_DATE
-		export  GIT_AUTHOR_NAME \
+		export GIT_AUTHOR_NAME \
 			GIT_AUTHOR_EMAIL \
 			GIT_AUTHOR_DATE \
 			GIT_COMMITTER_NAME \
@@ -328,7 +328,7 @@ add_msg()
 	fi
 	cat <<-EOF
 		$commit_message
-		
+
 		git-subtree-dir: $dir
 		git-subtree-mainline: $latest_old
 		git-subtree-split: $latest_new
@@ -356,7 +356,7 @@ rejoin_msg()
 	fi
 	cat <<-EOF
 		$commit_message
-		
+
 		git-subtree-dir: $dir
 		git-subtree-mainline: $latest_old
 		git-subtree-split: $latest_new
@@ -369,7 +369,7 @@ squash_msg()
 	oldsub="$2"
 	newsub="$3"
 	newsub_short=$(git rev-parse --short "$newsub")
-	
+
 	if [ -n "$oldsub" ]; then
 		oldsub_short=$(git rev-parse --short "$oldsub")
 		echo "Squashed '$dir/' changes from $oldsub_short..$newsub_short"
@@ -379,7 +379,7 @@ squash_msg()
 	else
 		echo "Squashed '$dir/' content from commit $newsub_short"
 	fi
-	
+
 	echo
 	echo "git-subtree-dir: $dir"
 	echo "git-subtree-split: $newsub"
@@ -428,7 +428,7 @@ new_squash_commit()
 	newsub="$3"
 	tree=$(toptree_for_commit $newsub) || exit $?
 	if [ -n "$old" ]; then
-		squash_msg "$dir" "$oldsub" "$newsub" | 
+		squash_msg "$dir" "$oldsub" "$newsub" |
 			git commit-tree "$tree" -p "$old" || exit $?
 	else
 		squash_msg "$dir" "" "$newsub" |
@@ -456,7 +456,7 @@ copy_or_skip()
 		else
 			nonidentical="$parent"
 		fi
-		
+
 		# sometimes both old parents map to the same newparent;
 		# eliminate duplicates
 		is_new=1
@@ -471,7 +471,7 @@ copy_or_skip()
 			p="$p -p $parent"
 		fi
 	done
-	
+
 	if [ -n "$identical" ]; then
 		echo $identical
 	else
@@ -496,7 +496,7 @@ cmd_add()
 	fi
 
 	ensure_clean
-	
+
 	if [ $# -eq 1 ]; then
 	    git rev-parse -q --verify "$1^{commit}" >/dev/null ||
 	    die "'$1' does not refer to a commit"
@@ -513,8 +513,8 @@ cmd_add()
 
 	    "cmd_add_repository" "$@"
 	else
-	    say "error: parameters were '$@'"
-	    die "Provide either a commit or a repository and commit."
+		say "error: parameters were '$@'"
+		die "Provide either a commit or a repository and commit."
 	fi
 }
 
@@ -534,19 +534,19 @@ cmd_add_commit()
 	revs=$(git rev-parse $default --revs-only "$@") || exit $?
 	set -- $revs
 	rev="$1"
-	
+
 	debug "Adding $dir as '$rev'..."
 	git read-tree --prefix="$dir" $rev || exit $?
 	git checkout -- "$dir" || exit $?
 	tree=$(git write-tree) || exit $?
-	
+
 	headrev=$(git rev-parse HEAD) || exit $?
 	if [ -n "$headrev" -a "$headrev" != "$rev" ]; then
 		headp="-p $headrev"
 	else
 		headp=
 	fi
-	
+
 	if [ -n "$squash" ]; then
 		rev=$(new_squash_commit "" "" "$rev") || exit $?
 		commit=$(add_squashed_msg "$rev" "$dir" |
@@ -556,7 +556,7 @@ cmd_add_commit()
 			 git commit-tree $tree $headp -p "$rev") || exit $?
 	fi
 	git reset "$commit" || exit $?
-	
+
 	say "Added dir '$dir'"
 }
 
@@ -564,7 +564,7 @@ cmd_split()
 {
 	debug "Splitting $dir..."
 	cache_setup || exit $?
-	
+
 	if [ -n "$onto" ]; then
 		debug "Reading history for --onto=$onto..."
 		git rev-list $onto |
@@ -575,13 +575,13 @@ cmd_split()
 			cache_set $rev $rev
 		done
 	fi
-	
+
 	if [ -n "$ignore_joins" ]; then
 		unrevs=
 	else
 		unrevs="$(find_existing_splits "$dir" "$revs")"
 	fi
-	
+
 	# We can't restrict rev-list to only $dir here, because some of our
 	# parents have the $dir contents the root, and those won't match.
 	# (and rev-list --follow doesn't seem to solve this)
@@ -603,12 +603,12 @@ cmd_split()
 		debug "  parents: $parents"
 		newparents=$(cache_get $parents)
 		debug "  newparents: $newparents"
-		
+
 		tree=$(subtree_for_commit $rev "$dir")
 		debug "  tree is: $tree"
 
 		check_parents $parents
-		
+
 		# ugly.  is there no better way to tell if this is a subtree
 		# vs. a mainline commit?  Does it matter?
 		if [ -z $tree ]; then
@@ -629,7 +629,7 @@ cmd_split()
 	if [ -z "$latest_new" ]; then
 		die "No new revisions were found"
 	fi
-	
+
 	if [ -n "$rejoin" ]; then
 		debug "Merging split branch into HEAD..."
 		latest_old=$(cache_get latest_old)
@@ -657,13 +657,13 @@ cmd_merge()
 {
 	revs=$(git rev-parse $default --revs-only "$@") || exit $?
 	ensure_clean
-	
+
 	set -- $revs
 	if [ $# -ne 1 ]; then
 		die "You must provide exactly one revision.  Got: '$revs'"
 	fi
 	rev="$1"
-	
+
 	if [ -n "$squash" ]; then
 		first_split="$(find_latest_squash "$dir")"
 		if [ -z "$first_split" ]; then
@@ -709,15 +709,15 @@ cmd_pull()
 cmd_push()
 {
 	if [ $# -ne 2 ]; then
-	    die "You must provide <repository> <refspec>"
+		die "You must provide <repository> <refspec>"
 	fi
 	if [ -e "$dir" ]; then
-	    repository=$1
-	    refspec=$2
-	    echo "git push using: " $repository $refspec
-	    git push $repository $(git subtree split --prefix=$prefix):refs/heads/$refspec
+		repository=$1
+		refspec=$2
+		echo "git push using: " $repository $refspec
+		git push $repository $(git subtree split --prefix=$prefix):refs/heads/$refspec
 	else
-	    die "'$dir' must already exist. Try 'git subtree add'."
+		die "'$dir' must already exist. Try 'git subtree add'."
 	fi
 }
 
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 7ba853e..aae9d19 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -87,15 +87,15 @@ merge::
 	go back in time from v2.5 to v2.4, for example.  If your
 	merge introduces a conflict, you can resolve it in the
 	usual ways.
-	
+
 pull::
 	Exactly like 'merge', but parallels 'git pull' in that
 	it fetches the given commit from the specified remote
 	repository.
-	
+
 push::
 	Does a 'split' (see below) using the <prefix> supplied
-	and then does a 'git push' to push the result to the 
+	and then does a 'git push' to push the result to the
 	repository and refspec. This can be used to push your
 	subtree to different branches of the remote repository.
 
@@ -107,19 +107,19 @@ split::
 	contents of <prefix> at the root of the project instead
 	of in a subdirectory.  Thus, the newly created history
 	is suitable for export as a separate git repository.
-	
+
 	After splitting successfully, a single commit id is
 	printed to stdout.  This corresponds to the HEAD of the
 	newly created tree, which you can manipulate however you
 	want.
-	
+
 	Repeated splits of exactly the same history are
 	guaranteed to be identical (ie. to produce the same
 	commit ids).  Because of this, if you add new commits
 	and then re-split, the new commits will be attached as
 	commits on top of the history you generated last time,
 	so 'git merge' and friends will work as expected.
-	
+
 	Note that if you use '--squash' when you merge, you
 	should usually not just '--rejoin' when you split.
 
@@ -156,26 +156,26 @@ OPTIONS FOR add, merge, push, pull
 	project, produce only a single commit that contains all
 	the differences you want to merge, and then merge that
 	new commit into your project.
-	
+
 	Using this option helps to reduce log clutter. People
 	rarely want to see every change that happened between
 	v1.0 and v1.1 of the library they're using, since none of the
 	interim versions were ever included in their application.
-	
+
 	Using '--squash' also helps avoid problems when the same
 	subproject is included multiple times in the same
 	project, or is removed and then re-added.  In such a
 	case, it doesn't make sense to combine the histories
 	anyway, since it's unclear which part of the history
 	belongs to which subtree.
-	
+
 	Furthermore, with '--squash', you can switch back and
 	forth between different versions of a subtree, rather
 	than strictly forward.  'git subtree merge --squash'
 	always adjusts the subtree to match the exactly
 	specified commit, even if getting to that commit would
 	require undoing some changes that were added earlier.
-	
+
 	Whether or not you use '--squash', changes made in your
 	local repository remain intact and can be later split
 	and send upstream to the subproject.
@@ -191,11 +191,11 @@ OPTIONS FOR split
 	commits with the same commit message, but possibly
 	different content, from the original commits, this can help
 	to differentiate them and avoid confusion.
-	
+
 	Whenever you split, you need to use the same
 	<annotation>, or else you don't have a guarantee that
 	the new re-created history will be identical to the old
-	one.  That will prevent merging from working correctly. 
+	one.  That will prevent merging from working correctly.
 	git subtree tries to make it work anyway, particularly
 	if you use --rejoin, but it may not always be effective.
 
@@ -204,8 +204,8 @@ OPTIONS FOR split
 	This option is only valid for the split command.
 
 	After generating the synthetic history, create a new
-	branch called <branch> that contains the new history. 
-	This is suitable for immediate pushing upstream. 
+	branch called <branch> that contains the new history.
+	This is suitable for immediate pushing upstream.
 	<branch> must not already exist.
 
 --ignore-joins::
@@ -228,7 +228,7 @@ OPTIONS FOR split
 	revision of the subproject's history that was imported
 	into your project, and git subtree will attempt to build
 	its history from there.
-	
+
 	If you used 'git subtree add', you should never need
 	this option.
 
@@ -239,18 +239,18 @@ OPTIONS FOR split
 	history back into your main project.  That way, future
 	splits can search only the part of history that has
 	been added since the most recent --rejoin.
-	
+
 	If your split commits end up merged into the upstream
 	subproject, and then you want to get the latest upstream
 	version, this will allow git's merge algorithm to more
 	intelligently avoid conflicts (since it knows these
 	synthetic commits are already part of the upstream
 	repository).
-	
+
 	Unfortunately, using this option results in 'git log'
 	showing an extra copy of every new commit that was
 	created (the original, and the synthetic one).
-	
+
 	If you do all your merges with '--squash', don't use
 	'--rejoin' when you split, because you don't want the
 	subproject's history to be part of your project anyway.
@@ -285,7 +285,7 @@ First, get your own copy of the git.git repository:
 	$ cd test-git
 
 gitweb (commit 1130ef3) was merged into git as of commit
-0a8f4f0, after which it was no longer maintained separately. 
+0a8f4f0, after which it was no longer maintained separately.
 But imagine it had been maintained separately, and we wanted to
 extract git's changes to gitweb since that time, to share with
 the upstream.  You could do this:
@@ -295,7 +295,7 @@ the upstream.  You could do this:
         	--branch gitweb-latest
         $ gitk gitweb-latest
         $ git push git@github.com:whatever/gitweb.git gitweb-latest:master
-        
+
 (We use '0a8f4f0^..' because that means "all the changes from
 0a8f4f0 to the current version, including 0a8f4f0 itself.")
 
@@ -329,7 +329,7 @@ And fast forward again:
 	$ git subtree merge --prefix=gitweb --squash gitweb-latest
 
 And notice that your change is still intact:
-	
+
 	$ ls -l gitweb/myfile
 
 And you can split it out and look at your changes versus
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 3f17f55..e6a3702 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -23,7 +23,7 @@ check_equal()
 {
 	test_debug 'echo'
 	test_debug "echo \"check a:\" \"{$1}\""
-	test_debug "echo \"      b:\" \"{$2}\""
+	test_debug "echo \"		 b:\" \"{$2}\""
 	if [ "$1" = "$2" ]; then
 		return 0
 	else
@@ -61,17 +61,17 @@ last_commit_message()
 }
 
 test_expect_success 'init subproj' '
-        test_create_repo subproj
+		test_create_repo subproj
 '
 
 # To the subproject!
 cd subproj
 
 test_expect_success 'add sub1' '
-        create sub1 &&
-        git commit -m "sub1" &&
-        git branch sub1 &&
-        git branch -m master subproj
+		create sub1 &&
+		git commit -m "sub1" &&
+		git branch sub1 &&
+		git branch -m master subproj
 '
 
 # Save this hash for testing later.
@@ -79,140 +79,140 @@ test_expect_success 'add sub1' '
 subdir_hash=`git rev-parse HEAD`
 
 test_expect_success 'add sub2' '
-        create sub2 &&
-        git commit -m "sub2" &&
-        git branch sub2
+		create sub2 &&
+		git commit -m "sub2" &&
+		git branch sub2
 '
 
 test_expect_success 'add sub3' '
-        create sub3 &&
-        git commit -m "sub3" &&
-        git branch sub3
+		create sub3 &&
+		git commit -m "sub3" &&
+		git branch sub3
 '
 
 # Back to mainline
 cd ..
 
 test_expect_success 'add main4' '
-        create main4 &&
-        git commit -m "main4" &&
-        git branch -m master mainline &&
-        git branch subdir
+		create main4 &&
+		git commit -m "main4" &&
+		git branch -m master mainline &&
+		git branch subdir
 '
 
 test_expect_success 'fetch subproj history' '
-        git fetch ./subproj sub1 &&
-        git branch sub1 FETCH_HEAD
+		git fetch ./subproj sub1 &&
+		git branch sub1 FETCH_HEAD
 '
 
 test_expect_success 'no subtree exists in main tree' '
-        test_must_fail git subtree merge --prefix=subdir sub1
+		test_must_fail git subtree merge --prefix=subdir sub1
 '
 
 test_expect_success 'no pull from non-existant subtree' '
-        test_must_fail git subtree pull --prefix=subdir ./subproj sub1
+		test_must_fail git subtree pull --prefix=subdir ./subproj sub1
 '
 
 test_expect_success 'check if --message works for add' '
-        git subtree add --prefix=subdir --message="Added subproject" sub1 &&
-        check_equal ''"$(last_commit_message)"'' "Added subproject" &&
-        undo
+		git subtree add --prefix=subdir --message="Added subproject" sub1 &&
+		check_equal ''"$(last_commit_message)"'' "Added subproject" &&
+		undo
 '
 
 test_expect_success 'check if --message works as -m and --prefix as -P' '
-        git subtree add -P subdir -m "Added subproject using git subtree" sub1 &&
-        check_equal ''"$(last_commit_message)"'' "Added subproject using git subtree" &&
-        undo
+		git subtree add -P subdir -m "Added subproject using git subtree" sub1 &&
+		check_equal ''"$(last_commit_message)"'' "Added subproject using git subtree" &&
+		undo
 '
 
 test_expect_success 'check if --message works with squash too' '
-        git subtree add -P subdir -m "Added subproject with squash" --squash sub1 &&
-        check_equal ''"$(last_commit_message)"'' "Added subproject with squash" &&
-        undo
+		git subtree add -P subdir -m "Added subproject with squash" --squash sub1 &&
+		check_equal ''"$(last_commit_message)"'' "Added subproject with squash" &&
+		undo
 '
 
 test_expect_success 'add subproj to mainline' '
-        git subtree add --prefix=subdir/ FETCH_HEAD &&
-        check_equal ''"$(last_commit_message)"'' "Add '"'subdir/'"' from commit '"'"'''"$(git rev-parse sub1)"'''"'"'"
+		git subtree add --prefix=subdir/ FETCH_HEAD &&
+		check_equal ''"$(last_commit_message)"'' "Add '"'subdir/'"' from commit '"'"'''"$(git rev-parse sub1)"'''"'"'"
 '
 
 # this shouldn't actually do anything, since FETCH_HEAD is already a parent
 test_expect_success 'merge fetched subproj' '
-        git merge -m "merge -s -ours" -s ours FETCH_HEAD
+		git merge -m "merge -s -ours" -s ours FETCH_HEAD
 '
 
 test_expect_success 'add main-sub5' '
-        create subdir/main-sub5 &&
-        git commit -m "main-sub5"
+		create subdir/main-sub5 &&
+		git commit -m "subproj: main-sub5"
 '
 
 test_expect_success 'add main6' '
-        create main6 &&
-        git commit -m "main6 boring"
+		create main6 &&
+		git commit -m "main6 boring"
 '
 
 test_expect_success 'add main-sub7' '
-        create subdir/main-sub7 &&
-        git commit -m "main-sub7"
+		create subdir/main-sub7 &&
+		git commit -m "subproj: main-sub7"
 '
 
 test_expect_success 'fetch new subproj history' '
-        git fetch ./subproj sub2 &&
-        git branch sub2 FETCH_HEAD
+		git fetch ./subproj sub2 &&
+		git branch sub2 FETCH_HEAD
 '
 
 test_expect_success 'check if --message works for merge' '
-        git subtree merge --prefix=subdir -m "Merged changes from subproject" sub2 &&
-        check_equal ''"$(last_commit_message)"'' "Merged changes from subproject" &&
-        undo
+		git subtree merge --prefix=subdir -m "Merged changes from subproject" sub2 &&
+		check_equal ''"$(last_commit_message)"'' "Merged changes from subproject" &&
+		undo
 '
 
 test_expect_success 'check if --message for merge works with squash too' '
-        git subtree merge --prefix subdir -m "Merged changes from subproject using squash" --squash sub2 &&
-        check_equal ''"$(last_commit_message)"'' "Merged changes from subproject using squash" &&
-        undo
+		git subtree merge --prefix subdir -m "Merged changes from subproject using squash" --squash sub2 &&
+		check_equal ''"$(last_commit_message)"'' "Merged changes from subproject using squash" &&
+		undo
 '
 
 test_expect_success 'merge new subproj history into subdir' '
-        git subtree merge --prefix=subdir FETCH_HEAD &&
-        git branch pre-split &&
-        check_equal ''"$(last_commit_message)"'' "Merge commit '"'"'"$(git rev-parse sub2)"'"'"' into mainline"
+		git subtree merge --prefix=subdir FETCH_HEAD &&
+		git branch pre-split &&
+		check_equal ''"$(last_commit_message)"'' "Merge commit '"'"'"$(git rev-parse sub2)"'"'"' into mainline"
 '
 
 test_expect_success 'Check that prefix argument is required for split' '
-        echo "You must provide the --prefix option." > expected &&
-        test_must_fail git subtree split > actual 2>&1 &&
-        test_debug "echo -n expected: " &&
-        test_debug "cat expected" &&
-        test_debug "echo -n actual: " &&
-        test_debug "cat actual" &&
-        test_cmp expected actual &&
-        rm -f expected actual
+		echo "You must provide the --prefix option." > expected &&
+		test_must_fail git subtree split > actual 2>&1 &&
+		test_debug "echo -n expected: " &&
+		test_debug "cat expected" &&
+		test_debug "echo -n actual: " &&
+		test_debug "cat actual" &&
+		test_cmp expected actual &&
+		rm -f expected actual
 '
 
 test_expect_success 'Check that the <prefix> exists for a split' '
-        echo "'"'"'non-existent-directory'"'"'" does not exist\; use "'"'"'git subtree add'"'"'" > expected &&
-        test_must_fail git subtree split --prefix=non-existent-directory > actual 2>&1 &&
-        test_debug "echo -n expected: " &&
-        test_debug "cat expected" &&
-        test_debug "echo -n actual: " &&
-        test_debug "cat actual" &&
-        test_cmp expected actual
-#        rm -f expected actual
+		echo "'"'"'non-existent-directory'"'"'" does not exist\; use "'"'"'git subtree add'"'"'" > expected &&
+		test_must_fail git subtree split --prefix=non-existent-directory > actual 2>&1 &&
+		test_debug "echo -n expected: " &&
+		test_debug "cat expected" &&
+		test_debug "echo -n actual: " &&
+		test_debug "cat actual" &&
+		test_cmp expected actual
+#		 rm -f expected actual
 '
 
 test_expect_success 'check if --message works for split+rejoin' '
-        spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
-        git branch spl1 "$spl1" &&
-        check_equal ''"$(last_commit_message)"'' "Split & rejoin" &&
-        undo
+		spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
+		git branch spl1 "$spl1" &&
+		check_equal ''"$(last_commit_message)"'' "Split & rejoin" &&
+		undo
 '
 
 test_expect_success 'check split with --branch' '
-        spl1=$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin) &&
-        undo &&
-        git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --branch splitbr1 &&
-        check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
+		spl1=$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin) &&
+		undo &&
+		git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --branch splitbr1 &&
+		check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
 '
 
 test_expect_success 'check hash of split' '
@@ -227,69 +227,69 @@ test_expect_success 'check hash of split' '
 '
 
 test_expect_success 'check split with --branch for an existing branch' '
-        spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
-        undo &&
-        git branch splitbr2 sub1 &&
-        git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --branch splitbr2 &&
-        check_equal ''"$(git rev-parse splitbr2)"'' "$spl1"
+		spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
+		undo &&
+		git branch splitbr2 sub1 &&
+		git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --branch splitbr2 &&
+		check_equal ''"$(git rev-parse splitbr2)"'' "$spl1"
 '
 
 test_expect_success 'check split with --branch for an incompatible branch' '
-        test_must_fail git subtree split --prefix subdir --onto FETCH_HEAD --branch subdir
+		test_must_fail git subtree split --prefix subdir --onto FETCH_HEAD --branch subdir
 '
 
 test_expect_success 'check split+rejoin' '
-        spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
-        undo &&
-        git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --rejoin &&
-        check_equal ''"$(last_commit_message)"'' "Split '"'"'subdir/'"'"' into commit '"'"'"$spl1"'"'"'"
+		spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
+		undo &&
+		git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --rejoin &&
+		check_equal ''"$(last_commit_message)"'' "Split '"'"'subdir/'"'"' into commit '"'"'"$spl1"'"'"'"
 '
 
 test_expect_success 'add main-sub8' '
-        create subdir/main-sub8 &&
-        git commit -m "main-sub8"
+		create subdir/main-sub8 &&
+		git commit -m "main-sub8"
 '
 
 # To the subproject!
 cd ./subproj
 
 test_expect_success 'merge split into subproj' '
-        git fetch .. spl1 &&
-        git branch spl1 FETCH_HEAD &&
-        git merge FETCH_HEAD
+		git fetch .. spl1 &&
+		git branch spl1 FETCH_HEAD &&
+		git merge FETCH_HEAD
 '
 
 test_expect_success 'add sub9' '
-        create sub9 &&
-        git commit -m "sub9"
+		create sub9 &&
+		git commit -m "sub9"
 '
 
 # Back to mainline
 cd ..
 
 test_expect_success 'split for sub8' '
-        split2=''"$(git subtree split --annotate='"'*'"' --prefix subdir/ --rejoin)"''
-        git branch split2 "$split2"
+		split2=''"$(git subtree split --annotate='"'*'"' --prefix subdir/ --rejoin)"''
+		git branch split2 "$split2"
 '
 
 test_expect_success 'add main-sub10' '
-        create subdir/main-sub10 &&
-        git commit -m "main-sub10"
+		create subdir/main-sub10 &&
+		git commit -m "main-sub10"
 '
 
 test_expect_success 'split for sub10' '
-        spl3=''"$(git subtree split --annotate='"'*'"' --prefix subdir --rejoin)"'' &&
-        git branch spl3 "$spl3"
+		spl3=''"$(git subtree split --annotate='"'*'"' --prefix subdir --rejoin)"'' &&
+		git branch spl3 "$spl3"
 '
 
 # To the subproject!
 cd ./subproj
 
 test_expect_success 'merge split into subproj' '
-        git fetch .. spl3 &&
-        git branch spl3 FETCH_HEAD &&
-        git merge FETCH_HEAD &&
-        git branch subproj-merge-spl3
+		git fetch .. spl3 &&
+		git branch spl3 FETCH_HEAD &&
+		git merge FETCH_HEAD &&
+		git branch subproj-merge-spl3
 '
 
 chkm="main4 main6"
@@ -299,44 +299,44 @@ chks="sub1 sub2 sub3 sub9"
 chks_sub=$(echo $chks | multiline | sed 's,^,subdir/,' | fixnl)
 
 test_expect_success 'make sure exactly the right set of files ends up in the subproj' '
-        subfiles=''"$(git ls-files | fixnl)"'' &&
-        check_equal "$subfiles" "$chkms $chks"
+		subfiles=''"$(git ls-files | fixnl)"'' &&
+		check_equal "$subfiles" "$chkms $chks"
 '
 
 test_expect_success 'make sure the subproj history *only* contains commits that affect the subdir' '
-        allchanges=''"$(git log --name-only --pretty=format:'"''"' | sort | fixnl)"'' &&
-        check_equal "$allchanges" "$chkms $chks"
+		allchanges=''"$(git log --name-only --pretty=format:'"''"' | sort | fixnl)"'' &&
+		check_equal "$allchanges" "$chkms $chks"
 '
 
 # Back to mainline
 cd ..
 
 test_expect_success 'pull from subproj' '
-        git fetch ./subproj subproj-merge-spl3 &&
-        git branch subproj-merge-spl3 FETCH_HEAD &&
-        git subtree pull --prefix=subdir ./subproj subproj-merge-spl3
+		git fetch ./subproj subproj-merge-spl3 &&
+		git branch subproj-merge-spl3 FETCH_HEAD &&
+		git subtree pull --prefix=subdir ./subproj subproj-merge-spl3
 '
 
 test_expect_success 'make sure exactly the right set of files ends up in the mainline' '
-        mainfiles=''"$(git ls-files | fixnl)"'' &&
-        check_equal "$mainfiles" "$chkm $chkms_sub $chks_sub"
+		mainfiles=''"$(git ls-files | fixnl)"'' &&
+		check_equal "$mainfiles" "$chkm $chkms_sub $chks_sub"
 '
 
 test_expect_success 'make sure each filename changed exactly once in the entire history' '
-        # main-sub?? and /subdir/main-sub?? both change, because those are the
-        # changes that were split into their own history.  And subdir/sub?? never
-        # change, since they were *only* changed in the subtree branch.
-        allchanges=''"$(git log --name-only --pretty=format:'"''"' | sort | fixnl)"'' &&
-        check_equal "$allchanges" ''"$(echo $chkms $chkm $chks $chkms_sub | multiline | sort | fixnl)"''
+		# main-sub?? and /subdir/main-sub?? both change, because those are the
+		# changes that were split into their own history.  And subdir/sub?? never
+		# change, since they were *only* changed in the subtree branch.
+		allchanges=''"$(git log --name-only --pretty=format:'"''"' | sort | fixnl)"'' &&
+		check_equal "$allchanges" ''"$(echo $chkms $chkm $chks $chkms_sub | multiline | sort | fixnl)"''
 '
 
 test_expect_success 'make sure the --rejoin commits never make it into subproj' '
-        check_equal ''"$(git log --pretty=format:'"'%s'"' HEAD^2 | grep -i split)"'' ""
+		check_equal ''"$(git log --pretty=format:'"'%s'"' HEAD^2 | grep -i split)"'' ""
 '
 
 test_expect_success 'make sure no "git subtree" tagged commits make it into subproj' '
-        # They are meaningless to subproj since one side of the merge refers to the mainline
-        check_equal ''"$(git log --pretty=format:'"'%s%n%b'"' HEAD^2 | grep "git-subtree.*:")"'' ""
+		# They are meaningless to subproj since one side of the merge refers to the mainline
+		check_equal ''"$(git log --pretty=format:'"'%s%n%b'"' HEAD^2 | grep "git-subtree.*:")"'' ""
 '
 
 # prepare second pair of repositories
@@ -344,27 +344,27 @@ mkdir test2
 cd test2
 
 test_expect_success 'init main' '
-        test_create_repo main
+		test_create_repo main
 '
 
 cd main
 
 test_expect_success 'add main1' '
-        create main1 &&
-        git commit -m "main1"
+		create main1 &&
+		git commit -m "main1"
 '
 
 cd ..
 
 test_expect_success 'init sub' '
-        test_create_repo sub
+		test_create_repo sub
 '
 
 cd sub
 
 test_expect_success 'add sub2' '
-        create sub2 &&
-        git commit -m "sub2"
+		create sub2 &&
+		git commit -m "sub2"
 '
 
 cd ../main
@@ -372,33 +372,33 @@ cd ../main
 # check if split can find proper base without --onto
 
 test_expect_success 'add sub as subdir in main' '
-        git fetch ../sub master &&
-        git branch sub2 FETCH_HEAD &&
-        git subtree add --prefix subdir sub2
+		git fetch ../sub master &&
+		git branch sub2 FETCH_HEAD &&
+		git subtree add --prefix subdir sub2
 '
 
 cd ../sub
 
 test_expect_success 'add sub3' '
-        create sub3 &&
-        git commit -m "sub3"
+		create sub3 &&
+		git commit -m "sub3"
 '
 
 cd ../main
 
 test_expect_success 'merge from sub' '
-        git fetch ../sub master &&
-        git branch sub3 FETCH_HEAD &&
-        git subtree merge --prefix subdir sub3
+		git fetch ../sub master &&
+		git branch sub3 FETCH_HEAD &&
+		git subtree merge --prefix subdir sub3
 '
 
 test_expect_success 'add main-sub4' '
-        create subdir/main-sub4 &&
-        git commit -m "main-sub4"
+		create subdir/main-sub4 &&
+		git commit -m "main-sub4"
 '
 
 test_expect_success 'split for main-sub4 without --onto' '
-        git subtree split --prefix subdir --branch mainsub4
+		git subtree split --prefix subdir --branch mainsub4
 '
 
 # at this point, the new commit parent should be sub3 if it is not,
@@ -407,21 +407,21 @@ test_expect_success 'split for main-sub4 without --onto' '
 # itself)
 
 test_expect_success 'check that the commit parent is sub3' '
-        check_equal ''"$(git log --pretty=format:%P -1 mainsub4)"'' ''"$(git rev-parse sub3)"''
+		check_equal ''"$(git log --pretty=format:%P -1 mainsub4)"'' ''"$(git rev-parse sub3)"''
 '
 
 test_expect_success 'add main-sub5' '
-        mkdir subdir2 &&
-        create subdir2/main-sub5 &&
-        git commit -m "main-sub5"
+		mkdir subdir2 &&
+		create subdir2/main-sub5 &&
+		git commit -m "main-sub5"
 '
 
 test_expect_success 'split for main-sub5 without --onto' '
-        # also test that we still can split out an entirely new subtree
-        # if the parent of the first commit in the tree is not empty,
-        # then the new subtree has accidently been attached to something
-        git subtree split --prefix subdir2 --branch mainsub5 &&
-        check_equal ''"$(git log --pretty=format:%P -1 mainsub5)"'' ""
+		# also test that we still can split out an entirely new subtree
+		# if the parent of the first commit in the tree is not empty,
+		# then the new subtree has accidently been attached to something
+		git subtree split --prefix subdir2 --branch mainsub5 &&
+		check_equal ''"$(git log --pretty=format:%P -1 mainsub5)"'' ""
 '
 
 # make sure no patch changes more than one file.  The original set of commits
@@ -449,20 +449,20 @@ joincommits()
 }
 
 test_expect_success 'verify one file change per commit' '
-        x= &&
-        list=''"$(git log --pretty=format:'"'commit: %H'"' | joincommits)"'' &&
-#        test_debug "echo HERE" &&
-#        test_debug "echo ''"$list"''" &&
-        (git log --pretty=format:'"'commit: %H'"' | joincommits |
-        (       while read commit a b; do
-		        test_debug "echo Verifying commit "''"$commit"''
-		        test_debug "echo a: "''"$a"''
-		        test_debug "echo b: "''"$b"''
-		        check_equal "$b" ""
-		        x=1
-	        done
-	        check_equal "$x" 1
-        ))
+		x= &&
+		list=''"$(git log --pretty=format:'"'commit: %H'"' | joincommits)"'' &&
+#		 test_debug "echo HERE" &&
+#		 test_debug "echo ''"$list"''" &&
+		(git log --pretty=format:'"'commit: %H'"' | joincommits |
+		(		while read commit a b; do
+				test_debug "echo Verifying commit "''"$commit"''
+				test_debug "echo a: "''"$a"''
+				test_debug "echo b: "''"$b"''
+				check_equal "$b" ""
+				x=1
+			done
+			check_equal "$x" 1
+		))
 '
 
 test_done
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 03/13] contrib/subtree: Better Error Handling for add
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: "David A. Greene" <greened@obbligato.org>

Check refspecs for validity before passing them on to other commands.
This lets us generate more helpful error messages.

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/git-subtree.sh |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 5598210..771f39d 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -497,12 +497,18 @@ cmd_add()
 	ensure_clean
 	
 	if [ $# -eq 1 ]; then
-		"cmd_add_commit" "$@"
+	    git rev-parse -q --verify "$1^{commit}" >/dev/null ||
+	    die "'$1' does not refer to a commit"
+
+	    "cmd_add_commit" "$@"
 	elif [ $# -eq 2 ]; then
-		"cmd_add_repository" "$@"
+	    git rev-parse -q --verify "$2^{commit}" >/dev/null ||
+	    die "'$2' does not refer to a commit"
+
+	    "cmd_add_repository" "$@"
 	else
 	    say "error: parameters were '$@'"
-	    die "Provide either a refspec or a repository and refspec."
+	    die "Provide either a commit or a repository and commit."
 	fi
 }
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 06/13] contrib/subtree: Make the Manual Directory if Needed
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: Jesper L. Nielsen, David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: "Jesper L. Nielsen" <lyager@gmail.com>

Before install git-subtree documentation, make sure the manpage
directory exists.

Signed-off-by: Jesper L. Nielsen <lyager@gmail.com>
Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/Makefile |    1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 36ae3e4..b507505 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -35,6 +35,7 @@ install: $(GIT_SUBTREE)
 install-doc: install-man
 
 install-man: $(GIT_SUBTREE_DOC)
+	$(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
 	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
 $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 05/13] contrib/subtree: Honor DESTDIR
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: Adam Tkac, David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: Adam Tkac <atkac@redhat.com>

Teach git-subtree's Makefile to honor DESTDIR.

Signed-off-by: Adam Tkac <atkac@redhat.com>
Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/Makefile |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 05cdd5c..36ae3e4 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -30,12 +30,12 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)
 doc: $(GIT_SUBTREE_DOC)
 
 install: $(GIT_SUBTREE)
-	$(INSTALL) -m 755 $(GIT_SUBTREE) $(libexecdir)
+	$(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(libexecdir)
 
 install-doc: install-man
 
 install-man: $(GIT_SUBTREE_DOC)
-	$(INSTALL) -m 644 $^ $(man1dir)
+	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
 $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
 	xmlto -m $(MANPAGE_NORMAL_XSL)  man $^
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 04/13] contrib/subtree: Fix Synopsis
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: "David A. Greene" <greened@obbligato.org>

Fix the documentation of add to show that a repository can be
specified along with a commit.

Suggested by Yann Dirson <dirson@bertin.fr>.

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/git-subtree.sh  |    6 ++++++
 contrib/subtree/git-subtree.txt |    3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 771f39d..8a23f58 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -9,6 +9,7 @@ if [ $# -eq 0 ]; then
 fi
 OPTS_SPEC="\
 git subtree add   --prefix=<prefix> <commit>
+git subtree add   --prefix=<prefix> <repository> <commit>
 git subtree merge --prefix=<prefix> <commit>
 git subtree pull  --prefix=<prefix> <repository> <refspec...>
 git subtree push  --prefix=<prefix> <repository> <refspec...>
@@ -502,6 +503,11 @@ cmd_add()
 
 	    "cmd_add_commit" "$@"
 	elif [ $# -eq 2 ]; then
+	    # Technically we could accept a refspec here but we're
+	    # just going to turn around and add FETCH_HEAD under the
+	    # specified directory.  Allowing a refspec might be
+	    # misleading because we won't do anything with any other
+	    # branches fetched via the refspec.
 	    git rev-parse -q --verify "$2^{commit}" >/dev/null ||
 	    die "'$2' does not refer to a commit"
 
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index c5bce41..7ba853e 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -9,7 +9,8 @@ git-subtree - Merge subtrees together and split repository into subtrees
 SYNOPSIS
 --------
 [verse]
-'git subtree' add   -P <prefix> <commit>
+'git subtree' add   -P <prefix> <refspec>
+'git subtree' add   -P <prefix> <repository> <refspec>
 'git subtree' pull  -P <prefix> <repository> <refspec...>
 'git subtree' push  -P <prefix> <repository> <refspec...>
 'git subtree' merge -P <prefix> <commit>
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 02/13] contrib/subtree: Use %B for Split Subject/Body
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: Techlive Zheng <techlivezheng@gmail.com>

Use %B to format the commit message and body to avoid an extra newline
if a commit only has a subject line.

Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/git-subtree.sh     |    2 +-
 contrib/subtree/t/t7900-subtree.sh |   15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 920c664..5598210 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -296,7 +296,7 @@ copy_commit()
 	# We're going to set some environment vars here, so
 	# do it in a subshell to get rid of them safely later
 	debug copy_commit "{$1}" "{$2}" "{$3}"
-	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' "$1" |
+	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" |
 	(
 		read GIT_AUTHOR_NAME
 		read GIT_AUTHOR_EMAIL
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 6cf9fb9..3f17f55 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -74,6 +74,10 @@ test_expect_success 'add sub1' '
         git branch -m master subproj
 '
 
+# Save this hash for testing later.
+
+subdir_hash=`git rev-parse HEAD`
+
 test_expect_success 'add sub2' '
         create sub2 &&
         git commit -m "sub2" &&
@@ -211,6 +215,17 @@ test_expect_success 'check split with --branch' '
         check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
 '
 
+test_expect_success 'check hash of split' '
+        spl1=$(git subtree split --prefix subdir) &&
+        undo &&
+        git subtree split --prefix subdir --branch splitbr1test &&
+        check_equal ''"$(git rev-parse splitbr1test)"'' "$spl1"
+        git checkout splitbr1test &&
+        new_hash=$(git rev-parse HEAD~2) &&
+        git checkout mainline &&
+        check_equal ''"$new_hash"'' "$subdir_hash"
+'
+
 test_expect_success 'check split with --branch for an existing branch' '
         spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
         undo &&
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 01/13] contrib/subtree: Remove Test Number Comments
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani
  Cc: David A. Greene
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

From: "David A. Greene" <greened@obbligato.org>

Delete the comments indicating test numbers as it causes maintenance
headaches.  t*.sh -i will help us find any broken tests.

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/t/t7900-subtree.sh |   55 ------------------------------------
 1 file changed, 55 deletions(-)

diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index bc2eeb0..6cf9fb9 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -60,7 +60,6 @@ last_commit_message()
 	git log --pretty=format:%s -1
 }
 
-# 1
 test_expect_success 'init subproj' '
         test_create_repo subproj
 '
@@ -68,7 +67,6 @@ test_expect_success 'init subproj' '
 # To the subproject!
 cd subproj
 
-# 2
 test_expect_success 'add sub1' '
         create sub1 &&
         git commit -m "sub1" &&
@@ -76,14 +74,12 @@ test_expect_success 'add sub1' '
         git branch -m master subproj
 '
 
-# 3
 test_expect_success 'add sub2' '
         create sub2 &&
         git commit -m "sub2" &&
         git branch sub2
 '
 
-# 4
 test_expect_success 'add sub3' '
         create sub3 &&
         git commit -m "sub3" &&
@@ -93,7 +89,6 @@ test_expect_success 'add sub3' '
 # Back to mainline
 cd ..
 
-# 5
 test_expect_success 'add main4' '
         create main4 &&
         git commit -m "main4" &&
@@ -101,101 +96,85 @@ test_expect_success 'add main4' '
         git branch subdir
 '
 
-# 6
 test_expect_success 'fetch subproj history' '
         git fetch ./subproj sub1 &&
         git branch sub1 FETCH_HEAD
 '
 
-# 7
 test_expect_success 'no subtree exists in main tree' '
         test_must_fail git subtree merge --prefix=subdir sub1
 '
 
-# 8
 test_expect_success 'no pull from non-existant subtree' '
         test_must_fail git subtree pull --prefix=subdir ./subproj sub1
 '
 
-# 9
 test_expect_success 'check if --message works for add' '
         git subtree add --prefix=subdir --message="Added subproject" sub1 &&
         check_equal ''"$(last_commit_message)"'' "Added subproject" &&
         undo
 '
 
-# 10
 test_expect_success 'check if --message works as -m and --prefix as -P' '
         git subtree add -P subdir -m "Added subproject using git subtree" sub1 &&
         check_equal ''"$(last_commit_message)"'' "Added subproject using git subtree" &&
         undo
 '
 
-# 11
 test_expect_success 'check if --message works with squash too' '
         git subtree add -P subdir -m "Added subproject with squash" --squash sub1 &&
         check_equal ''"$(last_commit_message)"'' "Added subproject with squash" &&
         undo
 '
 
-# 12
 test_expect_success 'add subproj to mainline' '
         git subtree add --prefix=subdir/ FETCH_HEAD &&
         check_equal ''"$(last_commit_message)"'' "Add '"'subdir/'"' from commit '"'"'''"$(git rev-parse sub1)"'''"'"'"
 '
 
-# 13
 # this shouldn't actually do anything, since FETCH_HEAD is already a parent
 test_expect_success 'merge fetched subproj' '
         git merge -m "merge -s -ours" -s ours FETCH_HEAD
 '
 
-# 14
 test_expect_success 'add main-sub5' '
         create subdir/main-sub5 &&
         git commit -m "main-sub5"
 '
 
-# 15
 test_expect_success 'add main6' '
         create main6 &&
         git commit -m "main6 boring"
 '
 
-# 16
 test_expect_success 'add main-sub7' '
         create subdir/main-sub7 &&
         git commit -m "main-sub7"
 '
 
-# 17
 test_expect_success 'fetch new subproj history' '
         git fetch ./subproj sub2 &&
         git branch sub2 FETCH_HEAD
 '
 
-# 18
 test_expect_success 'check if --message works for merge' '
         git subtree merge --prefix=subdir -m "Merged changes from subproject" sub2 &&
         check_equal ''"$(last_commit_message)"'' "Merged changes from subproject" &&
         undo
 '
 
-# 19
 test_expect_success 'check if --message for merge works with squash too' '
         git subtree merge --prefix subdir -m "Merged changes from subproject using squash" --squash sub2 &&
         check_equal ''"$(last_commit_message)"'' "Merged changes from subproject using squash" &&
         undo
 '
 
-# 20
 test_expect_success 'merge new subproj history into subdir' '
         git subtree merge --prefix=subdir FETCH_HEAD &&
         git branch pre-split &&
         check_equal ''"$(last_commit_message)"'' "Merge commit '"'"'"$(git rev-parse sub2)"'"'"' into mainline"
 '
 
-# 21
 test_expect_success 'Check that prefix argument is required for split' '
         echo "You must provide the --prefix option." > expected &&
         test_must_fail git subtree split > actual 2>&1 &&
@@ -207,7 +186,6 @@ test_expect_success 'Check that prefix argument is required for split' '
         rm -f expected actual
 '
 
-# 22
 test_expect_success 'Check that the <prefix> exists for a split' '
         echo "'"'"'non-existent-directory'"'"'" does not exist\; use "'"'"'git subtree add'"'"'" > expected &&
         test_must_fail git subtree split --prefix=non-existent-directory > actual 2>&1 &&
@@ -219,7 +197,6 @@ test_expect_success 'Check that the <prefix> exists for a split' '
 #        rm -f expected actual
 '
 
-# 23
 test_expect_success 'check if --message works for split+rejoin' '
         spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
         git branch spl1 "$spl1" &&
@@ -227,7 +204,6 @@ test_expect_success 'check if --message works for split+rejoin' '
         undo
 '
 
-# 24
 test_expect_success 'check split with --branch' '
         spl1=$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin) &&
         undo &&
@@ -235,7 +211,6 @@ test_expect_success 'check split with --branch' '
         check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
 '
 
-# 25
 test_expect_success 'check split with --branch for an existing branch' '
         spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
         undo &&
@@ -244,13 +219,10 @@ test_expect_success 'check split with --branch for an existing branch' '
         check_equal ''"$(git rev-parse splitbr2)"'' "$spl1"
 '
 
-# 26
 test_expect_success 'check split with --branch for an incompatible branch' '
         test_must_fail git subtree split --prefix subdir --onto FETCH_HEAD --branch subdir
 '
 
-
-# 27
 test_expect_success 'check split+rejoin' '
         spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
         undo &&
@@ -258,7 +230,6 @@ test_expect_success 'check split+rejoin' '
         check_equal ''"$(last_commit_message)"'' "Split '"'"'subdir/'"'"' into commit '"'"'"$spl1"'"'"'"
 '
 
-# 28
 test_expect_success 'add main-sub8' '
         create subdir/main-sub8 &&
         git commit -m "main-sub8"
@@ -267,14 +238,12 @@ test_expect_success 'add main-sub8' '
 # To the subproject!
 cd ./subproj
 
-# 29
 test_expect_success 'merge split into subproj' '
         git fetch .. spl1 &&
         git branch spl1 FETCH_HEAD &&
         git merge FETCH_HEAD
 '
 
-# 30
 test_expect_success 'add sub9' '
         create sub9 &&
         git commit -m "sub9"
@@ -283,19 +252,16 @@ test_expect_success 'add sub9' '
 # Back to mainline
 cd ..
 
-# 31
 test_expect_success 'split for sub8' '
         split2=''"$(git subtree split --annotate='"'*'"' --prefix subdir/ --rejoin)"''
         git branch split2 "$split2"
 '
 
-# 32
 test_expect_success 'add main-sub10' '
         create subdir/main-sub10 &&
         git commit -m "main-sub10"
 '
 
-# 33
 test_expect_success 'split for sub10' '
         spl3=''"$(git subtree split --annotate='"'*'"' --prefix subdir --rejoin)"'' &&
         git branch spl3 "$spl3"
@@ -304,7 +270,6 @@ test_expect_success 'split for sub10' '
 # To the subproject!
 cd ./subproj
 
-# 34
 test_expect_success 'merge split into subproj' '
         git fetch .. spl3 &&
         git branch spl3 FETCH_HEAD &&
@@ -318,13 +283,11 @@ chkms_sub=$(echo $chkms | multiline | sed 's,^,subdir/,' | fixnl)
 chks="sub1 sub2 sub3 sub9"
 chks_sub=$(echo $chks | multiline | sed 's,^,subdir/,' | fixnl)
 
-# 35
 test_expect_success 'make sure exactly the right set of files ends up in the subproj' '
         subfiles=''"$(git ls-files | fixnl)"'' &&
         check_equal "$subfiles" "$chkms $chks"
 '
 
-# 36
 test_expect_success 'make sure the subproj history *only* contains commits that affect the subdir' '
         allchanges=''"$(git log --name-only --pretty=format:'"''"' | sort | fixnl)"'' &&
         check_equal "$allchanges" "$chkms $chks"
@@ -333,20 +296,17 @@ test_expect_success 'make sure the subproj history *only* contains commits that
 # Back to mainline
 cd ..
 
-# 37
 test_expect_success 'pull from subproj' '
         git fetch ./subproj subproj-merge-spl3 &&
         git branch subproj-merge-spl3 FETCH_HEAD &&
         git subtree pull --prefix=subdir ./subproj subproj-merge-spl3
 '
 
-# 38
 test_expect_success 'make sure exactly the right set of files ends up in the mainline' '
         mainfiles=''"$(git ls-files | fixnl)"'' &&
         check_equal "$mainfiles" "$chkm $chkms_sub $chks_sub"
 '
 
-# 39
 test_expect_success 'make sure each filename changed exactly once in the entire history' '
         # main-sub?? and /subdir/main-sub?? both change, because those are the
         # changes that were split into their own history.  And subdir/sub?? never
@@ -355,12 +315,10 @@ test_expect_success 'make sure each filename changed exactly once in the entire
         check_equal "$allchanges" ''"$(echo $chkms $chkm $chks $chkms_sub | multiline | sort | fixnl)"''
 '
 
-# 40
 test_expect_success 'make sure the --rejoin commits never make it into subproj' '
         check_equal ''"$(git log --pretty=format:'"'%s'"' HEAD^2 | grep -i split)"'' ""
 '
 
-# 41
 test_expect_success 'make sure no "git subtree" tagged commits make it into subproj' '
         # They are meaningless to subproj since one side of the merge refers to the mainline
         check_equal ''"$(git log --pretty=format:'"'%s%n%b'"' HEAD^2 | grep "git-subtree.*:")"'' ""
@@ -370,14 +328,12 @@ test_expect_success 'make sure no "git subtree" tagged commits make it into subp
 mkdir test2
 cd test2
 
-# 42
 test_expect_success 'init main' '
         test_create_repo main
 '
 
 cd main
 
-# 43
 test_expect_success 'add main1' '
         create main1 &&
         git commit -m "main1"
@@ -385,14 +341,12 @@ test_expect_success 'add main1' '
 
 cd ..
 
-# 44
 test_expect_success 'init sub' '
         test_create_repo sub
 '
 
 cd sub
 
-# 45
 test_expect_success 'add sub2' '
         create sub2 &&
         git commit -m "sub2"
@@ -402,7 +356,6 @@ cd ../main
 
 # check if split can find proper base without --onto
 
-# 46
 test_expect_success 'add sub as subdir in main' '
         git fetch ../sub master &&
         git branch sub2 FETCH_HEAD &&
@@ -411,7 +364,6 @@ test_expect_success 'add sub as subdir in main' '
 
 cd ../sub
 
-# 47
 test_expect_success 'add sub3' '
         create sub3 &&
         git commit -m "sub3"
@@ -419,20 +371,17 @@ test_expect_success 'add sub3' '
 
 cd ../main
 
-# 48
 test_expect_success 'merge from sub' '
         git fetch ../sub master &&
         git branch sub3 FETCH_HEAD &&
         git subtree merge --prefix subdir sub3
 '
 
-# 49
 test_expect_success 'add main-sub4' '
         create subdir/main-sub4 &&
         git commit -m "main-sub4"
 '
 
-# 50
 test_expect_success 'split for main-sub4 without --onto' '
         git subtree split --prefix subdir --branch mainsub4
 '
@@ -442,19 +391,16 @@ test_expect_success 'split for main-sub4 without --onto' '
 # have been sub3, but it was not, because its cache was not set to
 # itself)
 
-# 51
 test_expect_success 'check that the commit parent is sub3' '
         check_equal ''"$(git log --pretty=format:%P -1 mainsub4)"'' ''"$(git rev-parse sub3)"''
 '
 
-# 52
 test_expect_success 'add main-sub5' '
         mkdir subdir2 &&
         create subdir2/main-sub5 &&
         git commit -m "main-sub5"
 '
 
-# 53
 test_expect_success 'split for main-sub5 without --onto' '
         # also test that we still can split out an entirely new subtree
         # if the parent of the first commit in the tree is not empty,
@@ -487,7 +433,6 @@ joincommits()
 	echo "$commit $all"
 }
 
-# 54
 test_expect_success 'verify one file change per commit' '
         x= &&
         list=''"$(git log --pretty=format:'"'commit: %H'"' | joincommits)"'' &&
-- 
1.7.10.4

^ permalink raw reply related

* Assorted contrib/subtree Patches
From: David A. Greene @ 2013-02-05  4:06 UTC (permalink / raw)
  To: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", Jakub Suder, John Yani

All of the patches I have received from others as well as a few of my
own follow.  Probably the most controversial is a patch to remove
--annotate.  After some discussion on the list it became clear that we
really want a more general commit rewrite feature.  Removing
--annotate means we don't have to also support --unannotate and carry
both forward as backward-compatibility baggage.

Before --annotate was added, git-subtree would force an annotation of
"*" on every split commit message.  It now does no such thing so
there's no need to unannotate anything.

Please review and integrate.  Thanks!

                        -David

^ permalink raw reply

* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Junio C Hamano @ 2013-02-05  0:21 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Jeff King, git
In-Reply-To: <CAJo=hJtZ64ER4X+axtFZJ5ArnEg3h_nCVEBdd8KmE0nUpskzBA@mail.gmail.com>

Shawn Pearce <spearce@spearce.org> writes:

> Looks fine to me too, but I think the test won't work now. :-)

Heh, that's amusing ;-)

 t/t5551-http-fetch.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index cb95b95..47eb769 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -158,9 +158,8 @@ test_expect_success 'GIT_SMART_HTTP can disable smart http' '
 '
 
 test_expect_success 'invalid Content-Type rejected' '
-	echo "fatal: invalid content-type text/html" >expect
 	test_must_fail git clone $HTTPD_URL/broken_smart/repo.git 2>actual
-	test_cmp expect actual
+	grep "not valid:" actual
 '
 
 test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE

^ permalink raw reply related

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Junio C Hamano @ 2013-02-05  0:15 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <87zjzjr5y4.fsf@lifelogs.com>

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Mon, 04 Feb 2013 15:40:32 -0800 Junio C Hamano <gitster@pobox.com> wrote: 
>
> JCH> "Sorry we couldn't" sounded like an error messag to me.  If this is
> JCH> a normal exit, then please make sure it is a normal exit.
>
> OK; done in PATCHv4: removed all "Sorry" because they are not abnormal
> exits.  I'll hold PATCHv4 until the below are known.
>
> JCH> The review cycle is not like reviewers give you instructions and
> JCH> designs and you blindly implement them.  It is a creative process
> JCH> where you show the design and a clear implementation of that design.
>
> OK.  I would like you to make the decisions I asked for, though:
>
> - do you want to support backslashed newlines?

What for?  netrc/authinfo is not a line oriented file format at all,
and

	machine k.org
	        	login me
                        password mysecret

is a single entry; you do not need backslash at the end of any line.

Perhaps you are asking something different?

> - do you want me to remove the statement modifiers?

I do not think we are at that "implementation nitpick" level yet.

> - should all die() calls just print to STDERR and exit(0)?

Where "when unhandled, the helper should silently exit with 0" is
expected by the invoker, we shouldn't say anything to error stream,
and exit with zero.  Please leave a comment to make it easy to
understand to the readers that is what is going on there.

If on the other hand it diagnosed an error (not a bug in the
implementation but a misconfiguration on the user's side), I _think_
it should loudly die() so that the user can notice and take
corrective action.

> - do you want to support multiple netrc files, as you and Peff suggested?

I didn't even suggest such thing IIRC---I expected it to iterate
from the most desirable (.authinfo.gpg) to the least (.netrc) and
stop at the first found one.  There may be use cases people use more
than one and expect an entry to be found in any file, but I suspect
that might be more confusing than it is worth.  But I do not care
very deeply myself either way.

^ permalink raw reply

* A fast alternative to git-filter-branch - The BFG Repo-Cleaner
From: Roberto Tyley @ 2013-02-05  0:04 UTC (permalink / raw)
  To: git

I recently released The BFG Repo-Cleaner, a new tool for cleansing bad
data out of Git repository histories. The BFG is typically at least
10-50x faster than git-filter-branch at these tasks:

* Removing Crazy Big Files from repo history
* Removing Passwords, Credentials & other Private data

http://rtyley.github.com/bfg-repo-cleaner/

As an example, these are timings for deleting an arbitrary file from
the large GCC repository (148495 commits):

The BFG : 3m29s
$ bfg -D README-fixinc

git filter-branch : 472m31s
$ git filter-branch --index-filter 'git rm --cached --ignore-unmatch
gcc/README-fixinc' --prune-empty --tag-name-filter cat -- --all

(roughly a 135x speed increase, reducing the task of processing a
large codebase from an overnight job to the work of a few minutes....
all timings done in a 4GB tmpfs ramdisk)


The BFG has some simple but very powerful command-line options, which
perform at similar speed:

remove all blobs bigger than 1 megabyte :
$ bfg --strip-blobs-bigger-than 1M  my-repo.git

replace all passwords (listed in a file 'passwords.txt') with ***REMOVED*** :
$ bfg --replace-banned-strings passwords.txt  my-repo.git


The main source of the BFG's performance advantage comes from
preventing repeated examination of the same tree objects. The approach
of git-filter-branch performs filtering for each commit, against the
complete file-hierarchy of each commit, one after the other, even
though commit trees are largely very similar. For the use-cases of The
BFG that's unnecessary- we don't care where, and in which commit, a
'bad' file exists - we just want it dealt with. Consequently the BFG
processes the Git object db on a memoised tree-by-tree basis,
processing each and every file & folder exactly once - the final
processing of the commit hierarchy is very quick. This _does_ mean
that it's not possible to delete files based on their absolute path
within the repo, but they can deleted based on their filename,
blob-id, or contents. This, and multi-core processing by default,
gives the dramatic speed-up while still providing the same results.
There's more performance data here:
https://docs.google.com/spreadsheet/ccc?key=0AsR1d5Zpes8HdER3VGU1a3dOcmVHMmtzT2dsS2xNenc

I'd welcome feedback, and if anyone has cause to filter a repository's
history in future, I'd appreciate you giving the BFG a try and letting
me know how you found it.

thanks,
Roberto Tyley
software dev @ The Guardian

http://rtyley.github.com/bfg-repo-cleaner/

^ permalink raw reply

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Ted Zlatanov @ 2013-02-04 23:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vy5f3zlzj.fsf@alter.siamese.dyndns.org>

On Mon, 04 Feb 2013 15:40:32 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> "Sorry we couldn't" sounded like an error messag to me.  If this is
JCH> a normal exit, then please make sure it is a normal exit.

OK; done in PATCHv4: removed all "Sorry" because they are not abnormal
exits.  I'll hold PATCHv4 until the below are known.

JCH> The review cycle is not like reviewers give you instructions and
JCH> designs and you blindly implement them.  It is a creative process
JCH> where you show the design and a clear implementation of that design.

OK.  I would like you to make the decisions I asked for, though:

- do you want to support backslashed newlines?
- do you want me to remove the statement modifiers?
- should all die() calls just print to STDERR and exit(0)?
- do you want to support multiple netrc files, as you and Peff suggested?

On all of those, I can go either way, it's just a little more work for me.

Ted

^ permalink raw reply

* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Shawn Pearce @ 2013-02-04 23:49 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20130204083824.GB30835@sigill.intra.peff.net>

On Mon, Feb 4, 2013 at 12:38 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Feb 03, 2013 at 11:17:33PM -0800, Junio C Hamano wrote:
>
> > Does this look good to both of you (relative to Shawn's patch)?
> >
> >  remote-curl.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/remote-curl.c b/remote-curl.c
> > index e6f3b63..933c69a 100644
> > --- a/remote-curl.c
> > +++ b/remote-curl.c
> > @@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char *service)
> >       last->buf_alloc = strbuf_detach(&buffer, &last->len);
> >       last->buf = last->buf_alloc;
> >
> > -     if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
> > +     strbuf_addf(&exp, "application/x-%s-advertisement", service);
> > +     if (maybe_smart &&
> > +         (5 <= last->len && last->buf[4] == '#') &&
> > +         !strbuf_cmp(&exp, &type)) {
> >               /*
> >                * smart HTTP response; validate that the service
> >                * pkt-line matches our request.
> >                */
> > -             strbuf_addf(&exp, "application/x-%s-advertisement", service);
> > -             if (strbuf_cmp(&exp, &type))
> > -                     die("invalid content-type %s", type.buf);
> >               if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
> >                       die("%s has invalid packet header", refs_url);
> >               if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
>
> Yeah, I think that's fine. Thanks.

Looks fine to me too, but I think the test won't work now. :-)

^ permalink raw reply

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Ted Zlatanov @ 2013-02-04 23:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20130204232317.GA17705@sigill.intra.peff.net>

On Mon, 4 Feb 2013 18:23:17 -0500 Jeff King <peff@peff.net> wrote: 

>> Perhaps "-r $file", if you say "is not accessible"?

JK> Even better: look at whether opening the file was successful. Though I
JK> guess that is complicated by the use of gpg, who will probably not
JK> distinguish ENOENT from other failures for us.

Yup.  I think the outcome for the user will be the same, so this is
mostly for debugging, right?  And we do look at the outcome of opening
the file, and die if that failed (which would change if your suggestion
below is implemented).

JK> I was trying not to be too nit-picky with my review, but here is how I
JK> would have written the outer logic of the script:

JK>   my $tokens = read_credential_data_from_stdin();
JK>   if ($options{file}) {
JK>           my @entries = load_netrc($options{file})
JK>                   or die "unable to open $options{file}: $!";
JK>           check_netrc($tokens, @entries);
JK>   }
JK>   else {
JK>           foreach my $ext ('.gpg', '') {
JK>                   foreach my $base (qw(authinfo netrc)) {
JK>                           my @entries = load_netrc("$base$ext")
JK>                                   or next;
JK>                           if (check_netrc($tokens, @entries)) {
JK>                                   last;
JK>                           }
JK>                   }
JK>           }
JK>   }

JK> I.e., to fail on "-f", but otherwise treat unreadable auto-selected
JK> files as a no-op, for whatever reason.

JK> I'd also consider checking all files if they are available, in case
JK> the user has multiple (e.g., they keep low-quality junk unencrypted
JK> but some high-security passwords in a .gpg file). Not that likely,
JK> but not any harder to implement.

I think that makes everything more complicated, and the user can name a
specific netrc file in the helper spec if he wants it.  It's too
automagic for me.  But if you and Junio feel this is the right approach,
I'll rewrite to basically allow --file to take a list of filenames and
default that list to the base list of ~/.{authinfo,netrc}{,.gpg}

Ted

^ permalink raw reply

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Junio C Hamano @ 2013-02-04 23:40 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <87bobzslke.fsf@lifelogs.com>

Ted Zlatanov <tzz@lifelogs.com> writes:

>>> +my $mode = shift @ARGV;
>>> +
>>> +# credentials may get 'get', 'store', or 'erase' as parameters but
>>> +# only acknowledge 'get'
>>> +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
>>> +
>>> +# only support 'get' mode
>>> +exit unless $mode eq 'get';
>
> JCH> The above looks strange.  Why does the invoker get the error message
> JCH> only when it runs this without arguments?  Did you mean to say more
> JCH> like this?
>
> JCH> 	unless (defined $mode && $mode eq 'get') {
> JCH> 		die "...";
> JCH> 	}
>
> I mean:
>
> - if the mode is not given, exit badly (since it's required)
>
> - if the mode is given but we don't support it, exit pleasantly
>
> I thought that was the right thing, according to my reading of the
> credentials API.  If not, I'll be glad to change it.

As Peff noted, I mistead what the code was doing, especially with
somewhat cryptic "only support x mode" comment, as if it is
rejecting other modes.

>>> +	print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
>>> +	exit;
>
> JCH> Is this really an error?  The file perhaps was empty.  Shouldn't
> JCH> that case treated the same way as the case where no entry that
> JCH> matches the criteria invoker gave you was found?
>
> exit(0) is not an error, so the behavior is exactly the same, we just
> don't print anything to STDOUT because there was no data, with a nicer
> error message.  I think that's what we want?

"Sorry we couldn't" sounded like an error messag to me.  If this is
a normal exit, then please make sure it is a normal exit.

The review cycle is not like reviewers give you instructions and
designs and you blindly implement them.  It is a creative process
where you show the design and a clear implementation of that design.

Thanks.

^ permalink raw reply

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Junio C Hamano @ 2013-02-04 23:36 UTC (permalink / raw)
  To: Jeff King; +Cc: Ted Zlatanov, git
In-Reply-To: <20130204232317.GA17705@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Feb 04, 2013 at 02:56:06PM -0800, Junio C Hamano wrote:
>
>> > +my $mode = shift @ARGV;
>> > +
>> > +# credentials may get 'get', 'store', or 'erase' as parameters but
>> > +# only acknowledge 'get'
>> > +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
>> > +
>> > +# only support 'get' mode
>> > +exit unless $mode eq 'get';
>> 
>> The above looks strange.  Why does the invoker get the error message
>> only when it runs this without arguments?  Did you mean to say more
>> like this?
>> 
>> 	unless (defined $mode && $mode eq 'get') {
>> 		die "...";
>> 	}
>
> Not having a mode is an invocation error; the credential-helper
> documentation indicates that the helper will always be invoked with an
> action. The likely culprit for not having one is the user invoking it
> manually, and showing the usage there is a sensible action.
>
> Whereas invoking it with a mode other than "get" is not an error at all.
> Git will run it with the "store" and "erase" actions, too. Those happen
> to be no-ops for this helper, so it exits silently. The credential docs
> specify that any other actions should be ignored, too, to allow for
> future expansion.

OK.  The code didn't express the above reasoning clearly enough.

> I was trying not to be too nit-picky with my review,...

I wasn't either.  Mine was still at design level review to get the
semantics right (e.g. what to consider as errors, the input is _not_
one entry per line, etc.), before reviewing the details of the
implementation.

> but here is how I
> would have written the outer logic of the script:
>
>   my $tokens = read_credential_data_from_stdin();
>   if ($options{file}) {
>           my @entries = load_netrc($options{file})
>                   or die "unable to open $options{file}: $!";
>           check_netrc($tokens, @entries);
>   }
>   else {
>           foreach my $ext ('.gpg', '') {
>                   foreach my $base (qw(authinfo netrc)) {
>                           my @entries = load_netrc("$base$ext")
>                                   or next;
>                           if (check_netrc($tokens, @entries)) {
>                                   last;
>                           }
>                   }
>           }
>   }
>
> I.e., to fail on "-f", but otherwise treat unreadable auto-selected
> files as a no-op, for whatever reason. I'd also consider checking all
> files if they are available, in case the user has multiple (e.g., they
> keep low-quality junk unencrypted but some high-security passwords in a
> .gpg file). Not that likely, but not any harder to implement.

Yeah, I think that looks like the right top-level codeflow.

^ permalink raw reply

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Ted Zlatanov @ 2013-02-04 23:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vd2wf1yex.fsf@alter.siamese.dyndns.org>

On Mon, 04 Feb 2013 14:56:06 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> I recall that netrc/authinfo files are _not_ line oriented.  Earlier
JCH> you said "looks for entries that match" which is a lot more correct,
JCH> but then we see "look for lines in authfile".

Hmm, do you mean backslashed newlines?  I think the Net::Netrc parser
doesn't support them, and I haven't seen them in the wild, but I could
support them if you think that's useful.

>> +my $mode = shift @ARGV;
>> +
>> +# credentials may get 'get', 'store', or 'erase' as parameters but
>> +# only acknowledge 'get'
>> +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
>> +
>> +# only support 'get' mode
>> +exit unless $mode eq 'get';

JCH> The above looks strange.  Why does the invoker get the error message
JCH> only when it runs this without arguments?  Did you mean to say more
JCH> like this?

JCH> 	unless (defined $mode && $mode eq 'get') {
JCH> 		die "...";
JCH> 	}

I mean:

- if the mode is not given, exit badly (since it's required)

- if the mode is given but we don't support it, exit pleasantly

I thought that was the right thing, according to my reading of the
credentials API.  If not, I'll be glad to change it.

JCH> By the way, I think statement modifiers tend to get overused and
JCH> make the resulting program harder to read.  die "..." at the
JCH> beginning of line makes the reader go "Whoa, it already is done and
JCH> existing on error", and then forces the eyes to scan the error
JCH> message to find "unless" and the condition.

JCH> It may be a cute syntax and some may find it even cool, but cuteness
JCH> or coolness is less valuable compared with the readability.

Your coding guidelines said you prefer one-line if statements, and I
thought it would be OK to lean on modifiers.  I changed many of the
modifiers but not all; please let me know if you'd like me to change
them all.  It's no problem.

JCH> Is it sensible to squelch the error message by default and force
JCH> user to specify --debug?  You could argue that the option is to
JCH> debug the user's configuration, but the name of the option sounds
JCH> more like it is for debugging this script itself.

It's both... without a clear separation because it's such a small
script.  Let me know how you'd like to change it, if at all.

JCH> I saw Peff already pointed out error conditions, but I am not sure
JCH> why all of these exit with 0.  If the user has configured

JCH> 	git config credential.helper 'netrc -f $HOME/.netcr'

JCH> shouldn't it be diagnosed as an error?  It is understandable to let
JCH> this go silently

JCH> 	git config credential.helper 'netrc'

JCH> and let other credential helpers take over when no $HOME/.{netrc,authinfo}{,.gpg}
JCH> file exist, but in that case the user may still want to remove the
JCH> config item that is not doing anything useful and erroring out with
JCH> a message may be a way to help the user know about the situation.

You and Peff should tell me how it should behave, or perhaps make the
changes after it's in.  I'm happy to change it any way you like, but at
this point I'm just following instructions, not really contributing,
about the exit statuses.  I thought I knew what you wanted 2 iterations
ago :)

>> +	print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
>> +	exit;

JCH> Is this really an error?  The file perhaps was empty.  Shouldn't
JCH> that case treated the same way as the case where no entry that
JCH> matches the criteria invoker gave you was found?

exit(0) is not an error, so the behavior is exactly the same, we just
don't print anything to STDOUT because there was no data, with a nicer
error message.  I think that's what we want?

PATCHv3 is out with the rest of your suggestions.  Thank you for the
thorough review.  I am happy to improve the script to meet your standards.

Thanks
Ted

^ permalink raw reply

* [PATCHv3] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-04 23:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vd2wf1yex.fsf@alter.siamese.dyndns.org>

Changes since PATCHv2:

- don't keep looking at netrc candidates if one good one is found

- fixed wording of "line" to "entry" everywhere suitable

- many (but not all) statement modifiers changed to block format

- use -r everywhere instead of -f

- move chomp to when we know @data has contents

Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
 contrib/credential/netrc/git-credential-netrc |  243 +++++++++++++++++++++++++
 1 files changed, 243 insertions(+), 0 deletions(-)
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..99ab204
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,243 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+               help => 0,
+               debug => 0,
+
+               # identical token maps, e.g. host -> host, will be inserted later
+               tmap => {
+                        port => 'protocol',
+                        machine => 'host',
+                        path => 'path',
+                        login => 'username',
+                        user => 'username',
+                        password => 'password',
+                       }
+              );
+
+# map each credential protocol token to itself on the netrc side
+foreach (values %{$options{tmap}}) {
+	$options{tmap}->{$_} = $_;
+}
+
+FILE:
+foreach my $suffix ('.gpg', '') {
+	foreach my $base (qw/authinfo netrc/) {
+		my $file = glob("~/.$base$suffix");
+		next unless (defined $file && -r $file);
+		$options{file} = $file;
+		last FILE;
+	}
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+           "help|h",
+           "debug|d",
+           "file|f=s",
+          );
+
+if ($options{help}) {
+	my $shortname = basename($0);
+	$shortname =~ s/git-credential-//;
+
+	print <<EOHIPPUS;
+
+$0 [-f AUTHFILE] [-d] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+  -f AUTHFILE: specify a netrc-style file
+  -d: turn on debugging
+
+To enable (note that Git will prepend "git-credential-" to the helper
+name and look for it in the path):
+
+  git config credential.helper '$shortname -f AUTHFILE'
+
+And if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+Only "get" mode is supported by this credential helper.  It opens
+AUTHFILE and looks for entries that match the requested search
+criteria:
+
+ 'port|protocol':
+   The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+   The remote hostname for a network credential. (host=X)
+
+ 'path':
+   The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+   The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+protocol=https
+username=tzz
+
+this credential helper will look for entries in AUTHFILE that match
+
+port https login tzz
+
+OR
+
+protocol https login tzz
+
+OR... etc. acceptable tokens as listed above.  Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the entry,
+including "password" tokens, mapping e.g. "port" back to "protocol".
+
+The first matching entry is used.  Tokens can be quoted as 'STRING' or
+"STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+	exit;
+}
+
+my $mode = shift @ARGV;
+
+# credentials may get 'get', 'store', or 'erase' as parameters but
+# only acknowledge 'get'
+die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
+
+# only support 'get' mode
+exit unless $mode eq 'get';
+
+my $debug = $options{debug};
+my $file = $options{file};
+
+unless (defined $file) {
+	print STDERR "Please specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\n" if $debug;
+	exit 0;
+}
+
+unless (-r $file) {
+	print STDERR "Sorry, the specified netrc $file is not accessible\n" if $debug;
+	exit 0;
+}
+
+my @data;
+if ($file =~ m/\.gpg$/) {
+	@data = load('-|', qw(gpg --decrypt), $file)
+}
+else {
+	@data = load('<', $file);
+}
+
+unless (scalar @data) {
+	print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
+	exit;
+}
+
+chomp @data;
+
+# the query: start with every token with no value
+my %q = map { $_ => undef } values(%{$options{tmap}});
+
+while (<STDIN>) {
+	next unless m/^([^=]+)=(.+)/;
+
+	my ($token, $value) = ($1, $2);
+	die "Unknown search token $1" unless exists $q{$token};
+	$q{$token} = $value;
+}
+
+# build reverse token map
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+	push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+# there are CPAN modules to do this better, but we want to avoid
+# dependencies and generally, complex netrc-style files are rare
+
+if ($debug) {
+	foreach (sort keys %q) {
+		printf STDERR "searching for %s = %s\n", $_, $q{$_} || '(any value)';
+	}
+}
+
+LINE: foreach my $line (@data) {
+
+	print STDERR "line [$line]\n" if $debug;
+	my @tok;
+	# gratefully stolen from Net::Netrc
+	while (length $line &&
+	       $line =~ s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+		(my $tok = $+) =~ s/\\(.)/$1/g;
+		push(@tok, $tok);
+	}
+
+	# skip blank lines, comments, etc.
+	next LINE unless scalar @tok;
+
+	my %tokens;
+	my $num_port;
+	while (@tok) {
+		my ($k, $v) = (shift @tok, shift @tok);
+		next unless defined $v;
+		next unless exists $options{tmap}->{$k};
+		$tokens{$options{tmap}->{$k}} = $v;
+		$num_port = ($k eq 'port' && $v =~ m/^\d+$/) ? $v : undef;
+	}
+
+	# for "host X port Y" where Y is an integer (captured by
+	# $num_port above), set the host to "X:Y"
+	if (defined $tokens{host} && defined $num_port) {
+		$tokens{host} = join(':', $tokens{host}, $num_port);
+	}
+
+	foreach my $check (sort keys %q) {
+		if (exists $tokens{$check} && defined $q{$check}) {
+			print STDERR "comparing [$tokens{$check}] to [$q{$check}] in entry [$line]\n" if $debug;
+			next LINE unless $tokens{$check} eq $q{$check};
+		}
+		else {
+			print STDERR "we could not find [$check] but it's OK\n" if $debug;
+		}
+	}
+
+	print STDERR "entry has passed all the search checks\n" if $debug;
+ TOKEN:
+	foreach my $token (sort keys %rmap) {
+		print STDERR "looking for useful token $token\n" if $debug;
+		next unless exists $tokens{$token}; # did we match?
+
+		foreach my $rctoken (@{$rmap{$token}}) {
+			# don't re-print given tokens
+			next TOKEN if defined $q{$rctoken};
+		}
+
+		print STDERR "FOUND: $token=$tokens{$token}\n" if $debug;
+		printf "%s=%s\n", $token, $tokens{$token};
+	}
+
+	last;
+}
+
+sub load {
+	# this supports pipes too
+	my $io = new IO::File(@_) or die "Could not open [@_]: $!\n";
+	return <$io>;                          # whole file
+}
-- 
1.7.9.rc2

^ permalink raw reply related

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Jeff King @ 2013-02-04 23:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ted Zlatanov, git
In-Reply-To: <7vd2wf1yex.fsf@alter.siamese.dyndns.org>

On Mon, Feb 04, 2013 at 02:56:06PM -0800, Junio C Hamano wrote:

> > +my $mode = shift @ARGV;
> > +
> > +# credentials may get 'get', 'store', or 'erase' as parameters but
> > +# only acknowledge 'get'
> > +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
> > +
> > +# only support 'get' mode
> > +exit unless $mode eq 'get';
> 
> The above looks strange.  Why does the invoker get the error message
> only when it runs this without arguments?  Did you mean to say more
> like this?
> 
> 	unless (defined $mode && $mode eq 'get') {
> 		die "...";
> 	}

Not having a mode is an invocation error; the credential-helper
documentation indicates that the helper will always be invoked with an
action. The likely culprit for not having one is the user invoking it
manually, and showing the usage there is a sensible action.

Whereas invoking it with a mode other than "get" is not an error at all.
Git will run it with the "store" and "erase" actions, too. Those happen
to be no-ops for this helper, so it exits silently. The credential docs
specify that any other actions should be ignored, too, to allow for
future expansion.

> > +my $debug = $options{debug};
> > +my $file = $options{file};
> > +
> > +unless (defined $file) {
> > +	print STDERR "Please specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\n" if $debug;
> > +	exit 0;
> > +}
> > +
> > +unless (-f $file) {
> > +	print STDERR "Sorry, the specified netrc $file is not accessible\n" if $debug;
> > +	exit 0;
> > +}
> 
> Perhaps "-r $file", if you say "is not accessible"?

Even better: look at whether opening the file was successful. Though I
guess that is complicated by the use of gpg, who will probably not
distinguish ENOENT from other failures for us.

> Is it sensible to squelch the error message by default and force
> user to specify --debug?  You could argue that the option is to
> debug the user's configuration, but the name of the option sounds
> more like it is for debugging this script itself.
> 
> I saw Peff already pointed out error conditions, but I am not sure
> why all of these exit with 0.  If the user has configured

It was from my suggestion to ignore missing files, which is that the
user might have the helper configured (e.g., via /etc/gitconfig, or by a
shared ~/.gitconfig) but not actually have a netrc.

It gets confusing because the contents of $file may have been
auto-detected, or it may have come from the command-line, and we do not
remember which at this point.

I was trying not to be too nit-picky with my review, but here is how I
would have written the outer logic of the script:

  my $tokens = read_credential_data_from_stdin();
  if ($options{file}) {
          my @entries = load_netrc($options{file})
                  or die "unable to open $options{file}: $!";
          check_netrc($tokens, @entries);
  }
  else {
          foreach my $ext ('.gpg', '') {
                  foreach my $base (qw(authinfo netrc)) {
                          my @entries = load_netrc("$base$ext")
                                  or next;
                          if (check_netrc($tokens, @entries)) {
                                  last;
                          }
                  }
          }
  }

I.e., to fail on "-f", but otherwise treat unreadable auto-selected
files as a no-op, for whatever reason. I'd also consider checking all
files if they are available, in case the user has multiple (e.g., they
keep low-quality junk unencrypted but some high-security passwords in a
.gpg file). Not that likely, but not any harder to implement.

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox