{"thread":{"id":"31671","subject":"[PATCH 00/21] git p4: work on cygwin","startedAt":"2012-09-28T12:04:04Z","lastAt":"2013-01-27T01:51:35Z","messageCount":29,"participants":["Pete Wyckoff","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":21},"messages":[{"id":"200086","messageId":"1348833865-6093-1-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":null,"subject":"[PATCH 00/21] git p4: work on cygwin","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:04Z","receivedAt":"2012-09-28T12:04:04Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"This series fixes problems in git-p4, and its tests, so that\ngit-p4 works on the cygwin platform.\n\nSee the wiki for info on how to get started on cygwin:\n\n    https://git.wiki.kernel.org/index.php/GitP4\n\nTesting by people who use cygwin would be appreciated.  It would\nbe good to support cygwin more regularly.  Anyone who had time\nto contribute to testing on cygwin, and reporting problems, would\nbe welcome.\n\nThere's more work requried to support msysgit.  Those patches\nare not in good enough shape to ship out yet, but a lot of what\nis in this series is required for msysgit too.\n\nThese patches:\n\n    - fix bugs in git-p4 related to issues found on cygwin\n    - cleanup some ugly code in git-p4 observed in error paths while\n      getting tests to work on cygwin\n    - simplify and refactor code and tests to make cygwin changes easier\n    - handle newline and path issues for cygwin platform\n    - speed up some aspects of git-p4 by removing extra shell invocations\n\nPete Wyckoff (21):\n  git p4: temp branch name should use / even on windows\n  git p4: remove unused imports\n  git p4: generate better error message for bad depot path\n  git p4: fix error message when \"describe -s\" fails\n  git p4 test: use client_view to build the initial client\n  git p4 test: use client_view in t9806\n  git p4 test: start p4d inside its db dir\n  git p4 test: translate windows paths for cygwin\n  git p4: remove unreachable windows \\r\\n conversion code\n  git p4: scrub crlf for utf16 files on windows\n  git p4 test: newline handling\n  git p4 test: use LineEnd unix in windows tests too\n  git p4 test: avoid wildcard * in windows\n  git p4: cygwin p4 client does not mark read-only\n  git p4 test: disable chmod test for cygwin\n  git p4: disable read-only attribute before deleting\n  git p4: avoid shell when mapping users\n  git p4: avoid shell when invoking git rev-list\n  git p4: avoid shell when invoking git config --get-all\n  git p4: avoid shell when calling git config\n  git p4: introduce gitConfigBool\n\n git-p4.py                     | 122 ++++++++++++++++++++++++++++--------------\n t/lib-git-p4.sh               |  60 +++++++++++++++------\n t/t9800-git-p4-basic.sh       |   5 ++\n t/t9802-git-p4-filetype.sh    | 117 ++++++++++++++++++++++++++++++++++++++++\n t/t9806-git-p4-options.sh     |  50 ++++++++---------\n t/t9807-git-p4-submit.sh      |  14 ++++-\n t/t9809-git-p4-client-view.sh |  14 +++--\n t/t9812-git-p4-wildcards.sh   |  37 ++++++++++---\n t/t9815-git-p4-submit-fail.sh |   4 +-\n t/test-lib.sh                 |   3 ++\n 10 files changed, 330 insertions(+), 96 deletions(-)\n\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200087","messageId":"1348833865-6093-2-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 01/21] git p4: temp branch name should use / even on windows","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:05Z","receivedAt":"2012-09-28T12:04:05Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Commit fed2369 (git-p4: Search for parent commit on branch creation,\n2012-01-25) uses temporary branches to help find the parent of a\nnew p4 branch.  The temp branches are of the form \"git-p4-tmp/%d\"\nfor some p4 change number.  Mistakenly, this string was made\nusing os.path.join() instead of just string concatenation.  On\nwindows, this turns into a backslash (\\), which is not allowed in\ngit branch names.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 882b1bb..1e7a22a 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2599,7 +2599,7 @@ class P4Sync(Command, P4UserMap):\n \n                         blob = None\n                         if len(parent) > 0:\n-                            tempBranch = os.path.join(self.tempBranchLocation, \"%d\" % (change))\n+                            tempBranch = \"%s/%d\" % (self.tempBranchLocation, change)\n                             if self.verbose:\n                                 print \"Creating temporary branch: \" + tempBranch\n                             self.commit(description, filesForCommit, tempBranch)\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200088","messageId":"1348833865-6093-3-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 02/21] git p4: remove unused imports","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:06Z","receivedAt":"2012-09-28T12:04:06Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Found by \"pyflakes\" checker tool.\nModules shelve, getopt were unused.\nModule os.path is exported by os.\nReformat one-per-line as is PEP008 suggested style.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 1e7a22a..97699ef 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -7,10 +7,16 @@\n #            2007 Trolltech ASA\n # License: MIT <http://www.opensource.org/licenses/mit-license.php>\n #\n-\n-import optparse, sys, os, marshal, subprocess, shelve\n-import tempfile, getopt, os.path, time, platform\n-import re, shutil\n+import sys\n+import os\n+import optparse\n+import marshal\n+import subprocess\n+import tempfile\n+import time\n+import platform\n+import re\n+import shutil\n \n verbose = False\n \n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200089","messageId":"1348833865-6093-4-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 03/21] git p4: generate better error message for bad depot path","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:07Z","receivedAt":"2012-09-28T12:04:07Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Depot paths must start with //.  Exit with a better explanation\nwhen a bad depot path is supplied.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py               | 1 +\n t/t9800-git-p4-basic.sh | 5 +++++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 97699ef..eef5c94 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3035,6 +3035,7 @@ class P4Clone(P4Sync):\n         self.cloneExclude = [\"/\"+p for p in self.cloneExclude]\n         for p in depotPaths:\n             if not p.startswith(\"//\"):\n+                sys.stderr.write('Depot paths must start with \"//\": %s\\n' % p)\n                 return False\n \n         if not self.cloneDestination:\ndiff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\nindex b7ad716..c5f4c88 100755\n--- a/t/t9800-git-p4-basic.sh\n+++ b/t/t9800-git-p4-basic.sh\n@@ -30,6 +30,11 @@ test_expect_success 'basic git p4 clone' '\n \t)\n '\n \n+test_expect_success 'depot typo error' '\n+\ttest_must_fail git p4 clone --dest=\"$git\" /depot 2>errs &&\n+\tgrep -q \"Depot paths must start with\" errs\n+'\n+\n test_expect_success 'git p4 clone @all' '\n \tgit p4 clone --dest=\"$git\" //depot@all &&\n \ttest_when_finished cleanup_git &&\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200090","messageId":"1348833865-6093-5-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 04/21] git p4: fix error message when \"describe -s\" fails","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:08Z","receivedAt":"2012-09-28T12:04:08Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"The output was a bit nonsensical, including a bare %d.  Fix it\nto make it easier to understand.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex eef5c94..d7ee4b4 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2679,7 +2679,8 @@ class P4Sync(Command, P4UserMap):\n             if r.has_key('time'):\n                 newestTime = int(r['time'])\n         if newestTime is None:\n-            die(\"\\\"describe -s\\\" on newest change %d did not give a time\")\n+            die(\"Output from \\\"describe -s\\\" on newest change %d did not give a time\" %\n+                newestRevision)\n         details[\"time\"] = newestTime\n \n         self.updateOptionDict(details)\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200091","messageId":"1348833865-6093-6-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 05/21] git p4 test: use client_view to build the initial client","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:09Z","receivedAt":"2012-09-28T12:04:09Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Simplify the code a bit by using an existing function.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/lib-git-p4.sh | 11 ++---------\n 1 file changed, 2 insertions(+), 9 deletions(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 7061dce..890ee60 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -74,15 +74,8 @@ start_p4d() {\n \tfi\n \n \t# build a client\n-\t(\n-\t\tcd \"$cli\" &&\n-\t\tp4 client -i <<-EOF\n-\t\tClient: client\n-\t\tDescription: client\n-\t\tRoot: $cli\n-\t\tView: //depot/... //client/...\n-\t\tEOF\n-\t)\n+\tclient_view \"//depot/... //client/...\" &&\n+\n \treturn 0\n }\n \n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200092","messageId":"1348833865-6093-7-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 06/21] git p4 test: use client_view in t9806","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:10Z","receivedAt":"2012-09-28T12:04:10Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Use the standard client_view function from lib-git-p4.sh\ninstead of building one by hand.  This requires a bit of\nrework, using the current value of $P4CLIENT for the client\nname.  It also reorganizes the test to isolate changes to\n$P4CLIENT and $cli in a subshell.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/lib-git-p4.sh           |  4 ++--\n t/t9806-git-p4-options.sh | 50 ++++++++++++++++++++++-------------------------\n 2 files changed, 25 insertions(+), 29 deletions(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 890ee60..d558dd0 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -116,8 +116,8 @@ marshal_dump() {\n client_view() {\n \t(\n \t\tcat <<-EOF &&\n-\t\tClient: client\n-\t\tDescription: client\n+\t\tClient: $P4CLIENT\n+\t\tDescription: $P4CLIENT\n \t\tRoot: $cli\n \t\tView:\n \t\tEOF\ndiff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\nindex fa40cc8..37ca30a 100755\n--- a/t/t9806-git-p4-options.sh\n+++ b/t/t9806-git-p4-options.sh\n@@ -126,37 +126,33 @@ test_expect_success 'clone --use-client-spec' '\n \t\texec >/dev/null &&\n \t\ttest_must_fail git p4 clone --dest=\"$git\" --use-client-spec\n \t) &&\n-\tcli2=$(test-path-utils real_path \"$TRASH_DIRECTORY/cli2\") &&\n+\t# build a different client\n+\tcli2=\"$TRASH_DIRECTORY/cli2\" &&\n \tmkdir -p \"$cli2\" &&\n \ttest_when_finished \"rmdir \\\"$cli2\\\"\" &&\n-\t(\n-\t\tcd \"$cli2\" &&\n-\t\tp4 client -i <<-EOF\n-\t\tClient: client2\n-\t\tDescription: client2\n-\t\tRoot: $cli2\n-\t\tView: //depot/sub/... //client2/bus/...\n-\t\tEOF\n-\t) &&\n-\tP4CLIENT=client2 &&\n \ttest_when_finished cleanup_git &&\n-\tgit p4 clone --dest=\"$git\" --use-client-spec //depot/... &&\n-\t(\n-\t\tcd \"$git\" &&\n-\t\ttest_path_is_file bus/dir/f4 &&\n-\t\ttest_path_is_missing file1\n-\t) &&\n-\tcleanup_git &&\n-\n-\t# same thing again, this time with variable instead of option\n \t(\n-\t\tcd \"$git\" &&\n-\t\tgit init &&\n-\t\tgit config git-p4.useClientSpec true &&\n-\t\tgit p4 sync //depot/... &&\n-\t\tgit checkout -b master p4/master &&\n-\t\ttest_path_is_file bus/dir/f4 &&\n-\t\ttest_path_is_missing file1\n+\t\t# group P4CLIENT and cli changes in a sub-shell\n+\t\tP4CLIENT=client2 &&\n+\t\tcli=\"$cli2\" &&\n+\t\tclient_view \"//depot/sub/... //client2/bus/...\" &&\n+\t\tgit p4 clone --dest=\"$git\" --use-client-spec //depot/... &&\n+\t\t(\n+\t\t\tcd \"$git\" &&\n+\t\t\ttest_path_is_file bus/dir/f4 &&\n+\t\t\ttest_path_is_missing file1\n+\t\t) &&\n+\t\tcleanup_git &&\n+\t\t# same thing again, this time with variable instead of option\n+\t\t(\n+\t\t\tcd \"$git\" &&\n+\t\t\tgit init &&\n+\t\t\tgit config git-p4.useClientSpec true &&\n+\t\t\tgit p4 sync //depot/... &&\n+\t\t\tgit checkout -b master p4/master &&\n+\t\t\ttest_path_is_file bus/dir/f4 &&\n+\t\t\ttest_path_is_missing file1\n+\t\t)\n \t)\n '\n \n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200093","messageId":"1348833865-6093-8-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 07/21] git p4 test: start p4d inside its db dir","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:11Z","receivedAt":"2012-09-28T12:04:11Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"This will avoid having to do native path conversion for\nwindows.  Also may be a bit cleaner always to know that p4d\nhas that working directory, instead of wherever the function\nwas called from.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/lib-git-p4.sh | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex d558dd0..402d736 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -40,8 +40,11 @@ start_p4d() {\n \tmkdir -p \"$db\" \"$cli\" \"$git\" &&\n \trm -f \"$pidfile\" &&\n \t(\n-\t\tp4d -q -r \"$db\" -p $P4DPORT &\n-\t\techo $! >\"$pidfile\"\n+\t\tcd \"$db\" &&\n+\t\t{\n+\t\t\tp4d -q -p $P4DPORT &\n+\t\t\techo $! >\"$pidfile\"\n+\t\t}\n \t) &&\n \n \t# This gives p4d a long time to start up, as it can be\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200094","messageId":"1348833865-6093-9-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 08/21] git p4 test: translate windows paths for cygwin","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:12Z","receivedAt":"2012-09-28T12:04:12Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Native windows binaries do not understand posix-like\npath mapping offered by cygwin.  Convert paths to native\nusing \"cygpath --windows\" before presenting them to p4d.\n\nThis is done using the AltRoots mechanism of p4.  Both the\nposix and windows forms are put in the client specification,\nallowing p4 to find its location by native path even though\nthe environment reports a different PWD.\n\nShell operations in tests will use the normal form of $cli,\nwhich will look like a posix path in cygwin, while p4 will\nuse AltRoots to match against the windows form of the working\ndirectory.\n\nThanks-to: Sebastian Schuberth <sschuberth@gmail.com>\nThanks-to: Johannes Sixt <j6t@kdbg.org>\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/lib-git-p4.sh | 24 ++++++++++++++++++++++--\n t/test-lib.sh   |  3 +++\n 2 files changed, 25 insertions(+), 2 deletions(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 402d736..e2941ac 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -8,7 +8,8 @@ TEST_NO_CREATE_REPO=NoThanks\n \n . ./test-lib.sh\n \n-if ! test_have_prereq PYTHON; then\n+if ! test_have_prereq PYTHON\n+then\n \tskip_all='skipping git p4 tests; python not available'\n \ttest_done\n fi\n@@ -17,6 +18,24 @@ fi\n \ttest_done\n }\n \n+# On cygwin, the NT version of Perforce can be used.  When giving\n+# it paths, either on the command-line or in client specifications,\n+# be sure to use the native windows form.\n+#\n+# Older versions of perforce were available compiled natively for\n+# cygwin.  Those do not accept native windows paths, so make sure\n+# not to convert for them.\n+native_path() {\n+\tpath=\"$1\" &&\n+\tif test_have_prereq CYGWIN && ! p4 -V | grep -q CYGWIN\n+\tthen\n+\t\tpath=$(cygpath --windows \"$path\")\n+\telse\n+\t\tpath=$(test-path-utils real_path \"$path\")\n+\tfi &&\n+\techo \"$path\"\n+}\n+\n # Try to pick a unique port: guess a large number, then hope\n # no more than one of each test is running.\n #\n@@ -32,7 +51,7 @@ P4EDITOR=:\n export P4PORT P4CLIENT P4EDITOR\n \n db=\"$TRASH_DIRECTORY/db\"\n-cli=$(test-path-utils real_path \"$TRASH_DIRECTORY/cli\")\n+cli=\"$TRASH_DIRECTORY/cli\"\n git=\"$TRASH_DIRECTORY/git\"\n pidfile=\"$TRASH_DIRECTORY/p4d.pid\"\n \n@@ -122,6 +141,7 @@ client_view() {\n \t\tClient: $P4CLIENT\n \t\tDescription: $P4CLIENT\n \t\tRoot: $cli\n+\t\tAltRoots: $(native_path \"$cli\")\n \t\tView:\n \t\tEOF\n \t\tfor arg ; do\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex f8e3733..fd04870 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -624,12 +624,14 @@ case $(uname -s) in\n \t# backslashes in pathspec are converted to '/'\n \t# exec does not inherit the PID\n \ttest_set_prereq MINGW\n+\ttest_set_prereq NOT_CYGWIN\n \ttest_set_prereq SED_STRIPS_CR\n \t;;\n *CYGWIN*)\n \ttest_set_prereq POSIXPERM\n \ttest_set_prereq EXECKEEPSPID\n \ttest_set_prereq NOT_MINGW\n+\ttest_set_prereq CYGWIN\n \ttest_set_prereq SED_STRIPS_CR\n \t;;\n *)\n@@ -637,6 +639,7 @@ case $(uname -s) in\n \ttest_set_prereq BSLASHPSPEC\n \ttest_set_prereq EXECKEEPSPID\n \ttest_set_prereq NOT_MINGW\n+\ttest_set_prereq NOT_CYGWIN\n \t;;\n esac\n \n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200095","messageId":"1348833865-6093-10-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 09/21] git p4: remove unreachable windows \\r\\n conversion code","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:13Z","receivedAt":"2012-09-28T12:04:13Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Replacing \\r\\n with \\n on windows was added in c1f9197 (Replace\n\\r\\n with \\n when importing from p4 on Windows, 2007-05-24), to\nwork around an oddity with \"p4 print\" on windows.  Text files\nare printed with \"\\r\\r\\n\" endings, regardless of whether they\nwere created on unix or windows, and regardless of the client\nLineEnd setting.\n\nAs of d2c6dd3 (use p4CmdList() to get file contents in Python\ndicts. This is more robust., 2007-05-23), git-p4 uses \"p4 -G\nprint\", which generates files in a raw format.  As the native\nline ending format if p4 is \\n, there will be no \\r\\n in the\nraw text.\n\nActually, it is possible to generate a text file so that the\np4 representation includes embedded \\r\\n, even though this is not\nnormal on either windows or unix.  In that case the code would\nhave mistakenly stripped them out, but now they will be left\nintact.\n\nMore information on how p4 deals with line endings is here:\n\n    http://kb.perforce.com/article/63\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 9 ---------\n 1 file changed, 9 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex d7ee4b4..b773b09 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2064,15 +2064,6 @@ class P4Sync(Command, P4UserMap):\n             print \"\\nIgnoring apple filetype file %s\" % file['depotFile']\n             return\n \n-        # Perhaps windows wants unicode, utf16 newlines translated too;\n-        # but this is not doing it.\n-        if self.isWindows and type_base == \"text\":\n-            mangled = []\n-            for data in contents:\n-                data = data.replace(\"\\r\\n\", \"\\n\")\n-                mangled.append(data)\n-            contents = mangled\n-\n         # Note that we do not try to de-mangle keywords on utf16 files,\n         # even though in theory somebody may want that.\n         pattern = p4_keywords_regexp_for_type(type_base, type_mods)\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200096","messageId":"1348833865-6093-11-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 10/21] git p4: scrub crlf for utf16 files on windows","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:14Z","receivedAt":"2012-09-28T12:04:14Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Files of type utf16 are handled with \"p4 print\" instead of the\nnormal \"p4 -G print\" interface due to how the latter does not\nproduce correct output.  See 55aa571 (git-p4: handle utf16\nfiletype properly, 2011-09-17) for details.\n\nOn windows, though, \"p4 print\" can not be told which line\nendings to use, as there is no underlying client, and always\nchooses crlf, even for utf16 files.  Convert the \\r\\n into \\n\nwhen importing utf16 files.\n\nThe fix for this is complex, in that the problem is a property\nof the NT version of p4.  There are old versions of p4 that\nwere compiled directly for cygwin that should not be subjected\nto text replacement.  The right check here, then, is to look\nat the p4 version, not the OS version.  Note also that on cygwin,\nplatform.system() is \"CYGWIN_NT-5.1\" or similar, not \"Windows\".\n\nAdd a function to memoize the p4 version string and use it to\ncheck for \"/NT\", indicating the Windows build of p4.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 24 +++++++++++++++++++++++-\n 1 file changed, 23 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex b773b09..5b2f73d 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -147,6 +147,22 @@ def p4_system(cmd):\n     expand = isinstance(real_cmd, basestring)\n     subprocess.check_call(real_cmd, shell=expand)\n \n+_p4_version_string = None\n+def p4_version_string():\n+    \"\"\"Read the version string, showing just the last line, which\n+       hopefully is the interesting version bit.\n+\n+       $ p4 -V\n+       Perforce - The Fast Software Configuration Management System.\n+       Copyright 1995-2011 Perforce Software.  All rights reserved.\n+       Rev. P4/NTX86/2011.1/393975 (2011/12/16).\n+    \"\"\"\n+    global _p4_version_string\n+    if not _p4_version_string:\n+        a = p4_read_pipe_lines([\"-V\"])\n+        _p4_version_string = a[-1].rstrip()\n+    return _p4_version_string\n+\n def p4_integrate(src, dest):\n     p4_system([\"integrate\", \"-Dt\", wildcard_encode(src), wildcard_encode(dest)])\n \n@@ -1903,7 +1919,6 @@ class P4Sync(Command, P4UserMap):\n         self.syncWithOrigin = True\n         self.importIntoRemotes = True\n         self.maxChanges = \"\"\n-        self.isWindows = (platform.system() == \"Windows\")\n         self.keepRepoPath = False\n         self.depotPaths = None\n         self.p4BranchesInGit = []\n@@ -2048,7 +2063,14 @@ class P4Sync(Command, P4UserMap):\n             # operations.  utf16 is converted to ascii or utf8, perhaps.\n             # But ascii text saved as -t utf16 is completely mangled.\n             # Invoke print -o to get the real contents.\n+            #\n+            # On windows, the newlines will always be mangled by print, so put\n+            # them back too.  This is not needed to the cygwin windows version,\n+            # just the native \"NT\" type.\n+            #\n             text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])\n+            if p4_version_string().find(\"/NT\") >= 0:\n+                text = text.replace(\"\\r\\n\", \"\\n\")\n             contents = [ text ]\n \n         if type_base == \"apple\":\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200097","messageId":"1348833865-6093-12-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 11/21] git p4 test: newline handling","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:15Z","receivedAt":"2012-09-28T12:04:15Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"P4 stores newlines in the depos as \\n.  By default, git does this\ntoo, both on unix and windows.  Test to make sure that this stays\ntrue.\n\nBoth git and p4 have mechanisms to use \\r\\n in the working\ndirectory.  Exercise these.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/t9802-git-p4-filetype.sh | 117 +++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 117 insertions(+)\n\ndiff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh\nindex 21924df..c5ab626 100755\n--- a/t/t9802-git-p4-filetype.sh\n+++ b/t/t9802-git-p4-filetype.sh\n@@ -8,6 +8,123 @@ test_expect_success 'start p4d' '\n \tstart_p4d\n '\n \n+#\n+# This series of tests checks newline handling  Both p4 and\n+# git store newlines as \\n, and have options to choose how\n+# newlines appear in checked-out files.\n+#\n+test_expect_success 'p4 client newlines, unix' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 client -o | sed \"/LineEnd/s/:.*/:unix/\" | p4 client -i &&\n+\t\tprintf \"unix\\ncrlf\\n\" >f-unix &&\n+\t\tprintf \"unix\\r\\ncrlf\\r\\n\" >f-unix-as-crlf &&\n+\t\tp4 add -t text f-unix &&\n+\t\tp4 submit -d f-unix &&\n+\n+\t\t# LineEnd: unix; should be no change after sync\n+\t\tcp f-unix f-unix-orig &&\n+\t\tp4 sync -f &&\n+\t\ttest_cmp f-unix-orig f-unix &&\n+\n+\t\t# make sure stored in repo as unix newlines\n+\t\t# use sed to eat python-appened newline\n+\t\tp4 -G print //depot/f-unix | marshal_dump data 2 |\\\n+\t\t    sed \\$d >f-unix-p4-print &&\n+\t\ttest_cmp f-unix-orig f-unix-p4-print &&\n+\n+\t\t# switch to win, make sure lf -> crlf\n+\t\tp4 client -o | sed \"/LineEnd/s/:.*/:win/\" | p4 client -i &&\n+\t\tp4 sync -f &&\n+\t\ttest_cmp f-unix-as-crlf f-unix\n+\t)\n+'\n+\n+test_expect_success 'p4 client newlines, win' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 client -o | sed \"/LineEnd/s/:.*/:win/\" | p4 client -i &&\n+\t\tprintf \"win\\r\\ncrlf\\r\\n\" >f-win &&\n+\t\tprintf \"win\\ncrlf\\n\" >f-win-as-lf &&\n+\t\tp4 add -t text f-win &&\n+\t\tp4 submit -d f-win &&\n+\n+\t\t# LineEnd: win; should be no change after sync\n+\t\tcp f-win f-win-orig &&\n+\t\tp4 sync -f &&\n+\t\ttest_cmp f-win-orig f-win &&\n+\n+\t\t# make sure stored in repo as unix newlines\n+\t\t# use sed to eat python-appened newline\n+\t\tp4 -G print //depot/f-win | marshal_dump data 2 |\\\n+\t\t    sed \\$d >f-win-p4-print &&\n+\t\ttest_cmp f-win-as-lf f-win-p4-print &&\n+\n+\t\t# switch to unix, make sure lf -> crlf\n+\t\tp4 client -o | sed \"/LineEnd/s/:.*/:unix/\" | p4 client -i &&\n+\t\tp4 sync -f &&\n+\t\ttest_cmp f-win-as-lf f-win\n+\t)\n+'\n+\n+test_expect_success 'ensure blobs store only lf newlines' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init &&\n+\t\tgit p4 sync //depot@all &&\n+\n+\t\t# verify the files in .git are stored only with newlines\n+\t\to=$(git ls-tree p4/master -- f-unix | cut -f1 | cut -d\\  -f3) &&\n+\t\tgit cat-file blob $o >f-unix-blob &&\n+\t\ttest_cmp \"$cli\"/f-unix-orig f-unix-blob &&\n+\n+\t\to=$(git ls-tree p4/master -- f-win | cut -f1 | cut -d\\  -f3) &&\n+\t\tgit cat-file blob $o >f-win-blob &&\n+\t\ttest_cmp \"$cli\"/f-win-as-lf f-win-blob &&\n+\n+\t\trm f-unix-blob f-win-blob\n+\t)\n+'\n+\n+test_expect_success 'gitattributes setting eol=lf produces lf newlines' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\t# checkout the files and make sure core.eol works as planned\n+\t\tcd \"$git\" &&\n+\t\tgit init &&\n+\t\techo \"* eol=lf\" >.gitattributes &&\n+\t\tgit p4 sync //depot@all &&\n+\t\tgit checkout master &&\n+\t\ttest_cmp \"$cli\"/f-unix-orig f-unix &&\n+\t\ttest_cmp \"$cli\"/f-win-as-lf f-win\n+\t)\n+'\n+\n+test_expect_success 'gitattributes setting eol=crlf produces crlf newlines' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\t# checkout the files and make sure core.eol works as planned\n+\t\tcd \"$git\" &&\n+\t\tgit init &&\n+\t\techo \"* eol=crlf\" >.gitattributes &&\n+\t\tgit p4 sync //depot@all &&\n+\t\tgit checkout master &&\n+\t\ttest_cmp \"$cli\"/f-unix-as-crlf f-unix &&\n+\t\ttest_cmp \"$cli\"/f-win-orig f-win\n+\t)\n+'\n+\n+test_expect_success 'crlf cleanup' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\trm f-unix-orig f-unix-as-crlf &&\n+\t\trm f-win-orig f-win-as-lf &&\n+\t\tp4 client -o | sed \"/LineEnd/s/:.*/:unix/\" | p4 client -i &&\n+\t\tp4 sync -f\n+\t)\n+'\n+\n test_expect_success 'utf-16 file create' '\n \t(\n \t\tcd \"$cli\" &&\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200098","messageId":"1348833865-6093-13-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 12/21] git p4 test: use LineEnd unix in windows tests too","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:16Z","receivedAt":"2012-09-28T12:04:16Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"In all clients, even those created on windows, use unix line\nendings.  This makes it possible to verify file contents without\ndoing OS-specific comparisons in all the tests.\n\nTests in t9802-git-p4-filetype.sh are used to make sure that\nthe other LineEnd options continue to work.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/lib-git-p4.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex e2941ac..fbd55ea 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -142,6 +142,7 @@ client_view() {\n \t\tDescription: $P4CLIENT\n \t\tRoot: $cli\n \t\tAltRoots: $(native_path \"$cli\")\n+\t\tLineEnd: unix\n \t\tView:\n \t\tEOF\n \t\tfor arg ; do\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200099","messageId":"1348833865-6093-14-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 13/21] git p4 test: avoid wildcard * in windows","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:17Z","receivedAt":"2012-09-28T12:04:17Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"This character is not valid in windows filenames, even though\nit can appear in p4 depot paths.  Avoid using it in tests on\nwindows, both mingw and cygwin.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/t9809-git-p4-client-view.sh | 10 ++++++++--\n t/t9812-git-p4-wildcards.sh   | 37 +++++++++++++++++++++++++++++--------\n 2 files changed, 37 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh\nindex 281be29..fd8fa89 100755\n--- a/t/t9809-git-p4-client-view.sh\n+++ b/t/t9809-git-p4-client-view.sh\n@@ -365,7 +365,10 @@ test_expect_success 'wildcard files submit back to p4, client-spec case' '\n \t(\n \t\tcd \"$git\" &&\n \t\techo git-wild-hash >dir1/git-wild#hash &&\n-\t\techo git-wild-star >dir1/git-wild\\*star &&\n+\t\tif test_have_prereq NOT_MINGW NOT_CYGWIN\n+\t\tthen\n+\t\t\techo git-wild-star >dir1/git-wild\\*star\n+\t\tfi &&\n \t\techo git-wild-at >dir1/git-wild@at &&\n \t\techo git-wild-percent >dir1/git-wild%percent &&\n \t\tgit add dir1/git-wild* &&\n@@ -376,7 +379,10 @@ test_expect_success 'wildcard files submit back to p4, client-spec case' '\n \t(\n \t\tcd \"$cli\" &&\n \t\ttest_path_is_file dir1/git-wild#hash &&\n-\t\ttest_path_is_file dir1/git-wild\\*star &&\n+\t\tif test_have_prereq NOT_MINGW NOT_CYGWIN\n+\t\tthen\n+\t\t\ttest_path_is_file dir1/git-wild\\*star\n+\t\tfi &&\n \t\ttest_path_is_file dir1/git-wild@at &&\n \t\ttest_path_is_file dir1/git-wild%percent\n \t) &&\ndiff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh\nindex 143d413..6763325 100755\n--- a/t/t9812-git-p4-wildcards.sh\n+++ b/t/t9812-git-p4-wildcards.sh\n@@ -14,7 +14,10 @@ test_expect_success 'add p4 files with wildcards in the names' '\n \t\tprintf \"file2\\nhas\\nsome\\nrandom\\ntext\\n\" >file2 &&\n \t\tp4 add file2 &&\n \t\techo file-wild-hash >file-wild#hash &&\n-\t\techo file-wild-star >file-wild\\*star &&\n+\t\tif test_have_prereq NOT_MINGW NOT_CYGWIN\n+\t\tthen\n+\t\t\techo file-wild-star >file-wild\\*star\n+\t\tfi &&\n \t\techo file-wild-at >file-wild@at &&\n \t\techo file-wild-percent >file-wild%percent &&\n \t\tp4 add -f file-wild* &&\n@@ -28,7 +31,10 @@ test_expect_success 'wildcard files git p4 clone' '\n \t(\n \t\tcd \"$git\" &&\n \t\ttest -f file-wild#hash &&\n-\t\ttest -f file-wild\\*star &&\n+\t\tif test_have_prereq NOT_MINGW NOT_CYGWIN\n+\t\tthen\n+\t\t\ttest -f file-wild\\*star\n+\t\tfi &&\n \t\ttest -f file-wild@at &&\n \t\ttest -f file-wild%percent\n \t)\n@@ -40,7 +46,10 @@ test_expect_success 'wildcard files submit back to p4, add' '\n \t(\n \t\tcd \"$git\" &&\n \t\techo git-wild-hash >git-wild#hash &&\n-\t\techo git-wild-star >git-wild\\*star &&\n+\t\tif test_have_prereq NOT_MINGW NOT_CYGWIN\n+\t\tthen\n+\t\t\techo git-wild-star >git-wild\\*star\n+\t\tfi &&\n \t\techo git-wild-at >git-wild@at &&\n \t\techo git-wild-percent >git-wild%percent &&\n \t\tgit add git-wild* &&\n@@ -51,7 +60,10 @@ test_expect_success 'wildcard files submit back to p4, add' '\n \t(\n \t\tcd \"$cli\" &&\n \t\ttest_path_is_file git-wild#hash &&\n-\t\ttest_path_is_file git-wild\\*star &&\n+\t\tif test_have_prereq NOT_MINGW NOT_CYGWIN\n+\t\tthen\n+\t\t\ttest_path_is_file git-wild\\*star\n+\t\tfi &&\n \t\ttest_path_is_file git-wild@at &&\n \t\ttest_path_is_file git-wild%percent\n \t)\n@@ -63,7 +75,10 @@ test_expect_success 'wildcard files submit back to p4, modify' '\n \t(\n \t\tcd \"$git\" &&\n \t\techo new-line >>git-wild#hash &&\n-\t\techo new-line >>git-wild\\*star &&\n+\t\tif test_have_prereq NOT_MINGW NOT_CYGWIN\n+\t\tthen\n+\t\t\techo new-line >>git-wild\\*star\n+\t\tfi &&\n \t\techo new-line >>git-wild@at &&\n \t\techo new-line >>git-wild%percent &&\n \t\tgit add git-wild* &&\n@@ -74,7 +89,10 @@ test_expect_success 'wildcard files submit back to p4, modify' '\n \t(\n \t\tcd \"$cli\" &&\n \t\ttest_line_count = 2 git-wild#hash &&\n-\t\ttest_line_count = 2 git-wild\\*star &&\n+\t\tif test_have_prereq NOT_MINGW NOT_CYGWIN\n+\t\tthen\n+\t\t\ttest_line_count = 2 git-wild\\*star\n+\t\tfi &&\n \t\ttest_line_count = 2 git-wild@at &&\n \t\ttest_line_count = 2 git-wild%percent\n \t)\n@@ -87,7 +105,7 @@ test_expect_success 'wildcard files submit back to p4, copy' '\n \t\tcd \"$git\" &&\n \t\tcp file2 git-wild-cp#hash &&\n \t\tgit add git-wild-cp#hash &&\n-\t\tcp git-wild\\*star file-wild-3 &&\n+\t\tcp git-wild#hash file-wild-3 &&\n \t\tgit add file-wild-3 &&\n \t\tgit commit -m \"wildcard copies\" &&\n \t\tgit config git-p4.detectCopies true &&\n@@ -134,7 +152,10 @@ test_expect_success 'wildcard files submit back to p4, delete' '\n \t(\n \t\tcd \"$cli\" &&\n \t\ttest_path_is_missing git-wild#hash &&\n-\t\ttest_path_is_missing git-wild\\*star &&\n+\t\tif test_have_prereq NOT_MINGW NOT_CYGWIN\n+\t\tthen\n+\t\t\ttest_path_is_missing git-wild\\*star\n+\t\tfi &&\n \t\ttest_path_is_missing git-wild@at &&\n \t\ttest_path_is_missing git-wild%percent\n \t)\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200100","messageId":"1348833865-6093-15-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 14/21] git p4: cygwin p4 client does not mark read-only","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:18Z","receivedAt":"2012-09-28T12:04:18Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"There are some old version of p4, compiled for cygwin, that\ntreat read-only files differently.\n\nNormally, a file that is not open is read-only, meaning that\n\"test -w\" on the file is false.  This works on unix, and it works\non windows using the NT version of p4.  The cygwin version\nof p4, though, changes the permissions, but does not set the\nwindows read-only attribute, so \"test -w\" returns false.\n\nNotice this oddity and make the tests work, even on cygiwn.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/lib-git-p4.sh               | 13 +++++++++++++\n t/t9807-git-p4-submit.sh      | 14 ++++++++++++--\n t/t9809-git-p4-client-view.sh |  4 ++--\n 3 files changed, 27 insertions(+), 4 deletions(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex fbd55ea..23d01fd 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -150,3 +150,16 @@ client_view() {\n \t\tdone\n \t) | p4 client -i\n }\n+\n+is_cli_file_writeable() {\n+\t# cygwin version of p4 does not set read-only attr,\n+\t# will be marked 444 but -w is true\n+\tfile=\"$1\" &&\n+\tif test_have_prereq CYGWIN && p4 -V | grep -q CYGWIN\n+\tthen\n+\t\tstat=$(stat --format=%a \"$file\") &&\n+\t\ttest $stat = 644\n+\telse\n+\t\ttest -w \"$file\"\n+\tfi\n+}\ndiff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\nindex 0ae048f..1fb7bc7 100755\n--- a/t/t9807-git-p4-submit.sh\n+++ b/t/t9807-git-p4-submit.sh\n@@ -17,6 +17,16 @@ test_expect_success 'init depot' '\n \t)\n '\n \n+test_expect_failure 'is_cli_file_writeable function' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo a >a &&\n+\t\tis_cli_file_writeable a &&\n+\t\t! is_cli_file_writeable file1 &&\n+\t\trm a\n+\t)\n+'\n+\n test_expect_success 'submit with no client dir' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone --dest=\"$git\" //depot &&\n@@ -200,7 +210,7 @@ test_expect_success 'submit copy' '\n \t(\n \t\tcd \"$cli\" &&\n \t\ttest_path_is_file file5.ta &&\n-\t\ttest ! -w file5.ta\n+\t\t! is_cli_file_writeable file5.ta\n \t)\n '\n \n@@ -219,7 +229,7 @@ test_expect_success 'submit rename' '\n \t\tcd \"$cli\" &&\n \t\ttest_path_is_missing file6.t &&\n \t\ttest_path_is_file file6.ta &&\n-\t\ttest ! -w file6.ta\n+\t\t! is_cli_file_writeable file6.ta\n \t)\n '\n \ndiff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh\nindex fd8fa89..e0eb6b0 100755\n--- a/t/t9809-git-p4-client-view.sh\n+++ b/t/t9809-git-p4-client-view.sh\n@@ -333,7 +333,7 @@ test_expect_success 'subdir clone, submit copy' '\n \t(\n \t\tcd \"$cli\" &&\n \t\ttest_path_is_file dir1/file11a &&\n-\t\ttest ! -w dir1/file11a\n+\t\t! is_cli_file_writeable dir1/file11a\n \t)\n '\n \n@@ -353,7 +353,7 @@ test_expect_success 'subdir clone, submit rename' '\n \t\tcd \"$cli\" &&\n \t\ttest_path_is_missing dir1/file13 &&\n \t\ttest_path_is_file dir1/file13a &&\n-\t\ttest ! -w dir1/file13a\n+\t\t! is_cli_file_writeable dir1/file13a\n \t)\n '\n \n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200102","messageId":"1348833865-6093-16-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 15/21] git p4 test: disable chmod test for cygwin","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:19Z","receivedAt":"2012-09-28T12:04:19Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"It does not notice chmod +x or -x; there is nothing\nfor this test to do.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/t9815-git-p4-submit-fail.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh\nindex d2b7b3d..2db1bf1 100755\n--- a/t/t9815-git-p4-submit-fail.sh\n+++ b/t/t9815-git-p4-submit-fail.sh\n@@ -400,7 +400,9 @@ test_expect_success 'cleanup rename after submit cancel' '\n \t)\n '\n \n-test_expect_success 'cleanup chmod after submit cancel' '\n+# chmods are not recognized in cygwin; git has nothing\n+# to commit\n+test_expect_success NOT_CYGWIN 'cleanup chmod after submit cancel' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone --dest=\"$git\" //depot &&\n \t(\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200103","messageId":"1348833865-6093-17-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 16/21] git p4: disable read-only attribute before deleting","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:20Z","receivedAt":"2012-09-28T12:04:20Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"On windows, p4 marks un-edited files as read-only.  Not only are\nthey read-only, but also they cannot be deleted.  Remove the\nread-only attribute before deleting in both the copy and rename\ncases.\n\nThis also happens in the RCS cleanup code, where a file is marked\nto be deleted, but must first be edited to remove adjust the\nkeyword lines.  Make sure it is editable before patching.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 5b2f73d..a6806bc 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -17,6 +17,7 @@ import time\n import platform\n import re\n import shutil\n+import stat\n \n verbose = False\n \n@@ -1163,6 +1164,9 @@ class P4Submit(Command, P4UserMap):\n                     p4_edit(dest)\n                     pureRenameCopy.discard(dest)\n                     filesToChangeExecBit[dest] = diff['dst_mode']\n+                if self.isWindows:\n+                    # turn off read-only attribute\n+                    os.chmod(dest, stat.S_IWRITE)\n                 os.unlink(dest)\n                 editedFiles.add(dest)\n             elif modifier == \"R\":\n@@ -1181,6 +1185,8 @@ class P4Submit(Command, P4UserMap):\n                         p4_edit(dest)   # with move: already open, writable\n                     filesToChangeExecBit[dest] = diff['dst_mode']\n                 if not self.p4HasMoveCommand:\n+                    if self.isWindows:\n+                        os.chmod(dest, stat.S_IWRITE)\n                     os.unlink(dest)\n                     filesToDelete.add(src)\n                 editedFiles.add(dest)\n@@ -1221,6 +1227,10 @@ class P4Submit(Command, P4UserMap):\n                 for file in kwfiles:\n                     if verbose:\n                         print \"zapping %s with %s\" % (line,pattern)\n+                    # File is being deleted, so not open in p4.  Must\n+                    # disable the read-only bit on windows.\n+                    if self.isWindows and file not in editedFiles:\n+                        os.chmod(file, stat.S_IWRITE)\n                     self.patchRCSKeywords(file, kwfiles[file])\n                     fixed_rcs_keywords = True\n \n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200104","messageId":"1348833865-6093-18-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 17/21] git p4: avoid shell when mapping users","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:21Z","receivedAt":"2012-09-28T12:04:21Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"The extra quoting and double-% are unneeded, just to work\naround the shell.  Instead, avoid the shell indirection.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex a6806bc..a92d84f 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -982,7 +982,8 @@ class P4Submit(Command, P4UserMap):\n     def p4UserForCommit(self,id):\n         # Return the tuple (perforce user,git email) for a given git commit id\n         self.getUserMapFromPerforceServer()\n-        gitEmail = read_pipe(\"git log --max-count=1 --format='%%ae' %s\" % id)\n+        gitEmail = read_pipe([\"git\", \"log\", \"--max-count=1\",\n+                              \"--format=%ae\", id])\n         gitEmail = gitEmail.strip()\n         if not self.emails.has_key(gitEmail):\n             return (None,gitEmail)\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200105","messageId":"1348833865-6093-19-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 18/21] git p4: avoid shell when invoking git rev-list","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:22Z","receivedAt":"2012-09-28T12:04:22Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Invoke git rev-list directly, avoiding the shell, in\nP4Submit and P4Sync.  The overhead of starting extra\nprocesses is significant in cygwin; this speeds things\nup on that platform.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex a92d84f..9c33af4 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1538,7 +1538,7 @@ class P4Submit(Command, P4UserMap):\n         self.check()\n \n         commits = []\n-        for line in read_pipe_lines(\"git rev-list --no-merges %s..%s\" % (self.origin, self.master)):\n+        for line in read_pipe_lines([\"git\", \"rev-list\", \"--no-merges\", \"%s..%s\" % (self.origin, self.master)]):\n             commits.append(line.strip())\n         commits.reverse()\n \n@@ -2558,7 +2558,8 @@ class P4Sync(Command, P4UserMap):\n \n     def searchParent(self, parent, branch, target):\n         parentFound = False\n-        for blob in read_pipe_lines([\"git\", \"rev-list\", \"--reverse\", \"--no-merges\", parent]):\n+        for blob in read_pipe_lines([\"git\", \"rev-list\", \"--reverse\",\n+                                     \"--no-merges\", parent]):\n             blob = blob.strip()\n             if len(read_pipe([\"git\", \"diff-tree\", blob, target])) == 0:\n                 parentFound = True\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200106","messageId":"1348833865-6093-20-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 19/21] git p4: avoid shell when invoking git config --get-all","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:23Z","receivedAt":"2012-09-28T12:04:23Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 9c33af4..c0c738a 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -525,7 +525,8 @@ def gitConfig(key, args = None): # set args to \"--bool\", for instance\n \n def gitConfigList(key):\n     if not _gitConfig.has_key(key):\n-        _gitConfig[key] = read_pipe(\"git config --get-all %s\" % key, ignore_error=True).strip().split(os.linesep)\n+        s = read_pipe([\"git\", \"config\", \"--get-all\", key], ignore_error=True)\n+        _gitConfig[key] = s.strip().split(os.linesep)\n     return _gitConfig[key]\n \n def p4BranchesInGit(branchesAreInRemotes = True):\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200107","messageId":"1348833865-6093-21-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 20/21] git p4: avoid shell when calling git config","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:24Z","receivedAt":"2012-09-28T12:04:24Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex c0c738a..007ef6b 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -514,13 +514,16 @@ def gitBranchExists(branch):\n     return proc.wait() == 0;\n \n _gitConfig = {}\n-def gitConfig(key, args = None): # set args to \"--bool\", for instance\n+\n+def gitConfig(key, args=None): # set args to \"--bool\", for instance\n     if not _gitConfig.has_key(key):\n-        argsFilter = \"\"\n-        if args != None:\n-            argsFilter = \"%s \" % args\n-        cmd = \"git config %s%s\" % (argsFilter, key)\n-        _gitConfig[key] = read_pipe(cmd, ignore_error=True).strip()\n+        cmd = [ \"git\", \"config\" ]\n+        if args:\n+            assert(args == \"--bool\")\n+            cmd.append(args)\n+        cmd.append(key)\n+        s = read_pipe(cmd, ignore_error=True)\n+        _gitConfig[key] = s.strip()\n     return _gitConfig[key]\n \n def gitConfigList(key):\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200108","messageId":"1348833865-6093-22-git-send-email-pw@padd.com","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"[PATCH 21/21] git p4: introduce gitConfigBool","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-09-28T12:04:25Z","receivedAt":"2012-09-28T12:04:25Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Make the intent of \"--bool\" more obvious by returning a direct True\nor False value.  Convert a couple non-bool users with obvious bool\nintent.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 45 ++++++++++++++++++++++++++-------------------\n 1 file changed, 26 insertions(+), 19 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 007ef6b..524df12 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -515,17 +515,25 @@ def gitBranchExists(branch):\n \n _gitConfig = {}\n \n-def gitConfig(key, args=None): # set args to \"--bool\", for instance\n+def gitConfig(key):\n     if not _gitConfig.has_key(key):\n-        cmd = [ \"git\", \"config\" ]\n-        if args:\n-            assert(args == \"--bool\")\n-            cmd.append(args)\n-        cmd.append(key)\n+        cmd = [ \"git\", \"config\", key ]\n         s = read_pipe(cmd, ignore_error=True)\n         _gitConfig[key] = s.strip()\n     return _gitConfig[key]\n \n+def gitConfigBool(key):\n+    \"\"\"Return a bool, using git config --bool.  It is True only if the\n+       variable is set to true, and False if set to false or not present\n+       in the config.\"\"\"\n+\n+    if not _gitConfig.has_key(key):\n+        cmd = [ \"git\", \"config\", \"--bool\", key ]\n+        s = read_pipe(cmd, ignore_error=True)\n+        v = s.strip()\n+        _gitConfig[key] = v == \"true\"\n+    return _gitConfig[key]\n+\n def gitConfigList(key):\n     if not _gitConfig.has_key(key):\n         s = read_pipe([\"git\", \"config\", \"--get-all\", key], ignore_error=True)\n@@ -656,8 +664,7 @@ def p4PathStartsWith(path, prefix):\n     #\n     # we may or may not have a problem. If you have core.ignorecase=true,\n     # we treat DirA and dira as the same directory\n-    ignorecase = gitConfig(\"core.ignorecase\", \"--bool\") == \"true\"\n-    if ignorecase:\n+    if gitConfigBool(\"core.ignorecase\"):\n         return path.lower().startswith(prefix.lower())\n     return path.startswith(prefix)\n \n@@ -892,7 +899,7 @@ class P4Submit(Command, P4UserMap):\n         self.usage += \" [name of git branch to submit into perforce depot]\"\n         self.origin = \"\"\n         self.detectRenames = False\n-        self.preserveUser = gitConfig(\"git-p4.preserveUser\").lower() == \"true\"\n+        self.preserveUser = gitConfigBool(\"git-p4.preserveUser\")\n         self.dry_run = False\n         self.prepare_p4_only = False\n         self.conflict_behavior = None\n@@ -1000,7 +1007,7 @@ class P4Submit(Command, P4UserMap):\n             (user,email) = self.p4UserForCommit(id)\n             if not user:\n                 msg = \"Cannot find p4 user for email %s in commit %s.\" % (email, id)\n-                if gitConfig('git-p4.allowMissingP4Users').lower() == \"true\":\n+                if gitConfigBool(\"git-p4.allowMissingP4Users\"):\n                     print \"%s\" % msg\n                 else:\n                     die(\"Error: %s\\nSet git-p4.allowMissingP4Users to true to allow this.\" % msg)\n@@ -1095,7 +1102,7 @@ class P4Submit(Command, P4UserMap):\n            message.  Return true if okay to continue with the submit.\"\"\"\n \n         # if configured to skip the editing part, just submit\n-        if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n+        if gitConfigBool(\"git-p4.skipSubmitEdit\"):\n             return True\n \n         # look at the modification time, to check later if the user saved\n@@ -1111,7 +1118,7 @@ class P4Submit(Command, P4UserMap):\n \n         # If the file was not saved, prompt to see if this patch should\n         # be skipped.  But skip this verification step if configured so.\n-        if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n+        if gitConfigBool(\"git-p4.skipSubmitEditCheck\"):\n             return True\n \n         # modification time updated means user saved the file\n@@ -1211,7 +1218,7 @@ class P4Submit(Command, P4UserMap):\n \n             # Patch failed, maybe it's just RCS keyword woes. Look through\n             # the patch to see if that's possible.\n-            if gitConfig(\"git-p4.attemptRCSCleanup\",\"--bool\") == \"true\":\n+            if gitConfigBool(\"git-p4.attemptRCSCleanup\"):\n                 file = None\n                 pattern = None\n                 kwfiles = {}\n@@ -1506,7 +1513,7 @@ class P4Submit(Command, P4UserMap):\n             sys.exit(128)\n \n         self.useClientSpec = False\n-        if gitConfig(\"git-p4.useclientspec\", \"--bool\") == \"true\":\n+        if gitConfigBool(\"git-p4.useclientspec\"):\n             self.useClientSpec = True\n         if self.useClientSpec:\n             self.clientSpecDirs = getClientSpec()\n@@ -1546,7 +1553,7 @@ class P4Submit(Command, P4UserMap):\n             commits.append(line.strip())\n         commits.reverse()\n \n-        if self.preserveUser or (gitConfig(\"git-p4.skipUserNameCheck\") == \"true\"):\n+        if self.preserveUser or gitConfigBool(\"git-p4.skipUserNameCheck\"):\n             self.checkAuthorship = False\n         else:\n             self.checkAuthorship = True\n@@ -1582,7 +1589,7 @@ class P4Submit(Command, P4UserMap):\n         else:\n             self.diffOpts += \" -C%s\" % detectCopies\n \n-        if gitConfig(\"git-p4.detectCopiesHarder\", \"--bool\") == \"true\":\n+        if gitConfigBool(\"git-p4.detectCopiesHarder\"):\n             self.diffOpts += \" --find-copies-harder\"\n \n         #\n@@ -1664,7 +1671,7 @@ class P4Submit(Command, P4UserMap):\n                                            \"--format=format:%h %s\",  c])\n                 print \"You will have to do 'git p4 sync' and rebase.\"\n \n-        if gitConfig(\"git-p4.exportLabels\", \"--bool\") == \"true\":\n+        if gitConfigBool(\"git-p4.exportLabels\"):\n             self.exportLabels = True\n \n         if self.exportLabels:\n@@ -2757,7 +2764,7 @@ class P4Sync(Command, P4UserMap):\n             # will use this after clone to set the variable\n             self.useClientSpec_from_options = True\n         else:\n-            if gitConfig(\"git-p4.useclientspec\", \"--bool\") == \"true\":\n+            if gitConfigBool(\"git-p4.useclientspec\"):\n                 self.useClientSpec = True\n         if self.useClientSpec:\n             self.clientSpecDirs = getClientSpec()\n@@ -2954,7 +2961,7 @@ class P4Sync(Command, P4UserMap):\n                             sys.stdout.write(\"%s \" % b)\n                         sys.stdout.write(\"\\n\")\n \n-        if gitConfig(\"git-p4.importLabels\", \"--bool\") == \"true\":\n+        if gitConfigBool(\"git-p4.importLabels\"):\n             self.importLabels = True\n \n         if self.importLabels:\n-- \n1.7.12.1.403.g28165e1\n"},{"id":"200125","messageId":"7vlifuklbz.fsf@alter.siamese.dyndns.org","threadId":"31671","inReplyTo":"1348833865-6093-4-git-send-email-pw@padd.com","subject":"Re: [PATCH 03/21] git p4: generate better error message for bad depot path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-28T18:58:01Z","receivedAt":"2012-09-28T18:58:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff <pw@padd.com> writes:\n\n> Depot paths must start with //.  Exit with a better explanation\n> when a bad depot path is supplied.\n>\n> Signed-off-by: Pete Wyckoff <pw@padd.com>\n> ---\n>  git-p4.py               | 1 +\n>  t/t9800-git-p4-basic.sh | 5 +++++\n>  2 files changed, 6 insertions(+)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 97699ef..eef5c94 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -3035,6 +3035,7 @@ class P4Clone(P4Sync):\n>          self.cloneExclude = [\"/\"+p for p in self.cloneExclude]\n>          for p in depotPaths:\n>              if not p.startswith(\"//\"):\n> +                sys.stderr.write('Depot paths must start with \"//\": %s\\n' % p)\n>                  return False\n>  \n>          if not self.cloneDestination:\n> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\n> index b7ad716..c5f4c88 100755\n> --- a/t/t9800-git-p4-basic.sh\n> +++ b/t/t9800-git-p4-basic.sh\n> @@ -30,6 +30,11 @@ test_expect_success 'basic git p4 clone' '\n>  \t)\n>  '\n>  \n> +test_expect_success 'depot typo error' '\n> +\ttest_must_fail git p4 clone --dest=\"$git\" /depot 2>errs &&\n> +\tgrep -q \"Depot paths must start with\" errs\n> +'\n\nUse of \"grep -q\" does not help ordinary testers, as the output will\nnot be shown when the tests are run normally.\n\n>  test_expect_success 'git p4 clone @all' '\n>  \tgit p4 clone --dest=\"$git\" //depot@all &&\n>  \ttest_when_finished cleanup_git &&\n"},{"id":"200126","messageId":"7vfw62klbx.fsf@alter.siamese.dyndns.org","threadId":"31671","inReplyTo":"1348833865-6093-5-git-send-email-pw@padd.com","subject":"Re: [PATCH 04/21] git p4: fix error message when \"describe -s\" fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-28T19:02:33Z","receivedAt":"2012-09-28T19:02:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff <pw@padd.com> writes:\n\n> The output was a bit nonsensical, including a bare %d.  Fix it\n> to make it easier to understand.\n>\n> Signed-off-by: Pete Wyckoff <pw@padd.com>\n> ---\n>  git-p4.py | 3 ++-\n>  1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index eef5c94..d7ee4b4 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -2679,7 +2679,8 @@ class P4Sync(Command, P4UserMap):\n>              if r.has_key('time'):\n>                  newestTime = int(r['time'])\n>          if newestTime is None:\n> -            die(\"\\\"describe -s\\\" on newest change %d did not give a time\")\n> +            die(\"Output from \\\"describe -s\\\" on newest change %d did not give a time\" %\n> +                newestRevision)\n\nShouldn't it say \"p4 describe -s\"?\n\n>          details[\"time\"] = newestTime\n>  \n>          self.updateOptionDict(details)\n"},{"id":"200124","messageId":"7va9waklbv.fsf@alter.siamese.dyndns.org","threadId":"31671","inReplyTo":"1348833865-6093-6-git-send-email-pw@padd.com","subject":"Re: [PATCH 05/21] git p4 test: use client_view to build the initial client","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-28T19:06:03Z","receivedAt":"2012-09-28T19:06:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff <pw@padd.com> writes:\n\n> Simplify the code a bit by using an existing function.\n>\n> Signed-off-by: Pete Wyckoff <pw@padd.com>\n> ---\n>  t/lib-git-p4.sh | 11 ++---------\n>  1 file changed, 2 insertions(+), 9 deletions(-)\n>\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> index 7061dce..890ee60 100644\n> --- a/t/lib-git-p4.sh\n> +++ b/t/lib-git-p4.sh\n> @@ -74,15 +74,8 @@ start_p4d() {\n>  \tfi\n>  \n>  \t# build a client\n> -\t(\n> -\t\tcd \"$cli\" &&\n> -\t\tp4 client -i <<-EOF\n> -\t\tClient: client\n> -\t\tDescription: client\n> -\t\tRoot: $cli\n> -\t\tView: //depot/... //client/...\n> -\t\tEOF\n> -\t)\n> +\tclient_view \"//depot/... //client/...\" &&\n> +\n>  \treturn 0\n>  }\n\nAssuming that writing //depot/... //client/... on the next line\nindented by a tab is equivalent to writing it on View: line (which I\nthink it is), this looks like an obviously good reuse of the code.\n\nI have to wonder if the use of printf in client_view implementation\nshould be tighted up, though.\n\ndiff --git i/t/lib-git-p4.sh w/t/lib-git-p4.sh\nindex 7061dce..4e58289 100644\n--- i/t/lib-git-p4.sh\n+++ w/t/lib-git-p4.sh\n@@ -128,8 +128,6 @@ client_view() {\n \t\tRoot: $cli\n \t\tView:\n \t\tEOF\n-\t\tfor arg ; do\n-\t\t\tprintf \"\\t$arg\\n\"\n-\t\tdone\n+\t\tprintf \"\\t%s\\n\" \"$@\"\n \t) | p4 client -i\n }\n"},{"id":"200123","messageId":"7v4nmiklbt.fsf@alter.siamese.dyndns.org","threadId":"31671","inReplyTo":"1348833865-6093-7-git-send-email-pw@padd.com","subject":"Re: [PATCH 06/21] git p4 test: use client_view in t9806","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-28T19:11:05Z","receivedAt":"2012-09-28T19:11:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff <pw@padd.com> writes:\n\n> Use the standard client_view function from lib-git-p4.sh\n> instead of building one by hand.  This requires a bit of\n> rework, using the current value of $P4CLIENT for the client\n> name.  It also reorganizes the test to isolate changes to\n> $P4CLIENT and $cli in a subshell.\n>\n> Signed-off-by: Pete Wyckoff <pw@padd.com>\n> ---\n>  t/lib-git-p4.sh           |  4 ++--\n>  t/t9806-git-p4-options.sh | 50 ++++++++++++++++++++++-------------------------\n>  2 files changed, 25 insertions(+), 29 deletions(-)\n>\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> index 890ee60..d558dd0 100644\n> --- a/t/lib-git-p4.sh\n> +++ b/t/lib-git-p4.sh\n> @@ -116,8 +116,8 @@ marshal_dump() {\n>  client_view() {\n>  \t(\n>  \t\tcat <<-EOF &&\n> -\t\tClient: client\n> -\t\tDescription: client\n> +\t\tClient: $P4CLIENT\n> +\t\tDescription: $P4CLIENT\n>  \t\tRoot: $cli\n>  \t\tView:\n>  \t\tEOF\n> diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\n> index fa40cc8..37ca30a 100755\n> --- a/t/t9806-git-p4-options.sh\n> +++ b/t/t9806-git-p4-options.sh\n> @@ -126,37 +126,33 @@ test_expect_success 'clone --use-client-spec' '\n>  \t\texec >/dev/null &&\n>  \t\ttest_must_fail git p4 clone --dest=\"$git\" --use-client-spec\n>  \t) &&\n> -\tcli2=$(test-path-utils real_path \"$TRASH_DIRECTORY/cli2\") &&\n> +\t# build a different client\n> +\tcli2=\"$TRASH_DIRECTORY/cli2\" &&\n>  \tmkdir -p \"$cli2\" &&\n>  \ttest_when_finished \"rmdir \\\"$cli2\\\"\" &&\n>  \ttest_when_finished cleanup_git &&\n> ...\n> -\t# same thing again, this time with variable instead of option\n>  \t(\n> ...\n> +\t\t# group P4CLIENT and cli changes in a sub-shell\n> +\t\tP4CLIENT=client2 &&\n> +\t\tcli=\"$cli2\" &&\n> +\t\tclient_view \"//depot/sub/... //client2/bus/...\" &&\n> +\t\tgit p4 clone --dest=\"$git\" --use-client-spec //depot/... &&\n> +\t\t(\n> +\t\t\tcd \"$git\" &&\n> +\t\t\ttest_path_is_file bus/dir/f4 &&\n> +\t\t\ttest_path_is_missing file1\n> +\t\t) &&\n> +\t\tcleanup_git &&\n\nHmm, the use of \"test-path-utils real_path\" to form cli2 in the\noriginal was not necessary at all?\n\n> +\t\t# same thing again, this time with variable instead of option\n> +\t\t(\n> +\t\t\tcd \"$git\" &&\n> +\t\t\tgit init &&\n> +\t\t\tgit config git-p4.useClientSpec true &&\n> +\t\t\tgit p4 sync //depot/... &&\n> +\t\t\tgit checkout -b master p4/master &&\n> +\t\t\ttest_path_is_file bus/dir/f4 &&\n> +\t\t\ttest_path_is_missing file1\n> +\t\t)\n\nDo you need a separate sub-shell inside a sub-shell we are already\nin that you called client_view in?\n\n>  \t)\n>  '\n"},{"id":"200122","messageId":"7vr4pmkld7.fsf@alter.siamese.dyndns.org","threadId":"31671","inReplyTo":"1348833865-6093-1-git-send-email-pw@padd.com","subject":"Re: [PATCH 00/21] git p4: work on cygwin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-28T19:17:40Z","receivedAt":"2012-09-28T19:17:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff <pw@padd.com> writes:\n\n> This series fixes problems in git-p4, and its tests, so that\n> git-p4 works on the cygwin platform.\n>\n> See the wiki for info on how to get started on cygwin:\n>\n>     https://git.wiki.kernel.org/index.php/GitP4\n>\n> Testing by people who use cygwin would be appreciated.  It would\n> be good to support cygwin more regularly.  Anyone who had time\n> to contribute to testing on cygwin, and reporting problems, would\n> be welcome.\n>\n> There's more work requried to support msysgit.  Those patches\n> are not in good enough shape to ship out yet, but a lot of what\n> is in this series is required for msysgit too.\n>\n> These patches:\n>\n>     - fix bugs in git-p4 related to issues found on cygwin\n>     - cleanup some ugly code in git-p4 observed in error paths while\n>       getting tests to work on cygwin\n>     - simplify and refactor code and tests to make cygwin changes easier\n>     - handle newline and path issues for cygwin platform\n>     - speed up some aspects of git-p4 by removing extra shell invocations\n>\n> Pete Wyckoff (21):\n>   git p4: temp branch name should use / even on windows\n>   git p4: remove unused imports\n>   git p4: generate better error message for bad depot path\n>   git p4: fix error message when \"describe -s\" fails\n>   git p4 test: use client_view to build the initial client\n>   git p4 test: use client_view in t9806\n>   git p4 test: start p4d inside its db dir\n>   git p4 test: translate windows paths for cygwin\n>   git p4: remove unreachable windows \\r\\n conversion code\n>   git p4: scrub crlf for utf16 files on windows\n>   git p4 test: newline handling\n>   git p4 test: use LineEnd unix in windows tests too\n>   git p4 test: avoid wildcard * in windows\n>   git p4: cygwin p4 client does not mark read-only\n>   git p4 test: disable chmod test for cygwin\n>   git p4: disable read-only attribute before deleting\n>   git p4: avoid shell when mapping users\n>   git p4: avoid shell when invoking git rev-list\n>   git p4: avoid shell when invoking git config --get-all\n>   git p4: avoid shell when calling git config\n>   git p4: introduce gitConfigBool\n\nVery nicely done.  I was impressed how easy to understand what the\nproblem each patch attempts to solve is and how it should be solved\nonly from the description and the changes apparently matched the\ndescription.\n\nI wish patches from everybody looked like these.\n"},{"id":"200129","messageId":"5065FB90.2070602@kdbg.org","threadId":"31671","inReplyTo":"1348833865-6093-16-git-send-email-pw@padd.com","subject":"Re: [PATCH 15/21] git p4 test: disable chmod test for cygwin","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-09-28T19:33:36Z","receivedAt":"2012-09-28T19:33:36Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 28.09.2012 14:04, schrieb Pete Wyckoff:\n> It does not notice chmod +x or -x; there is nothing\n> for this test to do.\n> \n> Signed-off-by: Pete Wyckoff <pw@padd.com>\n> ---\n>  t/t9815-git-p4-submit-fail.sh | 4 +++-\n>  1 file changed, 3 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh\n> index d2b7b3d..2db1bf1 100755\n> --- a/t/t9815-git-p4-submit-fail.sh\n> +++ b/t/t9815-git-p4-submit-fail.sh\n> @@ -400,7 +400,9 @@ test_expect_success 'cleanup rename after submit cancel' '\n>  \t)\n>  '\n>  \n> -test_expect_success 'cleanup chmod after submit cancel' '\n> +# chmods are not recognized in cygwin; git has nothing\n> +# to commit\n> +test_expect_success NOT_CYGWIN 'cleanup chmod after submit cancel' '\n>  \ttest_when_finished cleanup_git &&\n>  \tgit p4 clone --dest=\"$git\" //depot &&\n>  \t(\n> \n\nIn the git part, you could use test_chmod to change the executable bit.\nBut if you cannot test it in the p4 part later on, it is probably not\nworth it.\n\n-- Hannes\n"},{"id":"207903","messageId":"20130127015135.GA29157@padd.com","threadId":"31671","inReplyTo":"7v4nmiklbt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 06/21] git p4 test: use client_view in t9806","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-27T01:51:35Z","receivedAt":"2013-01-27T01:51:35Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Yes, this really is four months later.  Somehow I forgot all\nabout this series.\n\ngitster@pobox.com wrote on Fri, 28 Sep 2012 12:11 -0700:\n> Pete Wyckoff <pw@padd.com> writes:\n> \n> > Use the standard client_view function from lib-git-p4.sh\n> > instead of building one by hand.  This requires a bit of\n> > rework, using the current value of $P4CLIENT for the client\n> > name.  It also reorganizes the test to isolate changes to\n> > $P4CLIENT and $cli in a subshell.\n> >\n> > Signed-off-by: Pete Wyckoff <pw@padd.com>\n> > ---\n> >  t/lib-git-p4.sh           |  4 ++--\n> >  t/t9806-git-p4-options.sh | 50 ++++++++++++++++++++++-------------------------\n> >  2 files changed, 25 insertions(+), 29 deletions(-)\n> >\n> > diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> > index 890ee60..d558dd0 100644\n> > --- a/t/lib-git-p4.sh\n> > +++ b/t/lib-git-p4.sh\n> > @@ -116,8 +116,8 @@ marshal_dump() {\n> >  client_view() {\n> >  \t(\n> >  \t\tcat <<-EOF &&\n> > -\t\tClient: client\n> > -\t\tDescription: client\n> > +\t\tClient: $P4CLIENT\n> > +\t\tDescription: $P4CLIENT\n> >  \t\tRoot: $cli\n> >  \t\tView:\n> >  \t\tEOF\n> > diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\n> > index fa40cc8..37ca30a 100755\n> > --- a/t/t9806-git-p4-options.sh\n> > +++ b/t/t9806-git-p4-options.sh\n> > @@ -126,37 +126,33 @@ test_expect_success 'clone --use-client-spec' '\n> >  \t\texec >/dev/null &&\n> >  \t\ttest_must_fail git p4 clone --dest=\"$git\" --use-client-spec\n> >  \t) &&\n> > -\tcli2=$(test-path-utils real_path \"$TRASH_DIRECTORY/cli2\") &&\n> > +\t# build a different client\n> > +\tcli2=\"$TRASH_DIRECTORY/cli2\" &&\n> >  \tmkdir -p \"$cli2\" &&\n> >  \ttest_when_finished \"rmdir \\\"$cli2\\\"\" &&\n> >  \ttest_when_finished cleanup_git &&\n> > ...\n> > -\t# same thing again, this time with variable instead of option\n> >  \t(\n> > ...\n> > +\t\t# group P4CLIENT and cli changes in a sub-shell\n> > +\t\tP4CLIENT=client2 &&\n> > +\t\tcli=\"$cli2\" &&\n> > +\t\tclient_view \"//depot/sub/... //client2/bus/...\" &&\n> > +\t\tgit p4 clone --dest=\"$git\" --use-client-spec //depot/... &&\n> > +\t\t(\n> > +\t\t\tcd \"$git\" &&\n> > +\t\t\ttest_path_is_file bus/dir/f4 &&\n> > +\t\t\ttest_path_is_missing file1\n> > +\t\t) &&\n> > +\t\tcleanup_git &&\n> \n> Hmm, the use of \"test-path-utils real_path\" to form cli2 in the\n> original was not necessary at all?\n\nThanks, I will make this removal more explicit, putting it in\nwith 8/21 where it belongs, with explanation.\n\n> > +\t\t# same thing again, this time with variable instead of option\n> > +\t\t(\n> > +\t\t\tcd \"$git\" &&\n> > +\t\t\tgit init &&\n> > +\t\t\tgit config git-p4.useClientSpec true &&\n> > +\t\t\tgit p4 sync //depot/... &&\n> > +\t\t\tgit checkout -b master p4/master &&\n> > +\t\t\ttest_path_is_file bus/dir/f4 &&\n> > +\t\t\ttest_path_is_missing file1\n> > +\t\t)\n> \n> Do you need a separate sub-shell inside a sub-shell we are already\n> in that you called client_view in?\n> \n> >  \t)\n> >  '\n\nThe first subshell is to hide P4CLIENT and cli variable changes\nfrom the rest of the tests.\n\nThe second is to keep the \"cd $git\" from changing behavior of the\nfollowing \"cleanup_git\" call.  That does \"rm -rf $git\" which\nwould fail on some file systems if cwd is still in there.  With\njust one subshell it would look like:\n\n\t(\n\t\tP4CLIENT=client2 &&\n\t\tgit p4 clone .. &&\n\t\tcd \"$git\" &&\n\t\t... do test\n\t\tcd \"$TRASH_DIRECTORY\" &&\n\t\tcleanup_git &&\n\n\t\tcd \"$git\" &&\n\t\t... more test\n\t)\n\nIt's a bit easier to understand with an extra level of shell,\nand sticks to the pattern used in the rest of the t98*.\n\n\t\t-- Pete\n"}]}