git-subtree Patches to Apply

git-subtree Patches to Apply

[David A. Greene]

Here are all of the patches for git-subtree that have been posted to
the mailing list that I could apply and test in a reasonable amount of
time.  These are all rebased from trunk as of tonight.

Many apologies for being *so* behind.  Work has been a bear but I'm
hoping things will ease up in the new year and I can be more regularly
active.  But still, don't expect same-day service.  :)

These are also available on branch "toupstream" via

git clone gitolite@sources.obbligato.org:git.git

and

http://sources.obbligato.org
http://sources.obbligato.org/?p=git.git;a=summary

Junio, can you apply these?  Thanks!

                     -David

[PATCH 1/8] Use %B for Split Subject/Body

[David A. Greene]

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.

Author:    Techlive Zheng <techlivezheng@gmail.com>

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

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 920c664..f2b6d4a 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -296,7 +296,12 @@ 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}"
+	# Use %B rather than %s%n%n%b to handle the special case of a
+	# commit that only has a subject line.  We don't want to
+	# introduce a newline after the subject, causing generation of
+	# a new hash.
 	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 bc2eeb0..93eeb09 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -76,6 +76,10 @@ test_expect_success 'add sub1' '
         git branch -m master subproj
 '
 
+# Save this hash for testing later.
+
+subdir_hash=`git rev-parse HEAD`
+
 # 3
 test_expect_success 'add sub2' '
         create sub2 &&
@@ -155,7 +159,6 @@ test_expect_success 'add main-sub5' '
         create subdir/main-sub5 &&
         git commit -m "main-sub5"
 '
-
 # 15
 test_expect_success 'add main6' '
         create main6 &&
@@ -235,7 +238,19 @@ test_expect_success 'check split with --branch' '
         check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
 '
 
-# 25
+#25
+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"
+'
+
+# 26
 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 +259,13 @@ test_expect_success 'check split with --branch for an existing branch' '
         check_equal ''"$(git rev-parse splitbr2)"'' "$spl1"
 '
 
-# 26
+# 27
 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
+# 28
 test_expect_success 'check split+rejoin' '
         spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
         undo &&
@@ -258,7 +273,7 @@ test_expect_success 'check split+rejoin' '
         check_equal ''"$(last_commit_message)"'' "Split '"'"'subdir/'"'"' into commit '"'"'"$spl1"'"'"'"
 '
 
-# 28
+# 29
 test_expect_success 'add main-sub8' '
         create subdir/main-sub8 &&
         git commit -m "main-sub8"
@@ -267,14 +282,14 @@ test_expect_success 'add main-sub8' '
 # To the subproject!
 cd ./subproj
 
-# 29
+# 30
 test_expect_success 'merge split into subproj' '
         git fetch .. spl1 &&
         git branch spl1 FETCH_HEAD &&
         git merge FETCH_HEAD
 '
 
-# 30
+# 31
 test_expect_success 'add sub9' '
         create sub9 &&
         git commit -m "sub9"
@@ -283,19 +298,19 @@ test_expect_success 'add sub9' '
 # Back to mainline
 cd ..
 
-# 31
+# 32
 test_expect_success 'split for sub8' '
         split2=''"$(git subtree split --annotate='"'*'"' --prefix subdir/ --rejoin)"''
         git branch split2 "$split2"
 '
 
-# 32
+# 33
 test_expect_success 'add main-sub10' '
         create subdir/main-sub10 &&
         git commit -m "main-sub10"
 '
 
-# 33
+# 34
 test_expect_success 'split for sub10' '
         spl3=''"$(git subtree split --annotate='"'*'"' --prefix subdir --rejoin)"'' &&
         git branch spl3 "$spl3"
@@ -304,7 +319,7 @@ test_expect_success 'split for sub10' '
 # To the subproject!
 cd ./subproj
 
-# 34
+# 35
 test_expect_success 'merge split into subproj' '
         git fetch .. spl3 &&
         git branch spl3 FETCH_HEAD &&
@@ -318,13 +333,13 @@ chkms_sub=$(echo $chkms | multiline | sed 's,^,subdir/,' | fixnl)
 chks="sub1 sub2 sub3 sub9"
 chks_sub=$(echo $chks | multiline | sed 's,^,subdir/,' | fixnl)
 
-# 35
+# 36
 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
+# 37
 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 +348,20 @@ test_expect_success 'make sure the subproj history *only* contains commits that
 # Back to mainline
 cd ..
 
-# 37
+# 38
 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
+# 39
 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
+# 40
 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 +370,12 @@ 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
+# 41
 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
+# 42
 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 +385,14 @@ test_expect_success 'make sure no "git subtree" tagged commits make it into subp
 mkdir test2
 cd test2
 
-# 42
+# 43
 test_expect_success 'init main' '
         test_create_repo main
 '
 
 cd main
 
-# 43
+# 44
 test_expect_success 'add main1' '
         create main1 &&
         git commit -m "main1"
@@ -385,14 +400,14 @@ test_expect_success 'add main1' '
 
 cd ..
 
-# 44
+# 45
 test_expect_success 'init sub' '
         test_create_repo sub
 '
 
 cd sub
 
-# 45
+# 46
 test_expect_success 'add sub2' '
         create sub2 &&
         git commit -m "sub2"
@@ -402,7 +417,7 @@ cd ../main
 
 # check if split can find proper base without --onto
 
-# 46
+# 47
 test_expect_success 'add sub as subdir in main' '
         git fetch ../sub master &&
         git branch sub2 FETCH_HEAD &&
@@ -411,7 +426,7 @@ test_expect_success 'add sub as subdir in main' '
 
 cd ../sub
 
-# 47
+# 48
 test_expect_success 'add sub3' '
         create sub3 &&
         git commit -m "sub3"
