{"thread":{"id":"26557","subject":"[PATCH v2 1/8] git-p4: test script","startedAt":"2011-02-19T13:17:53Z","lastAt":"2011-02-27T21:16:22Z","messageCount":16,"participants":["Pete Wyckoff","Junio C Hamano","Tor Arvid Lund"],"isPatch":true,"patchVersion":2,"patchTotal":8},"messages":[{"id":"161611","messageId":"1298121481-7005-1-git-send-email-pw@padd.com","threadId":"26557","inReplyTo":null,"subject":"[PATCH v2 0/8] git-p4 fixes and enhancements","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-19T13:17:53Z","receivedAt":"2011-02-19T13:17:53Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"This is the second submission of git-p4 fixes and enhancements.\nThe first of these 8 patches adds a test script. The rest\nfix problems with git-p4 and add small featurest-p4: test script\n\n\tgit-p4: fix key error for p4 problem\n\tgit-p4: add missing newline in initial import message\n\tgit-p4: accommodate new move/delete type in p4\n\tgit-p4: reinterpret confusing p4 message\n\tgit-p4: better message for \"git-p4 sync\" when not cloned\n\tgit-p4: decode p4 wildcard characters\n\tgit-p4: support clone --bare\n\n contrib/fast-import/git-p4 | 60 ++++++++++++++++++++++-----\n t/t9800-git-p4.sh | 100 +++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 149 insertions(+), 11 deletions(-)\n\nChanges from v1.\n\n Includes acks from Tor Arvid for 3, 4, 6, 8.\n Includes review changes from Junio on 1, and from Tor Arvid\n on 7.\n\n1/8: test script\n\n Review comments from Junio.\n\n 1. Beautify icky p4 and p4d existence check.\n\n 2. Remove p4d initialization sleep. It binds and listens\n\tto the port nicely before daemonizing.\n\n\n7/8: decode p4 wildcard characters\n\n Tor Arvid points out that windows cannot have * in filenames.\n This is not something I can test, but at least avoid the situation\n on that platform.\n"},{"id":"161607","messageId":"1298121481-7005-2-git-send-email-pw@padd.com","threadId":"26557","inReplyTo":"1298121481-7005-1-git-send-email-pw@padd.com","subject":"[PATCH v2 1/8] git-p4: test script","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-19T13:17:54Z","receivedAt":"2011-02-19T13:17:54Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Signed-off-by: Pete Wyckoff \n---\n t/t9800-git-p4.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 55 insertions(+), 0 deletions(-)\n create mode 100755 t/t9800-git-p4.sh\n\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nnew file mode 100755\nindex 0000000..2d354f8\n--- /dev/null\n+++ b/t/t9800-git-p4.sh\n@@ -0,0 +1,55 @@\n+#!/bin/sh\n+\n+test_description='git-p4 tests'\n+\n+. ./test-lib.sh\n+\n+( p4 -h && p4d -h ) >/dev/null 2>&1 || {\n+\tskip_all='skipping git-p4 tests; no p4 or p4d'\n+\ttest_done\n+}\n+\n+GITP4=$GIT_BUILD_DIR/contrib/fast-import/git-p4\n+P4DPORT=10669\n+\n+db=\"$TRASH_DIRECTORY/db\"\n+cli=\"$TRASH_DIRECTORY/cli\"\n+git=\"$TRASH_DIRECTORY/git\"\n+\n+test_debug 'echo p4d -q -d -r \"$db\" -p $P4DPORT'\n+test_expect_success setup '\n+\tmkdir -p \"$db\" &&\n+\tp4d -q -d -r \"$db\" -p $P4DPORT &&\n+\tmkdir -p \"$cli\" &&\n+\tmkdir -p \"$git\" &&\n+\texport P4PORT=localhost:$P4DPORT\n+'\n+\n+test_expect_success 'add p4 files' '\n+\tcd \"$cli\" &&\n+\tp4 client -i <<-EOF &&\n+\tClient: client\n+\tDescription: client\n+\tRoot: $cli\n+\tView: //depot/... //client/...\n+\tEOF\n+\texport P4CLIENT=client &&\n+\techo file1 >file1 &&\n+\tp4 add file1 &&\n+\tp4 submit -d \"file1\" &&\n+\tcd \"$TRASH_DIRECTORY\"\n+'\n+\n+test_expect_success 'basic git-p4 clone' '\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\trm -rf \"$git\" && mkdir \"$git\"\n+'\n+\n+test_expect_success 'shutdown' '\n+\tpid=`pgrep -f p4d` &&\n+\ttest -n \"$pid\" &&\n+\ttest_debug \"ps wl `echo $pid`\" &&\n+\tkill $pid\n+'\n+\n+test_done\n-- \n1.7.4.1\n"},{"id":"161609","messageId":"1298121481-7005-3-git-send-email-pw@padd.com","threadId":"26557","inReplyTo":"1298121481-7005-1-git-send-email-pw@padd.com","subject":"[PATCH v2 2/8] git-p4: fix key error for p4 problem","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-19T13:17:55Z","receivedAt":"2011-02-19T13:17:55Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Some p4 failures result in an error, but the info['code'] is not\nset. These include a bad p4 executable, or a core dump from p4,\nand other odd internal errors where p4 fails to generate proper\nmarshaled output.\n\nMake sure the info key exists before using it to avoid a python\ntraceback.\n\nSigned-off-by: Pete Wyckoff \n---\n contrib/fast-import/git-p4 | 5 ++++-\n t/t9800-git-p4.sh | 13 +++++++++++++\n 2 files changed, 17 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 04ce7e3..2fefea4 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1440,10 +1440,13 @@ class P4Sync(Command):\n % (p, revision)\n for p in self.depotPaths])):\n \n- if info['code'] == 'error':\n+ if 'code' in info and info['code'] == 'error':\n sys.stderr.write(\"p4 returned an error: %s\\n\"\n % info['data'])\n sys.exit(1)\n+ if 'p4ExitCode' in info:\n+ sys.stderr.write(\"p4 exitcode: %s\\n\" % info['p4ExitCode'])\n+ sys.exit(1)\n \n \n change = int(info[\"change\"])\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex 2d354f8..c1ea4d4 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -45,6 +45,19 @@ test_expect_success 'basic git-p4 clone' '\n \trm -rf \"$git\" && mkdir \"$git\"\n '\n \n+test_expect_success 'exit when p4 fails to produce marshaled output' '\n+\tbadp4dir=\"$TRASH_DIRECTORY/badp4dir\" &&\n+\tmkdir -p \"$badp4dir\" &&\n+\tcat >\"$badp4dir\"/p4 <<-EOF &&\n+\t#!$SHELL_PATH\n+\texit 1\n+\tEOF\n+\tchmod 755 \"$badp4dir\"/p4 &&\n+\tPATH=\"$badp4dir:$PATH\" \"$GITP4\" clone --dest=\"$git\" //depot >errs 2>&1 ; retval=$? &&\n+\ttest $retval -eq 1 &&\n+\ttest_must_fail grep -q Traceback errs\n+'\n+\n test_expect_success 'shutdown' '\n \tpid=`pgrep -f p4d` &&\n \ttest -n \"$pid\" &&\n-- \n1.7.4.1\n"},{"id":"161608","messageId":"1298121481-7005-4-git-send-email-pw@padd.com","threadId":"26557","inReplyTo":"1298121481-7005-1-git-send-email-pw@padd.com","subject":"[PATCH v2 3/8] git-p4: add missing newline in initial import message","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-19T13:17:56Z","receivedAt":"2011-02-19T13:17:56Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Signed-off-by: Pete Wyckoff \nAcked-By: Tor Arvid Lund \n---\n contrib/fast-import/git-p4 | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 2fefea4..d2ba215 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1429,7 +1429,7 @@ class P4Sync(Command):\n print \"Doing initial import of %s from revision %s into %s\" % (' '.join(self.depotPaths), revision, self.branch)\n \n details = { \"user\" : \"git perforce import user\", \"time\" : int(time.time()) }\n- details[\"desc\"] = (\"Initial import of %s from the state at revision %s\"\n+ details[\"desc\"] = (\"Initial import of %s from the state at revision %s\\n\"\n % (' '.join(self.depotPaths), revision))\n details[\"change\"] = revision\n newestRevision = 0\n-- \n1.7.4.1\n"},{"id":"161610","messageId":"1298121481-7005-5-git-send-email-pw@padd.com","threadId":"26557","inReplyTo":"1298121481-7005-1-git-send-email-pw@padd.com","subject":"[PATCH v2 4/8] git-p4: accommodate new move/delete type in p4","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-19T13:17:57Z","receivedAt":"2011-02-19T13:17:57Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Change 562d53f (2010-11-21) recognized the new move/delete type\nfor git-p4 sync, but it can also show up in an initial clone and\nlabels output. Instead of replicating this in three places,\nhoist the definition somewhere global.\n\nSigned-off-by: Pete Wyckoff \nAcked-By: Tor Arvid Lund \n---\n contrib/fast-import/git-p4 | 12 +++++++-----\n 1 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex d2ba215..db19b17 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -834,6 +834,8 @@ class P4Submit(Command):\n return True\n \n class P4Sync(Command):\n+ delete_actions = ( \"delete\", \"move/delete\", \"purge\" )\n+\n def __init__(self):\n Command.__init__(self)\n self.options = [\n@@ -1038,10 +1040,10 @@ class P4Sync(Command):\n \n if includeFile:\n filesForCommit.append(f)\n- if f['action'] not in ('delete', 'move/delete', 'purge'):\n- filesToRead.append(f)\n- else:\n+ if f['action'] in self.delete_actions:\n filesToDelete.append(f)\n+ else:\n+ filesToRead.append(f)\n \n # deleted files...\n for f in filesToDelete:\n@@ -1127,7 +1129,7 @@ class P4Sync(Command):\n \n cleanedFiles = {}\n for info in files:\n- if info[\"action\"] in (\"delete\", \"purge\"):\n+ if info[\"action\"] in self.delete_actions:\n continue\n cleanedFiles[info[\"depotFile\"]] = info[\"rev\"]\n \n@@ -1453,7 +1455,7 @@ class P4Sync(Command):\n if change > newestRevision:\n newestRevision = change\n \n- if info[\"action\"] in (\"delete\", \"purge\"):\n+ if info[\"action\"] in self.delete_actions:\n # don't increase the file cnt, otherwise details[\"depotFile123\"] will have gaps!\n #fileCnt = fileCnt + 1\n continue\n-- \n1.7.4.1\n"},{"id":"161614","messageId":"1298121481-7005-6-git-send-email-pw@padd.com","threadId":"26557","inReplyTo":"1298121481-7005-1-git-send-email-pw@padd.com","subject":"[PATCH v2 5/8] git-p4: reinterpret confusing p4 message","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-19T13:17:58Z","receivedAt":"2011-02-19T13:17:58Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Error output will look like this:\n\nglom$ git p4 clone //deopt\nImporting from //deopt into .\nReinitialized existing Git repository in /tmp/x/.git/\nDoing initial import of //deopt from revision #head into refs/remotes/p4/master\np4 returned an error: //deopt/... - must refer to client glom.\n\nThis particular p4 error is misleading.\nPerhaps the depot path was misspelled.\nDepot path: //deopt\n\nSigned-off-by: Pete Wyckoff \n---\n contrib/fast-import/git-p4 | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex db19b17..6b847c4 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1445,6 +1445,10 @@ class P4Sync(Command):\n if 'code' in info and info['code'] == 'error':\n sys.stderr.write(\"p4 returned an error: %s\\n\"\n % info['data'])\n+ if info['data'].find(\"must refer to client\") >= 0:\n+ sys.stderr.write(\"This particular p4 error is misleading.\\n\")\n+ sys.stderr.write(\"Perhaps the depot path was misspelled.\\n\");\n+ sys.stderr.write(\"Depot path: %s\\n\" % \" \".join(self.depotPaths))\n sys.exit(1)\n if 'p4ExitCode' in info:\n sys.stderr.write(\"p4 exitcode: %s\\n\" % info['p4ExitCode'])\n-- \n1.7.4.1\n"},{"id":"161615","messageId":"1298121481-7005-7-git-send-email-pw@padd.com","threadId":"26557","inReplyTo":"1298121481-7005-1-git-send-email-pw@padd.com","subject":"[PATCH v2 6/8] git-p4: better message for \"git-p4 sync\" when not cloned","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-19T13:17:59Z","receivedAt":"2011-02-19T13:17:59Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"A common error is to do \"git-p4 sync\" in a repository that\nwas not initialized by \"git-p4 clone\". There will be no\np4 refs. The error message in this case is a traceback\nfor an assertion, which is confusing.\n\nChange it instead to explain the likely problem.\n\nSigned-off-by: Pete Wyckoff \nAcked-By: Tor Arvid Lund \n---\n contrib/fast-import/git-p4 | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 6b847c4..04e6c3d 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1676,6 +1676,8 @@ class P4Sync(Command):\n \n changes.sort()\n else:\n+ if not self.p4BranchesInGit:\n+ die(\"No remote p4 branches. Perhaps you never did \\\"git p4 clone\\\" in here.\");\n if self.verbose:\n print \"Getting p4 changes for %s...%s\" % (', '.join(self.depotPaths),\n self.changeRange)\n-- \n1.7.4.1\n"},{"id":"161612","messageId":"1298121481-7005-8-git-send-email-pw@padd.com","threadId":"26557","inReplyTo":"1298121481-7005-1-git-send-email-pw@padd.com","subject":"[PATCH v2 7/8] git-p4: decode p4 wildcard characters","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-19T13:18:00Z","receivedAt":"2011-02-19T13:18:00Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"There are four wildcard characters in p4. Files with these\ncharacters can be added to p4 repos using the \"-f\" option.\nThey are stored in %xx notation, and when checked out, p4\nconverts them back to normal.\n\nThis patch does the same thing when importing into git,\nconverting the four special characters. Without this change,\nthe files appear with literal %xx in their names.\n\nBe careful not to produce \"*\" in filenames on windows. That\nwill fail.\n\nSigned-off-by: Pete Wyckoff \n---\n contrib/fast-import/git-p4 | 18 ++++++++++++++++++\n t/t9800-git-p4.sh | 22 ++++++++++++++++++++++\n 2 files changed, 40 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 04e6c3d..98597d3 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -884,6 +884,23 @@ class P4Sync(Command):\n if gitConfig(\"git-p4.syncFromOrigin\") == \"false\":\n self.syncWithOrigin = False\n \n+ #\n+ # P4 wildcards are not allowed in filenames. P4 complains\n+ # if you simply add them, but you can force it with \"-f\", in\n+ # which case it translates them into %xx encoding internally.\n+ # Search for and fix just these four characters. Do % last so\n+ # that fixing it does not inadvertently create new %-escapes.\n+ #\n+ def wildcard_decode(self, path):\n+ # Cannot have * in a filename in windows; untested as to\n+ # what p4 would do in such a case.\n+ if not self.isWindows:\n+ path = path.replace(\"%2A\", \"*\")\n+ path = path.replace(\"%23\", \"#\") \\\n+ .replace(\"%40\", \"@\") \\\n+ .replace(\"%25\", \"%\")\n+ return path\n+\n def extractFilesFromCommit(self, commit):\n self.cloneExclude = [re.sub(r\"\\.\\.\\.$\", \"\", path)\n for path in self.cloneExclude]\n@@ -962,6 +979,7 @@ class P4Sync(Command):\n \t return\n \n relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n+ relPath = self.wildcard_decode(relPath)\n if verbose:\n sys.stderr.write(\"%s\\n\" % relPath)\n \ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex c1ea4d4..026277a 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -58,6 +58,28 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '\n \ttest_must_fail grep -q Traceback errs\n '\n \n+test_expect_success 'add p4 files with wildcards in the names' '\n+\tcd \"$cli\" &&\n+\techo file-wild-hash >file-wild#hash &&\n+\techo file-wild-star >file-wild\\*star &&\n+\techo file-wild-at >file-wild@at &&\n+\techo file-wild-percent >file-wild%percent &&\n+\tp4 add -f file-wild* &&\n+\tp4 submit -d \"file wildcards\" &&\n+\tcd \"$TRASH_DIRECTORY\"\n+'\n+\n+test_expect_success 'wildcard files git-p4 clone' '\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\tcd \"$git\" &&\n+\ttest -f file-wild#hash &&\n+\ttest -f file-wild\\*star &&\n+\ttest -f file-wild@at &&\n+\ttest -f file-wild%percent &&\n+\tcd \"$TRASH_DIRECTORY\" &&\n+\trm -rf \"$git\" && mkdir \"$git\"\n+'\n+\n test_expect_success 'shutdown' '\n \tpid=`pgrep -f p4d` &&\n \ttest -n \"$pid\" &&\n-- \n1.7.4.1\n"},{"id":"161613","messageId":"1298121481-7005-9-git-send-email-pw@padd.com","threadId":"26557","inReplyTo":"1298121481-7005-1-git-send-email-pw@padd.com","subject":"[PATCH v2 8/8] git-p4: support clone --bare","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-19T13:18:01Z","receivedAt":"2011-02-19T13:18:01Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Just like git clone --bare, build a .git directory but no\nchecked out files.\n\nSigned-off-by: Pete Wyckoff \nAcked-By: Tor Arvid Lund \n---\n contrib/fast-import/git-p4 | 17 +++++++++++++----\n t/t9800-git-p4.sh | 10 ++++++++++\n 2 files changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 98597d3..725af75 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1776,10 +1776,13 @@ class P4Clone(P4Sync):\n help=\"where to leave result of the clone\"),\n optparse.make_option(\"-/\", dest=\"cloneExclude\",\n action=\"append\", type=\"string\",\n- help=\"exclude depot path\")\n+ help=\"exclude depot path\"),\n+ optparse.make_option(\"--bare\", dest=\"cloneBare\",\n+ action=\"store_true\", default=False),\n ]\n self.cloneDestination = None\n self.needsGit = False\n+ self.cloneBare = False\n \n # This is required for the \"append\" cloneExclude action\n def ensure_value(self, attr, value):\n@@ -1819,11 +1822,16 @@ class P4Clone(P4Sync):\n self.cloneDestination = self.defaultDestination(args)\n \n print \"Importing from %s into %s\" % (', '.join(depotPaths), self.cloneDestination)\n+\n if not os.path.exists(self.cloneDestination):\n os.makedirs(self.cloneDestination)\n chdir(self.cloneDestination)\n- system(\"git init\")\n- self.gitdir = os.getcwd() + \"/.git\"\n+\n+ init_cmd = [ \"git\", \"init\" ]\n+ if self.cloneBare:\n+ init_cmd.append(\"--bare\")\n+ subprocess.check_call(init_cmd)\n+\n if not P4Sync.run(self, depotPaths):\n return False\n if self.branch != \"master\":\n@@ -1833,7 +1841,8 @@ class P4Clone(P4Sync):\n masterbranch = \"refs/heads/p4/master\"\n if gitBranchExists(masterbranch):\n system(\"git branch master %s\" % masterbranch)\n- system(\"git checkout -f\")\n+ if not self.cloneBare:\n+ system(\"git checkout -f\")\n else:\n print \"Could not detect main branch. No checkout/master branch created.\"\n \ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex 026277a..1969e6b 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -80,6 +80,16 @@ test_expect_success 'wildcard files git-p4 clone' '\n \trm -rf \"$git\" && mkdir \"$git\"\n '\n \n+test_expect_success 'clone bare' '\n+\t\"$GITP4\" clone --dest=\"$git\" --bare //depot &&\n+\tcd \"$git\" &&\n+\ttest ! -d .git &&\n+\tbare=`git config --get core.bare` &&\n+\ttest \"$bare\" = true &&\n+\tcd \"$TRASH_DIRECTORY\" &&\n+\trm -rf \"$git\" && mkdir \"$git\"\n+'\n+\n test_expect_success 'shutdown' '\n \tpid=`pgrep -f p4d` &&\n \ttest -n \"$pid\" &&\n-- \n1.7.4.1\n"},{"id":"161841","messageId":"7vhbbxhskd.fsf@alter.siamese.dyndns.org","threadId":"26557","inReplyTo":"1298121481-7005-5-git-send-email-pw@padd.com","subject":"Re: [PATCH v2 4/8] git-p4: accommodate new move/delete type in p4","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-21T23:32:50Z","receivedAt":"2011-02-21T23:32:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff writes:\n\n> Change 562d53f (2010-11-21) recognized the new move/delete type\n> for git-p4 sync, but it can also show up in an initial clone and\n> labels output. Instead of replicating this in three places,\n> hoist the definition somewhere global.\n\nI think the said commit is from January not November, though.\nNo need to resend---I'll fix the comment up in place.\n\nThanks.\n"},{"id":"161842","messageId":"7vbp25hsk6.fsf@alter.siamese.dyndns.org","threadId":"26557","inReplyTo":"1298121481-7005-8-git-send-email-pw@padd.com","subject":"Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-21T23:32:57Z","receivedAt":"2011-02-21T23:32:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff writes:\n\n> There are four wildcard characters in p4. Files with these\n> characters can be added to p4 repos using the \"-f\" option.\n> They are stored in %xx notation, and when checked out, p4\n> converts them back to normal.\n>\n> This patch does the same thing when importing into git,\n> converting the four special characters. Without this change,\n> the files appear with literal %xx in their names.\n>\n> Be careful not to produce \"*\" in filenames on windows. That\n> will fail.\n\n> + # P4 wildcards are not allowed in filenames. P4 complains\n> + # if you simply add them, but you can force it with \"-f\", in\n> + # which case it translates them into %xx encoding internally.\n> + # Search for and fix just these four characters. Do % last so\n> + # that fixing it does not inadvertently create new %-escapes.\n> + #\n> + def wildcard_decode(self, path):\n> + # Cannot have * in a filename in windows; untested as to\n> + # what p4 would do in such a case.\n> + if not self.isWindows:\n> + path = path.replace(\"%2A\", \"*\")\n\nI'll queue the patch as-is, but perhaps we can ask for help from people\nwho have access to P4 on both non-Windows and Windows to run a small test\nto determine what happens in the native client?\n\n 1. On a non-Windows client, add a path with '*' in it to the depot;\n perhaps \"p4 add\" might fail at this point, in which case we don't\n need to worry about this issue at all.\n \n 2. Create a p4 client on Windows against that depot, and sync it; unless\n the previous step failed, we will see what happens (I would imagine it\n either dies or mangles the pathname and warns), so that we have\n something to emulate.\n\nand then the quoted part can be further refined in a separate patch later.\n\nThanks.\n"},{"id":"162009","messageId":"AANLkTinbmqMfOTAqdG9Mdecpody2uy3wZOdaWbv25ph2@mail.gmail.com","threadId":"26557","inReplyTo":"1298121481-7005-6-git-send-email-pw@padd.com","subject":"Re: [PATCH v2 5/8] git-p4: reinterpret confusing p4 message","fromName":"Tor Arvid Lund","fromEmail":"torarvid@gmail.com","sentAt":"2011-02-23T08:26:34Z","receivedAt":"2011-02-23T08:26:34Z","isPatch":true,"sender":{"key":"torarvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/439758?v=4"},"body":"On Sat, Feb 19, 2011 at 2:17 PM, Pete Wyckoff wrote:\n> Error output will look like this:\n>\n> glom$ git p4 clone //deopt\n> Importing from //deopt into .\n> Reinitialized existing Git repository in /tmp/x/.git/\n> Doing initial import of //deopt from revision #head into refs/remotes/p4/master\n> p4 returned an error: //deopt/... - must refer to client glom.\n>\n> This particular p4 error is misleading.\n> Perhaps the depot path was misspelled.\n> Depot path:  //deopt\n>\n> Signed-off-by: Pete Wyckoff \n\nAcked-by: Tor Arvid Lund \n\n:-)\n\n> ---\n>  contrib/fast-import/git-p4 |    4 ++++\n>  1 files changed, 4 insertions(+), 0 deletions(-)\n>\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> index db19b17..6b847c4 100755\n> --- a/contrib/fast-import/git-p4\n> +++ b/contrib/fast-import/git-p4\n> @@ -1445,6 +1445,10 @@ class P4Sync(Command):\n>             if 'code' in info and info['code'] == 'error':\n>                 sys.stderr.write(\"p4 returned an error: %s\\n\"\n>                                  % info['data'])\n> +                if info['data'].find(\"must refer to client\") >= 0:\n> +                    sys.stderr.write(\"This particular p4 error is misleading.\\n\")\n> +                    sys.stderr.write(\"Perhaps the depot path was misspelled.\\n\");\n> +                    sys.stderr.write(\"Depot path:  %s\\n\" % \" \".join(self.depotPaths))\n>                 sys.exit(1)\n>             if 'p4ExitCode' in info:\n>                 sys.stderr.write(\"p4 exitcode: %s\\n\" % info['p4ExitCode'])\n> --\n> 1.7.4.1\n>\n>\n"},{"id":"162132","messageId":"20110224121210.GA16475@arf.padd.com","threadId":"26557","inReplyTo":"7vbp25hsk6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-24T12:12:10Z","receivedAt":"2011-02-24T12:12:10Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"gitster@pobox.com wrote on Mon, 21 Feb 2011 15:32 -0800:\n> Pete Wyckoff writes:\n> \n> > There are four wildcard characters in p4. Files with these\n> > characters can be added to p4 repos using the \"-f\" option.\n> > They are stored in %xx notation, and when checked out, p4\n> > converts them back to normal.\n> >\n> > This patch does the same thing when importing into git,\n> > converting the four special characters. Without this change,\n> > the files appear with literal %xx in their names.\n> >\n> > Be careful not to produce \"*\" in filenames on windows. That\n> > will fail.\n> \n> > + # P4 wildcards are not allowed in filenames. P4 complains\n> > + # if you simply add them, but you can force it with \"-f\", in\n> > + # which case it translates them into %xx encoding internally.\n> > + # Search for and fix just these four characters. Do % last so\n> > + # that fixing it does not inadvertently create new %-escapes.\n> > + #\n> > + def wildcard_decode(self, path):\n> > + # Cannot have * in a filename in windows; untested as to\n> > + # what p4 would do in such a case.\n> > + if not self.isWindows:\n> > + path = path.replace(\"%2A\", \"*\")\n> \n> I'll queue the patch as-is, but perhaps we can ask for help from people\n> who have access to P4 on both non-Windows and Windows to run a small test\n> to determine what happens in the native client?\n> \n> 1. On a non-Windows client, add a path with '*' in it to the depot;\n> perhaps \"p4 add\" might fail at this point, in which case we don't\n> need to worry about this issue at all.\n> \n> 2. Create a p4 client on Windows against that depot, and sync it; unless\n> the previous step failed, we will see what happens (I would imagine it\n> either dies or mangles the pathname and warns), so that we have\n> something to emulate.\n> \n> and then the quoted part can be further refined in a separate patch later.\n\nI tried this myself in a VM when Tor Arvid pointed out the\nproblem with Windows:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/166374\n\n1. \"*\" is acceptable in filenames, but users must use \"p4 add -f\"\n to indicate that they really want that wildcard character in\n the filename.\n\n2. Windows clients fail to create the file in \"p4 sync\". The\n error is:\n\n open for write: c:\\Documents and Settings\\Administrator\\Desktop\\file*star:\n The filename, directory name, or volume label syntax is incorrect.\n\nThe behavior for git-p4 I chose is to sync the file but leave\nthe name with its encoded %2A. If we think it is better to\nduplicate p4's failure, we can simply try to create the file\nand let the OS produce the same error message.\n\n\t\t-- Pete\n"},{"id":"162135","messageId":"AANLkTik=KWwszN9ETOh11btPvcPXCcoA=evT4Fgxt-Fm@mail.gmail.com","threadId":"26557","inReplyTo":"20110224121210.GA16475@arf.padd.com","subject":"Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters","fromName":"Tor Arvid Lund","fromEmail":"torarvid@gmail.com","sentAt":"2011-02-24T13:54:56Z","receivedAt":"2011-02-24T13:54:56Z","isPatch":true,"sender":{"key":"torarvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/439758?v=4"},"body":"On Thu, Feb 24, 2011 at 1:12 PM, Pete Wyckoff wrote:\n> gitster@pobox.com wrote on Mon, 21 Feb 2011 15:32 -0800:\n>> Pete Wyckoff writes:\n>>\n>> > There are four wildcard characters in p4.  Files with these\n>> > characters can be added to p4 repos using the \"-f\" option.\n>> > They are stored in %xx notation, and when checked out, p4\n>> > converts them back to normal.\n>> >\n>> > This patch does the same thing when importing into git,\n>> > converting the four special characters.  Without this change,\n>> > the files appear with literal %xx in their names.\n>> >\n>> > Be careful not to produce \"*\" in filenames on windows.  That\n>> > will fail.\n>>\n>> > +    # P4 wildcards are not allowed in filenames.  P4 complains\n>> > +    # if you simply add them, but you can force it with \"-f\", in\n>> > +    # which case it translates them into %xx encoding internally.\n>> > +    # Search for and fix just these four characters.  Do % last so\n>> > +    # that fixing it does not inadvertently create new %-escapes.\n>> > +    #\n>> > +    def wildcard_decode(self, path):\n>> > +        # Cannot have * in a filename in windows; untested as to\n>> > +        # what p4 would do in such a case.\n>> > +        if not self.isWindows:\n>> > +            path = path.replace(\"%2A\", \"*\")\n>>\n>> I'll queue the patch as-is, but perhaps we can ask for help from people\n>> who have access to P4 on both non-Windows and Windows to run a small test\n>> to determine what happens in the native client?\n>>\n>>  1. On a non-Windows client, add a path with '*' in it to the depot;\n>>     perhaps \"p4 add\" might fail at this point, in which case we don't\n>>     need to worry about this issue at all.\n>>\n>>  2. Create a p4 client on Windows against that depot, and sync it; unless\n>>     the previous step failed, we will see what happens (I would imagine it\n>>     either dies or mangles the pathname and warns), so that we have\n>>     something to emulate.\n>>\n>> and then the quoted part can be further refined in a separate patch later.\n>\n> I tried this myself in a VM when Tor Arvid pointed out the\n> problem with Windows:\n>\n> http://article.gmane.org/gmane.comp.version-control.git/166374\n>\n> 1.  \"*\" is acceptable in filenames, but users must use \"p4 add -f\"\n>    to indicate that they really want that wildcard character in\n>    the filename.\n>\n> 2.  Windows clients fail to create the file in \"p4 sync\".  The\n>    error is:\n>\n>    open for write: c:\\Documents and Settings\\Administrator\\Desktop\\file*star:\n>    The filename, directory name, or volume label syntax is incorrect.\n>\n> The behavior for git-p4 I chose is to sync the file but leave\n> the name with its encoded %2A.  If we think it is better to\n> duplicate p4's failure, we can simply try to create the file\n> and let the OS produce the same error message.\n\nYeah, I was thinking... what happens now if we do:\n\n1) Create \"my*file\" in linux, and submit.\n2) git-p4 sync from windows, and get my%2Afile on windows.\n3) modify my%2Afile and do git commit.\n4) git-p4 submit\n\nI haven't had time to test right now, but maybe p4 will not recognise\nmy%2Afile (or try to check it in as my%252Afile (replacing the '%'\ncharacter) or something like that? (Or maybe I just haven't had enough\ncoffee today :-/ )\n\n -- Tor Arvid\n"},{"id":"162166","messageId":"7voc612xns.fsf@alter.siamese.dyndns.org","threadId":"26557","inReplyTo":"AANLkTik=KWwszN9ETOh11btPvcPXCcoA=evT4Fgxt-Fm@mail.gmail.com","subject":"Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-24T16:40:55Z","receivedAt":"2011-02-24T16:40:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tor Arvid Lund writes:\n\n> On Thu, Feb 24, 2011 at 1:12 PM, Pete Wyckoff wrote:\n> ...\n> Yeah, I was thinking... what happens now if we do:\n>\n> 1) Create \"my*file\" in linux, and submit.\n> 2) git-p4 sync from windows, and get my%2Afile on windows.\n> 3) modify my%2Afile and do git commit.\n> 4) git-p4 submit\n>\n> I haven't had time to test right now, but maybe p4 will not recognise\n> my%2Afile (or try to check it in as my%252Afile (replacing the '%'\n> character) or something like that? (Or maybe I just haven't had enough\n> coffee today :-/ )\n\nThis shares the same issue as \"checking files out on case insensitive\nfilesystems\" topic in the other thread. \"my*file\" may not be usable by\nthe project when renamed to \"my%2Afile\", so \"git-p4 sync\" may want to warn\nthe user about the path when this happens.\n\nAnd you need to reverse this quoting upon \"git-p4 submit\". Does that\nhappen already?\n"},{"id":"162383","messageId":"20110227211622.GA15075@arf.padd.com","threadId":"26557","inReplyTo":"7voc612xns.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-02-27T21:16:22Z","receivedAt":"2011-02-27T21:16:22Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"gitster@pobox.com wrote on Thu, 24 Feb 2011 08:40 -0800:\n> Tor Arvid Lund writes:\n> \n> > On Thu, Feb 24, 2011 at 1:12 PM, Pete Wyckoff wrote:\n> > ...\n> > Yeah, I was thinking... what happens now if we do:\n> >\n> > 1) Create \"my*file\" in linux, and submit.\n> > 2) git-p4 sync from windows, and get my%2Afile on windows.\n> > 3) modify my%2Afile and do git commit.\n> > 4) git-p4 submit\n> >\n> > I haven't had time to test right now, but maybe p4 will not recognise\n> > my%2Afile (or try to check it in as my%252Afile (replacing the '%'\n> > character) or something like that? (Or maybe I just haven't had enough\n> > coffee today :-/ )\n> \n> This shares the same issue as \"checking files out on case insensitive\n> filesystems\" topic in the other thread. \"my*file\" may not be usable by\n> the project when renamed to \"my%2Afile\", so \"git-p4 sync\" may want to warn\n> the user about the path when this happens.\n> \n> And you need to reverse this quoting upon \"git-p4 submit\". Does that\n> happen already?\n\nI have not found any testers to try these things on windows.\n\nWe have a separate bug in git-p4 for submitting files with\nwildcards, windows or unix. This could be fixed anytime in\na separate patch; any takers?\n\nAnother unrelated p4-linux-only bug is this:\n\n arf$ echo hello > my%file\n arf$ p4 add -f my%file\n //depot/my%25file#1 - opened for add\n arf$ p4 submit -d 'add my file'\n Submitting change 1.\n Locking 1 files ...\n add //depot/my%25file#1\n Change 1 submitted.\n arf$ p4 sync\n File(s) up-to-date.\n arf$ p4 open my%file\n my%file - file(s) not on client.\n arf$ p4 open my%25file\n //depot/my%25file#1 - opened for edit\n\nI can create and add my%file, but cannot edit or delete it. The\n\"-f\" option does not help. It can be edited/deleted using the %25\nexpansion. I found this same problem on Windows.\n\nConclusion: p4 is buggy and incomplete with respect to wildcard\ncharacters in filenames already. This particular change does not make\nanything worse, and fixes a problem seen in the wild for a filename with\n\"@\". I'd like to hope p4 gets fixed at which point this % issue goes\naway, and we can decide what to do with * on windows, following their\nlead.\n\n\t\t-- Pete\n"}]}