{"thread":{"id":"40387","subject":"[PATCH v3 2/2] git-p4: handle \"Translation of file content failed\"","startedAt":"2015-09-20T16:22:09Z","lastAt":"2015-09-21T17:43:48Z","messageCount":12,"participants":["larsxschneider@gmail.com","Eric Sunshine","Lars Schneider","Luke Diamand","Torsten Bögershausen","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"270340","messageId":"1442766131-45017-1-git-send-email-larsxschneider@gmail.com","threadId":"40387","inReplyTo":null,"subject":"[PATCH v3 0/2] git-p4: handle \"Translation of file content failed\"","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-20T16:22:09Z","receivedAt":"2015-09-20T16:22:09Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\ndiff to v2:\n* remove Perl calls for printing characters (Thanks Torsten!)\n* remove whitespaces after \">\" (Thanks Torsten!)\n* add test case to show that the fix is not working for non \"--verbose\" mode (Thanks Luke!)\n* add P4 knowledge base link with detailed error description to commit message (Thanks Luke!)\n* remove unnecessary .gitattributes file in test data\n\nCheers,\nLars\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 | 49 ++++++++++++++++++++++++++++++\n 2 files changed, 65 insertions(+), 11 deletions(-)\n create mode 100755 t/t9824-git-p4-handle-utf16-without-bom.sh\n\n--\n2.5.1\n"},{"id":"270341","messageId":"1442766131-45017-2-git-send-email-larsxschneider@gmail.com","threadId":"40387","inReplyTo":"1442766131-45017-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v3 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-20T16:22:10Z","receivedAt":"2015-09-20T16:22:10Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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\nMore info here: http://answers.perforce.com/articles/KB/3117\n\nSigned-off-by: Lars Schneider <larsxschneider@gmail.com>\n---\n t/t9824-git-p4-handle-utf16-without-bom.sh | 49 ++++++++++++++++++++++++++++++\n 1 file changed, 49 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..517f6da\n--- /dev/null\n+++ b/t/t9824-git-p4-handle-utf16-without-bom.sh\n@@ -0,0 +1,49 @@\n+#!/bin/sh\n+\n+test_description='git p4 handling of UTF-16 files without BOM'\n+\n+. ./lib-git-p4.sh\n+\n+UTF16=\"\\227\\000\\227\\000\"\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\tprintf \"$UTF16\" >file1 &&\n+\t\tp4 add -t utf16 file1 &&\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\tsed -i.bak \"$ d\" depot/file1,v &&\n+\t\tprintf \"@$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 in verbose mode' '\n+\tgit p4 clone --dest=\"$git\" --verbose //depot &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tprintf \"$UTF16\" >expect &&\n+\t\ttest_cmp_bin expect file1\n+\t)\n+'\n+\n+test_expect_failure 'clone depot with invalid UTF-16 file in non-verbose mode' '\n+\tgit p4 clone --dest=\"$git\" //depot\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n2.5.1\n"},{"id":"270339","messageId":"1442766131-45017-3-git-send-email-larsxschneider@gmail.com","threadId":"40387","inReplyTo":"1442766131-45017-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v3 2/2] git-p4: handle \"Translation of file content failed\"","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-20T16:22:11Z","receivedAt":"2015-09-20T16:22:11Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\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\nMore info here: http://answers.perforce.com/articles/KB/3117\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 <larsxschneider@gmail.com>\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":"270365","messageId":"CAPig+cThcNa2Xg6GDkNKKeOMOzApTi_FQfQz_KRAbF8-PWY5Eg@mail.gmail.com","threadId":"40387","inReplyTo":"1442766131-45017-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v3 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-20T21:16:51Z","receivedAt":"2015-09-20T21:16:51Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Sep 20, 2015 at 12:22 PM,  <larsxschneider@gmail.com> wrote:\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\nHmm, are these tests going to succeed only after patch 2/2 is applied?\nIf so, the order of these patches is backward since you want each\npatch to be able to stand on its own and not introduce any sort of\nbreakage.\n\n> More info here: http://answers.perforce.com/articles/KB/3117\n>\n> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>\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..517f6da\n> --- /dev/null\n> +++ b/t/t9824-git-p4-handle-utf16-without-bom.sh\n> @@ -0,0 +1,49 @@\n> +#!/bin/sh\n> +\n> +test_description='git p4 handling of UTF-16 files without BOM'\n> +\n> +. ./lib-git-p4.sh\n> +\n> +UTF16=\"\\227\\000\\227\\000\"\n> +\n> +test_expect_success 'start p4d' '\n> +       start_p4d\n> +'\n> +\n> +test_expect_success 'init depot with UTF-16 encoded file and artificially remove BOM' '\n> +       (\n> +               cd \"$cli\" &&\n> +               printf \"$UTF16\" >file1 &&\n> +               p4 add -t utf16 file1 &&\n> +               p4 submit -d \"file1\"\n> +       ) &&\n> +\n> +       (\n> +               cd \"db\" &&\n> +               p4d -jc &&\n> +               # P4D automatically adds a BOM. Remove it here to make the file invalid.\n> +               sed -i.bak \"$ d\" depot/file1,v &&\n> +               printf \"@$UTF16@\" >>depot/file1,v &&\n> +               p4d -jrF checkpoint.1\n> +       )\n> +'\n> +\n> +test_expect_success 'clone depot with invalid UTF-16 file in verbose mode' '\n> +       git p4 clone --dest=\"$git\" --verbose //depot &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\" &&\n> +               printf \"$UTF16\" >expect &&\n> +               test_cmp_bin expect file1\n> +       )\n> +'\n> +\n> +test_expect_failure 'clone depot with invalid UTF-16 file in non-verbose mode' '\n> +       git p4 clone --dest=\"$git\" //depot\n> +'\n> +\n> +test_expect_success 'kill p4d' '\n> +       kill_p4d\n> +'\n> +\n> +test_done\n> --\n> 2.5.1\n"},{"id":"270367","messageId":"4DB8A17D-A670-4E84-8FA8-9B05F2DCE1CB@gmail.com","threadId":"40387","inReplyTo":"CAPig+cThcNa2Xg6GDkNKKeOMOzApTi_FQfQz_KRAbF8-PWY5Eg@mail.gmail.com","subject":"Re: [PATCH v3 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-20T21:34:23Z","receivedAt":"2015-09-20T21:34:23Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 20 Sep 2015, at 23:16, Eric Sunshine <sunshine@sunshineco.com> wrote:\n\n> On Sun, Sep 20, 2015 at 12:22 PM,  <larsxschneider@gmail.com> wrote:\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> Hmm, are these tests going to succeed only after patch 2/2 is applied?\n> If so, the order of these patches is backward since you want each\n> patch to be able to stand on its own and not introduce any sort of\n> breakage.\nYes, these tests succeed only after 2/2. I think I saw this approach somewhere in the Git history. I thought it would ease the reviewing process: show the problem in the first commit, fix it in a subsequent commit.\nHowever, I understand your point as 1/2 would break the build.\n\nWhat is the preferred way by the Git community? Combine patch and test in one commit or a patch commit followed by a test commit? I would prefer to have everything in one commit.\n\n- Lars\n"},{"id":"270368","messageId":"CAPig+cSFbYzLi8daS55B_sdvDPW4q6yOC5Hm75+Rmr-s1aFRHw@mail.gmail.com","threadId":"40387","inReplyTo":"4DB8A17D-A670-4E84-8FA8-9B05F2DCE1CB@gmail.com","subject":"Re: [PATCH v3 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-20T22:29:41Z","receivedAt":"2015-09-20T22:29:41Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Sep 20, 2015 at 5:34 PM, Lars Schneider\n<larsxschneider@gmail.com> wrote:\n> On 20 Sep 2015, at 23:16, Eric Sunshine <sunshine@sunshineco.com> wrote:\n>> On Sun, Sep 20, 2015 at 12:22 PM,  <larsxschneider@gmail.com> wrote:\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>> Hmm, are these tests going to succeed only after patch 2/2 is applied?\n>> If so, the order of these patches is backward since you want each\n>> patch to be able to stand on its own and not introduce any sort of\n>> breakage.\n>\n> Yes, these tests succeed only after 2/2. I think I saw this approach\n> somewhere in the Git history. I thought it would ease the reviewing\n> process: show the problem in the first commit, fix it in a\n> subsequent commit.  However, I understand your point as 1/2 would\n> break the build.\n\nYes, people sometimes do that, however, the patch which demonstrates\nthe problem uses test_expect_failure, and the follow-up patch which\nfixes the problem flips it to test_expect_success.\n\n> What is the preferred way by the Git community? Combine patch and\n> test in one commit or a patch commit followed by a test commit? I\n> would prefer to have everything in one commit.\n\nIf the tests are in a separate patch, Junio seems to prefer adding\nthem after the problem is fixes; the idea being that tests are added\nto ensure that some future change doesn't break the feature, as\nopposed to showing that your patch fixes a bug.\n\nWhether or not to combine the fix with the new tests often depends\nupon the length of the patches and how easy or hard it is to review\nthem. In this case, the fix itself is fairly short, but the tests are\nslightly lengthy, so there may not be a clear cut answer. As a\nreviewer, I tend to prefer smaller patches, however, this situation\ndoesn't demand it, so use your best judgment.\n"},{"id":"270378","messageId":"55FFB614.7000908@diamand.org","threadId":"40387","inReplyTo":"CAPig+cSFbYzLi8daS55B_sdvDPW4q6yOC5Hm75+Rmr-s1aFRHw@mail.gmail.com","subject":"Re: [PATCH v3 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-09-21T07:47:32Z","receivedAt":"2015-09-21T07:47:32Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 20/09/15 23:29, Eric Sunshine wrote:\n> On Sun, Sep 20, 2015 at 5:34 PM, Lars Schneider\n> <larsxschneider@gmail.com> wrote:\n>\n>> What is the preferred way by the Git community? Combine patch and\n>> test in one commit or a patch commit followed by a test commit? I\n>> would prefer to have everything in one commit.\n>\n> If the tests are in a separate patch, Junio seems to prefer adding\n> them after the problem is fixes; the idea being that tests are added\n> to ensure that some future change doesn't break the feature, as\n> opposed to showing that your patch fixes a bug.\n>\n> Whether or not to combine the fix with the new tests often depends\n> upon the length of the patches and how easy or hard it is to review\n> them. In this case, the fix itself is fairly short, but the tests are\n> slightly lengthy, so there may not be a clear cut answer. As a\n> reviewer, I tend to prefer smaller patches, however, this situation\n> doesn't demand it, so use your best judgment.\n\nI think in the past we have a test added that demonstrates the problem, \nwith \"test_expect_failure\", followed by the fix, which also flips the \ntest to \"test_expect_success\".\n\nLuke\n"},{"id":"270379","messageId":"55FFB681.50500@diamand.org","threadId":"40387","inReplyTo":"1442766131-45017-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v3 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-09-21T07:49:21Z","receivedAt":"2015-09-21T07:49:21Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 20/09/15 17:22, larsxschneider@gmail.com wrote:\n> From: Lars Schneider <larsxschneider@gmail.com>\n\nWhen I run this, I get errors reported on the sed usage:\n\nt9824-git-p4-handle-utf16-without-bom.sh:25: error: sed -i is not \nportable:             sed -i.bak \"$ d\" depot/file1,v &&\nt9824-git-p4-handle-utf16-without-bom.sh:25: error: sed -i is not \nportable:             sed -i.bak \"$ d\" depot/file1,v &&\n\n\nLuke\n\n\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> More info here: http://answers.perforce.com/articles/KB/3117\n>\n> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>\n> ---\n>   t/t9824-git-p4-handle-utf16-without-bom.sh | 49 ++++++++++++++++++++++++++++++\n>   1 file changed, 49 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..517f6da\n> --- /dev/null\n> +++ b/t/t9824-git-p4-handle-utf16-without-bom.sh\n> @@ -0,0 +1,49 @@\n> +#!/bin/sh\n> +\n> +test_description='git p4 handling of UTF-16 files without BOM'\n> +\n> +. ./lib-git-p4.sh\n> +\n> +UTF16=\"\\227\\000\\227\\000\"\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\tprintf \"$UTF16\" >file1 &&\n> +\t\tp4 add -t utf16 file1 &&\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\tsed -i.bak \"$ d\" depot/file1,v &&\n\nThis line is the problem I think.\n\n\n> +\t\tprintf \"@$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 in verbose mode' '\n> +\tgit p4 clone --dest=\"$git\" --verbose //depot &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\tprintf \"$UTF16\" >expect &&\n> +\t\ttest_cmp_bin expect file1\n> +\t)\n> +'\n> +\n> +test_expect_failure 'clone depot with invalid UTF-16 file in non-verbose mode' '\n> +\tgit p4 clone --dest=\"$git\" //depot\n> +'\n> +\n> +test_expect_success 'kill p4d' '\n> +\tkill_p4d\n> +'\n> +\n> +test_done\n>\n"},{"id":"270384","messageId":"0BCEFDB9-9042-4D42-8775-AC22C3658F71@gmail.com","threadId":"40387","inReplyTo":"55FFB681.50500@diamand.org","subject":"Re: [PATCH v3 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-21T09:05:06Z","receivedAt":"2015-09-21T09:05:06Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 21 Sep 2015, at 09:49, Luke Diamand <luke@diamand.org> wrote:\n\n> On 20/09/15 17:22, larsxschneider@gmail.com wrote:\n>> From: Lars Schneider <larsxschneider@gmail.com>\n> \n> When I run this, I get errors reported on the sed usage:\n> \n> t9824-git-p4-handle-utf16-without-bom.sh:25: error: sed -i is not portable:             sed -i.bak \"$ d\" depot/file1,v &&\n> t9824-git-p4-handle-utf16-without-bom.sh:25: error: sed -i is not portable:             sed -i.bak \"$ d\" depot/file1,v &&\n\nI tried it on OS X 10.9.5 and on Ubuntu Linux 14.04.1 (sed version 4.2.2). \n\nThe “-i” option is mentioned in the GNU sed docs here: \nhttps://www.gnu.org/software/sed/manual/sed.html\n\nThe OS X sed man page indeed lists “-i” as non-standard option:\nhttps://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man1/sed.1.html\n\nWhat OS/sed version are you using?\n\nThanks,\nLars\n\n\n> \n> \n> Luke\n> \n> \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>> More info here: http://answers.perforce.com/articles/KB/3117\n>> \n>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>\n>> ---\n>>  t/t9824-git-p4-handle-utf16-without-bom.sh | 49 ++++++++++++++++++++++++++++++\n>>  1 file changed, 49 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..517f6da\n>> --- /dev/null\n>> +++ b/t/t9824-git-p4-handle-utf16-without-bom.sh\n>> @@ -0,0 +1,49 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='git p4 handling of UTF-16 files without BOM'\n>> +\n>> +. ./lib-git-p4.sh\n>> +\n>> +UTF16=\"\\227\\000\\227\\000\"\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\tprintf \"$UTF16\" >file1 &&\n>> +\t\tp4 add -t utf16 file1 &&\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\tsed -i.bak \"$ d\" depot/file1,v &&\n> \n> This line is the problem I think.\n> \n> \n>> +\t\tprintf \"@$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 in verbose mode' '\n>> +\tgit p4 clone --dest=\"$git\" --verbose //depot &&\n>> +\ttest_when_finished cleanup_git &&\n>> +\t(\n>> +\t\tcd \"$git\" &&\n>> +\t\tprintf \"$UTF16\" >expect &&\n>> +\t\ttest_cmp_bin expect file1\n>> +\t)\n>> +'\n>> +\n>> +test_expect_failure 'clone depot with invalid UTF-16 file in non-verbose mode' '\n>> +\tgit p4 clone --dest=\"$git\" //depot\n>> +'\n>> +\n>> +test_expect_success 'kill p4d' '\n>> +\tkill_p4d\n>> +'\n>> +\n>> +test_done\n"},{"id":"270385","messageId":"55FFCA89.601@web.de","threadId":"40387","inReplyTo":"0BCEFDB9-9042-4D42-8775-AC22C3658F71@gmail.com","subject":"Re: [PATCH v3 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-21T09:14:49Z","receivedAt":"2015-09-21T09:14:49Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 09/21/2015 11:05 AM, Lars Schneider wrote:\n> I tried it on OS X 10.9.5 and on Ubuntu Linux 14.04.1 (sed version 4.2.2).\n>\n> The “-i” option is mentioned in the GNU sed docs here:\n> https://www.gnu.org/software/sed/manual/sed.html\n>\n> The OS X sed man page indeed lists “-i” as non-standard option:\n> https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man1/sed.1.html\n>\n> What OS/sed version are you using?\n>\n> Thanks,\n> Lars\n>\n>\nsed -i is not portable.\n(I think I missed that in the review :-()\n\nThe gnu version of sed supports \"-i\", but the POSIX doesn't:\n<http://pubs.opengroup.org/onlinepubs/007904975/utilities/sed.html>\n"},{"id":"270411","messageId":"xmqqzj0facvr.fsf@gitster.mtv.corp.google.com","threadId":"40387","inReplyTo":"4DB8A17D-A670-4E84-8FA8-9B05F2DCE1CB@gmail.com","subject":"Re: [PATCH v3 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-21T17:41:28Z","receivedAt":"2015-09-21T17:41:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider <larsxschneider@gmail.com> writes:\n\n> On 20 Sep 2015, at 23:16, Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n>> On Sun, Sep 20, 2015 at 12:22 PM,  <larsxschneider@gmail.com> wrote:\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>> Hmm, are these tests going to succeed only after patch 2/2 is applied?\n>> If so, the order of these patches is backward since you want each\n>> patch to be able to stand on its own and not introduce any sort of\n>> breakage.\n> Yes, these tests succeed only after 2/2. I think I saw this approach\n> somewhere in the Git history. I thought it would ease the reviewing\n> process: show the problem in the first commit, fix it in a subsequent\n> commit.\n> However, I understand your point as 1/2 would break the build.\n>\n> What is the preferred way by the Git community? Combine patch and test\n> in one commit or a patch commit followed by a test commit? I would\n> prefer to have everything in one commit.\n\nA single patch is fine and usually preferable when the patch does\nnot span all over the tree.\n"},{"id":"270412","messageId":"xmqqvbb3acrv.fsf@gitster.mtv.corp.google.com","threadId":"40387","inReplyTo":"0BCEFDB9-9042-4D42-8775-AC22C3658F71@gmail.com","subject":"Re: [PATCH v3 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-21T17:43:48Z","receivedAt":"2015-09-21T17:43:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider <larsxschneider@gmail.com> writes:\n\n> What OS/sed version are you using?\n\nYou should go with POSIX.1\n\nhttp://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n"}]}