@@ -419,20 +434,20 @@ test_expect_success 'add sub3' '
 
 cd ../main
 
-# 48
+# 49
 test_expect_success 'merge from sub' '
         git fetch ../sub master &&
         git branch sub3 FETCH_HEAD &&
         git subtree merge --prefix subdir sub3
 '
 
-# 49
+# 50
 test_expect_success 'add main-sub4' '
         create subdir/main-sub4 &&
         git commit -m "main-sub4"
 '
 
-# 50
+# 51
 test_expect_success 'split for main-sub4 without --onto' '
         git subtree split --prefix subdir --branch mainsub4
 '
@@ -442,12 +457,12 @@ 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
+# 52
 test_expect_success 'check that the commit parent is sub3' '
         check_equal ''"$(git log --pretty=format:%P -1 mainsub4)"'' ''"$(git rev-parse sub3)"''
 '
 
-# 52
+# 53
 test_expect_success 'add main-sub5' '
         mkdir subdir2 &&
         create subdir2/main-sub5 &&
-- 
1.7.10.4

[PATCH 2/8] Add --unannotate

[David A. Greene]

From: James Nylen <jnylen@gmail.com>

Teach git-subtree about --unannotate.  This option strips a prefix
from a commit message when doing a subtree split.

Author:    James Nylen <jnylen@gmail.com>

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/git-subtree.sh     |   11 +++++-
 contrib/subtree/git-subtree.txt    |   15 ++++++++
 contrib/subtree/t/t7900-subtree.sh |   73 ++++++++++++++++++++----------------
 3 files changed, 65 insertions(+), 34 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index f2b6d4a..7ceb413 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -21,6 +21,7 @@ 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
+unannotate=   remove a prefix from new commit messages (supports bash globbing)
 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,6 +44,7 @@ onto=
 rejoin=
 ignore_joins=
 annotate=
+unannotate=
 squash=
 message=
 
@@ -80,6 +82,8 @@ while [ $# -gt 0 ]; do
 		-d) debug=1 ;;
 		--annotate) annotate="$1"; shift ;;
 		--no-annotate) annotate= ;;
+		--unannotate) unannotate="$1"; shift ;;
+		--no-unannotate) unannotate= ;;
 		-b) branch="$1"; shift ;;
 		-P) prefix="$1"; shift ;;
 		-m) message="$1"; shift ;;
@@ -315,8 +319,11 @@ copy_commit()
 			GIT_COMMITTER_NAME \
 			GIT_COMMITTER_EMAIL \
 			GIT_COMMITTER_DATE
-		(echo -n "$annotate"; cat ) |
-		git commit-tree "$2" $3  # reads the rest of stdin
+		(
+			read FIRST_LINE
+			echo "$annotate${FIRST_LINE#$unannotate}"
+			cat  # reads the rest of stdin
+		) | git commit-tree "$2" $3
 	) || die "Can't copy commit $1"
 }
 
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 0c44fda..ae420aa 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -198,6 +198,21 @@ OPTIONS FOR split
 	git subtree tries to make it work anyway, particularly
 	if you use --rejoin, but it may not always be effective.
 
+--unannotate=<annotation>::
+	This option is only valid for the split command.
+
+	When generating synthetic history, try to remove the prefix
+	<annotation> from each commit message (using bash's "strip
+	shortest match from beginning" command, which supports
+	globbing).  This makes sense if you format library commits
+	like "library: Change something or other" when you're working
+	in your project's repository, but you want to remove this
+	prefix when pushing back to the library's upstream repository.
+	(In this case --unannotate='*: ' would work well.)
+	
+	Like --annotate,  you need to use the same <annotation>
+	whenever you split, or you may run into problems.
+
 -b <branch>::
 --branch=<branch>::
 	This option is only valid for the split command.
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 93eeb09..9816da5 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -157,7 +157,7 @@ test_expect_success 'merge fetched subproj' '
 # 14
 test_expect_success 'add main-sub5' '
         create subdir/main-sub5 &&
-        git commit -m "main-sub5"
+        git commit -m "subproj: main-sub5"
 '
 # 15
 test_expect_success 'add main6' '
@@ -168,7 +168,7 @@ test_expect_success 'add main6' '
 # 16
 test_expect_success 'add main-sub7' '
         create subdir/main-sub7 &&
-        git commit -m "main-sub7"
+        git commit -m "subproj: main-sub7"
 '
 
 # 17
@@ -238,7 +238,7 @@ test_expect_success 'check split with --branch' '
         check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
 '
 
-#25
+# 25
 test_expect_success 'check hash of split' '
         spl1=$(git subtree split --prefix subdir) &&
         undo &&
@@ -251,6 +251,15 @@ test_expect_success 'check hash of split' '
 '
 
 # 26
