Git development
 help / color / mirror / Atom feed
* [PATCH 07/13] contrib/subtree: Fix whitespaces
From: David A. Greene @ 2013-02-05 11:36 UTC (permalink / raw)
  To: git; +Cc: Techlive Zheng, David A. Greene
In-Reply-To: <1360064219-28789-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 05/13] contrib/subtree: Honor DESTDIR
From: David A. Greene @ 2013-02-05 11:36 UTC (permalink / raw)
  To: git; +Cc: Adam Tkac, David A. Greene
In-Reply-To: <1360064219-28789-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 08/13] contrib/subtree: Add vim modeline
From: David A. Greene @ 2013-02-05 11:36 UTC (permalink / raw)
  To: git; +Cc: Techlive Zheng, David A. Greene
In-Reply-To: <1360064219-28789-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 11:36 UTC (permalink / raw)
  To: git; +Cc: Techlive Zheng, David A. Greene
In-Reply-To: <1360064219-28789-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 04/13] contrib/subtree: Fix Synopsis
From: David A. Greene @ 2013-02-05 11:36 UTC (permalink / raw)
  To: git; +Cc: David A. Greene
In-Reply-To: <1360064219-28789-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 06/13] contrib/subtree: Make the Manual Directory if Needed
From: David A. Greene @ 2013-02-05 11:36 UTC (permalink / raw)
  To: git; +Cc: Jesper L. Nielsen, David A. Greene
In-Reply-To: <1360064219-28789-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 03/13] contrib/subtree: Better Error Handling for add
From: David A. Greene @ 2013-02-05 11:36 UTC (permalink / raw)
  To: git; +Cc: David A. Greene
In-Reply-To: <1360064219-28789-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 02/13] contrib/subtree: Use %B for Split Subject/Body
From: David A. Greene @ 2013-02-05 11:36 UTC (permalink / raw)
  To: git; +Cc: Techlive Zheng, David A. Greene
In-Reply-To: <1360064219-28789-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 11:36 UTC (permalink / raw)
  To: git; +Cc: David A. Greene
In-Reply-To: <1360064219-28789-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

* Rebased git-subtree changes
From: David A. Greene @ 2013-02-05 11:36 UTC (permalink / raw)
  To: git

Here's a re-send of the git-subtree patches after rebasing onto
master.  Hopefully Junio will have better luck applying these.

                            -David

^ permalink raw reply

* Re: [RFC] Should "log --cc" imply "log --cc -p"?
From: Jeff King @ 2013-02-05 11:22 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Junio C Hamano, Michael J Gruber, git
In-Reply-To: <CACBZZX7FJ39sereptAQ40Q4MwEpf15v0H=++WvB7+BpMknxoYw@mail.gmail.com>

On Tue, Feb 05, 2013 at 11:16:52AM +0100, Ævar Arnfjörð Bjarmason wrote:

> On Mon, Feb 4, 2013 at 5:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > "git log/diff-files -U8" do not need "-p" to enable textual patches,
> > for example.  It is "I already told you that I want 8-line context.
> > For what else, other than showing textual diff, do you think I told
> > you that for?" and replacing "8-line context" with various other
> > options that affect patch generation will give us a variety of end
> > user complaints that would tell us that C) is more intuitive to
> > them.
> 
> On a related note I think "--full-diff" should imply "-p" too.

I don't think that is in the same class. --full-diff is quite useful for
many other diff formats. E.g.:

  git log --full-diff --raw Makefile

If you are proposing to default to "-p" when "--full-diff" is used but
no format is given, that is a slightly different thing. The --full-diff
in such a case is indeed useless, but I do not think it necessarily
follows that "-p" was what the user wanted.

-Peff

^ permalink raw reply

* Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names
From: Jeff King @ 2013-02-05 11:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Shawn Pearce
In-Reply-To: <20130205091938.GB24973@sigill.intra.peff.net>

On Tue, Feb 05, 2013 at 04:19:38AM -0500, Jeff King wrote:

> But taking a step back, this really seems quite inferior to an
> extension that would allow the client to share its refspecs with the
> server.  That would solve the bandwidth efficiency problem for normal
> fetchers who are

I should have read your cover letter more closely, as I see you make the
point there that this is no longer about the efficiency, but about
uncluttering.

I'm not sure I like it as an uncluttering tool, though; for the reasons
I stated in my previous mail, it makes it much more awkward for the
moments when you actually do want to fetch some of the "clutter".

-Peff

^ permalink raw reply

* Re: Is anyone working on a next-gen Git protocol (Re: [PATCH v3 0/8] Hiding refs)
From: Ævar Arnfjörð Bjarmason @ 2013-02-05 11:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Shawn Pearce, Johannes Schindelin

