{"thread":{"id":"9054","subject":"[PATCH 1/2] git-p4: use subprocess in p4CmdList","startedAt":"2007-07-16T03:58:10Z","lastAt":"2007-07-16T18:33:56Z","messageCount":3,"participants":["Scott Lamb","Simon Hausmann"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"47505","messageId":"11845582912155-git-send-email-slamb@slamb.org","threadId":"9054","inReplyTo":null,"subject":"[PATCH 1/2] git-p4: use subprocess in p4CmdList","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-07-16T03:58:10Z","receivedAt":"2007-07-16T03:58:10Z","isPatch":true,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"This allows bidirectional piping - useful for \"-x -\" to avoid commandline\narguments - and is a step toward bypassing the shell.\n\nSigned-off-by: Scott Lamb <slamb@slamb.org>\n---\n contrib/fast-import/git-p4 |   23 ++++++++++++++++++-----\n 1 files changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex d877150..d93e656 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -63,21 +63,34 @@ def system(cmd):\n     if os.system(cmd) != 0:\n         die(\"command failed: %s\" % cmd)\n \n-def p4CmdList(cmd):\n+def p4CmdList(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-    pipe = os.popen(cmd, \"rb\")\n+\n+    # Use a temporary file to avoid deadlocks without\n+    # subprocess.communicate(), which would put another copy\n+    # of stdout into memory.\n+    stdin_file = None\n+    if stdin is not None:\n+        stdin_file = tempfile.TemporaryFile(prefix='p4-stdin', mode=stdin_mode)\n+        stdin_file.write(stdin)\n+        stdin_file.flush()\n+        stdin_file.seek(0)\n+\n+    p4 = subprocess.Popen(cmd, shell=True,\n+                          stdin=stdin_file,\n+                          stdout=subprocess.PIPE)\n \n     result = []\n     try:\n         while True:\n-            entry = marshal.load(pipe)\n+            entry = marshal.load(p4.stdout)\n             result.append(entry)\n     except EOFError:\n         pass\n-    exitCode = pipe.close()\n-    if exitCode != None:\n+    exitCode = p4.wait()\n+    if exitCode != 0:\n         entry = {}\n         entry[\"p4ExitCode\"] = exitCode\n         result.append(entry)\n-- \n1.5.2.2.238.g7cbf2f2-dirty\n"},{"id":"47506","messageId":"11845582942124-git-send-email-slamb@slamb.org","threadId":"9054","inReplyTo":"11845582912155-git-send-email-slamb@slamb.org","subject":"[PATCH 2/2] git-p4: input to \"p4 files\" by stdin instead of arguments","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-07-16T03:58:11Z","receivedAt":"2007-07-16T03:58:11Z","isPatch":true,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"This approach, suggested by Alex Riesen, bypasses the need for xargs-style\nargument list handling. The handling in question looks broken in a corner\ncase with SC_ARG_MAX=4096 and final argument over 96 characters.\n\nSigned-off-by: Scott Lamb <slamb@slamb.org>\n---\n contrib/fast-import/git-p4 |   28 +++++++---------------------\n 1 files changed, 7 insertions(+), 21 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex d93e656..54053e3 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -725,27 +725,13 @@ class P4Sync(Command):\n         if not files:\n             return\n \n-        # We cannot put all the files on the command line\n-        # OS have limitations on the max lenght of arguments\n-        # POSIX says it's 4096 bytes, default for Linux seems to be 130 K.\n-        # and all OS from the table below seems to be higher than POSIX.\n-        # See http://www.in-ulm.de/~mascheck/various/argmax/\n-        if (self.isWindows):\n-            argmax = 2000\n-        else:\n-            argmax = min(4000, os.sysconf('SC_ARG_MAX'))\n-\n-        chunk = ''\n-        filedata = []\n-        for i in xrange(len(files)):\n-            f = files[i]\n-            chunk += '\"%s#%s\" ' % (f['path'], f['rev'])\n-            if len(chunk) > argmax or i == len(files)-1:\n-                data = p4CmdList('print %s' % chunk)\n-                if \"p4ExitCode\" in data[0]:\n-                    die(\"Problems executing p4. Error: [%d].\" % (data[0]['p4ExitCode']));\n-                filedata.extend(data)\n-                chunk = ''\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-- \n1.5.2.2.238.g7cbf2f2-dirty\n"},{"id":"47578","messageId":"200707162033.56888.simon@lst.de","threadId":"9054","inReplyTo":"11845582912155-git-send-email-slamb@slamb.org","subject":"Re: [PATCH 1/2] git-p4: use subprocess in p4CmdList","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2007-07-16T18:33:56Z","receivedAt":"2007-07-16T18:33:56Z","isPatch":true,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"On Monday 16 July 2007 05:58:10 Scott Lamb wrote:\n> This allows bidirectional piping - useful for \"-x -\" to avoid commandline\n> arguments - and is a step toward bypassing the shell.\n\nThanks! I have pushed your two patches into\n\n\thttp://gitweb.freedesktop.org/?p=users/hausmann/git-p4;a=summary\n\nUnless somebody else wants to try earlier I intend to ask Junio to pull your \nchanges from there after 1.5.3.\n\n\nSimon\n"}]}