Merge branch 'pw/git-p4-on-cygwin'

Improve "git p4" on Cygwin.

* pw/git-p4-on-cygwin: (21 commits)
  git p4: introduce gitConfigBool
  git p4: avoid shell when calling git config
  git p4: avoid shell when invoking git config --get-all
  git p4: avoid shell when invoking git rev-list
  git p4: avoid shell when mapping users
  git p4: disable read-only attribute before deleting
  git p4 test: use test_chmod for cygwin
  git p4: cygwin p4 client does not mark read-only
  git p4 test: avoid wildcard * in windows
  git p4 test: use LineEnd unix in windows tests too
  git p4 test: newline handling
  git p4: scrub crlf for utf16 files on windows
  git p4: remove unreachable windows \r\n conversion code
  git p4 test: translate windows paths for cygwin
  git p4 test: start p4d inside its db dir
  git p4 test: use client_view in t9806
  git p4 test: avoid loop in client_view
  git p4 test: use client_view to build the initial client
  git p4: generate better error message for bad depot path
  git p4: remove unused imports
  ...

9 files changed, 252 insertions, 66 deletions

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 7061dce7e5..2098b9ba05 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -8,7 +8,8 @@ TEST_NO_CREATE_REPO=NoThanks
 
 . ./test-lib.sh
 
-if ! test_have_prereq PYTHON; then
+if ! test_have_prereq PYTHON
+then
 	skip_all='skipping git p4 tests; python not available'
 	test_done
 fi
@@ -17,6 +18,24 @@ fi
 	test_done
 }
 
+# On cygwin, the NT version of Perforce can be used.  When giving
+# it paths, either on the command-line or in client specifications,
+# be sure to use the native windows form.
+#
+# Older versions of perforce were available compiled natively for
+# cygwin.  Those do not accept native windows paths, so make sure
+# not to convert for them.
+native_path() {
+	path="$1" &&
+	if test_have_prereq CYGWIN && ! p4 -V | grep -q CYGWIN
+	then
+		path=$(cygpath --windows "$path")
+	else
+		path=$(test-path-utils real_path "$path")
+	fi &&
+	echo "$path"
+}
+
 # Try to pick a unique port: guess a large number, then hope
 # no more than one of each test is running.
 #
@@ -32,7 +51,7 @@ P4EDITOR=:
 export P4PORT P4CLIENT P4EDITOR
 
 db="$TRASH_DIRECTORY/db"
-cli=$(test-path-utils real_path "$TRASH_DIRECTORY/cli")
+cli="$TRASH_DIRECTORY/cli"
 git="$TRASH_DIRECTORY/git"
 pidfile="$TRASH_DIRECTORY/p4d.pid"
 
