[PATCH v1 0/2] contrib/subtree: make it respect spaces in a repository path

[PATCH v1 0/2] contrib/subtree: make it respect spaces in a repository path

[Alexey Shumkin]

Some repositories may have spaces in their paths. Currently `git-subtree`
raises an error in such cases.
Also, `git-subtree` currently does not have tests for its 'push' command.
Following patches are to fix these statements.

Alexey Shumkin (2):
  t7900-subtree: test the "space in a subdirectory name" case
  contrib/subtree: respect spaces in a repository path

 contrib/subtree/git-subtree.sh     |   4 +-
 contrib/subtree/t/t7900-subtree.sh | 194 +++++++++++++++++++++++--------------
 2 files changed, 124 insertions(+), 74 deletions(-)

-- 
2.4.1-21

[PATCH v1 1/2] t7900-subtree: test the "space in a subdirectory name" case

[Alexey Shumkin]

In common case there can be spaces in a subdirectory name. Change tests
accorgingly to this statement.

Also, as far as a call to the `rejoin_msg` function (in `cmd_split`)
does not take into account such a case this patch fixes commit message
when `--rejoin` option is set .

Besides, as `fixnl` and `multiline` functions did not take into account
the "new" tested "space in a subdirectory name" case they become unused
and redundant, so they are removed.

Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
---
 contrib/subtree/git-subtree.sh     |   2 +-
 contrib/subtree/t/t7900-subtree.sh | 147 +++++++++++++++++++------------------
 2 files changed, 76 insertions(+), 73 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 9f06571..72a20c0 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -648,7 +648,7 @@ cmd_split()
 		debug "Merging split branch into HEAD..."
 		latest_old=$(cache_get latest_old)
 		git merge -s ours \
-			-m "$(rejoin_msg $dir $latest_old $latest_new)" \
+			-m "$(rejoin_msg "$dir" $latest_old $latest_new)" \
 			$latest_new >&2 || exit $?
 	fi
 	if [ -n "$branch" ]; then
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 9051982..9979827 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -32,25 +32,6 @@ check_equal()
 	fi
 }
 
-fixnl()
-{
-	t=""
-	while read x; do
-		t="$t$x "
-	done
-	echo $t
-}
-
-multiline()
-{
-	while read x; do
-		set -- $x
-		for d in "$@"; do
-			echo "$d"
-		done
-	done
-}
-
 undo()
 {
 	git reset --hard HEAD~
@@ -62,11 +43,11 @@ last_commit_message()
 }
 
 test_expect_success 'init subproj' '
-	test_create_repo subproj
+	test_create_repo "sub proj"
 '
 
 # To the subproject!
-cd subproj
+cd ./"sub proj"
 
 test_expect_success 'add sub1' '
 	create sub1 &&
@@ -106,39 +87,39 @@ test_expect_success 'add main4' '
 '
 
 test_expect_success 'fetch subproj history' '
-	git fetch ./subproj sub1 &&
+	git fetch ./"sub proj" 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="sub dir" 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="sub dir" ./"sub proj" sub1
 '
 
 test_expect_success 'check if --message works for add' '