On Wed, Jan 30, 2013 at 7:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The third round.
>
>  - Multi-valued variable transfer.hiderefs lists prefixes of ref
>    hierarchies to be hidden from the requests coming over the
>    network.
>
>  - A configuration optionally allows uploadpack to accept fetch
>    requests for an object at the tip of a hidden ref.
>
> Elsewhere, we discussed "delaying ref advertisement" (aka "expand
> refs"), but it is an orthogonal feature and this "hiding refs
> completely from advertisement" series does not attempt to address.

I'm a bit late to this so sorry if this has been covered before.

In the initial draft of this series the rationale for it was "reducing
the network cost while talking with a repository with tons of
refs"[1]. But later you seem to have changed your mind, and "network
bandwidth reduction of advertisement is a side effect of clutter
reduction, and not necessarily the primary goal".

Do you have any plans for something that *does* have the reduction of
network bandwidth as a primary goal?

In October I asked if anyone was working on a next-gen Git protocol[3]
that would provide clients with the ability to specify what refs they
wanted. You replied to me off-list saying "Yes".

Is this what you've been working on? Because if so I misunderstood you
thinking you were going to work on something that gave clients the
ability specify what they wanted before the initial ref advertisement.

I'm still very keen to have that ability, so if you're not working on
it I just might give it a go.

1. http://article.gmane.org/gmane.comp.version-control.git/213951
2. http://article.gmane.org/gmane.comp.version-control.git/213984
3. http://article.gmane.org/gmane.comp.version-control.git/214025
4. http://thread.gmane.org/gmane.comp.version-control.git/207190

^ permalink raw reply

* Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters
From: Jeff King @ 2013-02-05 11:13 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, git
In-Reply-To: <2c0641ea4df6a872a4466efe0c0124f304f44c3e.1359991521.git.git@drmicha.warpmail.net>

On Mon, Feb 04, 2013 at 04:27:31PM +0100, Michael J Gruber wrote:

> Recently and not so recently, we made sure that log/grep type operations
> use textconv filters when a userfacing diff would do the same:
> 
> ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
> b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
> 0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)
> 
> "git grep" currently does not use textconv filters at all, that is
> neither for displaying the match and context nor for the actual grepping.
> 
> Introduce a binary mode "--textconv" (in addition to "--text" and "-I")
> which makes git grep use any configured textconv filters for grepping
> and output purposes.

Sounds like a reasonable goal.

>     The difficulty is in getting the different cases (blob/sha1 vs.
>     worktree) right, and in making the changes minimally invasive. It seems
>     that some more refactoring could help: "git show --textconv" does not
>     use textconv filters when used on blobs either. (It does for diffs, of
>     course.) Most existing helper functions are tailored for diffs.

I think "git show" with blobs originally did not because we have no
filename with which to look up the attributes. IIRC, the patches to
support "cat-file --textconv" taught get_sha1_with_context to report the
path at which we found a blob. I suspect it is mostly a matter of
plumbing that information from the revision parser through to
show_blob_object.

>     Nota bene: --textconv does not affect "diff --stat" either...

Yeah, though I wonder if it should be on by default. The diffstat for a
binary file, unlike the diff, is already useful. The diffstat of the
textconv'd data may _also_ be useful, of course.

> @@ -659,6 +659,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  		OPT_SET_INT('I', NULL, &opt.binary,
>  			N_("don't match patterns in binary files"),
>  			GREP_BINARY_NOMATCH),
> +		OPT_SET_INT(0, "textconv", &opt.binary,
> +			N_("process binary files with textconv filters"),
> +			GREP_BINARY_TEXTCONV),

Is this really a new form of GREP_BINARY_*? What happens when a file
does not have a textconv filter?

I would expect this to be more like diff's "--textconv" flag, which is
really "allow textconv to be respected". Then you could do:

  git grep -I --textconv foo

to grep in the text version of files which support it, and ignore the
rest.

> -static int grep_source_load(struct grep_source *gs);
> -static int grep_source_is_binary(struct grep_source *gs);
> +static int grep_source_load(struct grep_source *gs, struct grep_opt *opt);
> +static int grep_source_is_binary(struct grep_source *gs, struct grep_opt *opt);

Hmm. grep_source_load is more or less the analogue of
diff_populate_filespec, which does not know about textconv at all. So I
feel like this might be going in at the wrong layer...

> @@ -1354,14 +1356,15 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
>  
>  	switch (opt->binary) {
>  	case GREP_BINARY_DEFAULT:
> -		if (grep_source_is_binary(gs))
> +		if (grep_source_is_binary(gs, opt))
>  			binary_match_only = 1;
>  		break;
>  	case GREP_BINARY_NOMATCH:
> -		if (grep_source_is_binary(gs))
> +		if (grep_source_is_binary(gs, opt))
>  			return 0; /* Assume unmatch */
>  		break;

The is_binary function learned about "opt" so that it could pass it to
grep_source_load, which might do the textconv for us. But we _know_ that
we will not here, because we see that we have other GREP_BINARY flags.

And when we do have _TEXTCONV:

>  	case GREP_BINARY_TEXT:
> +	case GREP_BINARY_TEXTCONV:
>  		break;

We do not call is_binary. :)

