[PATCH 1/2] git-p4: use subprocess in p4CmdList

[PATCH 1/2] git-p4: use subprocess in p4CmdList

[Scott Lamb]

This allows bidirectional piping - useful for "-x -" to avoid commandline
arguments - and is a step toward bypassing the shell.

Signed-off-by: Scott Lamb <slamb@slamb.org>
---
 contrib/fast-import/git-p4 |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index d877150..d93e656 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -63,21 +63,34 @@ def system(cmd):
     if os.system(cmd) != 0:
         die("command failed: %s" % cmd)
 
-def p4CmdList(cmd):
+def p4CmdList(cmd, stdin=None, stdin_mode='w+b'):
     cmd = "p4 -G %s" % cmd
     if verbose:
         sys.stderr.write("Opening pipe: %s\n" % cmd)
-    pipe = os.popen(cmd, "rb")
+
+    # Use a temporary file to avoid deadlocks without
+    # subprocess.communicate(), which would put another copy
+    # of stdout into memory.
+    stdin_file = None
+    if stdin is not None:
+        stdin_file = tempfile.TemporaryFile(prefix='p4-stdin', mode=stdin_mode)
+        stdin_file.write(stdin)
+        stdin_file.flush()
+        stdin_file.seek(0)
+
+    p4 = subprocess.Popen(cmd, shell=True,
+                          stdin=stdin_file,
+                          stdout=subprocess.PIPE)
 
     result = []
     try:
         while True:
-            entry = marshal.load(pipe)
+            entry = marshal.load(p4.stdout)
             result.append(entry)
     except EOFError:
         pass
-    exitCode = pipe.close()
-    if exitCode != None:
+    exitCode = p4.wait()
+    if exitCode != 0:
         entry = {}
         entry["p4ExitCode"] = exitCode
         result.append(entry)
-- 
1.5.2.2.238.g7cbf2f2-dirty

[PATCH 2/2] git-p4: input to "p4 files" by stdin instead of arguments

[Scott Lamb]

This approach, suggested by Alex Riesen, bypasses the need for xargs-style
argument list handling. The handling in question looks broken in a corner
case with SC_ARG_MAX=4096 and final argument over 96 characters.

Signed-off-by: Scott Lamb <slamb@slamb.org>
---
 contrib/fast-import/git-p4 |   28 +++++++---------------------
 1 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index d93e656..54053e3 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -725,27 +725,13 @@ class P4Sync(Command):
         if not files:
             return
 
-        # We cannot put all the files on the command line
-        # OS have limitations on the max lenght of arguments
-        # POSIX says it's 4096 bytes, default for Linux seems to be 130 K.
-        # and all OS from the table below seems to be higher than POSIX.
-        # See http://www.in-ulm.de/~mascheck/various/argmax/
-        if (self.isWindows):
-            argmax = 2000
-        else:
-            argmax = min(4000, os.sysconf('SC_ARG_MAX'))
-
-        chunk = ''
-        filedata = []
-        for i in xrange(len(files)):
-            f = files[i]
-            chunk += '"%s#%s" ' % (f['path'], f['rev'])
-            if len(chunk) > argmax or i == len(files)-1:
-                data = p4CmdList('print %s' % chunk)
-                if "p4ExitCode" in data[0]:
-                    die("Problems executing p4. Error: [%d]." % (data[0]['p4ExitCode']));
-                filedata.extend(data)
-                chunk = ''
+        filedata = p4CmdList('-x - print',
+                             stdin='\n'.join(['%s#%s' % (f['path'], f['rev'])
+                                              for f in files]),
+                             stdin_mode='w+')
+        if "p4ExitCode" in filedata[0]:
+            die("Problems executing p4. Error: [%d]."
+                % (filedata[0]['p4ExitCode']));
 
         j = 0;
         contents = {}
-- 
1.5.2.2.238.g7cbf2f2-dirty

Re: [PATCH 1/2] git-p4: use subprocess in p4CmdList

[Simon Hausmann]

[-- Attachment #1: Type: text/plain, Size: 426 bytes --]

On Monday 16 July 2007 05:58:10 Scott Lamb wrote:
> This allows bidirectional piping - useful for "-x -" to avoid commandline
> arguments - and is a step toward bypassing the shell.

Thanks! I have pushed your two patches into

	http://gitweb.freedesktop.org/?p=users/hausmann/git-p4;a=summary

Unless somebody else wants to try earlier I intend to ask Junio to pull your 
changes from there after 1.5.3.


Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]