@matheusbsbs / matheus-bagliano / commits / 6cec21a82f
Fork 0

git-p4: encode/decode communication with p4 for python3

The marshalled dict in the response given on STDOUT by p4 uses `str` for keys and string values. When run using python3, these values are deserialized as `bytes`, leading to a whole host of problems as the rest of the code assumes `str` is used throughout. This patch changes the deserialization behaviour such that, as much as possible, text output from p4 is decoded to native unicode strings. Exceptions are made for the field `data` as it is usually arbitrary binary data. `depotFile[0-9]*`, `path`, and `clientFile` are also exempt as they contain path strings not encoded with UTF-8, and must survive round-trip back to p4. Conversely, text data being piped to p4 must always be encoded when running under python3. encode_text_stream() and decode_text_stream() were added to make these transformations more convenient. Signed-off-by: Yang Zhao <yang.zhao@skyboxlabs.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Yang Zhao committed Dec 13, 2019 at 15:52 UTC 6cec21a82f437184c69cce2b8fe6011d02acfa06
1 file changed +46 -13
git-p4.py
+46 -13
@@ -135,6 +135,21 @@ def die(msg):
135 sys.stderr.write(msg + "\n")
136 sys.exit(1)
137
138 +# We need different encoding/decoding strategies for text data being passed
139 +# around in pipes depending on python version
140 +if bytes is not str:
141 + # For python3, always encode and decode as appropriate
142 + def decode_text_stream(s):
143 + return s.decode() if isinstance(s, bytes) else s
144 + def encode_text_stream(s):
145 + return s.encode() if isinstance(s, str) else s
146 +else:
147 + # For python2.7, pass read strings as-is, but also allow writing unicode
148 + def decode_text_stream(s):
149 + return s
150 + def encode_text_stream(s):
151 + return s.encode('utf_8') if isinstance(s, unicode) else s
152 +
153 def write_pipe(c, stdin):
154 if verbose:
155 sys.stderr.write('Writing pipe: %s\n' % str(c))
@@ -151,6 +166,8 @@ def write_pipe(c, stdin):
166
167 def p4_write_pipe(c, stdin):
168 real_cmd = p4_build_cmd(c)
169 + if bytes is not str and isinstance(stdin, str):
170 + stdin = encode_text_stream(stdin)
171 return write_pipe(real_cmd, stdin)
172
173 def read_pipe_full(c):
@@ -164,7 +181,7 @@ def read_pipe_full(c):
181 expand = not isinstance(c, list)
182 p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)
183 (out, err) = p.communicate()
167 - return (p.returncode, out, err)
184 + return (p.returncode, out, decode_text_stream(err))
185
186 def read_pipe(c, ignore_error=False):
187 """ Read output from command. Returns the output text on
@@ -187,11 +204,11 @@ def read_pipe_text(c):
204 if retcode != 0:
205 return None
206 else:
190 - return out.rstrip()
207 + return decode_text_stream(out).rstrip()
208
192 -def p4_read_pipe(c, ignore_error=False):
209 +def p4_read_pipe(c, ignore_error=False, raw=False):
210 real_cmd = p4_build_cmd(c)
194 - return read_pipe(real_cmd, ignore_error)
211 + return read_pipe(real_cmd, ignore_error, raw=raw)
212
213 def read_pipe_lines(c):
214 if verbose:
@@ -200,7 +217,7 @@ def read_pipe_lines(c):
217 expand = not isinstance(c, list)
218 p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)
219 pipe = p.stdout
203 - val = pipe.readlines()
220 + val = [decode_text_stream(line) for line in pipe.readlines()]
221 if pipe.close() or p.wait():
222 die('Command failed: %s' % str(c))
223
@@ -231,6 +248,7 @@ def p4_has_move_command():
248 cmd = p4_build_cmd(["move", "-k", "@from", "@to"])
249 p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
250 (out, err) = p.communicate()
251 + err = decode_text_stream(err)
252 # return code will be 1 in either case
253 if err.find("Invalid option") >= 0:
254 return False
@@ -611,6 +629,20 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
629 try:
630 while True:
631 entry = marshal.load(p4.stdout)
632 + if bytes is not str:
633 + # Decode unmarshalled dict to use str keys and values, except for:
634 + # - `data` which may contain arbitrary binary data
635 + # - `depotFile[0-9]*`, `path`, or `clientFile` which may contain non-UTF8 encoded text
636 + decoded_entry = {}
637 + for key, value in entry.items():
638 + key = key.decode()
639 + if isinstance(value, bytes) and not (key in ('data', 'path', 'clientFile') or key.startswith('depotFile')):
640 + value = value.decode()
641 + decoded_entry[key] = value
642 + # Parse out data if it's an error response
643 + if decoded_entry.get('code') == 'error' and 'data' in decoded_entry:
644 + decoded_entry['data'] = decoded_entry['data'].decode()
645 + entry = decoded_entry
646 if skip_info:
647 if 'code' in entry and entry['code'] == 'info':
648 continue
@@ -828,6 +860,7 @@ def branch_exists(branch):
860 cmd = [ "git", "rev-parse", "--symbolic", "--verify", branch ]
861 p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
862 out, _ = p.communicate()
863 + out = decode_text_stream(out)
864 if p.returncode:
865 return False
866 # expect exactly one line of output: the branch name
@@ -1971,7 +2004,7 @@ class P4Submit(Command, P4UserMap):
2004 tmpFile = os.fdopen(handle, "w+b")
2005 if self.isWindows:
2006 submitTemplate = submitTemplate.replace("\n", "\r\n")
1974 - tmpFile.write(submitTemplate)
2007 + tmpFile.write(encode_text_stream(submitTemplate))
2008 tmpFile.close()
2009
2010 if self.prepare_p4_only:
@@ -2018,7 +2051,7 @@ class P4Submit(Command, P4UserMap):
2051 if self.edit_template(fileName):
2052 # read the edited message and submit
2053 tmpFile = open(fileName, "rb")
2021 - message = tmpFile.read()
2054 + message = decode_text_stream(tmpFile.read())
2055 tmpFile.close()
2056 if self.isWindows:
2057 message = message.replace("\r\n", "\n")
@@ -2707,7 +2740,7 @@ class P4Sync(Command, P4UserMap):
2740 return branches
2741
2742 def writeToGitStream(self, gitMode, relPath, contents):
2710 - self.gitStream.write('M %s inline %s\n' % (gitMode, relPath))
2743 + self.gitStream.write(encode_text_stream(u'M {} inline {}\n'.format(gitMode, relPath)))
2744 self.gitStream.write('data %d\n' % sum(len(d) for d in contents))
2745 for d in contents:
2746 self.gitStream.write(d)
@@ -2748,7 +2781,7 @@ class P4Sync(Command, P4UserMap):
2781 git_mode = "120000"
2782 # p4 print on a symlink sometimes contains "target\n";
2783 # if it does, remove the newline
2751 - data = ''.join(contents)
2784 + data = ''.join(decode_text_stream(c) for c in contents)
2785 if not data:
2786 # Some version of p4 allowed creating a symlink that pointed
2787 # to nothing. This causes p4 errors when checking out such
@@ -2802,7 +2835,7 @@ class P4Sync(Command, P4UserMap):
2835 pattern = p4_keywords_regexp_for_type(type_base, type_mods)
2836 if pattern:
2837 regexp = re.compile(pattern, re.VERBOSE)
2805 - text = ''.join(contents)
2838 + text = ''.join(decode_text_stream(c) for c in contents)
2839 text = regexp.sub(r'$\1$', text)
2840 contents = [ text ]
2841
@@ -2817,7 +2850,7 @@ class P4Sync(Command, P4UserMap):
2850 if verbose:
2851 sys.stdout.write("delete %s\n" % relPath)
2852 sys.stdout.flush()
2820 - self.gitStream.write("D %s\n" % relPath)
2853 + self.gitStream.write(encode_text_stream(u'D {}\n'.format(relPath)))
2854
2855 if self.largeFileSystem and self.largeFileSystem.isLargeFile(relPath):
2856 self.largeFileSystem.removeLargeFile(relPath)
@@ -2917,9 +2950,9 @@ class P4Sync(Command, P4UserMap):
2950 if 'shelved_cl' in f:
2951 # Handle shelved CLs using the "p4 print file@=N" syntax to print
2952 # the contents
2920 - fileArg = '%s@=%d' % (f['path'], f['shelved_cl'])
2953 + fileArg = f['path'] + encode_text_stream('@={}'.format(f['shelved_cl']))
2954 else:
2922 - fileArg = '%s#%s' % (f['path'], f['rev'])
2955 + fileArg = f['path'] + encode_text_stream('#{}'.format(f['rev']))
2956
2957 fileArgs.append(fileArg)
2958