> -static int grep_source_load_sha1(struct grep_source *gs)
> +static int grep_source_load_sha1(struct grep_source *gs, struct grep_opt *opt)
>  {
>  	enum object_type type;
> -
>  	grep_read_lock();
> -	gs->buf = read_sha1_file(gs->identifier, &type, &gs->size);
> +	if (opt->binary == GREP_BINARY_TEXTCONV) {
> +		struct diff_filespec *df = alloc_filespec(gs->name);
> +		gs->size = fill_textconv(gs->driver, df, &gs->buf);
> +		free_filespec(df);
> +	} else {
> +		gs->buf = read_sha1_file(gs->identifier, &type, &gs->size);
> +	}

So here we do the textconv for the sha1 case. But what about file
sources?

This is why I think the layer is wrong; you want the fill_textconv
function to call your abstract _load function (in the diff_filespec
world, it is diff_filespec_populate, but it is the moral equivalent).
And you want it to hold off as long as possible in case we can pull the
value from cache, or feed the working tree version of a file straight to
the filter.

> -void grep_source_load_driver(struct grep_source *gs)
> +void grep_source_load_driver(struct grep_source *gs, struct grep_opt *opt)
>  {
>  	if (gs->driver)
>  		return;
>  
> -	grep_attr_lock();
> +	grep_attr_lock(); //TODO
> +	printf("Looking up userdiff driver for: %s", gs->path);
>  	if (gs->path)
>  		gs->driver = userdiff_find_by_path(gs->path);
>  	if (!gs->driver)
>  		gs->driver = userdiff_find_by_name("default");
> +	if (opt->binary == GREP_BINARY_TEXTCONV)
> +		gs->driver = userdiff_get_textconv(gs->driver);
>  	grep_attr_unlock();
>  }

This is wrong. The point of userdiff_get_textconv is that it will return
NULL when we are not doing textconv for this path. So you can use it
like:

  struct userdiff_driver *textconv = userdiff_get_textconv(gs->driver);

  if (textconv) {
          /* ok, we are doing textconv. Call our fill_textconv
           * equivalent. */
  }
  else {
          /* nope, plain old file. */
  }

But by assigning it on top of gs->driver, you're going to end up with a
NULL driver sometimes. And the post-condition of the load_driver
function is that gs->driver always points to a valid driver (even if it
is the default one). I wouldn't be surprised if this causes segfaults.


So I would do it more like the patch below. Only lightly tested by me.
There are some refactoring opportunities if you want to bring
grep_source and diff_filespec closer together.

---
diff --git a/builtin/grep.c b/builtin/grep.c
index 8025964..915c8ef 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -659,6 +659,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('I', NULL, &opt.binary,
 			N_("don't match patterns in binary files"),
 			GREP_BINARY_NOMATCH),
+		OPT_BOOL(0, "textconv", &opt.allow_textconv,
+			 N_("process binary files with textconv filters")),
 		{ OPTION_INTEGER, 0, "max-depth", &opt.max_depth, N_("depth"),
 			N_("descend at most <depth> levels"), PARSE_OPT_NONEG,
 			NULL, 1 },
diff --git a/grep.c b/grep.c
index 4bd1b8b..3880d64 100644
--- a/grep.c
+++ b/grep.c
@@ -2,6 +2,8 @@
 #include "grep.h"
 #include "userdiff.h"
 #include "xdiff-interface.h"
+#include "diff.h"
+#include "diffcore.h"
 
 static int grep_source_load(struct grep_source *gs);
 static int grep_source_is_binary(struct grep_source *gs);
@@ -1321,6 +1323,58 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
 	fwrite(buf, size, 1, stdout);
 }
 
+static int fill_textconv_grep(struct userdiff_driver *driver,
+			      struct grep_source *gs)
+{
+	struct diff_filespec *df;
+	char *buf;
+	size_t size;
+
+	if (!driver || !driver->textconv)
+		return grep_source_load(gs);
+
+	/*
+	 * The textconv interface is intimately tied to diff_filespecs, so we
+	 * have to pretend to be one. If we could unify the grep_source
+	 * and diff_filespec structs, this mess could just go away.
+	 */
+	df = alloc_filespec(gs->path);
+	switch (gs->type) {
+	case GREP_SOURCE_SHA1:
+		fill_filespec(df, gs->identifier, 1, 0100644);
+		break;
+	case GREP_SOURCE_FILE:
+		fill_filespec(df, null_sha1, 0, 0100644);
+		break;
+	default:
+		die("BUG: attempt to textconv something without a path?");
+	}
+
+	/*
+	 * fill_textconv is not remotely thread-safe; it may load objects
+	 * behind the scenes, and it modifies the global diff tempfile
+	 * structure.
+	 */
+	grep_read_lock();
+	size = fill_textconv(driver, df, &buf);
+	grep_read_unlock();
+	free_filespec(df);
+
+	/*
+	 * The normal fill_textconv usage by the diff machinery would just keep
+	 * the textconv'd buf separate from the diff_filespec. But much of the
+	 * grep code passes around a grep_source and assumes that its "buf"
+	 * pointer is the beginning of the thing we are searching. So let's
+	 * install our textconv'd version into the grep_source, taking care not
+	 * to leak any existing buffer.
+	 */
+	grep_source_clear_data(gs);
+	gs->buf = buf;
+	gs->size = size;
+
+	return 0;
+}
+
 static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int collect_hits)
 {
 	char *bol;
@@ -1331,6 +1385,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 	unsigned count = 0;
 	int try_lookahead = 0;
 	int show_function = 0;
+	struct userdiff_driver *textconv = NULL;
 	enum grep_context ctx = GREP_CONTEXT_HEAD;
 	xdemitconf_t xecfg;
 
@@ -1352,19 +1407,36 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 	}
 	opt->last_shown = 0;
 
