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

Subsystems: the rest

DORMANTno replies

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Scott Lamb <hidden>
Date: 2016-06-15 22:43:21

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

Signed-off-by: Scott Lamb <redacted>
---
 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

From: Scott Lamb <hidden>
Date: 2016-06-15 22:43:21

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 <redacted>
---
 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

From: Simon Hausmann <hidden>
Date: 2016-06-15 22:43:21

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help