{"thread":{"id":"40292","subject":"[PATCH v1 0/2] git-p4: handle \"Translation of file content failed\"","startedAt":"2015-09-07T12:42:01Z","lastAt":"2015-09-09T18:50:15Z","messageCount":5,"participants":["larsxschneider@gmail.com","Junio C Hamano","Lars Schneider"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"269563","messageId":"1441629723-90604-1-git-send-email-larsxschneider@gmail.com","threadId":"40292","inReplyTo":null,"subject":"[PATCH v1 0/2] git-p4: handle \"Translation of file content failed\"","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-07T12:42:01Z","receivedAt":"2015-09-07T12:42:01Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nHi,\n\nthis patch fixes the P4 \"Translation of file content failed\" error. Unfortuantly\nI was not able to generate a P4 test repository to reproduce the error with a\ntest case.\n\nAn Internet search shows that this error happens in the wild:\nhttps://stackoverflow.com/questions/5156909/translation-of-file-content-failed-error-in-perforce\nhttps://stackoverflow.com/questions/887006/perforce-translation-of-file-content-failed-error\n\nThanks,\nLars\n\nLars Schneider (2):\n git-p4: print stderr if P4 read_pipe operation fails\n git-p4: handle \"Translation of file content failed\"\n\n git-p4.py | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\n--\n2.5.1\n"},{"id":"269565","messageId":"1441629723-90604-2-git-send-email-larsxschneider@gmail.com","threadId":"40292","inReplyTo":"1441629723-90604-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v1 1/2] git-p4: print stderr if P4 read_pipe operation fails","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-07T12:42:02Z","receivedAt":"2015-09-07T12:42:02Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nIf read_pipe crashes then the caller can inspect the error and handle\nit appropriately.\n\nSigned-off-by: Lars Schneider \n---\n git-p4.py | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 073f87b..36a4bcb 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -134,11 +134,11 @@ def read_pipe(c, ignore_error=False):\n sys.stderr.write('Reading pipe: %s\\n' % str(c))\n \n expand = isinstance(c,basestring)\n- p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)\n+ p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)\n pipe = p.stdout\n val = pipe.read()\n if p.wait() and not ignore_error:\n- die('Command failed: %s' % str(c))\n+ die('Command failed: %s\\nError: %s' % (str(c), p.stderr.read()))\n \n return val\n \n-- \n2.5.1\n"},{"id":"269564","messageId":"1441629723-90604-3-git-send-email-larsxschneider@gmail.com","threadId":"40292","inReplyTo":"1441629723-90604-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v1 2/2] git-p4: handle \"Translation of file content failed\"","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-07T12:42:03Z","receivedAt":"2015-09-07T12:42:03Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nA P4 repository can get into a state where it contains a file with file\ntype UTF16 that does not not contain valid UTF16 characters. If git-p4\nattempts to retrieve the file as UTF16 from P4 then the process crashes\nwith a \"Translation of file content failed\" error.\n\nFix this by detecting this error and retrieving the file as binary\ninstead. The result in Git is the same.\n---\n git-p4.py | 15 +++++++++++----\n 1 file changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 36a4bcb..aaa0ad9 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2186,10 +2186,17 @@ class P4Sync(Command, P4UserMap):\n # them back too. This is not needed to the cygwin windows version,\n # just the native \"NT\" type.\n #\n- text = p4_read_pipe(['print', '-q', '-o', '-', \"%s@%s\" % (file['depotFile'], file['change']) ])\n- if p4_version_string().find(\"/NT\") >= 0:\n- text = text.replace(\"\\r\\n\", \"\\n\")\n- contents = [ text ]\n+ try:\n+ text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % (file['depotFile'], file['change'])])\n+ except Exception as e:\n+ if 'Translation of file content failed' in str(e):\n+ type_base = 'binary'\n+ else:\n+ raise e\n+ else:\n+ if p4_version_string().find('/NT') >= 0:\n+ text = text.replace('\\r\\n', '\\n')\n+ contents = [ text ]\n \n if type_base == \"apple\":\n # Apple filetype files will be streamed as a concatenation of\n-- \n2.5.1\n"},{"id":"269659","messageId":"xmqqbndb7fc0.fsf@gitster.mtv.corp.google.com","threadId":"40292","inReplyTo":"1441629723-90604-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v1 1/2] git-p4: print stderr if P4 read_pipe operation fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-09T16:00:31Z","receivedAt":"2015-09-09T16:00:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"larsxschneider@gmail.com writes:\n\n> From: Lars Schneider \n>\n> If read_pipe crashes then the caller can inspect the error and handle\n> it appropriately.\n>\n> Signed-off-by: Lars Schneider \n> ---\n> git-p4.py | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 073f87b..36a4bcb 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -134,11 +134,11 @@ def read_pipe(c, ignore_error=False):\n> sys.stderr.write('Reading pipe: %s\\n' % str(c))\n> \n> expand = isinstance(c,basestring)\n> - p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)\n> + p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)\n> pipe = p.stdout\n> val = pipe.read()\n> if p.wait() and not ignore_error:\n> - die('Command failed: %s' % str(c))\n> + die('Command failed: %s\\nError: %s' % (str(c), p.stderr.read()))\n\nI don't know enough about the callers of this helper function to\ntell offhand if that is an issue, but this looks unsafe depending on\nwhat the process on the other side of these pipes are doing.\n\nIf it attempts to spew a lot on its standard error stream first and\nthen write some to its standard output, I would not be surprised it\nwould get stuck waiting for us to read and drain its standard error\nbefore it can proceed to write to its standard output, and in the\nmeantime we would be waiting for it to say something on its standard\noutput, no?\n"},{"id":"269664","messageId":"6A7A961A-9DB6-4B12-91E1-D6FE1F5E0358@gmail.com","threadId":"40292","inReplyTo":"xmqqbndb7fc0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v1 1/2] git-p4: print stderr if P4 read_pipe operation fails","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-09T18:50:15Z","receivedAt":"2015-09-09T18:50:15Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 09 Sep 2015, at 18:00, Junio C Hamano wrote:\n\n> larsxschneider@gmail.com writes:\n> \n>> From: Lars Schneider \n>> \n>> If read_pipe crashes then the caller can inspect the error and handle\n>> it appropriately.\n>> \n>> Signed-off-by: Lars Schneider \n>> ---\n>> git-p4.py | 4 ++--\n>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/git-p4.py b/git-p4.py\n>> index 073f87b..36a4bcb 100755\n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -134,11 +134,11 @@ def read_pipe(c, ignore_error=False):\n>> sys.stderr.write('Reading pipe: %s\\n' % str(c))\n>> \n>> expand = isinstance(c,basestring)\n>> - p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)\n>> + p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)\n>> pipe = p.stdout\n>> val = pipe.read()\n>> if p.wait() and not ignore_error:\n>> - die('Command failed: %s' % str(c))\n>> + die('Command failed: %s\\nError: %s' % (str(c), p.stderr.read()))\n> \n> I don't know enough about the callers of this helper function to\n> tell offhand if that is an issue, but this looks unsafe depending on\n> what the process on the other side of these pipes are doing.\n> \n> If it attempts to spew a lot on its standard error stream first and\n> then write some to its standard output, I would not be surprised it\n> would get stuck waiting for us to read and drain its standard error\n> before it can proceed to write to its standard output, and in the\n> meantime we would be waiting for it to say something on its standard\n> output, no?\n> \nYou are right. I will use the “communicate” function here as recommended in the Python docs:\nhttps://docs.python.org/2/library/subprocess.html#subprocess.Popen.wait\n\nThanks!\n"}]}