-	switch (opt->binary) {
-	case GREP_BINARY_DEFAULT:
-		if (grep_source_is_binary(gs))
-			binary_match_only = 1;
-		break;
-	case GREP_BINARY_NOMATCH:
-		if (grep_source_is_binary(gs))
-			return 0; /* Assume unmatch */
-		break;
-	case GREP_BINARY_TEXT:
-		break;
-	default:
-		die("bug: unknown binary handling mode");
+	if (opt->allow_textconv) {
+		grep_source_load_driver(gs);
+		/*
+		 * We might set up the shared textconv cache data here, which
+		 * is not thread-safe.
+		 */
+		grep_attr_lock();
+		textconv = userdiff_get_textconv(gs->driver);
+		grep_attr_unlock();
+	}
+
+	/*
+	 * We know the result of a textconv is text, so we only have to care
+	 * about binary handling if we are not using it.
+	 */
+	if (!textconv) {
+		switch (opt->binary) {
+		case GREP_BINARY_DEFAULT:
+			if (grep_source_is_binary(gs))
+				binary_match_only = 1;
+			break;
+		case GREP_BINARY_NOMATCH:
+			if (grep_source_is_binary(gs))
+				return 0; /* Assume unmatch */
+			break;
+		case GREP_BINARY_TEXT:
+			break;
+		default:
+			die("bug: unknown binary handling mode");
+		}
 	}
 
 	memset(&xecfg, 0, sizeof(xecfg));
@@ -1372,7 +1444,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 
 	try_lookahead = should_lookahead(opt);
 
-	if (grep_source_load(gs) < 0)
+	if (fill_textconv_grep(textconv, gs) < 0)
 		return 0;
 
 	bol = gs->buf;
diff --git a/grep.h b/grep.h
index 8fc854f..94a7ac2 100644
--- a/grep.h
+++ b/grep.h
@@ -106,6 +106,7 @@ struct grep_opt {
 #define GREP_BINARY_NOMATCH	1
 #define GREP_BINARY_TEXT	2
 	int binary;
+	int allow_textconv;
 	int extended;
 	int use_reflog_filter;
 	int pcre;

^ permalink raw reply related

* will git provide `submodule remove` option ?‏
From: Lingcha X @ 2013-02-05 10:32 UTC (permalink / raw)
  To: git@vger.kernel.org

As we all know, git provides `submodule add , init, update, sync, sumary, foreach, status`, but where is `submodule remove`?

will
 I not delete one of them sometime in the future? Although most people 
will not use submodule or one who uses it can remove submodule by hand, providing complete options may be a good idea.

I am not familiar with C, but i can help test it. will anyone finish this feature?

----
Thanks and Best Regards,
lingchax 		 	   		  

^ permalink raw reply

* Re: [PATCH v3 0/8] Hiding refs
From: Michael Haggerty @ 2013-02-05 10:29 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git, Jeff King, Shawn Pearce
In-Reply-To: <20130205083327.GA4931@elie.Belkin>

On 02/05/2013 09:33 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
> 
>> I would again like to express my discomfort about this feature, which is
>> already listed as "will merge to next".  Frankly, I have the feeling
>> that this feature is being steamrolled in before a community consensus
>> has been reached and indeed before many valid points raised by other
>> members of the community have even been addressed.  For example:
> 
> In $dayjob I work with Gerrit, so I think I can start to answer some
> of these questions.
> 
>> * I didn't see a response to Peff's convincing arguments that this
>> should be a client-side feature rather than a server-side feature [1].
> 
> The client can't control the size of the ref advertisement.  That is
> the main motivation if I understood correctly.

Not according to Junio [4]:

  Look at this as a mechanism for the repository owner to control the
  clutter in what is shown to the intended audience of what s/he
  publishes in the repository.  Network bandwidth reduction of
  advertisement is a side effect of clutter reduction, and not
  necessarily the primary goal.

>> * I didn't see an answer to Duy's question [2] about what is different
>> between the proposed feature and gitnamespaces.
> 
> Namespaces are more complicated and don't sit well in existing setups
> involving git repositories whose refs are not namespaced.

Thanks.

>> * I didn't see a response to my worries that this feature could be
>> abused [3].
> 
> Can you elaborate?  Do you mean that through social engineering an
> attacker would convince the server admin to store secrets using a
> hidden ref and enable the upload-archive service?

The upload-archive service is not needed; after patch v3 "7/8" remote
clients are able to retrieve hidden content by SHA1.

Hiderefs creates a "dark" corner of a remote git repo that can hold
arbitrary content that is impossible for anybody to discover but
nevertheless possible for anybody to download (if they know the name of
a hidden reference).  In earlier versions of the patch series I believe
that it was possible to push to a hidden reference hierarchy, which made
it possible to upload dark content.  The new version appears (from the
code) to prohibit adding references in a hidden hierarchy, which would
close the main loophole that I was worried about.  But the documentation
and the unit tests only explicitly say that updates and deletes are
prohibited; nothing is said about adding references (unless "update" is
understood to include "add").  I think the true behavior should be
clarified and tested.

I was worried that somehow this "dark" content could be used for
malicious purposes; for example, pushing compromised code then
convincing somebody to download it by SHA1 with the implicit argument
"it's safe since it comes directly from the project's official
repository".  If it is indeed impossible to populate the dark namespace
remotely then I can't think of a way to exploit it.

> That does sound like a reasonable concern.  Perhaps the documentation
> should be updated along these lines
> 
> 	transfer.hiderefs::
> 		String(s) `upload-pack` and `receive-pack` use to decide
> 		which refs to omit from their initial advertisement.  Use
> 		more than one transfer.hiderefs configuration variables to
> 		specify multiple prefix strings. A ref that are under the
> 		hierarchies listed on the value of this variable is excluded,
> 		and is hidden from `git ls-remote`, `git fetch`, `git push :`,
> 		etc.  An attempt to update or delete a hidden ref by `git push`
> 		is rejected, and an attempt to fetch a hidden ref by `git fetch`
> 		will fail.
> 	+
> 	This setting does not currently affect the `upload-archive` service.
> 
> until someone interested implements the same for upload-archive.

Yes, this sounds reasonable.

>> I also think that the feature is poorly designed.  For example:
> 
> That's another reasonable concern.  It's very hard to get a design
> correct right away, which is presumably part of the motivation of
> getting this into the hands of interested users who can give feedback
> on it.  What would potentially be worth blocking even that is concerns
> about the wire protocol, since it is hard to take back mistakes there.
> 
>> * Why should a repository have exactly one setting for what refs should
>> be hidden?  Wouldn't it make more sense to allow multiple "views" to be
>> defined?:
> 
> How do I request a different view of the repository at
> /path/to/repo.git over the network?  How can we make the common case
> of only one view easy to achieve?  Isn't the multiple-views case
> exactly what gitnamespaces is for?

Hidden references can only be configured by somebody with local access
to the repository being served.  Somebody with that access could also
configure views.  He could also probably organize a mapping from URL ->
(GIT_PATH, GIT_VIEW) and offer several different views of the same
repository.  If the setting of environment variables is thought to
sometimes be too problematic, the default view could be defined via
configuration, too:

[view]
	default = official
[view "official"]
	hiderefs = refs/pull

Or perhaps, if views are made available locally too, then one view could
be designated as the default for transfer purposes:

[transfer]
	defaultview = official

gitnamespaces have the disadvantage that you mentioned yourself earlier.

> [...]
>> * Is it enough to support only reference exclusion (as opposed to
>> exclusion and inclusion rules)?
> 
> The motivating example is turning off advertisement of the
> refs/changes hierarchy.  If and when more complicated cases come up,
> that would presumably be the time to support more complicated
> configuration.

I suppose should the need arise we could later introduce
"transfer.showrefs" and let the longest match to a given reference
(i.e., "transfer.hiderefs" vs. "transfer.showrefs") win.

> [...]
>> * Why should this feature only be available remotely?
> 
> It is about transport.  Ref namespaces have their own set of use cases
> and are a distinct feature.

gitnamespaces offer multiple perspectives of a single object database,
using independent sets of references to define the subset.  Changing a
reference in one namespace has no effect on similarly-named references
in another namespace.

The "views" feature that I vaguely suggested, on the other hand, would
offer multiple subsets of a single set of references, a bit like an SQL
view.  Changing a reference in one view would automatically affect it in
other views that include the reference.  It could be a handy way to
reduce clutter *locally* in the same way that hiderefs would reduce
clutter *remotely*.  But more importantly, both features could be built
on the same foundation.

Michael

[4] http://article.gmane.org/gmane.comp.version-control.git/213984

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [RFC] Should "log --cc" imply "log --cc -p"?
From: Ævar Arnfjörð Bjarmason @ 2013-02-05 10:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, git
In-Reply-To: <7vfw1c3ujo.fsf@alter.siamese.dyndns.org>

On Mon, Feb 4, 2013 at 5:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "git log/diff-files -U8" do not need "-p" to enable textual patches,
> for example.  It is "I already told you that I want 8-line context.
> For what else, other than showing textual diff, do you think I told
> you that for?" and replacing "8-line context" with various other
> options that affect patch generation will give us a variety of end
> user complaints that would tell us that C) is more intuitive to
> them.

On a related note I think "--full-diff" should imply "-p" too.

^ permalink raw reply

* Re: [RFC] Should "log --cc" imply "log --cc -p"?
From: Jeff King @ 2013-02-05  9:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, git
In-Reply-To: <7vfw1c3ujo.fsf@alter.siamese.dyndns.org>

On Mon, Feb 04, 2013 at 08:36:43AM -0800, Junio C Hamano wrote:

> "git log/diff-files -U8" do not need "-p" to enable textual patches,
> for example.  It is "I already told you that I want 8-line context.
> For what else, other than showing textual diff, do you think I told
> you that for?" and replacing "8-line context" with various other
> options that affect patch generation will give us a variety of end
> user complaints that would tell us that C) is more intuitive to
> them.