+test_expect_success 'check --unannotate' '
+        spl1=$(git subtree split --unannotate='"subproj:"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin) &&
+        undo &&
+        git subtree split --unannotate='"subproj:"' --prefix subdir --onto FETCH_HEAD --branch splitunann &&
+        check_equal ''"$(git rev-parse splitunann)"'' "$spl1" &&
+        check_equal ''"$(git log splitunann | grep subproj)"'' ""
+'
+
+# 27
 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 &&
@@ -259,13 +268,13 @@ test_expect_success 'check split with --branch for an existing branch' '
         check_equal ''"$(git rev-parse splitbr2)"'' "$spl1"
 '
 
-# 27
+# 28
 test_expect_success 'check split with --branch for an incompatible branch' '
         test_must_fail git subtree split --prefix subdir --onto FETCH_HEAD --branch subdir
 '
 
 
-# 28
+# 29
 test_expect_success 'check split+rejoin' '
         spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
         undo &&
@@ -273,7 +282,7 @@ test_expect_success 'check split+rejoin' '
         check_equal ''"$(last_commit_message)"'' "Split '"'"'subdir/'"'"' into commit '"'"'"$spl1"'"'"'"
 '
 
-# 29
+# 30
 test_expect_success 'add main-sub8' '
         create subdir/main-sub8 &&
         git commit -m "main-sub8"
@@ -282,14 +291,14 @@ test_expect_success 'add main-sub8' '
 # To the subproject!
 cd ./subproj
 
-# 30
+# 31
 test_expect_success 'merge split into subproj' '
         git fetch .. spl1 &&
         git branch spl1 FETCH_HEAD &&
         git merge FETCH_HEAD
 '
 
-# 31
+# 32
 test_expect_success 'add sub9' '
         create sub9 &&
         git commit -m "sub9"
@@ -298,19 +307,19 @@ test_expect_success 'add sub9' '
 # Back to mainline
 cd ..
 
-# 32
+# 33
 test_expect_success 'split for sub8' '
         split2=''"$(git subtree split --annotate='"'*'"' --prefix subdir/ --rejoin)"''
         git branch split2 "$split2"
 '
 
-# 33
+# 34
 test_expect_success 'add main-sub10' '
         create subdir/main-sub10 &&
         git commit -m "main-sub10"
 '
 
-# 34
+# 35
 test_expect_success 'split for sub10' '
         spl3=''"$(git subtree split --annotate='"'*'"' --prefix subdir --rejoin)"'' &&
         git branch spl3 "$spl3"
@@ -319,7 +328,7 @@ test_expect_success 'split for sub10' '
 # To the subproject!
 cd ./subproj
 
-# 35
+# 36
 test_expect_success 'merge split into subproj' '
         git fetch .. spl3 &&
         git branch spl3 FETCH_HEAD &&
@@ -333,13 +342,13 @@ chkms_sub=$(echo $chkms | multiline | sed 's,^,subdir/,' | fixnl)
 chks="sub1 sub2 sub3 sub9"
 chks_sub=$(echo $chks | multiline | sed 's,^,subdir/,' | fixnl)
 
-# 36
+# 37
 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"
 '
 
-# 37
+# 38
 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"
@@ -348,20 +357,20 @@ test_expect_success 'make sure the subproj history *only* contains commits that
 # Back to mainline
 cd ..
 
-# 38
+# 39
 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
 '
 
-# 39
+# 40
 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"
 '
 
-# 40
+# 41
 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
@@ -370,12 +379,12 @@ 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)"''
 '
 
-# 41
+# 42
 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)"'' ""
 '
 
-# 42
+# 43
 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.*:")"'' ""
@@ -385,14 +394,14 @@ test_expect_success 'make sure no "git subtree" tagged commits make it into subp
 mkdir test2
 cd test2
 
-# 43
+# 44
 test_expect_success 'init main' '
         test_create_repo main
 '
 
 cd main
 
-# 44
+# 45
 test_expect_success 'add main1' '
         create main1 &&
         git commit -m "main1"
@@ -400,14 +409,14 @@ test_expect_success 'add main1' '
 
 cd ..
 
-# 45
+# 46
 test_expect_success 'init sub' '
         test_create_repo sub
 '
 
 cd sub
 
-# 46
+# 47
 test_expect_success 'add sub2' '
         create sub2 &&
         git commit -m "sub2"
@@ -417,7 +426,7 @@ cd ../main
 
 # check if split can find proper base without --onto
 
-# 47
+# 48
 test_expect_success 'add sub as subdir in main' '
         git fetch ../sub master &&
         git branch sub2 FETCH_HEAD &&
@@ -426,7 +435,7 @@ test_expect_success 'add sub as subdir in main' '
 
 cd ../sub
 
-# 48
+# 49
 test_expect_success 'add sub3' '
         create sub3 &&
         git commit -m "sub3"
@@ -434,20 +443,20 @@ test_expect_success 'add sub3' '
 
 cd ../main
 
-# 49
+# 50
 test_expect_success 'merge from sub' '
         git fetch ../sub master &&
         git branch sub3 FETCH_HEAD &&
         git subtree merge --prefix subdir sub3
 '
 
-# 50
+# 51
 test_expect_success 'add main-sub4' '
         create subdir/main-sub4 &&
         git commit -m "main-sub4"
 '
 
-# 51
+# 52
 test_expect_success 'split for main-sub4 without --onto' '
         git subtree split --prefix subdir --branch mainsub4
 '
@@ -457,19 +466,19 @@ test_expect_success 'split for main-sub4 without --onto' '
 # have been sub3, but it was not, because its cache was not set to
 # itself)
 
-# 52
+# 53
 test_expect_success 'check that the commit parent is sub3' '
         check_equal ''"$(git log --pretty=format:%P -1 mainsub4)"'' ''"$(git rev-parse sub3)"''
 '
 
-# 53
+# 54
 test_expect_success 'add main-sub5' '
         mkdir subdir2 &&
         create subdir2/main-sub5 &&
         git commit -m "main-sub5"
 '
 
-# 53
+# 55
 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,
@@ -502,7 +511,7 @@ joincommits()
 	echo "$commit $all"
 }
 
-# 54
+# 56
 test_expect_success 'verify one file change per commit' '
         x= &&
         list=''"$(git log --pretty=format:'"'commit: %H'"' | joincommits)"'' &&
-- 
1.7.10.4

[PATCH 3/8] Better Error Handling for add