@@ -40,8 +59,11 @@ start_p4d() {
 	mkdir -p "$db" "$cli" "$git" &&
 	rm -f "$pidfile" &&
 	(
-		p4d -q -r "$db" -p $P4DPORT &
-		echo $! >"$pidfile"
+		cd "$db" &&
+		{
+			p4d -q -p $P4DPORT &
+			echo $! >"$pidfile"
+		}
 	) &&
 
 	# This gives p4d a long time to start up, as it can be
@@ -74,15 +96,8 @@ start_p4d() {
 	fi
 
 	# build a client
-	(
-		cd "$cli" &&
-		p4 client -i <<-EOF
-		Client: client
-		Description: client
-		Root: $cli
-		View: //depot/... //client/...
-		EOF
-	)
+	client_view "//depot/... //client/..." &&
+
 	return 0
 }
 
@@ -123,13 +138,26 @@ marshal_dump() {
 client_view() {
 	(
 		cat <<-EOF &&
-		Client: client
-		Description: client
+		Client: $P4CLIENT
+		Description: $P4CLIENT
 		Root: $cli
+		AltRoots: $(native_path "$cli")
+		LineEnd: unix
 		View:
 		EOF
-		for arg ; do
-			printf "\t$arg\n"
-		done
+		printf "\t%s\n" "$@"
 	) | p4 client -i
 }
+
+is_cli_file_writeable() {
+	# cygwin version of p4 does not set read-only attr,
+	# will be marked 444 but -w is true
+	file="$1" &&
+	if test_have_prereq CYGWIN && p4 -V | grep -q CYGWIN
+	then
+		stat=$(stat --format=%a "$file") &&
+		test $stat = 644
+	else
+		test -w "$file"
+	fi
+}
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 166e75209f..665607c9cb 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -30,6 +30,11 @@ test_expect_success 'basic git p4 clone' '
 	)
 '
 
+test_expect_success 'depot typo error' '
+	test_must_fail git p4 clone --dest="$git" /depot 2>errs &&
+	grep "Depot paths must start with" errs
+'
+
 test_expect_success 'git p4 clone @all' '
 	git p4 clone --dest="$git" //depot@all &&
 	test_when_finished cleanup_git &&
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index aae1a3f816..eeefa67904 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -8,6 +8,123 @@ test_expect_success 'start p4d' '
 	start_p4d
 '
 
+#
+# This series of tests checks newline handling  Both p4 and
+# git store newlines as \n, and have options to choose how
+# newlines appear in checked-out files.
+#
+test_expect_success 'p4 client newlines, unix' '
+	(
+		cd "$cli" &&
+		p4 client -o | sed "/LineEnd/s/:.*/:unix/" | p4 client -i &&
+		printf "unix\ncrlf\n" >f-unix &&
+		printf "unix\r\ncrlf\r\n" >f-unix-as-crlf &&
+		p4 add -t text f-unix &&
+		p4 submit -d f-unix &&
+
+		# LineEnd: unix; should be no change after sync
+		cp f-unix f-unix-orig &&
+		p4 sync -f &&
+		test_cmp f-unix-orig f-unix &&
+
+		# make sure stored in repo as unix newlines
+		# use sed to eat python-appened newline
+		p4 -G print //depot/f-unix | marshal_dump data 2 |\
+		    sed \$d >f-unix-p4-print &&
+		test_cmp f-unix-orig f-unix-p4-print &&
+
+		# switch to win, make sure lf -> crlf
+		p4 client -o | sed "/LineEnd/s/:.*/:win/" | p4 client -i &&
+		p4 sync -f &&
+		test_cmp f-unix-as-crlf f-unix
+	)
+'
+
+test_expect_success 'p4 client newlines, win' '
+	(
+		cd "$cli" &&
+		p4 client -o | sed "/LineEnd/s/:.*/:win/" | p4 client -i &&
+		printf "win\r\ncrlf\r\n" >f-win &&
+		printf "win\ncrlf\n" >f-win-as-lf &&
+		p4 add -t text f-win &&
+		p4 submit -d f-win &&
+
+		# LineEnd: win; should be no change after sync
+		cp f-win f-win-orig &&
+		p4 sync -f &&
+		test_cmp f-win-orig f-win &&
+
+		# make sure stored in repo as unix newlines
+		# use sed to eat python-appened newline
+		p4 -G print //depot/f-win | marshal_dump data 2 |\
+		    sed \$d >f-win-p4-print &&
+		test_cmp f-win-as-lf f-win-p4-print &&
+
+		# switch to unix, make sure lf -> crlf
+		p4 client -o | sed "/LineEnd/s/:.*/:unix/" | p4 client -i &&
+		p4 sync -f &&
+		test_cmp f-win-as-lf f-win
+	)
+'
+
+test_expect_success 'ensure blobs store only lf newlines' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git init &&
+		git p4 sync //depot@all &&
+
+		# verify the files in .git are stored only with newlines
+		o=$(git ls-tree p4/master -- f-unix | cut -f1 | cut -d\  -f3) &&
+		git cat-file blob $o >f-unix-blob &&
+		test_cmp "$cli"/f-unix-orig f-unix-blob &&
+
+		o=$(git ls-tree p4/master -- f-win | cut -f1 | cut -d\  -f3) &&
+		git cat-file blob $o >f-win-blob &&
+		test_cmp "$cli"/f-win-as-lf f-win-blob &&
+
+		rm f-unix-blob f-win-blob
+	)
+'
+
+test_expect_success 'gitattributes setting eol=lf produces lf newlines' '
+	test_when_finished cleanup_git &&
+	(
+		# checkout the files and make sure core.eol works as planned
+		cd "$git" &&
+		git init &&
+		echo "* eol=lf" >.gitattributes &&
+		git p4 sync //depot@all &&
+		git checkout master &&
+		test_cmp "$cli"/f-unix-orig f-unix &&
+		test_cmp "$cli"/f-win-as-lf f-win
+	)
+'
+
+test_expect_success 'gitattributes setting eol=crlf produces crlf newlines' '
+	test_when_finished cleanup_git &&
+	(
+		# checkout the files and make sure core.eol works as planned
+		cd "$git" &&
+		git init &&
+		echo "* eol=crlf" >.gitattributes &&
+		git p4 sync //depot@all &&
+		git checkout master &&
+		test_cmp "$cli"/f-unix-as-crlf f-unix &&
+		test_cmp "$cli"/f-win-orig f-win
+	)
+'
+
+test_expect_success 'crlf cleanup' '
+	(
+		cd "$cli" &&
+		rm f-unix-orig f-unix-as-crlf &&
+		rm f-win-orig f-win-as-lf &&
+		p4 client -o | sed "/LineEnd/s/:.*/:unix/" | p4 client -i &&
+		p4 sync -f
+	)
+'
+
 test_expect_success 'utf-16 file create' '
 	(
 		cd "$cli" &&
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 4f077eeca8..254d428b73 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -214,40 +214,33 @@ test_expect_success 'clone --use-client-spec' '
 		exec >/dev/null &&
 		test_must_fail git p4 clone --dest="$git" --use-client-spec
 	) &&