Yeah, I'd agree with this. My feeling is that when there are two
options, A and B, and A is a no-op if B is not also specified, that it
makes sense for A to imply B. We do it in several places already (and I
just added some for "git branch --list" recently).

Is "--cc" a no-op when "-p" is not specified? Certainly "-c" is not, but
I do not think you are proposing that. At first glance, "--cc" is
nonsensical without "-p", but what about other xdiff callers? For
example, in:

  git log --cc --stat

the "--cc" is significant. So I don't think it is right for "--cc" to
always imply "-p". But if the rule kicked in only when no other format
had been specified, that might make sense.

-Peff

^ permalink raw reply

* Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names
From: Jeff King @ 2013-02-05  9:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Shawn Pearce
In-Reply-To: <1359571542-19852-8-git-send-email-gitster@pobox.com>

On Wed, Jan 30, 2013 at 10:45:41AM -0800, Junio C Hamano wrote:

> Teach "git fetch" to accept an exact SHA-1 object name the user may
> obtain out of band on the LHS of a pathspec, and send it on a "want"
> message when the server side advertises the allow-tip-sha1-in-want
> capability.

Hmm. The UI on this is a little less nice than I would have hoped. Right
now if you want a ref outside of refs/heads, it's up to you to configure
a refspec or do a one-off fetch of the ref:

  git config --add remote.origin.fetch '+refs/pull/*:refs/pull/*'
  git fetch
  git checkout refs/pull/123/head
  ... inspect the contents ...

