Fix unicode errors generting diffs with git-p4

Review Request #12255 — Created April 25, 2022 and submitted — Latest diff uploaded

Information

RBTools
master

Reviewers

When using git-p4 as the underlying client with python 3 runtime, a
UnicodeDecodeError is thrown if any of the files to be diffed contained
non-ASCII text.

This was because the `make_perforce_diff` function was using a mixture
of bytes and str objects. The solution is to keep everything in bytes
and never try to intepret the characters returned from the diff other
than in very specific ways it always has.

This was a frequent error for us at work. I posted the solution to the issue tracker a long time ago, but I've since been unable to find the specific issue that was raised. I have been using this patch for at least 3 years.


Add a file named test-Œ.txt with contents ŒŒŒŒ.
rbt post that.

You can see inthe --debug output that the commands were successful:

>>> Running: p4 files //depot/MMA/core/main/test/test-Œ.txt@2399980
>>> Running: p4 files //depot/MMA/core/main/test/test.txt@2399980

I added print() statement and inspected the generated diff too:

Unicode: --- //depot/MMA/core/main/test/test-Œ.txt      //depot/MMA/core/main/test/test-Œ.txt#1
+++ //depot/MMA/core/main/test/test-Œ.txt       TIMESTAMP
@@ -0,0 +1 @@
+ŒŒŒŒ
--- //depot/MMA/core/main/test/test.txt //depot/MMA/core/main/test/test.txt#1
+++ //depot/MMA/core/main/test/test.txt TIMESTAMP
@@ -0,0 +1 @@
+ŒŒŒŒŒŒŒ

b'Raw: --- //depot/MMA/core/main/test/test-\xc5\x92.txt\t//depot/MMA/core/main/test/test-\xc5\x92.txt#1\n+++ //depot/MMA/core/main/test/test-\xc5\x92.txt\tTIMESTAMP\n@@ -0,0 +1 @@\n+\xc5\x92\xc5\x92\xc5\x92\xc5\x92\n--- //depot/MMA/core/main/test/test.txt\t//depot/MMA/core/main/test/test.txt#1\n+++ //depot/MMA/core/main/test/test.txt\tTIMESTAMP\n@@ -0,0 +1 @@\n+\xc5\x92\xc5\x92\xc5\x92\xc5\x92\xc5\x92\xc5\x92\xc5\x92\n'

Funny enought he server version we're on just aborts the request, but i think the client behaved correctly:

>>> Command line: rbt post --debug -r 93808 HEAD
>>> Running: git rev-parse HEAD
>>> Running: git rev-parse 48ecac22ee23eb524754fd08b1c651584104786c^
>>> Running: git branch --remotes
>>> Running: git rev-list 6e25de453e378d6f3a5927a5ed1d17b15a7517b9 --not --remotes=p4
>>> Making HTTP GET request to http://reviews.eu.fidessa.priv/api/review-requests/93808/?only-fields=absolute_url%2Cbugs_closed%2Cid%2Cstatus%2Cpublic&only-links=diffs%2Cdraft
>>> Cached response for HTTP GET http://reviews.eu.fidessa.priv/api/review-requests/93808/?only-fields=absolute_url%2Cbugs_closed%2Cid%2Cstatus%2Cpublic&only-links=diffs%2Cdraft expired and was not modified
>>> Running: git version
>>> Running: git -c core.quotepath=false diff --no-color --no-prefix -r -u --no-ext-diff 6e25de453e378d6f3a5927a5ed1d17b15a7517b9..48ecac22ee23eb524754fd08b1c651584104786c
>>> Running: git log 6e25de453e378d6f3a5927a5ed1d17b15a7517b9
>>> Running: p4 files //depot/MMA/core/main/test/test-Œ.txt@2399980
>>> Running: p4 files //depot/MMA/core/main/test/test.txt@2399980
>>> Making HTTP GET request to http://reviews.eu.fidessa.priv/api/validation/diffs/
>>> Cached response for HTTP GET http://reviews.eu.fidessa.priv/api/validation/diffs/ expired and was modified
>>> Making HTTP POST request to http://reviews.eu.fidessa.priv/api/validation/diffs/
Traceback (most recent call last):
  File "/mma/users/benj/.local/bin/rbt", line 33, in <module>
    sys.exit(load_entry_point('RBTools', 'console_scripts', 'rbt')())
  File "/mma/users/benj/proj/rbtools/rbtools/commands/main.py", line 207, in main
    command.run_from_argv([RB_MAIN, command_name] + args)
  File "/mma/users/benj/proj/rbtools/rbtools/commands/__init__.py", line 1043, in run_from_argv
    exit_code = self.main(*args) or 0
  File "/mma/users/benj/proj/rbtools/rbtools/commands/post.py", line 1022, in main
    self._validate_squashed_diff(squashed_diff)
  File "/mma/users/benj/proj/rbtools/rbtools/commands/post.py", line 1383, in _validate_squashed_diff
    **validate_kwargs)
  File "/mma/users/benj/proj/rbtools/rbtools/api/decorators.py", line 27, in request_method
    *args, **kwargs)
  File "/mma/users/benj/proj/rbtools/rbtools/api/transport/sync.py", line 83, in execute_request_method
    return self._execute_request(request)
  File "/mma/users/benj/proj/rbtools/rbtools/api/transport/sync.py", line 92, in _execute_request
    rsp = self.server.make_request(request)
  File "/mma/users/benj/proj/rbtools/rbtools/api/request.py", line 827, in make_request
    request.url, body, headers, request.method))
  File "/mma/users/benj/proj/rbtools/rbtools/api/cache.py", line 209, in make_request
    return self.urlopen(request)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 223, in urlopen
    return opener.open(url, data, timeout)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 526, in open
    response = self._open(req, data)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 544, in _open
    '_open', req)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 504, in _call_chain
    result = func(*args)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 1346, in http_open
    return self.do_open(http.client.HTTPConnection, req)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 1321, in do_open
    r = h.getresponse()
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/http/client.py", line 1331, in getresponse
    response.begin()
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/http/client.py", line 297, in begin
    version, status, reason = self._read_status()
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/http/client.py", line 266, in _read_status
    raise RemoteDisconnected("Remote end closed connection without"
http.client.RemoteDisconnected: Remote end closed connection without response

Commits

Files