-	cli2=$(test-path-utils real_path "$TRASH_DIRECTORY/cli2") &&
+	# build a different client
+	cli2="$TRASH_DIRECTORY/cli2" &&
 	mkdir -p "$cli2" &&
 	test_when_finished "rmdir \"$cli2\"" &&
-	(
-		cd "$cli2" &&
-		p4 client -i <<-EOF
-		Client: client2
-		Description: client2
-		Root: $cli2
-		View: //depot/sub/... //client2/bus/...
-		EOF
-	) &&
 	test_when_finished cleanup_git &&
 	(
+		# group P4CLIENT and cli changes in a sub-shell
 		P4CLIENT=client2 &&
-		git p4 clone --dest="$git" --use-client-spec //depot/...
-	) &&
-	(
-		cd "$git" &&
-		test_path_is_file bus/dir/f4 &&
-		test_path_is_missing file1
-	) &&
-	cleanup_git &&
-
-	# same thing again, this time with variable instead of option
-	(
-		cd "$git" &&
-		git init &&
-		git config git-p4.useClientSpec true &&
-		P4CLIENT=client2 &&
-		git p4 sync //depot/... &&
-		git checkout -b master p4/master &&
-		test_path_is_file bus/dir/f4 &&
-		test_path_is_missing file1
+		cli="$cli2" &&
+		client_view "//depot/sub/... //client2/bus/..." &&
+		git p4 clone --dest="$git" --use-client-spec //depot/... &&
+		(
+			cd "$git" &&
+			test_path_is_file bus/dir/f4 &&
+			test_path_is_missing file1
+		) &&
+		cleanup_git &&
+		# same thing again, this time with variable instead of option
+		(
+			cd "$git" &&
+			git init &&
+			git config git-p4.useClientSpec true &&
+			git p4 sync //depot/... &&
+			git checkout -b master p4/master &&
+			test_path_is_file bus/dir/f4 &&
+			test_path_is_missing file1
+		)
 	)
 '
 
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 0ae048f29f..1fb7bc7cf9 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -17,6 +17,16 @@ test_expect_success 'init depot' '
 	)
 '
 
+test_expect_failure 'is_cli_file_writeable function' '
+	(
+		cd "$cli" &&
+		echo a >a &&
+		is_cli_file_writeable a &&
+		! is_cli_file_writeable file1 &&
+		rm a
+	)
+'
+
 test_expect_success 'submit with no client dir' '
 	test_when_finished cleanup_git &&
 	git p4 clone --dest="$git" //depot &&
@@ -200,7 +210,7 @@ test_expect_success 'submit copy' '
 	(
 		cd "$cli" &&
 		test_path_is_file file5.ta &&
-		test ! -w file5.ta
+		! is_cli_file_writeable file5.ta
 	)
 '
 
@@ -219,7 +229,7 @@ test_expect_success 'submit rename' '
 		cd "$cli" &&
 		test_path_is_missing file6.t &&
 		test_path_is_file file6.ta &&