Without advertisement, we have to learn that refs/pull/123/head exists
out of band. We can no longer fetch all of the refs/pull hierarchy
preemptively, but we can in theory grab at least that one ref like this:

  git fetch refs/pull/123/head
  git checkout FETCH_HEAD
  ... inspect the contents ...

But that does not work with your patch; instead you have to learn not
just the existence of the ref, but also its sha1. This may seem like a
little thing, since you are already learning of the ref out-of-band,
but:

  1. The full sha1 is more annoying to work with. You'd have to cut and
     paste or otherwise script getting it to fetch.  A human-readable
     ref, though, is much easier to remember. The "refs/pull/N/head"
     pattern is simple to learn and type.

  2. Related to (1) above, is that it may be easier to come up with a
     hidden ref name out of band than the full sha1. E.g., if I am
     looking at https://github.com/me/foo.git/pulls/123, I can easily
     construct the ref from that. Getting the sha1 will take extra
     steps.

  3. You have to do the out-of-band step, which may be inconvenient,
     every time the ref is updated. There is no way to say "just give me
     what is at the tip of refs/pull/123/head".

I think you could solve it by teaching upload-pack to understand refs on
"want" lines and convert them into the pointed-to object.

But taking a step back, this really seems quite inferior to an extension
that would allow the client to share its refspecs with the server. That
would solve the bandwidth efficiency problem for normal fetchers who are
looking at "refs/heads/*", while still giving people who are interested
in "refs/pull/*" (or even a specific refs/pull tip) the information they
need to fetch.

The obvious problem is that the server speaks first. But I recall
somebody suggested a combination of:

  1. For git-over-ssh and git-over-tcp, the server advertises
     tell-me-your-refspecs as it starts advertising.  Client interrupts
     advertisement with refspecs once it sees that it is OK to do so.

     We waste some bandwidth during the round-trip, but there will still
     be a benefit for repos with many refs (I wonder if we could even
     re-order the advertisement to show refs/heads/ first, as they are
     the most likely case to be requested). And as time goes on and the
     majority of clients support tell-me-your-refspecs, the server side
     can introduce a short delay after the first advertisement.

  2. For git-over-http, the client speaks first via the http protocol.
     We can stuff the refspecs into extra query parameters.

