{"thread":{"id":"40348","subject":"[PATCH v2 2/2] git-p4: handle \"Translation of file content failed\"","startedAt":"2015-09-14T16:55:26Z","lastAt":"2015-09-16T11:38:52Z","messageCount":9,"participants":["larsxschneider@gmail.com","Torsten Bögershausen","Luke Diamand","Lars Schneider"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"269933","messageId":"1442249728-89494-1-git-send-email-larsxschneider@gmail.com","threadId":"40348","inReplyTo":null,"subject":"[PATCH v2 0/2] git-p4: handle \"Translation of file content failed\"","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-14T16:55:26Z","receivedAt":"2015-09-14T16:55:26Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\ndiff to v1:\n* add a test case\n* use Popen \"communication\" function instead of \"wait\"\n\nThanks to Junio for feedback!\n\nKnown issue: My fix works only if git-p4 is executed in verbose mode.\nIn normal mode no exceptions are thrown and git-p4 just exits.\n\nLars Schneider (2):\n git-p4: add test case for \"Translation of file content failed\" error\n git-p4: handle \"Translation of file content failed\"\n\n git-p4.py | 27 ++++++++++-------\n t/t9824-git-p4-handle-utf16-without-bom.sh | 47 ++++++++++++++++++++++++++++++\n 2 files changed, 63 insertions(+), 11 deletions(-)\n create mode 100755 t/t9824-git-p4-handle-utf16-without-bom.sh\n\n--\n2.5.1\n"},{"id":"269934","messageId":"1442249728-89494-2-git-send-email-larsxschneider@gmail.com","threadId":"40348","inReplyTo":"1442249728-89494-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v2 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-14T16:55:27Z","receivedAt":"2015-09-14T16:55:27Z","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\ntype UTF-16 that does not contain a valid UTF-16 BOM. If git-p4\nattempts to retrieve the file then the process crashes with a\n\"Translation of file content failed\" error.\n\nSigned-off-by: Lars Schneider \n---\n t/t9824-git-p4-handle-utf16-without-bom.sh | 47 ++++++++++++++++++++++++++++++\n 1 file changed, 47 insertions(+)\n create mode 100755 t/t9824-git-p4-handle-utf16-without-bom.sh\n\ndiff --git a/t/t9824-git-p4-handle-utf16-without-bom.sh b/t/t9824-git-p4-handle-utf16-without-bom.sh\nnew file mode 100755\nindex 0000000..fa8043b\n--- /dev/null\n+++ b/t/t9824-git-p4-handle-utf16-without-bom.sh\n@@ -0,0 +1,47 @@\n+#!/bin/sh\n+\n+test_description='git p4 handle UTF-16 without BOM'\n+\n+. ./lib-git-p4.sh\n+\n+UTF16=\"\\\\x97\\\\x0\\\\x97\\\\x0\"\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+test_expect_success 'init depot with UTF-16 encoded file and artificially remove BOM' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo \"file1 -text\" > .gitattributes &&\n+\t\tperl -e \"printf \\\"$UTF16\\\"\" >file1 &&\n+\t\tp4 add -t utf16 file1 &&\n+\n+\t\tp4 add .gitattributes &&\n+\t\tp4 submit -d \"file1\"\n+\t) &&\n+\n+\t(\n+\t\tcd \"db\" &&\n+\t\tp4d -jc &&\n+\t\t# P4D automatically adds a BOM. Remove it here to make the file invalid.\n+\t\tperl -i -ne \"print unless eof\" depot/file1,v &&\n+\t\tperl -e \"printf \\\"@$UTF16@\\\"\" >> depot/file1,v &&\n+\t\tp4d -jrF checkpoint.1\n+\t)\n+'\n+\n+test_expect_success 'clone depot with invalid UTF-16 file' '\n+\tgit p4 clone --dest=\"$git\" --verbose //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tperl -e \"printf \\\"$UTF16\\\"\" > expect &&\n+\t\ttest_cmp_bin expect file1\n+\t)\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n2.5.1\n"},{"id":"269932","messageId":"1442249728-89494-3-git-send-email-larsxschneider@gmail.com","threadId":"40348","inReplyTo":"1442249728-89494-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v2 2/2] git-p4: handle \"Translation of file content failed\"","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-14T16:55:28Z","receivedAt":"2015-09-14T16:55:28Z","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\ntype UTF-16 that does not contain a valid UTF-16 BOM. If git-p4\nattempts to retrieve the file then the process crashes with a\n\"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\nKnown issue: This works only if git-p4 is executed in verbose mode.\nIn normal mode no exceptions are thrown and git-p4 just exits.\n\nSigned-off-by: Lars Schneider \n---\n git-p4.py | 27 ++++++++++++++++-----------\n 1 file changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 073f87b..5ae25a6 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -134,13 +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- pipe = p.stdout\n- val = pipe.read()\n- if p.wait() and not ignore_error:\n- die('Command failed: %s' % str(c))\n-\n- return val\n+ p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)\n+ (out, err) = p.communicate()\n+ if p.returncode != 0 and not ignore_error:\n+ die('Command failed: %s\\nError: %s' % (str(c), err))\n+ return out\n \n def p4_read_pipe(c, ignore_error=False):\n real_cmd = p4_build_cmd(c)\n@@ -2186,10 +2184,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":"269981","messageId":"55F7A12D.9020603@web.de","threadId":"40348","inReplyTo":"1442249728-89494-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v2 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-09-15T04:40:13Z","receivedAt":"2015-09-15T04:40:13Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 09/14/2015 06:55 PM, larsxschneider@gmail.com wrote:\n> From: Lars Schneider \n>\n> A P4 repository can get into a state where it contains a file with\n> type UTF-16 that does not contain a valid UTF-16 BOM. If git-p4\n> attempts to retrieve the file then the process crashes with a\n> \"Translation of file content failed\" error.\n>\n> Signed-off-by: Lars Schneider \n> ---\n> t/t9824-git-p4-handle-utf16-without-bom.sh | 47 ++++++++++++++++++++++++++++++\n> 1 file changed, 47 insertions(+)\n> create mode 100755 t/t9824-git-p4-handle-utf16-without-bom.sh\n>\n> diff --git a/t/t9824-git-p4-handle-utf16-without-bom.sh b/t/t9824-git-p4-handle-utf16-without-bom.sh\n> new file mode 100755\n> index 0000000..fa8043b\n> --- /dev/null\n> +++ b/t/t9824-git-p4-handle-utf16-without-bom.sh\n> @@ -0,0 +1,47 @@\n> +#!/bin/sh\n> +\n> +test_description='git p4 handle UTF-16 without BOM'\n> +\n> +. ./lib-git-p4.sh\n> +\n> +UTF16=\"\\\\x97\\\\x0\\\\x97\\\\x0\"\n> +\n> +test_expect_success 'start p4d' '\n> +\tstart_p4d\n> +'\n> +\n> +test_expect_success 'init depot with UTF-16 encoded file and artificially remove BOM' '\n> +\t(\n> +\t\tcd \"$cli\" &&\n> +\t\techo \"file1 -text\" > .gitattributes &&\nPlease no space between '>' and the filename,\n(this is our coding standard, and the same further down)\n\n> +\t\tperl -e \"printf \\\"$UTF16\\\"\" >file1 &&\nEhh, do we need perl here ?\nThis will invoke a process-fork, which costs time and cpu load.\nThe following works for me:\nprintf '\\227\\000\\227\\000' >file1\n"},{"id":"269983","messageId":"55F7BE2A.80609@diamand.org","threadId":"40348","inReplyTo":"1442249728-89494-3-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v2 2/2] git-p4: handle \"Translation of file content failed\"","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-09-15T06:43:54Z","receivedAt":"2015-09-15T06:43:54Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 14/09/15 17:55, larsxschneider@gmail.com wrote:\n> From: Lars Schneider \n>\n> A P4 repository can get into a state where it contains a file with\n> type UTF-16 that does not contain a valid UTF-16 BOM. If git-p4\n\nSorry - what's a BOM? I'm assuming it's not a Bill of Materials?\n\nDo we know the mechanism by which we end up in this state?\n\n\n\n> attempts to retrieve the file then the process crashes with a\n> \"Translation of file content failed\" error.\n>\n> Fix this by detecting this error and retrieving the file as binary\n> instead. The result in Git is the same.\n>\n> Known issue: This works only if git-p4 is executed in verbose mode.\n> In normal mode no exceptions are thrown and git-p4 just exits.\n\nDoes that mean that the error will only be detected in verbose mode? \nThat doesn't seem right!\n\n>\n> Signed-off-by: Lars Schneider \n> ---\n> git-p4.py | 27 ++++++++++++++++-----------\n> 1 file changed, 16 insertions(+), 11 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 073f87b..5ae25a6 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -134,13 +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> - pipe = p.stdout\n> - val = pipe.read()\n> - if p.wait() and not ignore_error:\n> - die('Command failed: %s' % str(c))\n> -\n> - return val\n> + p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)\n> + (out, err) = p.communicate()\n> + if p.returncode != 0 and not ignore_error:\n> + die('Command failed: %s\\nError: %s' % (str(c), err))\n> + return out\n>\n> def p4_read_pipe(c, ignore_error=False):\n> real_cmd = p4_build_cmd(c)\n> @@ -2186,10 +2184,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\nWould it be better to specify which kind of Exception you are catching? \nLooks like you could get OSError, ValueError and CalledProcessError; \nit's the last of these you want (I think).\n\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\nThe indentation on this bit doesn't look right to me.\n\n>\n> if type_base == \"apple\":\n> # Apple filetype files will be streamed as a concatenation of\n>\n\nLuke\n"},{"id":"269996","messageId":"EA7A521F-1E7B-4169-A0EA-65F16904EE92@gmail.com","threadId":"40348","inReplyTo":"55F7A12D.9020603@web.de","subject":"Re: [PATCH v2 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-15T14:49:44Z","receivedAt":"2015-09-15T14:49:44Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 15 Sep 2015, at 06:40, Torsten Bögershausen wrote:\n\n> On 09/14/2015 06:55 PM, larsxschneider@gmail.com wrote:\n>> From: Lars Schneider \n>> \n>> A P4 repository can get into a state where it contains a file with\n>> type UTF-16 that does not contain a valid UTF-16 BOM. If git-p4\n>> attempts to retrieve the file then the process crashes with a\n>> \"Translation of file content failed\" error.\n>> \n>> Signed-off-by: Lars Schneider \n>> ---\n>> t/t9824-git-p4-handle-utf16-without-bom.sh | 47 ++++++++++++++++++++++++++++++\n>> 1 file changed, 47 insertions(+)\n>> create mode 100755 t/t9824-git-p4-handle-utf16-without-bom.sh\n>> \n>> diff --git a/t/t9824-git-p4-handle-utf16-without-bom.sh b/t/t9824-git-p4-handle-utf16-without-bom.sh\n>> new file mode 100755\n>> index 0000000..fa8043b\n>> --- /dev/null\n>> +++ b/t/t9824-git-p4-handle-utf16-without-bom.sh\n>> @@ -0,0 +1,47 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='git p4 handle UTF-16 without BOM'\n>> +\n>> +. ./lib-git-p4.sh\n>> +\n>> +UTF16=\"\\\\x97\\\\x0\\\\x97\\\\x0\"\n>> +\n>> +test_expect_success 'start p4d' '\n>> +\tstart_p4d\n>> +'\n>> +\n>> +test_expect_success 'init depot with UTF-16 encoded file and artificially remove BOM' '\n>> +\t(\n>> +\t\tcd \"$cli\" &&\n>> +\t\techo \"file1 -text\" > .gitattributes &&\n> Please no space between '>' and the filename,\n> (this is our coding standard, and the same further down)\nCorrect! Sorry, I still need to get used to this style. Thanks for the reminder!\n\n> \n>> +\t\tperl -e \"printf \\\"$UTF16\\\"\" >file1 &&\n> Ehh, do we need perl here ?\n> This will invoke a process-fork, which costs time and cpu load.\n> The following works for me:\n> printf '\\227\\000\\227\\000' >file1\nI agree this is better.\n\nBoth issues will be fixed v3.\n\nThanks!"},{"id":"270016","messageId":"5FF97F9B-E2FB-4919-BCAF-5462ED9D1EEE@gmail.com","threadId":"40348","inReplyTo":"55F7BE2A.80609@diamand.org","subject":"Re: [PATCH v2 2/2] git-p4: handle \"Translation of file content failed\"","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-15T15:38:41Z","receivedAt":"2015-09-15T15:38:41Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 15 Sep 2015, at 08:43, Luke Diamand wrote:\n\n> On 14/09/15 17:55, larsxschneider@gmail.com wrote:\n>> From: Lars Schneider \n>> \n>> A P4 repository can get into a state where it contains a file with\n>> type UTF-16 that does not contain a valid UTF-16 BOM. If git-p4\n> \n> Sorry - what's a BOM? I'm assuming it's not a Bill of Materials?\nBOM stands for Byte Order Mark. The UTF-16 BOM is a two byte sequence at the beginning of a UTF-16 file. It is not part of the actual content. It is only used to define the encoding of the remaining file. FEFF stands for UTF-16 big-endian encoding and FFFE for little-endian encoding.\n\nMore info here: http://www.unicode.org/faq/utf_bom.html#bom1\n\n\n> Do we know the mechanism by which we end up in this state?\nUnfortunately no. I tried hard to reproduce the error with “conventional” methods. As you can see I ended up manipulating the P4 database…\n\nHowever, it looks like this error happens in the wild, too:\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\n\n>> attempts to retrieve the file then the process crashes with a\n>> \"Translation of file content failed\" error.\n>> \n>> Fix this by detecting this error and retrieving the file as binary\n>> instead. The result in Git is the same.\n>> \n>> Known issue: This works only if git-p4 is executed in verbose mode.\n>> In normal mode no exceptions are thrown and git-p4 just exits.\n> \n> Does that mean that the error will only be detected in verbose mode? That doesn't seem right!\nCorrect. I don’t like this either but I also don’t want to make huge changes to git-p4.\nYou can see the root problem here:\nhttps://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L110-L114\n\nAny idea how to approach that best?\n\n\n> \n>> \n>> Signed-off-by: Lars Schneider \n>> ---\n>> git-p4.py | 27 ++++++++++++++++-----------\n>> 1 file changed, 16 insertions(+), 11 deletions(-)\n>> \n>> diff --git a/git-p4.py b/git-p4.py\n>> index 073f87b..5ae25a6 100755\n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -134,13 +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>> - pipe = p.stdout\n>> - val = pipe.read()\n>> - if p.wait() and not ignore_error:\n>> - die('Command failed: %s' % str(c))\n>> -\n>> - return val\n>> + p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)\n>> + (out, err) = p.communicate()\n>> + if p.returncode != 0 and not ignore_error:\n>> + die('Command failed: %s\\nError: %s' % (str(c), err))\n>> + return out\n>> \n>> def p4_read_pipe(c, ignore_error=False):\n>> real_cmd = p4_build_cmd(c)\n>> @@ -2186,10 +2184,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> \n> Would it be better to specify which kind of Exception you are catching? Looks like you could get OSError, ValueError and CalledProcessError; it's the last of these you want (I think).\nI think it is just a plain exception. See here:\nhttps://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L111\n\n\n> \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> The indentation on this bit doesn't look right to me.\nI believe it is exactly how it was:\nhttps://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L2397-L2399\n\n\nIn general, what is the appropriate way to reference code in this email list? Are GitHub links OK?\n\n\nThanks,\nLars\n"},{"id":"270082","messageId":"55F897D0.9000100@diamand.org","threadId":"40348","inReplyTo":"5FF97F9B-E2FB-4919-BCAF-5462ED9D1EEE@gmail.com","subject":"Re: [PATCH v2 2/2] git-p4: handle \"Translation of file content failed\"","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-09-15T22:12:32Z","receivedAt":"2015-09-15T22:12:32Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 15/09/15 16:38, Lars Schneider wrote:\n>\n> On 15 Sep 2015, at 08:43, Luke Diamand wrote:\n>\n\n\n>> Do we know the mechanism by which we end up in this state?\n> Unfortunately no. I tried hard to reproduce the error with “conventional” methods. As you can see I ended up manipulating the P4 database…\n>\n> However, it looks like this error happens in the wild, too:\n> https://stackoverflow.com/questions/5156909/translation-of-file-content-failed-error-in-perforce\n> https://stackoverflow.com/questions/887006/perforce-translation-of-file-content-failed-error\n\nIt's described in the Perforce FAQ here:\n\nhttp://answers.perforce.com/articles/KB/3117\n\ni.e. it looks to be caused by mixing old and new P4 clients.\n\n>>>\n>>> Known issue: This works only if git-p4 is executed in verbose mode.\n>>> In normal mode no exceptions are thrown and git-p4 just exits.\n>>\n>> Does that mean that the error will only be detected in verbose mode? That doesn't seem right!\n> Correct. I don’t like this either but I also don’t want to make huge changes to git-p4.\n> You can see the root problem here:\n> https://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L110-L114\n>\n> Any idea how to approach that best?\n\nI guess what we have is not ideal but probably good enough.\n\n\n>>> + try:\n>>> + text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % (file['depotFile'], file['change'])])\n>>> + except Exception as e:\n>>\n>> Would it be better to specify which kind of Exception you are catching? Looks like you could get OSError, ValueError and CalledProcessError; it's the last of these you want (I think).\n> I think it is just a plain exception. See here:\n> https://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L111\n\nOK, you're right (probably less than ideal behaviour from read_pipe() \nand die() but let's not try to fix that).\n\n\n>>> + if p4_version_string().find('/NT') >= 0:\n>>> + text = text.replace('\\r\\n', '\\n')\n>>> + contents = [ text ]\n>>\n>> The indentation on this bit doesn't look right to me.\n> I believe it is exactly how it was:\n> https://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L2397-L2399\n\nOK.\n\n>\n>\n> In general, what is the appropriate way to reference code in this email list? Are GitHub links OK?\n\nI'm not an expert, but it feels possibly a bit ephemeral - if someone is \ndigging through email archives in a future where that github project has \nbeen moved elsewhere, the links will all be dead.\n\nLuke\n"},{"id":"270118","messageId":"C8BD1D98-21BB-420B-884E-79D0FC71D119@gmail.com","threadId":"40348","inReplyTo":"55F897D0.9000100@diamand.org","subject":"Re: [PATCH v2 2/2] git-p4: handle \"Translation of file content failed\"","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-16T11:38:52Z","receivedAt":"2015-09-16T11:38:52Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 16 Sep 2015, at 00:12, Luke Diamand wrote:\n\n> On 15/09/15 16:38, Lars Schneider wrote:\n>> \n>> On 15 Sep 2015, at 08:43, Luke Diamand wrote:\n>> \n> \n> \n>>> Do we know the mechanism by which we end up in this state?\n>> Unfortunately no. I tried hard to reproduce the error with “conventional” methods. As you can see I ended up manipulating the P4 database…\n>> \n>> However, it looks like this error happens in the wild, too:\n>> https://stackoverflow.com/questions/5156909/translation-of-file-content-failed-error-in-perforce\n>> https://stackoverflow.com/questions/887006/perforce-translation-of-file-content-failed-error\n> \n> It's described in the Perforce FAQ here:\n> \n> http://answers.perforce.com/articles/KB/3117\n> \n> i.e. it looks to be caused by mixing old and new P4 clients.\nGood find! No idea why I did not find this article before… I will copy the text from the KB into the git commit message to explain the problem.\n \n> \n>>>> \n>>>> Known issue: This works only if git-p4 is executed in verbose mode.\n>>>> In normal mode no exceptions are thrown and git-p4 just exits.\n>>> \n>>> Does that mean that the error will only be detected in verbose mode? That doesn't seem right!\n>> Correct. I don’t like this either but I also don’t want to make huge changes to git-p4.\n>> You can see the root problem here:\n>> https://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L110-L114\n>> \n>> Any idea how to approach that best?\n> \n> I guess what we have is not ideal but probably good enough.\nok. thanks!\n\nI will add another test case without “—verbose\" to document that there is work to do :-)\n\n> \n> \n>>>> + try:\n>>>> + text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % (file['depotFile'], file['change'])])\n>>>> + except Exception as e:\n>>> \n>>> Would it be better to specify which kind of Exception you are catching? Looks like you could get OSError, ValueError and CalledProcessError; it's the last of these you want (I think).\n>> I think it is just a plain exception. See here:\n>> https://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L111\n> \n> OK, you're right (probably less than ideal behaviour from read_pipe() and die() but let's not try to fix that).\nok\n\n> \n> \n>>>> + if p4_version_string().find('/NT') >= 0:\n>>>> + text = text.replace('\\r\\n', '\\n')\n>>>> + contents = [ text ]\n>>> \n>>> The indentation on this bit doesn't look right to me.\n>> I believe it is exactly how it was:\n>> https://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L2397-L2399\n> \n> OK.\n> \n>> \n>> \n>> In general, what is the appropriate way to reference code in this email list? Are GitHub links OK?\n> \n> I'm not an expert, but it feels possibly a bit ephemeral - if someone is digging through email archives in a future where that github project has been moved elsewhere, the links will all be dead.\n\nRight. However, you could disassemble the URL and use the commit hash, the filename and the line number. They are not ephemeral because they are part of the repo.\n\nThanks,\nLars"}]}