[David A. Greene]

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, 12 insertions(+)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7ceb413..b8a807a 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -509,8 +509,20 @@ cmd_add()
 	ensure_clean
 	
 	if [ $# -eq 1 ]; then
+	        ref=$(git check-ref-format --normalize "refs/heads/$1") ||
+                die "'$1' is not a valid refspec.  Are you missing a branch?"
+
+	        rev=$(git rev-parse --verify $1) ||
+                die "'$1' is not a valid refspec.  Are you missing a branch?"
+
 		"cmd_add_commit" "$@"
 	elif [ $# -eq 2 ]; then
+	        ref=$(git check-ref-format --normalize "refs/heads/$2") ||
+                die "'$2' is not a valid refspec."
+
+	        rev=$(git rev-parse --verify $2) ||
+                die "'$2' is not a valid refspec."
+
 		"cmd_add_repository" "$@"
 	else
 	    say "error: parameters were '$@'"
-- 
1.7.10.4

[PATCH 4/8] Fix Synopsis

[David A. Greene]

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

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

Change "commit" to "refspec" in the synopsis for add.

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

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

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index b8a807a..ad62dfb 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -8,7 +8,8 @@ if [ $# -eq 0 ]; then
     set -- -h
 fi
 OPTS_SPEC="\
-git subtree add   --prefix=<prefix> <commit>
+git subtree add   --prefix=<prefix> <refspec>
+git subtree add   --prefix=<prefix> <repository> <refspec>
 git subtree merge --prefix=<prefix> <commit>
 git subtree pull  --prefix=<prefix> <repository> <refspec...>
 git subtree push  --prefix=<prefix> <repository> <refspec...>
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index ae420aa..89c2d6e 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

[PATCH 5/8] Honor DESTDIR

[David A. Greene]

From: Adam Tkac <atkac@redhat.com>

Teach git-subtree's Makefile to honor DESTDIR.

Author:    Adam Tkac <atkac@redhat.com>

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

[PATCH 6/8] Make the Manual Directory if Needed

[David A. Greene]

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

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

Author:    Jesper L. Nielsen <lyager@gmail.com>

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..52d6fb9 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)
+	mkdir -p $(man1dir)
 	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
 $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
-- 
1.7.10.4

[PATCH 7/8] Ignore git-subtree

[David A. Greene]

From: Michael Schubert <mschub@elegosoft.com>

Add the git-subtree command executable to .gitignore.

Author:    Michael Schubert <mschub@elegosoft.com>

Signed-off-by:    Michael Schubert <mschub@elegosoft.com>

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/.gitignore |    1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/subtree/.gitignore b/contrib/subtree/.gitignore
index 7e77c9d..91360a3 100644
--- a/contrib/subtree/.gitignore
+++ b/contrib/subtree/.gitignore
@@ -1,4 +1,5 @@
 *~
+git-subtree
 git-subtree.xml
 git-subtree.1
 mainline
-- 
1.7.10.4

[PATCH 8/8] Fix Documentation Typo

[David A. Greene]

From: Michael Schubert <mschub@elegosoft.com>

"split" is documented below "push" so fix the reference to it in
push's documentation.

Author:    Michael Schubert <mschub@elegosoft.com>

Signed-off-by:    Michael Schubert <mschub@elegosoft.com>

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 contrib/subtree/git-subtree.txt |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 89c2d6e..078d4ac 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -94,7 +94,7 @@ pull::
 	repository.
 	
 push::
-	Does a 'split' (see above) using the <prefix> supplied
+	Does a 'split' (see below) using the <prefix> supplied
 	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.
-- 
1.7.10.4

Re: [PATCH 1/8] Use %B for Split Subject/Body

[greened]

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

> 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.

Wow.  So that was a spectacular fail.  Sorry about th duplicate patch
e-mails.  I have no idea how that happened.

                        -David

Re: [PATCH 1/8] Use %B for Split Subject/Body

[Junio C Hamano]

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

> 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.

Is this an unconditional improvement, or is it generally an
improvement but for some users it may be a regression?  I am
guessing it is the former but am just making sure.

> Author:    Techlive Zheng <techlivezheng@gmail.com>
>
> Signed-off-by: David A. Greene <greened@obbligato.org>

Please don't do "Author: " which does not add anything new.  That is
what "From: " is for.  Instead it needs to be a sign-off.

Also, is that a real name, I have to wonder?

Thanks.

Re: [PATCH 1/8] Use %B for Split Subject/Body

[Junio C Hamano]

greened@obbligato.org writes:

> "David A. Greene" <greened@obbligato.org> writes:
>
>> 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.
>
> Wow.  So that was a spectacular fail.  Sorry about th duplicate patch
> e-mails.  I have no idea how that happened.
>
>                         -David

Also, please be careful about the subject line.  I doubt that these
8 patches will stand out as relating to "contrib/subtree", when mixed
in 200 line output of "git shortlog --no-merges".

Thanks.

Re: [PATCH 1/8] Use %B for Split Subject/Body

[Junio C Hamano]

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

> Subject: Re: [PATCH 1/8] Use %B for Split Subject/Body

This needs to say "contrib/subtree" somewhere (applies to all
patches in this series).

> 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.
>
> Author:    Techlive Zheng <techlivezheng@gmail.com>

This needs to be a S-o-b instead; is it a real name, by the way?

> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
>  contrib/subtree/git-subtree.sh     |    5 +++
>  contrib/subtree/t/t7900-subtree.sh |   73 ++++++++++++++++++++++--------------
>  2 files changed, 49 insertions(+), 29 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 920c664..f2b6d4a 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -296,7 +296,12 @@ 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}"
> +	# Use %B rather than %s%n%n%b to handle the special case of a
> +	# commit that only has a subject line.  We don't want to
> +	# introduce a newline after the subject, causing generation of
> +	# a new hash.
>  	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" |

Is it really replacing %s%n%n%b with %B, or is it still an
experiment that is disabled?

>  	(
>  		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 bc2eeb0..93eeb09 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -76,6 +76,10 @@ test_expect_success 'add sub1' '
>          git branch -m master subproj
>  '
>  
> +# Save this hash for testing later.
> +
> +subdir_hash=`git rev-parse HEAD`
> +

We prefer $() over ``; much more readable.

>  # 3
>  test_expect_success 'add sub2' '
>          create sub2 &&
> @@ -155,7 +159,6 @@ test_expect_success 'add main-sub5' '
>          create subdir/main-sub5 &&
>          git commit -m "main-sub5"
>  '
> -
>  # 15
>  test_expect_success 'add main6' '
>          create main6 &&

Why?

> @@ -235,7 +238,19 @@ test_expect_success 'check split with --branch' '
>          check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"

Is quoting screwed up around here (and in many other places in this
patch)?  What are these no-op '' doing?

>  '
>  
> -# 25
> +#25

Why the lossage of a SP?

It may make sense to lose these "# num" that will have to be touched
every time somebody inserts new test pieces in the middle, as a
preparatory step before any of these patches, by the way.  That will
reduce noise in the patches for real changes.

Re: [PATCH 2/8] Add --unannotate

[Junio C Hamano]

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

> From: James Nylen <jnylen@gmail.com>
>
> Teach git-subtree about --unannotate.  This option strips a prefix
> from a commit message when doing a subtree split.

Hrm.  This looks like a workaround for a short-sighted misdesign of
the annotate option that only allowed prefixing a fixed string.  I
have to wonder if it is better to deprecate --annotate and replace
it with a more general "commit log rewriting" facility that can
cover both use cases?

Re: [PATCH 3/8] Better Error Handling for add

[Junio C Hamano]

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

> 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, 12 insertions(+)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 7ceb413..b8a807a 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -509,8 +509,20 @@ cmd_add()
>  	ensure_clean
>  	
>  	if [ $# -eq 1 ]; then
> +	        ref=$(git check-ref-format --normalize "refs/heads/$1") ||
> +                die "'$1' is not a valid refspec.  Are you missing a branch?"

Is a user forbidden from passing a commit that is not at the tip of
an existing branch?  In other words, is

	$ subtree add origin/next~4^2

forbidden?

> +	        rev=$(git rev-parse --verify $1) ||
> +                die "'$1' is not a valid refspec.  Are you missing a branch?"
> +
>  		"cmd_add_commit" "$@"

If you want to make sure you give a comit to add_commit, you can
probably say something like this:

	git rev-parse -q --verify "$1^{commit}" >/dev/null ||
        die "'$1' does not refer to a commit"

>  	elif [ $# -eq 2 ]; then
> +	        ref=$(git check-ref-format --normalize "refs/heads/$2") ||
> +                die "'$2' is not a valid refspec."
> +
> +	        rev=$(git rev-parse --verify $2) ||
> +                die "'$2' is not a valid refspec."
> +

Likewise.

>  		"cmd_add_repository" "$@"
>  	else
>  	    say "error: parameters were '$@'"

Re: [PATCH 4/8] Fix Synopsis

[Junio C Hamano]

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

> From: "David A. Greene" <greened@obbligato.org>
>
> Fix the documentation of add to show that a repository can be
> specified along with a commit.
>
> Change "commit" to "refspec" in the synopsis for add.
>
> Suggested by Yann Dirson <dirson@bertin.fr>.
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
>  contrib/subtree/git-subtree.sh  |    3 ++-
>  contrib/subtree/git-subtree.txt |    3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index b8a807a..ad62dfb 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -8,7 +8,8 @@ if [ $# -eq 0 ]; then
>      set -- -h
>  fi
>  OPTS_SPEC="\
> -git subtree add   --prefix=<prefix> <commit>
> +git subtree add   --prefix=<prefix> <refspec>

Again, this is not <refspec> but <commit>.

> +git subtree add   --prefix=<prefix> <repository> <refspec>

This is given to "fetch" and it seems to acccept any <refspec>, so
it is probably a good change (I didn't fully follow the codepath,
though).

>  git subtree merge --prefix=<prefix> <commit>
>  git subtree pull  --prefix=<prefix> <repository> <refspec...>
>  git subtree push  --prefix=<prefix> <repository> <refspec...>
> diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
> index ae420aa..89c2d6e 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>

Re: [PATCH 5/8] Honor DESTDIR

[Junio C Hamano]

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

> From: Adam Tkac <atkac@redhat.com>
>
> Teach git-subtree's Makefile to honor DESTDIR.
>
> Author:    Adam Tkac <atkac@redhat.com>
>
> Signed-off-by:    Adam Tkac <atkac@redhat.com>
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---

The contents of the patch looks sensible; the above is questionable
as all the other messages in this series, though.  Did any of our
tools cause this failure?  If so I would like to know more about it.

>  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 $^

Re: [PATCH 6/8] Make the Manual Directory if Needed

[Junio C Hamano]

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

> From: "Jesper L. Nielsen" <lyager@gmail.com>
>
> Before install git-subtree documentation, make sure the manpage
> directory exists.
>
> Author:    Jesper L. Nielsen <lyager@gmail.com>
>
> 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..52d6fb9 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)
> +	mkdir -p $(man1dir)

We seem to use "$(INSTALL) -d -m 755" for this kind of thing (see
the Documentation/Makefile).

>  	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
>  
>  $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)

Re: [PATCH 7/8] Ignore git-subtree

[Junio C Hamano]

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

> From: Michael Schubert <mschub@elegosoft.com>
>
> Add the git-subtree command executable to .gitignore.
>
> Author:    Michael Schubert <mschub@elegosoft.com>
>
> Signed-off-by:    Michael Schubert <mschub@elegosoft.com>
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---

Seems sensible; I think I have this already in my tree.

>  contrib/subtree/.gitignore |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/contrib/subtree/.gitignore b/contrib/subtree/.gitignore
> index 7e77c9d..91360a3 100644
> --- a/contrib/subtree/.gitignore
> +++ b/contrib/subtree/.gitignore
> @@ -1,4 +1,5 @@
>  *~
> +git-subtree
>  git-subtree.xml
>  git-subtree.1
>  mainline

Re: [PATCH 1/8] Use %B for Split Subject/Body

[greened]

Junio C Hamano <gitster@pobox.com> writes:

> "David A. Greene" <greened@obbligato.org> writes:
>
>> 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.
>
> Is this an unconditional improvement, or is it generally an
> improvement but for some users it may be a regression?  I am
> guessing it is the former but am just making sure.

The former.

>> Author:    Techlive Zheng <techlivezheng@gmail.com>
>>
>> Signed-off-by: David A. Greene <greened@obbligato.org>
>
> Please don't do "Author: " which does not add anything new.  That is
> what "From: " is for.  Instead it needs to be a sign-off.

Ok.  Unfortunately I sent a number of patches like that.  Do you want me
to re-send them?

> Also, is that a real name, I have to wonder?

No idea.  Not likely, I'd say.

                        -David

Re: [PATCH 1/8] Use %B for Split Subject/Body

[greened]

Junio C Hamano <gitster@pobox.com> writes:

> Also, please be careful about the subject line.  I doubt that these
> 8 patches will stand out as relating to "contrib/subtree", when mixed
> in 200 line output of "git shortlog --no-merges".

Ah, ok.  I'll be more careful next time.

                          -David

Re: [PATCH 1/8] Use %B for Split Subject/Body

[greened]

Junio C Hamano <gitster@pobox.com> writes:

> "David A. Greene" <greened@obbligato.org> writes:
>
>> Subject: Re: [PATCH 1/8] Use %B for Split Subject/Body
>
> This needs to say "contrib/subtree" somewhere (applies to all
> patches in this series).

Ok.  Shall I re-send everything?

>> 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.
>>
>> Author:    Techlive Zheng <techlivezheng@gmail.com>
>
> This needs to be a S-o-b instead; is it a real name, by the way?

Ok.  No idea about the name but his online presence seems consistent at
least.

>> +# Save this hash for testing later.
>> +
>> +subdir_hash=`git rev-parse HEAD`
>> +
>
> We prefer $() over ``; much more readable.

Ack, of course.  I don't know how I missed that.

>>  # 15
>>  test_expect_success 'add main6' '
>>          create main6 &&
>
> Why?

It was in the original testsuite from Avery.  I didn't add or remove any
tests when I first integrated git-subtree.

>> @@ -235,7 +238,19 @@ test_expect_success 'check split with --branch' '
>>          check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
>
> Is quoting screwed up around here (and in many other places in this
> patch)?  What are these no-op '' doing?

I assumed they are there to get the double-quotes around the command.
I'll see about removing them.

>> -# 25
>> +#25
>
> Why the lossage of a SP?

I think this got fixed later in the series.

> It may make sense to lose these "# num" that will have to be touched
> every time somebody inserts new test pieces in the middle, as a
> preparatory step before any of these patches, by the way.  That will
> reduce noise in the patches for real changes.

Yeah, I know, but it makes it really easy to find a test when something
goes wrong.

                            -David

Re: [PATCH 2/8] Add --unannotate

[greened]

Junio C Hamano <gitster@pobox.com> writes:

> "David A. Greene" <greened@obbligato.org> writes:
>
>> From: James Nylen <jnylen@gmail.com>
>>
>> Teach git-subtree about --unannotate.  This option strips a prefix
>> from a commit message when doing a subtree split.
>
> Hrm.  This looks like a workaround for a short-sighted misdesign of
> the annotate option that only allowed prefixing a fixed string.  I
> have to wonder if it is better to deprecate --annotate and replace
> it with a more general "commit log rewriting" facility that can
> cover both use cases?

That's not a bad idea.  I'd have to think a bit about a sensible design.
Do you have any ideas, James?

In the meantime, will you apply the patch or do you prefer a new design?

                      -David

Re: [PATCH 3/8] Better Error Handling for add

[greened]

Junio C Hamano <gitster@pobox.com> writes:

>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
>> index 7ceb413..b8a807a 100755
>> --- a/contrib/subtree/git-subtree.sh
>> +++ b/contrib/subtree/git-subtree.sh
>> @@ -509,8 +509,20 @@ cmd_add()
>>  	ensure_clean
>>  	
>>  	if [ $# -eq 1 ]; then
>> +	        ref=$(git check-ref-format --normalize "refs/heads/$1") ||
>> +                die "'$1' is not a valid refspec.  Are you missing a branch?"
>
> Is a user forbidden from passing a commit that is not at the tip of
> an existing branch?  In other words, is
>
> 	$ subtree add origin/next~4^2
>
> forbidden?

Good point.  It probably shouldn't be.  I think rev-parse should be
enough of a check.

>> +	        rev=$(git rev-parse --verify $1) ||
>> +                die "'$1' is not a valid refspec.  Are you missing a branch?"
>> +
>>  		"cmd_add_commit" "$@"
>
> If you want to make sure you give a comit to add_commit, you can
> probably say something like this:
>
> 	git rev-parse -q --verify "$1^{commit}" >/dev/null ||
>         die "'$1' does not refer to a commit"

What does $1^{commit} mean?  I think your suggestion is what I want but
I don't know what it means yet.  :)

                           -David

Re: [PATCH 4/8] Fix Synopsis

[greened]

Junio C Hamano <gitster@pobox.com> writes:

>> -git subtree add   --prefix=<prefix> <commit>
>> +git subtree add   --prefix=<prefix> <refspec>
>
> Again, this is not <refspec> but <commit>.

Ok, I need to study the terminology.  :)

>> +git subtree add   --prefix=<prefix> <repository> <refspec>
>
> This is given to "fetch" and it seems to acccept any <refspec>, so
> it is probably a good change (I didn't fully follow the codepath,
> though).

I think you are correct.

                          -David

Re: [PATCH 5/8] Honor DESTDIR

[greened]

Junio C Hamano <gitster@pobox.com> writes:

> "David A. Greene" <greened@obbligato.org> writes:
>
>> From: Adam Tkac <atkac@redhat.com>
>>
>> Teach git-subtree's Makefile to honor DESTDIR.
>>
>> Author:    Adam Tkac <atkac@redhat.com>
>>
>> Signed-off-by:    Adam Tkac <atkac@redhat.com>
>>
>> Signed-off-by: David A. Greene <greened@obbligato.org>
>> ---
>
> The contents of the patch looks sensible; the above is questionable
> as all the other messages in this series, though.  Did any of our
> tools cause this failure?  If so I would like to know more about it.

What failure are you referring to?  When I used git send-email --author,
the Author: line was commented out.  I assumed I was supposed to
uncomment it.  Guess I was wrong.

I'll re-send the series since you pointed out a number of improvements.

                       -David

Re: [PATCH 6/8] Make the Manual Directory if Needed

[greened]

Junio C Hamano <gitster@pobox.com> writes:

>> diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
>> index 36ae3e4..52d6fb9 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)
>> +	mkdir -p $(man1dir)
>
> We seem to use "$(INSTALL) -d -m 755" for this kind of thing (see
> the Documentation/Makefile).

Ok, will fix.

                             -Dave

Re: [PATCH 1/8] Use %B for Split Subject/Body

[Junio C Hamano]

greened@obbligato.org writes:

> Ack, of course.  I don't know how I missed that.
>
>>>  # 15
>>>  test_expect_success 'add main6' '
>>>          create main6 &&
>>
>> Why?
>
> It was in the original testsuite from Avery.  I didn't add or remove any
> tests when I first integrated git-subtree.

The question was about the lossage of the blank line, which does not
seem to be related to what this patch wants to do.

>>> -# 25
>>> +#25
>>
>> Why the lossage of a SP?
>
> I think this got fixed later in the series.

That is not a good excuse to introduce breakages in the first place, no?

>> It may make sense to lose these "# num" that will have to be touched
>> every time somebody inserts new test pieces in the middle, as a
>> preparatory step before any of these patches, by the way.  That will
>> reduce noise in the patches for real changes.
>
> Yeah, I know, but it makes it really easy to find a test when something
> goes wrong.

That is what "tXXXX-*.sh -i" is for, isn't it?

Re: [PATCH 2/8] Add --unannotate

[Junio C Hamano]

greened@obbligato.org writes:

> In the meantime, will you apply the patch or do you prefer a new design?

The --unannotate option will become a baggage you will have to keep
working until the end of time, if we applied it.  I think it is not
too uch a baggage, so it probably is OK.

Re: [PATCH 3/8] Better Error Handling for add

[Junio C Hamano]

greened@obbligato.org writes:

>> If you want to make sure you give a comit to add_commit, you can
>> probably say something like this:
>>
>> 	git rev-parse -q --verify "$1^{commit}" >/dev/null ||
>>         die "'$1' does not refer to a commit"
>
> What does $1^{commit} mean?

"$thing^{type}" tells Git to interpret the $thing as that type (and
error out if it can't).

So v1.0.0^{commit} is a less cryptic way to say v1.0.0^0 (there is
no need to say "zeroth parent of a commit is the commit itself?
Yeah, it makes sort of sense" when you learn it).

"git cat-file -t junio-gpg-pub^{blob}" will say "blob", but you will
get a failure from "git rev-parse v1.0.0^{blob}" as you can only
dereference a tag that refers to a commit down to the comit and then
to its top-level tree, but not to a single blob.

And you can ask for the tree object with v1.0.0^{tree}, for example.

        

Re: [PATCH 1/8] Use %B for Split Subject/Body

[郑文辉(Techlive Zheng)]

 2013/1/1 Junio C Hamano <gitster@pobox.com>:
> "David A. Greene" <greened@obbligato.org> writes:
>
>> 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.
>
> Is this an unconditional improvement, or is it generally an
> improvement but for some users it may be a regression?  I am
> guessing it is the former but am just making sure.
>
>> Author:    Techlive Zheng <techlivezheng@gmail.com>
>>
>> Signed-off-by: David A. Greene <greened@obbligato.org>
>
> Please don't do "Author: " which does not add anything new.  That is
> what "From: " is for.  Instead it needs to be a sign-off.
>
> Also, is that a real name, I have to wonder?
>
Hmm, sorry about the confusing.

I am a Chinese, I coined that first name a couple years ago when I
decided to have a unique name across the web. My real name is "郑文辉" in
Chinese，translate to English by its pronucation is "Wenhui
Zheng"，which means "Zheng" is acturally my real last name. The first
name "Wenhui" does not have any meaning in English, so I coined it by
"Tech" + "Live", which I interprate it as "Technological Living",
thus, "Techlive Zheng" is the name I am currently using online.

Here are some links:

* Let the code talks. https://github.com/techlivezheng
* I cross the great GFW to use twitter. https://twitter.com/techlivezheng
* Also search "Techlive Zheng" in Google, the result should be unique to me.

So, no doubt, I am a real person, just with kind of an uncommon name.

>> Thanks.

Re: [PATCH 1/8] Use %B for Split Subject/Body

[郑文辉(Techlive Zheng)]

2013/1/1 Junio C Hamano <gitster@pobox.com>:
> "David A. Greene" <greened@obbligato.org> writes:
>
>> 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.
>
> Is this an unconditional improvement, or is it generally an
> improvement but for some users it may be a regression?  I am
> guessing it is the former but am just making sure.

This patch will make sure the commits in the result branch by using
`git-subtree split` stays intact as they were in the original branch.

This patch will break the current existing branch that splitted before
this patch, becuase these branches were splitted with the wrongly
altered commit messages.

Maybe a fallback option should be added to make sure these branches
could still be updated.

Though, this patch defintely should be merged, becuase no one expects
his commit message be altered durging the splitting process.

Re: [PATCH 1/8] Use %B for Split Subject/Body

[Junio C Hamano]

"郑文辉(Techlive Zheng)"  <techlivezheng@gmail.com> writes:

> Though, this patch defintely should be merged, becuase no one expects
> his commit message be altered durging the splitting process.

Are you saying that after double-checking what was posted?  It said
something like this below, which does not look like 'definitely
should be' to me.

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 920c664..f2b6d4a 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -296,7 +296,12 @@ 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}"
+	# Use %B rather than %s%n%n%b to handle the special case of a
+	# commit that only has a subject line.  We don't want to
+	# introduce a newline after the subject, causing generation of
+	# a new hash.
 	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

Re: [PATCH 1/8] Use %B for Split Subject/Body

[greened]

Junio C Hamano <gitster@pobox.com> writes:

> The question was about the lossage of the blank line, which does not
> seem to be related to what this patch wants to do.

Ah, missed that.

>>>> -# 25
>>>> +#25
>>>
>>> Why the lossage of a SP?
>>
>> I think this got fixed later in the series.
>
> That is not a good excuse to introduce breakages in the first place, no?

Oh, I agree.  I wasn't making excuses.  :)

>>> It may make sense to lose these "# num" that will have to be touched
>>> every time somebody inserts new test pieces in the middle, as a
>>> preparatory step before any of these patches, by the way.  That will
>>> reduce noise in the patches for real changes.
>>
>> Yeah, I know, but it makes it really easy to find a test when something
>> goes wrong.
>
> That is what "tXXXX-*.sh -i" is for, isn't it?

Oh, I didn't know about that!

                        -David

Re: [PATCH 2/8] Add --unannotate

[greened]

Junio C Hamano <gitster@pobox.com> writes:

> greened@obbligato.org writes:
>
>> In the meantime, will you apply the patch or do you prefer a new design?
>
> The --unannotate option will become a baggage you will have to keep
> working until the end of time, if we applied it.  I think it is not
> too uch a baggage, so it probably is OK.

Ok.  I think it's worth applying since people find it useful.  It's not
very complicated.

                       -David

Re: [PATCH 2/8] Add --unannotate

[James Nylen]

On Tue, Jan 1, 2013 at 5:31 PM,  <greened@obbligato.org> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> "David A. Greene" <greened@obbligato.org> writes:
>>
>>> From: James Nylen <jnylen@gmail.com>
>>>
>>> Teach git-subtree about --unannotate.  This option strips a prefix
>>> from a commit message when doing a subtree split.
>>
>> Hrm.  This looks like a workaround for a short-sighted misdesign of
>> the annotate option that only allowed prefixing a fixed string.  I
>> have to wonder if it is better to deprecate --annotate and replace
>> it with a more general "commit log rewriting" facility that can
>> cover both use cases?
>
> That's not a bad idea.  I'd have to think a bit about a sensible design.
> Do you have any ideas, James?

I just now saw these emails.  I'm having a hard time thinking of any
good use case other than:

 - add "fancylib" as a subtree of "myprog"
 - commit to myprog repo: "fancylib: don't crash as much"
 - split these commits back out to fancylib's main repo, and remove
the "fancylib: " prefix

You could potentially have something like "Don't crash as much
(fancylib)" but that's awkward.  What might you want to do with a
pattern-based rewrite that doesn't involve removing a prefix when
splitting commits?

In fact, I don't see the use of the original --annotate option at all,
since it causes more detailed commit messages in the smaller of the
two repositories.

Re: [PATCH 2/8] Add --unannotate

[greened]

James Nylen <jnylen@gmail.com> writes:

> I just now saw these emails.  I'm having a hard time thinking of any
> good use case other than:
>
>  - add "fancylib" as a subtree of "myprog"
>  - commit to myprog repo: "fancylib: don't crash as much"
>  - split these commits back out to fancylib's main repo, and remove
> the "fancylib: " prefix

That does seem to me to be the common case, at least.

> You could potentially have something like "Don't crash as much
> (fancylib)" but that's awkward.  What might you want to do with a
> pattern-based rewrite that doesn't involve removing a prefix when
> splitting commits?

I'm not really sure.  I've never used --annotate in my own work.

> In fact, I don't see the use of the original --annotate option at all,
> since it causes more detailed commit messages in the smaller of the
> two repositories.

I'll have to look back through Avery's logs and see if I can puzzle out
why this was added.  If it's not useful, perhaps we can remove it before
migrating to mainline.

Junio, is there a policy for backward-compatability in contrib?  I hope
that since that directory is for stuff not yet in mainline, there is
some room to massage the user interface.

                            -David

Re: [PATCH 2/8] Add --unannotate

[Junio C Hamano]

greened@obbligato.org writes:

> Junio, is there a policy for backward-compatability in contrib?  I hope
> that since that directory is for stuff not yet in mainline, there is
> some room to massage the user interface.

I do not think there is anything more than "we wish there were fewer
such end-user facing changes."

The purpose of having anything in contrib/ is to give them a way to
gain more visibility than they otherwise would as standalone
projects on their own. I don't want to control the day-to-day
quality issues in them.  That is what we have "guilty parties" for
contrib/ subdirectories for, so that they can decide how mature
their pieces in contrib/ are and what kind of b/c break is
acceptable ;-)

Thanks.