It's a little more complicated as a solution, but I feel like it gets
the efficiency without a loss of functionality. And it helps in more
situations than the hidden refs proposal (e.g., fetching refs/heads/foo
can avoid enumerating all of refs/heads/*).

-Peff

^ permalink raw reply

* Re: Assorted contrib/subtree Patches
From: Jakub Suder @ 2013-02-05  9:14 UTC (permalink / raw)
  To: David A. Greene
  Cc: James Nylen, git, Techlive Zheng, Wayne Walter,
	Avery Pennarun ", John Yani
In-Reply-To: <1360037173-23291-1-git-send-email-greened@obbligato.org>

Can I please unsubscribe from this thing?...

Thanks.

Jakub Suder

On 5 February 2013 05:06, David A. Greene <greened@obbligato.org> wrote:
> 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: [RFC] Should "log --cc" imply "log --cc -p"?
From: Michael J Gruber @ 2013-02-05  8:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfw1c3ujo.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 04.02.2013 17:36:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> But diffs are on here ("-p"), it's just that the default diff option for
>> merges is to not display them. Well, I admit there's two different ways
>> of thinking here:
>>
>> A) "git log -p" turns on diffs for all commits, and the default diff
>> options is the (non-existing) "--no-show" diff-option for merges.
>>
>> B) "git log -p" applies "-p" to all commits except merge commits.
>>
>> I'm strongly in the A camp,...
> 
> I can personally be trained to think either way, but I suspect that
> we already came halfway to
> 
>   C) "-p" asks for two-way patches, and "-c/--cc" ask for n-way
>      patches (including but not limited to n==2); it is not that -p
>      asks for patch generation and others modify the finer behaviour
>      of patch generation.
> 
> "git log/diff-files -U8" do not need "-p" to enable textual patches,
> for example.  It is "I already told you that I want 8-line context.

That's a good point that I wasn't aware of.

> For what else, other than showing textual diff, do you think I told
> you that for?" and replacing "8-line context" with various other
> options that affect patch generation will give us a variety of end
> user complaints that would tell us that C) is more intuitive to
> them.
> 
> But I do not feel very strongly that I am *right* on this issue, so
> I won't do anything about it hastily right now.

I'm not sure how many of these we have already, but to me it indicates
that we should turn diffs on for log with any diff option that only
makes sense with a diff. That would make the ui more consistent without
taking anything away.

For scripting/aliasing purposes, we have "-s" in order to override any
implied "-p" (as I just learned).

Michael

^ permalink raw reply

* Re: Feature request: Allow extracting revisions into directories
From: Sitaram Chamarty @ 2013-02-05  8:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Tomas Carnecky, Robert Clausecker, Michael J Gruber, git
In-Reply-To: <7v7gmo3tte.fsf@alter.siamese.dyndns.org>

On Mon, Feb 4, 2013 at 10:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tomas Carnecky <tomas.carnecky@gmail.com> writes:
>
>> That's what `git checkout` is for. And I would even argue that it's the better
>> choice in your situation because it would delete files from /var/www/foo which
>> you have deleted in your repo. git archive|tar wouldn't do that.
>
> The point about removal is an interesting one.  From that /var/www
> location I guess that you are discussing some webapp, but if you let
> it _write_ into it, you may also have to worry about how to handle
> the case where an update from the source end that comes from the
> checkout and an update by the webapp collide with each other.
>
> You also need to maintain the .git/index file that corresponds to
> what should be in /var/www/foo/ if you go that route.
>
> Just to be sure, I am not saying "checkout" is an inappropriate
> solution to whatever problem you are trying to solve. I am just
> pointing out things you need to be aware of if you take that
> approach.

http://sitaramc.github.com/the-list-and-irc/deploy.html is a fairly
popular URL on #git, where the bot responds to "!deploy" with some
text and this URL.

Just sayin'... :)

^ permalink raw reply

* Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies
From: Jeff King @ 2013-02-05  8:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Shawn Pearce
In-Reply-To: <1359571542-19852-4-git-send-email-gitster@pobox.com>

On Wed, Jan 30, 2013 at 10:45:37AM -0800, Junio C Hamano wrote:

> Teach upload-pack and receive-pack to omit some refs from their
> initial advertisements by paying attention to the transfer.hiderefs
> multi-valued configuration variable.  Any ref that is under the
> hierarchies listed on the value of this variable is excluded from
> responses to requests made by "ls-remote", "fetch", "clone", "push",
> etc.
> 
> A typical use case may be
> 
> 	[transfer]
> 		hiderefs = refs/pull
> 
> to hide the refs that are internally used by the hosting site and
> should not be exposed over the network.

In the earlier review, I mentioned making this per-service, but I see
that is not the case here. Do you have an argument against doing so?

I'm specifically thinking of the way we do refs/pull at GitHub (which we
hide only from receive-pack).  I know that you think it would be cleaner
to hide those, and at some level I agree. But at the same time, the
current mechanism has been in place for some time; changing what we
present via upload-pack is likely to break people's workflows. And I
have not seen complaints about the current system. So unless there is a
compelling reason to do so, I'd rather let the fetcher make the
decision.

Gerrit's refs/changes may be a different story, if they have a large
enough number of them to make upload-pack's ref advertisement
overwhelming.

I'm happy to do the per-service patch on top, but I just expected it
here, so I'm wondering if you are against having the feature.

-Peff

^ permalink raw reply

* Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters
From: Michael J Gruber @ 2013-02-05  8:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vtxps2eb2.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 04.02.2013 18:12:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Recently and not so recently, we made sure that log/grep type operations
>> use textconv filters when a userfacing diff would do the same:
>>
>> ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
>> b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
>> 0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)
>>
>> "git grep" currently does not use textconv filters at all, that is
>> neither for displaying the match and context nor for the actual grepping.
>>
>> Introduce a binary mode "--textconv" (in addition to "--text" and "-I")
>> which makes git grep use any configured textconv filters for grepping
>> and output purposes.
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>>
>> Notes:
>>     I'm somehow stuck in textconv/filespec/... hell, so I'm sending this out
>>     in request for help. I'm sure there are people for whom it's a breeze to
>>     get this right.
> 
> Looks like the patch touches the right places in the codepaths that
> need to be updated from a quick read.

I should have added that this coredumps when used on blobs and that
textconv isn't invoked in any case (because it crashes on blobs, and I
haven't implemented tetxconv on worktree files yet).

I'm somehow struggling to use just the right bits from the more diff
centric helpers like fill_textconv or fill_one (which is static so far).

>>     The difficulty is in getting the different cases (blob/sha1 vs.
>>     worktree) right, and in making the changes minimally invasive.
> 
> I think grep_source abstraction was intended to be a way for that,
> and if what it gives you is not sufficient for your needs, extending
> it should not be seen as "invasive" at all.

It seems to me that we textconvified the diff versions of these
abstractions, but not the grep_source abstractions. Doing it at the
source appears to be the right thing.

Michael

^ permalink raw reply

* Re: [PATCH v3 0/8] Hiding refs
From: Jonathan Nieder @ 2013-02-05  8:33 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Junio C Hamano, git, Jeff King, Shawn Pearce
In-Reply-To: <5110BD18.3080608@alum.mit.edu>

Hi Michael,

Michael Haggerty wrote:

> I would again like to express my discomfort about this feature, which is
> already listed as "will merge to next".  Frankly, I have the feeling
> that this feature is being steamrolled in before a community consensus
> has been reached and indeed before many valid points raised by other
> members of the community have even been addressed.  For example:

In $dayjob I work with Gerrit, so I think I can start to answer some
of these questions.

> * I didn't see a response to Peff's convincing arguments that this
> should be a client-side feature rather than a server-side feature [1].

The client can't control the size of the ref advertisement.  That is
the main motivation if I understood correctly.

> * I didn't see an answer to Duy's question [2] about what is different
> between the proposed feature and gitnamespaces.

Namespaces are more complicated and don't sit well in existing setups
involving git repositories whose refs are not namespaced.

> * I didn't see a response to my worries that this feature could be
> abused [3].

Can you elaborate?  Do you mean that through social engineering an
attacker would convince the server admin to store secrets using a
hidden ref and enable the upload-archive service?

That does sound like a reasonable concern.  Perhaps the documentation
should be updated along these lines

	transfer.hiderefs::
		String(s) `upload-pack` and `receive-pack` use to decide
		which refs to omit from their initial advertisement.  Use
		more than one transfer.hiderefs configuration variables to
		specify multiple prefix strings. A ref that are under the
		hierarchies listed on the value of this variable is excluded,
		and is hidden from `git ls-remote`, `git fetch`, `git push :`,
		etc.  An attempt to update or delete a hidden ref by `git push`
		is rejected, and an attempt to fetch a hidden ref by `git fetch`
		will fail.
	+
	This setting does not currently affect the `upload-archive` service.

until someone interested implements the same for upload-archive.

> I also think that the feature is poorly designed.  For example:

That's another reasonable concern.  It's very hard to get a design
correct right away, which is presumably part of the motivation of
getting this into the hands of interested users who can give feedback
on it.  What would potentially be worth blocking even that is concerns
about the wire protocol, since it is hard to take back mistakes there.

> * Why should a repository have exactly one setting for what refs should
> be hidden?  Wouldn't it make more sense to allow multiple "views" to be
> defined?:

How do I request a different view of the repository at
/path/to/repo.git over the network?  How can we make the common case
of only one view easy to achieve?  Isn't the multiple-views case
exactly what gitnamespaces is for?

[...]
> * Is it enough to support only reference exclusion (as opposed to
> exclusion and inclusion rules)?

The motivating example is turning off advertisement of the
refs/changes hierarchy.  If and when more complicated cases come up,
that would presumably be the time to support more complicated
configuration.

[...]
> * Why should this feature only be available remotely?

It is about transport.  Ref namespaces have their own set of use cases
and are a distinct feature.

Hoping that clarifies,
Jonathan

^ 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