-		test ! -w file6.ta
+		! is_cli_file_writeable file6.ta
 	)
 '
 
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 281be29174..77f63492d9 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -196,7 +196,7 @@ test_expect_success 'exclusion single file' '
 
 test_expect_success 'overlay wildcard' '
 	client_view "//depot/dir1/... //client/cli/..." \
-		    "+//depot/dir2/... //client/cli/...\n" &&
+		    "+//depot/dir2/... //client/cli/..." &&
 	files="cli/file11 cli/file12 cli/file21 cli/file22" &&
 	client_verify $files &&
 	test_when_finished cleanup_git &&
@@ -333,7 +333,7 @@ test_expect_success 'subdir clone, submit copy' '
 	(
 		cd "$cli" &&
 		test_path_is_file dir1/file11a &&
-		test ! -w dir1/file11a
+		! is_cli_file_writeable dir1/file11a
 	)
 '
 
@@ -353,7 +353,7 @@ test_expect_success 'subdir clone, submit rename' '
 		cd "$cli" &&
 		test_path_is_missing dir1/file13 &&
 		test_path_is_file dir1/file13a &&
-		test ! -w dir1/file13a
+		! is_cli_file_writeable dir1/file13a
 	)
 '
 
@@ -365,7 +365,10 @@ test_expect_success 'wildcard files submit back to p4, client-spec case' '
 	(
 		cd "$git" &&
 		echo git-wild-hash >dir1/git-wild#hash &&
-		echo git-wild-star >dir1/git-wild\*star &&
+		if test_have_prereq NOT_MINGW NOT_CYGWIN
+		then
+			echo git-wild-star >dir1/git-wild\*star
+		fi &&
 		echo git-wild-at >dir1/git-wild@at &&
 		echo git-wild-percent >dir1/git-wild%percent &&
 		git add dir1/git-wild* &&
@@ -376,7 +379,10 @@ test_expect_success 'wildcard files submit back to p4, client-spec case' '
 	(
 		cd "$cli" &&
 		test_path_is_file dir1/git-wild#hash &&
-		test_path_is_file dir1/git-wild\*star &&
+		if test_have_prereq NOT_MINGW NOT_CYGWIN
+		then
+			test_path_is_file dir1/git-wild\*star
+		fi &&
 		test_path_is_file dir1/git-wild@at &&
 		test_path_is_file dir1/git-wild%percent
 	) &&
diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh
index 143d413057..67633257f3 100755
--- a/t/t9812-git-p4-wildcards.sh
+++ b/t/t9812-git-p4-wildcards.sh
@@ -14,7 +14,10 @@ test_expect_success 'add p4 files with wildcards in the names' '
 		printf "file2\nhas\nsome\nrandom\ntext\n" >file2 &&
 		p4 add file2 &&
 		echo file-wild-hash >file-wild#hash &&
-		echo file-wild-star >file-wild\*star &&
+		if test_have_prereq NOT_MINGW NOT_CYGWIN
+		then
+			echo file-wild-star >file-wild\*star
+		fi &&
 		echo file-wild-at >file-wild@at &&
 		echo file-wild-percent >file-wild%percent &&
 		p4 add -f file-wild* &&
@@ -28,7 +31,10 @@ test_expect_success 'wildcard files git p4 clone' '
 	(
 		cd "$git" &&
 		test -f file-wild#hash &&
-		test -f file-wild\*star &&
+		if test_have_prereq NOT_MINGW NOT_CYGWIN
+		then
+			test -f file-wild\*star
+		fi &&
 		test -f file-wild@at &&
 		test -f file-wild%percent
 	)
@@ -40,7 +46,10 @@ test_expect_success 'wildcard files submit back to p4, add' '
 	(
 		cd "$git" &&
 		echo git-wild-hash >git-wild#hash &&
-		echo git-wild-star >git-wild\*star &&
+		if test_have_prereq NOT_MINGW NOT_CYGWIN
+		then
+			echo git-wild-star >git-wild\*star
+		fi &&
 		echo git-wild-at >git-wild@at &&
 		echo git-wild-percent >git-wild%percent &&
 		git add git-wild* &&
@@ -51,7 +60,10 @@ test_expect_success 'wildcard files submit back to p4, add' '
 	(
 		cd "$cli" &&
 		test_path_is_file git-wild#hash &&
-		test_path_is_file git-wild\*star &&
+		if test_have_prereq NOT_MINGW NOT_CYGWIN
+		then
+			test_path_is_file git-wild\*star
+		fi &&
 		test_path_is_file git-wild@at &&
 		test_path_is_file git-wild%percent
 	)
@@ -63,7 +75,10 @@ test_expect_success 'wildcard files submit back to p4, modify' '
 	(
 		cd "$git" &&
 		echo new-line >>git-wild#hash &&
-		echo new-line >>git-wild\*star &&
+		if test_have_prereq NOT_MINGW NOT_CYGWIN
+		then
+			echo new-line >>git-wild\*star
+		fi &&
 		echo new-line >>git-wild@at &&
 		echo new-line >>git-wild%percent &&
 		git add git-wild* &&
@@ -74,7 +89,10 @@ test_expect_success 'wildcard files submit back to p4, modify' '
 	(
 		cd "$cli" &&
 		test_line_count = 2 git-wild#hash &&
-		test_line_count = 2 git-wild\*star &&
+		if test_have_prereq NOT_MINGW NOT_CYGWIN
+		then
+			test_line_count = 2 git-wild\*star
+		fi &&
 		test_line_count = 2 git-wild@at &&
 		test_line_count = 2 git-wild%percent
 	)
@@ -87,7 +105,7 @@ test_expect_success 'wildcard files submit back to p4, copy' '
 		cd "$git" &&
 		cp file2 git-wild-cp#hash &&
 		git add git-wild-cp#hash &&
-		cp git-wild\*star file-wild-3 &&
+		cp git-wild#hash file-wild-3 &&
 		git add file-wild-3 &&
 		git commit -m "wildcard copies" &&
 		git config git-p4.detectCopies true &&
@@ -134,7 +152,10 @@ test_expect_success 'wildcard files submit back to p4, delete' '
 	(
 		cd "$cli" &&
 		test_path_is_missing git-wild#hash &&
-		test_path_is_missing git-wild\*star &&
+		if test_have_prereq NOT_MINGW NOT_CYGWIN
+		then
+			test_path_is_missing git-wild\*star
+		fi &&
 		test_path_is_missing git-wild@at &&
 		test_path_is_missing git-wild%percent
 	)
diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh
index d2b7b3d98d..1243d96092 100755
--- a/t/t9815-git-p4-submit-fail.sh
+++ b/t/t9815-git-p4-submit-fail.sh
@@ -405,8 +405,8 @@ test_expect_success 'cleanup chmod after submit cancel' '
 	git p4 clone --dest="$git" //depot &&
 	(
 		cd "$git" &&
-		chmod u+x text &&
-		chmod u-x text+x &&
+		test_chmod +x text &&
+		test_chmod -x text+x &&
 		git add text text+x &&
 		git commit -m "chmod texts" &&
 		echo n | test_expect_code 1 git p4 submit
@@ -415,10 +415,13 @@ test_expect_success 'cleanup chmod after submit cancel' '
 		cd "$cli" &&
 		test_path_is_file text &&
 		! p4 fstat -T action text &&
-		stat --format=%A text | egrep ^-r-- &&
 		test_path_is_file text+x &&
 		! p4 fstat -T action text+x &&
-		stat --format=%A text+x | egrep ^-r-x
+		if test_have_prereq NOT_CYGWIN
+		then
+			stat --format=%A text | egrep ^-r-- &&
+			stat --format=%A text+x | egrep ^-r-x
+		fi
 	)
 '
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1a6c4ab08c..9e7f6b424d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -666,12 +666,14 @@ case $(uname -s) in
 	# backslashes in pathspec are converted to '/'
 	# exec does not inherit the PID
 	test_set_prereq MINGW
+	test_set_prereq NOT_CYGWIN
 	test_set_prereq SED_STRIPS_CR
 	;;
 *CYGWIN*)
 	test_set_prereq POSIXPERM
 	test_set_prereq EXECKEEPSPID
 	test_set_prereq NOT_MINGW
+	test_set_prereq CYGWIN
 	test_set_prereq SED_STRIPS_CR
 	;;
 *)
@@ -679,6 +681,7 @@ case $(uname -s) in
 	test_set_prereq BSLASHPSPEC
 	test_set_prereq EXECKEEPSPID
 	test_set_prereq NOT_MINGW
+	test_set_prereq NOT_CYGWIN
 	;;
 esac