{"thread":{"id":"11848","subject":"[PATCH 1/3] git-p4: Fix an obvious typo","startedAt":"2008-02-03T09:21:04Z","lastAt":"2008-02-03T18:55:28Z","messageCount":5,"participants":["Tommy Thorn","Simon Hausmann"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"67198","messageId":"9439626e72a267ff29cb6eaa1c733ec4641341d9.1202029604.git.tommy-git@thorn.ws","threadId":"11848","inReplyTo":null,"subject":"[PATCH 1/3] git-p4: Fix an obvious typo","fromName":"Tommy Thorn","fromEmail":"tommy-git@thorn.ws","sentAt":"2008-02-03T09:21:04Z","receivedAt":"2008-02-03T09:21:04Z","isPatch":true,"sender":{"key":"tommy-git@thorn.ws","avatar":null},"body":"The regexp \"$,\" can't match anything. Clearly not intended.\n\nThis was introduced in ce6f33c8 which is quite a while ago.\n\nSigned-off-by: Tommy Thorn \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 c80a6da..553e237 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1670,7 +1670,7 @@ class P4Clone(P4Sync):\n depotPath = args[0]\n depotDir = re.sub(\"(@[^@]*)$\", \"\", depotPath)\n depotDir = re.sub(\"(#[^#]*)$\", \"\", depotDir)\n- depotDir = re.sub(r\"\\.\\.\\.$,\", \"\", depotDir)\n+ depotDir = re.sub(r\"\\.\\.\\.$\", \"\", depotDir)\n depotDir = re.sub(r\"/$\", \"\", depotDir)\n return os.path.split(depotDir)[1]\n \n-- \n1.5.4.rc5.17.g22b645\n"},{"id":"67200","messageId":"dd96ea0b47e8ec67ef14e4e954aa9ec7bec3c295.1202029604.git.tommy-git@thorn.ws","threadId":"11848","inReplyTo":"9439626e72a267ff29cb6eaa1c733ec4641341d9.1202029604.git.tommy-git@thorn.ws","subject":"[PATCH 2/3] git-p4: support exclude paths","fromName":"Tommy Thorn","fromEmail":"tommy-git@thorn.ws","sentAt":"2008-02-03T09:21:05Z","receivedAt":"2008-02-03T09:21:05Z","isPatch":true,"sender":{"key":"tommy-git@thorn.ws","avatar":null},"body":"Teach git-p4 about the -/ option which adds depot paths to the exclude\nlist, used when cloning. The option is chosen such that the natural\nPerforce syntax works, eg:\n\n git p4 clone //branch/path/... -//branch/path/{large,old}/...\n\nTrailing ... on exclude paths are optional.\n\nThis is a generalization of a change by Dmitry Kakurin (thanks).\n\nSigned-off-by: Tommy Thorn \n---\n contrib/fast-import/git-p4 | 26 ++++++++++++++++++++++----\n 1 files changed, 22 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 553e237..2340876 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -876,18 +876,25 @@ class P4Sync(Command):\n self.keepRepoPath = False\n self.depotPaths = None\n self.p4BranchesInGit = []\n+ self.cloneExclude = []\n \n if gitConfig(\"git-p4.syncFromOrigin\") == \"false\":\n self.syncWithOrigin = False\n \n def extractFilesFromCommit(self, commit):\n+ self.cloneExclude = [re.sub(r\"\\.\\.\\.$\", \"\", path)\n+ for path in self.cloneExclude]\n files = []\n fnum = 0\n while commit.has_key(\"depotFile%s\" % fnum):\n path = commit[\"depotFile%s\" % fnum]\n \n- found = [p for p in self.depotPaths\n- if path.startswith (p)]\n+ if [p for p in self.cloneExclude\n+ if path.startswith (p)]:\n+ found = False\n+ else:\n+ found = [p for p in self.depotPaths\n+ if path.startswith (p)]\n if not found:\n fnum = fnum + 1\n continue\n@@ -1658,13 +1665,23 @@ class P4Clone(P4Sync):\n P4Sync.__init__(self)\n self.description = \"Creates a new git repository and imports from Perforce into it\"\n self.usage = \"usage: %prog [options] //depot/path[@revRange]\"\n- self.options.append(\n+ self.options += [\n optparse.make_option(\"--destination\", dest=\"cloneDestination\",\n action='store', default=None,\n- help=\"where to leave result of the clone\"))\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+ ]\n self.cloneDestination = None\n self.needsGit = False\n \n+ # This is required for the \"append\" cloneExclude action\n+ def ensure_value(self, attr, value):\n+ if not hasattr(self, attr) or getattr(self, attr) is None:\n+ setattr(self, attr, value)\n+ return getattr(self, attr)\n+\n def defaultDestination(self, args):\n ## TODO: use common prefix of args?\n depotPath = args[0]\n@@ -1688,6 +1705,7 @@ class P4Clone(P4Sync):\n self.cloneDestination = depotPaths[-1]\n depotPaths = depotPaths[:-1]\n \n+ self.cloneExclude = [\"/\"+p for p in self.cloneExclude]\n for p in depotPaths:\n if not p.startswith(\"//\"):\n return False\n-- \n1.5.4.rc5.17.g22b645\n"},{"id":"67199","messageId":"98fe358f9523dec637dbe87a0258e3327cd901cd.1202029604.git.tommy-git@thorn.ws","threadId":"11848","inReplyTo":"dd96ea0b47e8ec67ef14e4e954aa9ec7bec3c295.1202029604.git.tommy-git@thorn.ws","subject":"[PATCH 3/3] git-p4: no longer keep all file contents while cloning","fromName":"Tommy Thorn","fromEmail":"tommy-git@thorn.ws","sentAt":"2008-02-03T09:21:06Z","receivedAt":"2008-02-03T09:21:06Z","isPatch":true,"sender":{"key":"tommy-git@thorn.ws","avatar":null},"body":"- Factor out the pipe creation part of p4CmdList() as p4CmdListPipe()\n\n- Factor readP4Files() into openP4Files() and readP4File() and\n changed P4Sync to use this.\n\nThe upshot is that git-p4 now read only one file at a time and\nimmediately pass it on to fast-import. This massively reduces the\nmemory requirement of git-p4.\n\nNote, git-p4 still reads in the whole file in memory -- this can and\nshould be fixed in future.\n\nSigned-off-by: Tommy Thorn \n---\n contrib/fast-import/git-p4 | 102 ++++++++++++++++++++++++++++++-------------\n 1 files changed, 71 insertions(+), 31 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 2340876..78a5d02 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -144,7 +144,8 @@ def isModeExec(mode):\n def isModeExecChanged(src_mode, dst_mode):\n return isModeExec(src_mode) != isModeExec(dst_mode)\n \n-def p4CmdList(cmd, stdin=None, stdin_mode='w+b'):\n+# p4CmdListPipe returns a pipe delivering the result of the p4 command\n+def p4CmdListPipe(cmd, stdin=None, stdin_mode='w+b'):\n cmd = \"p4 -G %s\" % cmd\n if verbose:\n sys.stderr.write(\"Opening pipe: %s\\n\" % cmd)\n@@ -162,7 +163,11 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b'):\n p4 = subprocess.Popen(cmd, shell=True,\n stdin=stdin_file,\n stdout=subprocess.PIPE)\n+ return p4\n \n+# p4CmdList returns the stdout result of the p4 command\n+def p4CmdList(cmd, stdin=None, stdin_mode='w+b'):\n+ p4 = p4CmdListPipe(cmd, stdin, stdin_mode)\n result = []\n try:\n while True:\n@@ -950,42 +955,62 @@ class P4Sync(Command):\n return branches\n \n ## Should move this out, doesn't use SELF.\n- def readP4Files(self, files):\n+ def openP4Files(self, files):\n files = [f for f in files\n if f['action'] != 'delete']\n \n if not files:\n return\n \n- filedata = p4CmdList('-x - print',\n- stdin='\\n'.join(['%s#%s' % (f['path'], f['rev'])\n- for f in files]),\n- stdin_mode='w+')\n- if \"p4ExitCode\" in filedata[0]:\n- die(\"Problems executing p4. Error: [%d].\"\n- % (filedata[0]['p4ExitCode']));\n-\n- j = 0;\n- contents = {}\n- while j < len(filedata):\n- stat = filedata[j]\n- j += 1\n- text = ''\n- while j < len(filedata) and filedata[j]['code'] in ('text',\n- 'binary'):\n- text += filedata[j]['data']\n- j += 1\n+ p4 = p4CmdListPipe('-x - print',\n+ stdin='\\n'.join(['%s#%s' % (f['path'], f['rev'])\n+ for f in files]),\n+ stdin_mode='w+')\n \n+ self.curDepotFile = None\n+ return p4\n \n- if not stat.has_key('depotFile'):\n- sys.stderr.write(\"p4 print fails with: %s\\n\" % repr(stat))\n- continue\n+ # Uisng the pipe handle provided by openP4File, read in a file and\n+ # return the pair of (depot file name, contents)\n+ def readP4File(self, p4):\n+ text = ''\n \n- contents[stat['depotFile']] = text\n+ try:\n+ while True:\n+ entry = marshal.load(p4.stdout)\n+\n+ if entry['code'] in ('text', 'binary'):\n+ text += entry['data']\n+ elif entry['code'] == 'stat':\n+ # We are done with the previous file\n+ if self.curDepotFile is not None:\n+ if not entry.has_key('depotFile'):\n+ sys.stderr.write(\"p4 print fails with: %s\\n\" % repr(entry))\n+ self.curDepotFile = None\n+ continue\n+\n+ depotFile = self.curDepotFile\n+ self.curDepotFile = entry['depotFile']\n+ if verbose: sys.stderr.write('Read %s\\n' % depotFile)\n+ return (depotFile, text)\n+\n+ text = ''\n+ self.curDepotFile = entry['depotFile']\n+ else:\n+ sys.stderr.write(\"p4 print returned unexpected code: %s\\n\" % entry['code'])\n+\n+ except EOFError:\n+ pass\n+\n+ exitCode = p4.wait()\n+ if exitCode != 0:\n+ die(\"Problems executing p4. Error: [%d].\" % exitCode);\n+\n+ depotFile = self.curDepotFile\n+ self.curDepotFile = None\n+ if verbose: sys.stderr.write('Read %s\\n' % depotFile)\n+ return (depotFile, text)\n \n- for f in files:\n- assert not f.has_key('data')\n- f['data'] = contents[f['path']]\n \n def commit(self, details, files, branch, branchPrefixes, parent = \"\"):\n epoch = details[\"time\"]\n@@ -996,6 +1021,10 @@ class P4Sync(Command):\n \n # start with reading files; if that fails, we should not\n # create a commit.\n+\n+ # XXX No, reading all file contents into memory is to tall a\n+ # price to pay. Right now if an error is found, it will abort,\n+ # leaving cruft for git gc to prune.\n new_files = []\n for f in files:\n if [p for p in branchPrefixes if f['path'].startswith(p)]:\n@@ -1003,7 +1032,6 @@ class P4Sync(Command):\n else:\n sys.stderr.write(\"Ignoring file outside of prefix: %s\\n\" % path)\n files = new_files\n- self.readP4Files(files)\n \n \n \n@@ -1034,17 +1062,26 @@ class P4Sync(Command):\n print \"parent %s\" % parent\n self.gitStream.write(\"from %s\\n\" % parent)\n \n- for file in files:\n+ # Create a depotFileName -> file mapping\n+ revFile = {}\n+ for f in files:\n+ revFile[f['path']] = f\n+\n+ p4 = self.openP4Files(files)\n+ (depotFile, data) = self.readP4File(p4)\n+\n+ while depotFile is not None:\n+\n+ file = revFile[depotFile]\n if file[\"type\"] == \"apple\":\n print \"\\nfile %s is a strange apple file that forks. Ignoring!\" % file['path']\n+ (depotFile, data) = self.readP4File(p4)\n continue\n \n relPath = self.stripRepoPath(file['path'], branchPrefixes)\n if file[\"action\"] == \"delete\":\n self.gitStream.write(\"D %s\\n\" % relPath)\n else:\n- data = file['data']\n-\n mode = \"644\"\n if isP4Exec(file[\"type\"]):\n mode = \"755\"\n@@ -1061,6 +1098,9 @@ class P4Sync(Command):\n self.gitStream.write(data)\n self.gitStream.write(\"\\n\")\n \n+ (depotFile, data) = self.readP4File(p4)\n+\n+\n self.gitStream.write(\"\\n\")\n \n change = int(details[\"change\"])\n-- \n1.5.4.rc5.17.g22b645\n"},{"id":"67246","messageId":"200802031941.17010.simon@lst.de","threadId":"11848","inReplyTo":"dd96ea0b47e8ec67ef14e4e954aa9ec7bec3c295.1202029604.git.tommy-git@thorn.ws","subject":"Re: [PATCH 2/3] git-p4: support exclude paths","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2008-02-03T18:41:16Z","receivedAt":"2008-02-03T18:41:16Z","isPatch":true,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"On Sunday 03 February 2008 10:21:05 Tommy Thorn wrote:\n> Teach git-p4 about the -/ option which adds depot paths to the exclude\n> list, used when cloning. The option is chosen such that the natural\n> Perforce syntax works, eg:\n>\n> git p4 clone //branch/path/... -//branch/path/{large,old}/...\n>\n> Trailing ... on exclude paths are optional.\n>\n> This is a generalization of a change by Dmitry Kakurin (thanks).\n>\n> Signed-off-by: Tommy Thorn \n\nAcked-By: Simon Hausmann \n\nI like it, Perforce'ish syntax. (Not that I like p4 though ;)\n\n\nSimon\n\n> ---\n> contrib/fast-import/git-p4 | 26 ++++++++++++++++++++++----\n> 1 files changed, 22 insertions(+), 4 deletions(-)\n>\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> index 553e237..2340876 100755\n> --- a/contrib/fast-import/git-p4\n> +++ b/contrib/fast-import/git-p4\n> @@ -876,18 +876,25 @@ class P4Sync(Command):\n> self.keepRepoPath = False\n> self.depotPaths = None\n> self.p4BranchesInGit = []\n> + self.cloneExclude = []\n>\n> if gitConfig(\"git-p4.syncFromOrigin\") == \"false\":\n> self.syncWithOrigin = False\n>\n> def extractFilesFromCommit(self, commit):\n> + self.cloneExclude = [re.sub(r\"\\.\\.\\.$\", \"\", path)\n> + for path in self.cloneExclude]\n> files = []\n> fnum = 0\n> while commit.has_key(\"depotFile%s\" % fnum):\n> path = commit[\"depotFile%s\" % fnum]\n>\n> - found = [p for p in self.depotPaths\n> - if path.startswith (p)]\n> + if [p for p in self.cloneExclude\n> + if path.startswith (p)]:\n> + found = False\n> + else:\n> + found = [p for p in self.depotPaths\n> + if path.startswith (p)]\n> if not found:\n> fnum = fnum + 1\n> continue\n> @@ -1658,13 +1665,23 @@ class P4Clone(P4Sync):\n> P4Sync.__init__(self)\n> self.description = \"Creates a new git repository and imports from\n> Perforce into it\" self.usage = \"usage: %prog [options]\n> //depot/path[@revRange]\" - self.options.append(\n> + self.options += [\n> optparse.make_option(\"--destination\", dest=\"cloneDestination\",\n> action='store', default=None,\n> - help=\"where to leave result of the\n> clone\")) + help=\"where to leave result of\n> the clone\"), + optparse.make_option(\"-/\", dest=\"cloneExclude\",\n> + action=\"append\", type=\"string\",\n> + help=\"exclude depot path\")\n> + ]\n> self.cloneDestination = None\n> self.needsGit = False\n>\n> + # This is required for the \"append\" cloneExclude action\n> + def ensure_value(self, attr, value):\n> + if not hasattr(self, attr) or getattr(self, attr) is None:\n> + setattr(self, attr, value)\n> + return getattr(self, attr)\n> +\n> def defaultDestination(self, args):\n> ## TODO: use common prefix of args?\n> depotPath = args[0]\n> @@ -1688,6 +1705,7 @@ class P4Clone(P4Sync):\n> self.cloneDestination = depotPaths[-1]\n> depotPaths = depotPaths[:-1]\n>\n> + self.cloneExclude = [\"/\"+p for p in self.cloneExclude]\n> for p in depotPaths:\n> if not p.startswith(\"//\"):\n> return False\n"},{"id":"67249","messageId":"47A60E20.7060909@thorn.ws","threadId":"11848","inReplyTo":"200802031941.17010.simon@lst.de","subject":"Re: [PATCH 2/3] git-p4: support exclude paths","fromName":"Tommy Thorn","fromEmail":"tommy-git@thorn.ws","sentAt":"2008-02-03T18:55:28Z","receivedAt":"2008-02-03T18:55:28Z","isPatch":true,"sender":{"key":"tommy-git@thorn.ws","avatar":null},"body":"Simon Hausmann wrote:\n> On Sunday 03 February 2008 10:21:05 Tommy Thorn wrote:\n> \n>> Teach git-p4 about the -/ option which adds depot paths to the exclude\n>> list, used when cloning. The option is chosen such that the natural\n>> Perforce syntax works, eg:\n>>\n>> git p4 clone //branch/path/... -//branch/path/{large,old}/...\n>>\n>> Trailing ... on exclude paths are optional.\n>>\n>> This is a generalization of a change by Dmitry Kakurin (thanks).\n>>\n>> Signed-off-by: Tommy Thorn \n>> \n>\n> Acked-By: Simon Hausmann \n>\n> I like it, Perforce'ish syntax. (Not that I like p4 though ;)\n> \nThank you.\n\nI would appear that I have some mail server problems, so apologies if \nyou get multiple copies.\n\nAlso, this is the first Python hacking I've tried, so it's likely that \nmy changes needs improvement.\n\nWith these two patches, I can now use git-p4. However in a perfect \nworld, git-p4 would:\n\n- include support everything that a (ugly) Perforce client can do, \nincluding naming individual files\n and remapping things around (a sick feature that never should be used \nIMO), and\n\n- not consume memory proportional to the imported files.\n\nThe former would require pervasive changes and likely break some \nassumptions currently made.\n\nThe latter is easy enough.\n\n\nRegards\nTommy\n"}]}