-	git subtree add --prefix=subdir --message="Added subproject" sub1 &&
+	git subtree add --prefix="sub dir" --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 &&
+	git subtree add -P "sub dir" -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 &&
+	git subtree add -P "sub dir" -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="sub dir"/ FETCH_HEAD &&
+	check_equal ''"$(last_commit_message)"'' "Add '"'sub dir/'"' from commit '"'"'''"$(git rev-parse sub1)"'''"'"'"
 '
 
 # this shouldn't actually do anything, since FETCH_HEAD is already a parent
@@ -147,7 +128,7 @@ test_expect_success 'merge fetched subproj' '
 '
 
 test_expect_success 'add main-sub5' '
-	create subdir/main-sub5 &&
+	create "sub dir/main-sub5" &&
 	git commit -m "main-sub5"
 '
 
@@ -157,29 +138,29 @@ test_expect_success 'add main6' '
 '
 
 test_expect_success 'add main-sub7' '
-	create subdir/main-sub7 &&
+	create "sub dir/main-sub7" &&
 	git commit -m "main-sub7"
 '
 
 test_expect_success 'fetch new subproj history' '
-	git fetch ./subproj sub2 &&
+	git fetch ./"sub proj" 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 &&
+	git subtree merge --prefix="sub dir" -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 &&
+	git subtree merge --prefix "sub dir" -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 subtree merge --prefix="sub dir" FETCH_HEAD &&
 	git branch pre-split &&
 	check_equal ''"$(last_commit_message)"'' "Merge commit '"'"'"$(git rev-parse sub2)"'"'"' into mainline" &&
 	undo
@@ -208,53 +189,53 @@ test_expect_success 'Check that the <prefix> exists for a split' '
 '
 
 test_expect_success 'check if --message works for split+rejoin' '
-	spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
+	spl1=''"$(git subtree split --annotate='"'*'"' --prefix "sub dir" --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) &&
+	spl1=$(git subtree split --annotate='"'*'"' --prefix "sub dir" --onto FETCH_HEAD --message "Split & rejoin" --rejoin) &&
 	undo &&
-	git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --branch splitbr1 &&
+	git subtree split --annotate='"'*'"' --prefix "sub dir" --onto FETCH_HEAD --branch splitbr1 &&
 	check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
 '
 
 test_expect_success 'check hash of split' '
-	spl1=$(git subtree split --prefix subdir) &&
-	git subtree split --prefix subdir --branch splitbr1test &&
+	spl1=$(git subtree split --prefix "sub dir") &&
+	git subtree split --prefix "sub dir" --branch splitbr1test &&
 	check_equal ''"$(git rev-parse splitbr1test)"'' "$spl1" &&
 	new_hash=$(git rev-parse splitbr1test~2) &&
 	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)"'' &&
+	spl1=''"$(git subtree split --annotate='"'*'"' --prefix "sub dir" --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
 	undo &&
 	git branch splitbr2 sub1 &&
-	git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --branch splitbr2 &&
+	git subtree split --annotate='"'*'"' --prefix "sub dir" --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 "sub dir" --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)"'' &&
+	spl1=''"$(git subtree split --annotate='"'*'"' --prefix "sub dir" --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"'"'"'"
+	git subtree split --annotate='"'*'"' --prefix "sub dir" --onto FETCH_HEAD --rejoin &&
+	check_equal ''"$(last_commit_message)"'' "Split '"'"'sub dir/'"'"' into commit '"'"'"$spl1"'"'"'"
 '
 
 test_expect_success 'add main-sub8' '
-	create subdir/main-sub8 &&
+	create "sub dir/main-sub8" &&
 	git commit -m "main-sub8"
 '
 
 # To the subproject!
-cd ./subproj
+cd ./"sub proj"
 
 test_expect_success 'merge split into subproj' '
 	git fetch .. spl1 &&
@@ -271,22 +252,22 @@ test_expect_success 'add sub9' '
 cd ..
 
 test_expect_success 'split for sub8' '
-	split2=''"$(git subtree split --annotate='"'*'"' --prefix subdir/ --rejoin)"'' &&
+	split2=''"$(git subtree split --annotate='"'*'"' --prefix "sub dir/" --rejoin)"'' &&
 	git branch split2 "$split2"
 '
 
 test_expect_success 'add main-sub10' '
-	create subdir/main-sub10 &&
+	create "sub dir/main-sub10" &&
 	git commit -m "main-sub10"
 '
 
 test_expect_success 'split for sub10' '
-	spl3=''"$(git subtree split --annotate='"'*'"' --prefix subdir --rejoin)"'' &&
+	spl3=''"$(git subtree split --annotate='"'*'"' --prefix "sub dir" --rejoin)"'' &&
 	git branch spl3 "$spl3"
 '
 
 # To the subproject!
-cd ./subproj
+cd ./"sub proj"
 
 test_expect_success 'merge split into subproj' '
 	git fetch .. spl3 &&
@@ -295,42 +276,64 @@ test_expect_success 'merge split into subproj' '
 	git branch subproj-merge-spl3
 '
 
-chkm="main4 main6"
-chkms="main-sub10 main-sub5 main-sub7 main-sub8"
-chkms_sub=$(echo $chkms | multiline | sed 's,^,subdir/,' | fixnl)
-chks="sub1 sub2 sub3 sub9"
-chks_sub=$(echo $chks | multiline | sed 's,^,subdir/,' | fixnl)
+chkm="main4
+main6"
+chkms="main-sub10
+main-sub5
+main-sub7
+main-sub8"
+chkms_sub=$(cat <<TXT | sed 's,^,sub dir/,'
+$chkms
+TXT
+)
+chks="sub1
+sub2
+sub3
+sub9"
+chks_sub=$(cat <<TXT | sed 's,^,sub dir/,'
+$chks
+TXT
+)
 
 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)" &&
+	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 | sed "/^$/d")"'' &&
+	check_equal "$allchanges" "$chkms
+$chks"
 '
 
 # Back to mainline
 cd ..
 
 test_expect_success 'pull from subproj' '
-	git fetch ./subproj subproj-merge-spl3 &&
+	git fetch ./"sub proj" subproj-merge-spl3 &&
 	git branch subproj-merge-spl3 FETCH_HEAD &&
-	git subtree pull --prefix=subdir ./subproj subproj-merge-spl3
+	git subtree pull --prefix="sub dir" ./"sub proj" 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) &&
+	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)"''
+	allchanges=''"$(git log --name-only --pretty=format:'"''"' | sort | sed "/^$/d")"'' &&
+	check_equal "$allchanges" ''"$(cat <<TXT | sort
+$chkms
+$chkm
+$chks
+$chkms_sub
+TXT
+)"''
 '
 
 test_expect_success 'make sure the --rejoin commits never make it into subproj' '
@@ -377,7 +380,7 @@ cd ../main
 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 subtree add --prefix "sub dir" sub2
 '
 
 cd ../sub
@@ -392,16 +395,16 @@ cd ../main
 test_expect_success 'merge from sub' '
 	git fetch ../sub master &&
 	git branch sub3 FETCH_HEAD &&
-	git subtree merge --prefix subdir sub3
+	git subtree merge --prefix "sub dir" sub3
 '
 
 test_expect_success 'add main-sub4' '
-	create subdir/main-sub4 &&
+	create "sub dir/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 "sub dir" --branch mainsub4
 '
 
 # at this point, the new commit parent should be sub3 if it is not,
-- 
2.4.1-21

[PATCH v1 2/2] contrib/subtree: respect spaces in a repository path

[Alexey Shumkin]

Remote repository may have spaces in its path, so take it into account.

Also, as far as there are no tests for the `push` command, add them.

Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
---
 contrib/subtree/git-subtree.sh     |  2 +-
 contrib/subtree/t/t7900-subtree.sh | 47 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 72a20c0..308b777 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -735,7 +735,7 @@ cmd_push()
 	    refspec=$2
 	    echo "git push using: " $repository $refspec
 	    localrev=$(git subtree split --prefix="$prefix") || die
-	    git push $repository $localrev:refs/heads/$refspec
+	    git push "$repository" $localrev:refs/heads/$refspec
 	else
 	    die "'$dir' must already exist. Try 'git subtree add'."
 	fi
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 9979827..dfbe443 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -1,6 +1,7 @@
 #!/bin/sh
 #
 # Copyright (c) 2012 Avery Pennaraum
+# Copyright (c) 2015 Alexey Shumkin
 #
 test_description='Basic porcelain support for subtrees
 
@@ -471,4 +472,50 @@ test_expect_success 'verify one file change per commit' '
 	))
 '
 
+# test push
+
+cd ../..
+
+mkdir test-push
+
+cd test-push
+
+test_expect_success 'init main' '
+	test_create_repo main
+'
+
+test_expect_success 'init sub' '
+	test_create_repo "sub project"
+'
+
+cd ./"sub project"
+
+test_expect_success 'add subproject' '
+	create "sub project" &&
+	git commit -m "Sub project: 1" &&
+	git branch sub-branch-1
+'
+
+cd ../main
+
+test_expect_success 'make first commit and add subproject' '
+	create "main-1" &&
+	git commit -m "main: 1" &&
+	git subtree add "../sub project" --prefix "sub dir" --message "Added subproject" sub-branch-1 &&
+	check_equal "$(last_commit_message)" "Added subproject"
+'
+
+test_expect_success 'make second commit to a subproject file and push it into a sub project' '
+	create "sub dir/sub1" &&
+	git commit -m "Sub project: 2" &&
+	git subtree push "../sub project" --prefix "sub dir" sub-branch-1
+'
+
+cd ../"sub project"
+
+test_expect_success 'Test second commit is pushed' '
+	git checkout sub-branch-1 &&
+	check_equal "$(last_commit_message)" "Sub project: 2"
+'
+
 test_done
-- 
2.4.1-21

Re: [PATCH v1 0/2] contrib/subtree: make it respect spaces in a repository path

[Junio C Hamano]

Alexey Shumkin <alex.crezoff@gmail.com> writes:

> Some repositories may have spaces in their paths. Currently `git-subtree`
> raises an error in such cases.
> Also, `git-subtree` currently does not have tests for its 'push' command.
> Following patches are to fix these statements.
>
> Alexey Shumkin (2):
>   t7900-subtree: test the "space in a subdirectory name" case
>   contrib/subtree: respect spaces in a repository path

Doesn't this order break bisection?  It seems that you turn "subdir"
to "sub dir" in existing tests, and I understand that the whole
point of this series is that such a change will expose that the tool
is broken, making tests fail.

Also, if you feel up to it, it might be a good idea to clean t7900
test up to the current best practice before doing any other changes
as a pure preparatory clean-up patch.

Namely, using cd outside a subshell of the tests to move around is a
bad thing to do, and you are adding more instance of it in this
series.  If one test with such a cd to go down fails before it has a
chance to come back up (or go up and then fail to come back down),
the later tests will be left in an unexpected place.

>  contrib/subtree/git-subtree.sh     |   4 +-
>  contrib/subtree/t/t7900-subtree.sh | 194 +++++++++++++++++++++++--------------
>  2 files changed, 124 insertions(+), 74 deletions(-)

Thanks.

Re: [PATCH v1 0/2] contrib/subtree: make it respect spaces in a repository path

[Alexey Shumkin]

On Fri, Sep 04, 2015 at 04:08:06PM -0700, Junio C Hamano wrote:
> Alexey Shumkin <alex.crezoff@gmail.com> writes:
> 
> > Some repositories may have spaces in their paths. Currently `git-subtree`
> > raises an error in such cases.
> > Also, `git-subtree` currently does not have tests for its 'push' command.
> > Following patches are to fix these statements.
> >
> > Alexey Shumkin (2):
> >   t7900-subtree: test the "space in a subdirectory name" case
> >   contrib/subtree: respect spaces in a repository path
> 
> Doesn't this order break bisection?  It seems that you turn "subdir"
> to "sub dir" in existing tests, and I understand that the whole
> point of this series is that such a change will expose that the tool
> is broken, making tests fail.
It seems I have to reword commit messages to avoid such an interpretation.
Because, the first commit does not break anything. It is to change the
tests for `git-subtree` "to show" that `git-subtree`s already tested
commands (almost) work correctly if there are spaces in paths (except
--rejoin-msg issue).
And the second commit adds missing tests and the fix.
Should I add/commit the breaking test first and then commit the fix?
> 
> Also, if you feel up to it, it might be a good idea to clean t7900
> test up to the current best practice before doing any other changes
> as a pure preparatory clean-up patch.
> 
> Namely, using cd outside a subshell of the tests to move around is a
> bad thing to do, and you are adding more instance of it in this
> series.  If one test with such a cd to go down fails before it has a
> chance to come back up (or go up and then fail to come back down),
> the later tests will be left in an unexpected place.
I understand this issue with "cd" (I've just followed the existing t7900
tests "code style").
> 
> >  contrib/subtree/git-subtree.sh     |   4 +-
> >  contrib/subtree/t/t7900-subtree.sh | 194 +++++++++++++++++++++++--------------
> >  2 files changed, 124 insertions(+), 74 deletions(-)
> 
> Thanks.

-- 
Alexey Shumkin
E-mail: Alex.Crezoff@gmail.com
ICQ: 118001447
Jabber (GoogleTalk): Alex.Crezoff@gmail.com
Skype: crezoff