{"thread":{"id":"40394","subject":"[PATCH v4 0/2] git-p4: handle \"Translation of file content failed\"","startedAt":"2015-09-21T10:01:39Z","lastAt":"2015-09-23T07:34:37Z","messageCount":14,"participants":["larsxschneider@gmail.com","Junio C Hamano","Lars Schneider","Eric Sunshine","Michael Blume"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"270389","messageId":"1442829701-2347-1-git-send-email-larsxschneider@gmail.com","threadId":"40394","inReplyTo":null,"subject":"[PATCH v4 0/2] git-p4: handle \"Translation of file content failed\"","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-21T10:01:39Z","receivedAt":"2015-09-21T10:01:39Z","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 v3:\n* replace non portable \"sed -i\" call in test case (thanks Luke and Torsten!)\n* use \"test_expect_failure\" in the first commit that adds the test case, flip it to \"test_expect_success\" in subsequent commit (thanks Eric and Luke!)\n* rename test case from t9824... to t9825... to avoid clashes with \"git-p4: add Git LFS backend for large file system\" patch\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/t9825-git-p4-handle-utf16-without-bom.sh | 50 ++++++++++++++++++++++++++++++\n 2 files changed, 66 insertions(+), 11 deletions(-)\n create mode 100755 t/t9825-git-p4-handle-utf16-without-bom.sh\n\n--\n2.5.1\n"},{"id":"270390","messageId":"1442829701-2347-2-git-send-email-larsxschneider@gmail.com","threadId":"40394","inReplyTo":"1442829701-2347-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v4 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-21T10:01:40Z","receivedAt":"2015-09-21T10:01:40Z","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/t9825-git-p4-handle-utf16-without-bom.sh | 50 ++++++++++++++++++++++++++++++\n 1 file changed, 50 insertions(+)\n create mode 100755 t/t9825-git-p4-handle-utf16-without-bom.sh\n\ndiff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh\nnew file mode 100755\nindex 0000000..65c3c4e\n--- /dev/null\n+++ b/t/t9825-git-p4-handle-utf16-without-bom.sh\n@@ -0,0 +1,50 @@\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 -e \"$ d\" depot/file1,v >depot/file1,v.new &&\n+\t\tmv -- depot/file1,v.new depot/file1,v &&\n+\t\tprintf \"@$UTF16@\" >>depot/file1,v &&\n+\t\tp4d -jrF checkpoint.1\n+\t)\n+'\n+\n+test_expect_failure '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":"270391","messageId":"1442829701-2347-3-git-send-email-larsxschneider@gmail.com","threadId":"40394","inReplyTo":"1442829701-2347-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v4 2/2] git-p4: handle \"Translation of file content failed\"","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-21T10:01:41Z","receivedAt":"2015-09-21T10:01:41Z","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 t/t9825-git-p4-handle-utf16-without-bom.sh |  2 +-\n 2 files changed, 17 insertions(+), 12 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\ndiff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh\nindex 65c3c4e..fd2edce 100755\n--- a/t/t9825-git-p4-handle-utf16-without-bom.sh\n+++ b/t/t9825-git-p4-handle-utf16-without-bom.sh\n@@ -29,7 +29,7 @@ test_expect_success 'init depot with UTF-16 encoded file and artificially remove\n \t)\n '\n \n-test_expect_failure 'clone depot with invalid UTF-16 file in verbose mode' '\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-- \n2.5.1\n"},{"id":"270415","messageId":"xmqqio73abl0.fsf@gitster.mtv.corp.google.com","threadId":"40394","inReplyTo":"1442829701-2347-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 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-21T18:09:31Z","receivedAt":"2015-09-21T18:09: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 <larsxschneider@gmail.com>\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/t9825-git-p4-handle-utf16-without-bom.sh | 50 ++++++++++++++++++++++++++++++\n>  1 file changed, 50 insertions(+)\n>  create mode 100755 t/t9825-git-p4-handle-utf16-without-bom.sh\n>\n> diff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh\n> b/t/t9825-git-p4-handle-utf16-without-bom.sh\n> new file mode 100755\n> index 0000000..65c3c4e\n> --- /dev/null\n> +++ b/t/t9825-git-p4-handle-utf16-without-bom.sh\n> @@ -0,0 +1,50 @@\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 -e \"$ d\" depot/file1,v >depot/file1,v.new &&\n\nDo you need the space between the address $ (i.e. the last line) and\noperation 'd' (i.e. delete it)?  I am asking because that looks very\nunusual at least in our codebase.\n\n> +\t\tmv -- depot/file1,v.new depot/file1,v &&\n> +\t\tprintf \"@$UTF16@\" >>depot/file1,v &&\n> +\t\tp4d -jrF checkpoint.1\n> +\t)\n> +'\n> +\n> +test_expect_failure '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":"270452","messageId":"E47DE9F0-6017-4E96-AC29-E6C60C4D85CB@gmail.com","threadId":"40394","inReplyTo":"xmqqio73abl0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-21T23:03:17Z","receivedAt":"2015-09-21T23:03:17Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 21 Sep 2015, at 20:09, Junio C Hamano <gitster@pobox.com> wrote:\n\n> larsxschneider@gmail.com writes:\n> \n>> From: Lars Schneider <larsxschneider@gmail.com>\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/t9825-git-p4-handle-utf16-without-bom.sh | 50 ++++++++++++++++++++++++++++++\n>> 1 file changed, 50 insertions(+)\n>> create mode 100755 t/t9825-git-p4-handle-utf16-without-bom.sh\n>> \n>> diff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh\n>> b/t/t9825-git-p4-handle-utf16-without-bom.sh\n>> new file mode 100755\n>> index 0000000..65c3c4e\n>> --- /dev/null\n>> +++ b/t/t9825-git-p4-handle-utf16-without-bom.sh\n>> @@ -0,0 +1,50 @@\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 -e \"$ d\" depot/file1,v >depot/file1,v.new &&\n> \n> Do you need the space between the address $ (i.e. the last line) and\n> operation 'd' (i.e. delete it)?  I am asking because that looks very\n> unusual at least in our codebase.\nWell, I am no “sed” pro. I have to admit that I found this snippet on the Internet and it just worked. If I remove the space then it does not work. I was not yet able to figure out why… anyone an idea?\n\nThanks,\nLars\n\n\n\n> \n>> +\t\tmv -- depot/file1,v.new depot/file1,v &&\n>> +\t\tprintf \"@$UTF16@\" >>depot/file1,v &&\n>> +\t\tp4d -jrF checkpoint.1\n>> +\t)\n>> +'\n>> +\n>> +test_expect_failure '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":"270454","messageId":"CAPig+cRV-RCdcmAHG+bRL6_yYYNCRqQPQ+v3KCXwC81StGKibg@mail.gmail.com","threadId":"40394","inReplyTo":"E47DE9F0-6017-4E96-AC29-E6C60C4D85CB@gmail.com","subject":"Re: [PATCH v4 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-21T23:54:29Z","receivedAt":"2015-09-21T23:54:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Sep 21, 2015 at 7:03 PM, Lars Schneider\n<larsxschneider@gmail.com> wrote:\n> On 21 Sep 2015, at 20:09, Junio C Hamano <gitster@pobox.com> wrote:\n>> larsxschneider@gmail.com writes:\n>>> +test_expect_success 'init depot with UTF-16 encoded file and artificially remove BOM' '\n>>> +    (\n>>> +            cd \"db\" &&\n>>> +            p4d -jc &&\n>>> +            # P4D automatically adds a BOM. Remove it here to make the file invalid.\n>>> +            sed -e \"$ d\" depot/file1,v >depot/file1,v.new &&\n>>\n>> Do you need the space between the address $ (i.e. the last line) and\n>> operation 'd' (i.e. delete it)?  I am asking because that looks very\n>> unusual at least in our codebase.\n>\n> Well, I am no “sed” pro. I have to admit that I found this snippet\n> on the Internet and it just worked. If I remove the space then it\n> does not work. I was not yet able to figure out why… anyone an idea?\n\nYes, it's because $d is a variable reference, even within double\nquotes. Typically, one uses single quotes around the sed argument to\nsuppress this sort of undesired behavior. Since the entire test body\nis already within single quotes, however, changing the sed argument to\nuse single quotes, rather than double, will require escaping them:\n\n    sed -e \\'$d\\' depot/file...\n\nAside: You could also drop the unnecessary quotes from the 'cd' argument.\n"},{"id":"270466","messageId":"xmqqbncv6yym.fsf@gitster.mtv.corp.google.com","threadId":"40394","inReplyTo":"CAPig+cRV-RCdcmAHG+bRL6_yYYNCRqQPQ+v3KCXwC81StGKibg@mail.gmail.com","subject":"Re: [PATCH v4 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-22T01:10:25Z","receivedAt":"2015-09-22T01:10:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> Yes, it's because $d is a variable reference, even within double\n> quotes.\n\ns/even/especially/ ;-)\n\nHere is what I queued as SQUASH???\n\ndiff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh\nindex 65c3c4e..735c0bb 100644\n--- a/t/t9825-git-p4-handle-utf16-without-bom.sh\n+++ b/t/t9825-git-p4-handle-utf16-without-bom.sh\n@@ -22,8 +22,8 @@ test_expect_success 'init depot with UTF-16 encoded file and artificially remove\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 -e \"$ d\" depot/file1,v >depot/file1,v.new &&\n-\t\tmv -- depot/file1,v.new depot/file1,v &&\n+\t\tsed -e \"\\$d\" depot/file1,v >depot/file1,v.new &&\n+\t\tmv depot/file1,v.new depot/file1,v &&\n \t\tprintf \"@$UTF16@\" >>depot/file1,v &&\n \t\tp4d -jrF checkpoint.1\n \t)\n"},{"id":"270479","messageId":"9F835973-7045-4AA7-A0B0-D3D3C6F25D73@gmail.com","threadId":"40394","inReplyTo":"xmqqbncv6yym.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-22T10:09:20Z","receivedAt":"2015-09-22T10:09:20Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 22 Sep 2015, at 03:10, Junio C Hamano <gitster@pobox.com> wrote:\n\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n> \n>> Yes, it's because $d is a variable reference, even within double\n>> quotes.\n> \n> s/even/especially/ ;-)\n> \n> Here is what I queued as SQUASH???\n> \n> diff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh\n> index 65c3c4e..735c0bb 100644\n> --- a/t/t9825-git-p4-handle-utf16-without-bom.sh\n> +++ b/t/t9825-git-p4-handle-utf16-without-bom.sh\n> @@ -22,8 +22,8 @@ test_expect_success 'init depot with UTF-16 encoded file and artificially remove\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 -e \"$ d\" depot/file1,v >depot/file1,v.new &&\n> -\t\tmv -- depot/file1,v.new depot/file1,v &&\n> +\t\tsed -e \"\\$d\" depot/file1,v >depot/file1,v.new &&\n> +\t\tmv depot/file1,v.new depot/file1,v &&\n> \t\tprintf \"@$UTF16@\" >>depot/file1,v &&\n> \t\tp4d -jrF checkpoint.1\n> \t)\n\nThis works. I even tested successfully this one:\n\nsed \\$d depot/file1,v >depot/file1,v.new &&\n\nDo we need the “-e” option?\n\nThanks,\nLars\n"},{"id":"270480","messageId":"FAA838A1-CBD8-4716-ADDE-6CC912C19BE6@gmail.com","threadId":"40394","inReplyTo":"CAPig+cRV-RCdcmAHG+bRL6_yYYNCRqQPQ+v3KCXwC81StGKibg@mail.gmail.com","subject":"Re: [PATCH v4 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-22T10:12:22Z","receivedAt":"2015-09-22T10:12:22Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 22 Sep 2015, at 01:54, Eric Sunshine <sunshine@sunshineco.com> wrote:\n\n> On Mon, Sep 21, 2015 at 7:03 PM, Lars Schneider\n> <larsxschneider@gmail.com> wrote:\n>> On 21 Sep 2015, at 20:09, Junio C Hamano <gitster@pobox.com> wrote:\n>>> larsxschneider@gmail.com writes:\n>>>> +test_expect_success 'init depot with UTF-16 encoded file and artificially remove BOM' '\n>>>> +    (\n>>>> +            cd \"db\" &&\n>>>> +            p4d -jc &&\n>>>> +            # P4D automatically adds a BOM. Remove it here to make the file invalid.\n>>>> +            sed -e \"$ d\" depot/file1,v >depot/file1,v.new &&\n>>> \n>>> Do you need the space between the address $ (i.e. the last line) and\n>>> operation 'd' (i.e. delete it)?  I am asking because that looks very\n>>> unusual at least in our codebase.\n>> \n>> Well, I am no “sed” pro. I have to admit that I found this snippet\n>> on the Internet and it just worked. If I remove the space then it\n>> does not work. I was not yet able to figure out why… anyone an idea?\n> \n> Yes, it's because $d is a variable reference, even within double\n> quotes. Typically, one uses single quotes around the sed argument to\n> suppress this sort of undesired behavior. Since the entire test body\n> is already within single quotes, however, changing the sed argument to\n> use single quotes, rather than double, will require escaping them:\n> \n>    sed -e \\'$d\\' depot/file...\n> \n> Aside: You could also drop the unnecessary quotes from the 'cd' argument.\n\nThanks for the explanation. Plus you are correct with the quotes around “db”… just a habit.\n\n@Junio: \nIf it is no extra work for you, you can remove the quotes around “db”. I can also create a new patch roll including the sed change and the quote change if it is easier for you.\n\nBest,\nLars\n"},{"id":"270492","messageId":"xmqq8u7y5toe.fsf@gitster.mtv.corp.google.com","threadId":"40394","inReplyTo":"9F835973-7045-4AA7-A0B0-D3D3C6F25D73@gmail.com","subject":"Re: [PATCH v4 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-22T16:02:09Z","receivedAt":"2015-09-22T16:02:09Z","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> This works.\n\nOK, and thanks; as I don't do perforce, the squash was without any\ntesting.\n\n> Do we need the “-e” option?\n\nIn syntactic sense, no, but our codebase tends to prefer to have\none, because it is easier to spot which ones are the instructions if\nyou consistently have \"-e\" even when you give only one.\n"},{"id":"270493","messageId":"xmqq4mim5tn1.fsf@gitster.mtv.corp.google.com","threadId":"40394","inReplyTo":"FAA838A1-CBD8-4716-ADDE-6CC912C19BE6@gmail.com","subject":"Re: [PATCH v4 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-22T16:02:58Z","receivedAt":"2015-09-22T16:02:58Z","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> If it is no extra work for you, you can remove the quotes around\n> “db”. I can also create a new patch roll including the sed change\n> and the quote change if it is easier for you.\n\nNow you've tested the SQUASH??? for me, I can just squash that into\nyour original without resend.\n\nThanks.\n"},{"id":"270512","messageId":"CAO2U3QgehMcBrDUtChLLrn5VrH4jLE0CF5xDSShY72yycLryCg@mail.gmail.com","threadId":"40394","inReplyTo":"xmqq8u7y5toe.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Michael Blume","fromEmail":"blume.mike@gmail.com","sentAt":"2015-09-22T19:11:11Z","receivedAt":"2015-09-22T19:11:11Z","isPatch":true,"sender":{"key":"blume.mike@gmail.com","avatar":"https://gravatar.com/avatar/1a7b440e1d942425ff4098ac7fc15b86b30cecaa56e1692a7ef8b5939ba25ea7?d=mp&s=160"},"body":"I'm seeing test failures\n\nnon-executable tests: t9825-git-p4-handle-utf16-without-bom.sh\n\nls -l shows that all the other tests are executable but t9825 isn't.\n\nOn Tue, Sep 22, 2015 at 9:02 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Lars Schneider <larsxschneider@gmail.com> writes:\n>\n>> This works.\n>\n> OK, and thanks; as I don't do perforce, the squash was without any\n> testing.\n>\n>> Do we need the “-e” option?\n>\n> In syntactic sense, no, but our codebase tends to prefer to have\n> one, because it is easier to spot which ones are the instructions if\n> you consistently have \"-e\" even when you give only one.\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"270513","messageId":"CAPc5daXm9sBGAgrqz12d5a=zhR3PUXbFpPvOkBCoNQcQVhyOhw@mail.gmail.com","threadId":"40394","inReplyTo":"CAO2U3QgehMcBrDUtChLLrn5VrH4jLE0CF5xDSShY72yycLryCg@mail.gmail.com","subject":"Re: [PATCH v4 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-22T19:17:52Z","receivedAt":"2015-09-22T19:17:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yup, this was privately reported and I just squashed a fix in right now ;-)\n\nThanks. \"cd t && make test-lint\" would have caught it.\n\nOn Tue, Sep 22, 2015 at 12:11 PM, Michael Blume <blume.mike@gmail.com> wrote:\n> I'm seeing test failures\n>\n> non-executable tests: t9825-git-p4-handle-utf16-without-bom.sh\n>\n> ls -l shows that all the other tests are executable but t9825 isn't.\n>\n> On Tue, Sep 22, 2015 at 9:02 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Lars Schneider <larsxschneider@gmail.com> writes:\n>>\n>>> This works.\n>>\n>> OK, and thanks; as I don't do perforce, the squash was without any\n>> testing.\n>>\n>>> Do we need the “-e” option?\n>>\n>> In syntactic sense, no, but our codebase tends to prefer to have\n>> one, because it is easier to spot which ones are the instructions if\n>> you consistently have \"-e\" even when you give only one.\n>> --\n>> To unsubscribe from this list: send the line \"unsubscribe git\" in\n>> the body of a message to majordomo@vger.kernel.org\n>> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"270563","messageId":"3E330347-0F89-4E6F-8663-694AD3A559CC@gmail.com","threadId":"40394","inReplyTo":"CAPc5daXm9sBGAgrqz12d5a=zhR3PUXbFpPvOkBCoNQcQVhyOhw@mail.gmail.com","subject":"Re: [PATCH v4 1/2] git-p4: add test case for \"Translation of file content failed\" error","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-23T07:34:37Z","receivedAt":"2015-09-23T07:34:37Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Thanks a lot for taking care of this!\n\n- Lars\n\nOn 22 Sep 2015, at 21:17, Junio C Hamano <gitster@pobox.com> wrote:\n\n> Yup, this was privately reported and I just squashed a fix in right now ;-)\n> \n> Thanks. \"cd t && make test-lint\" would have caught it.\n> \n> On Tue, Sep 22, 2015 at 12:11 PM, Michael Blume <blume.mike@gmail.com> wrote:\n>> I'm seeing test failures\n>> \n>> non-executable tests: t9825-git-p4-handle-utf16-without-bom.sh\n>> \n>> ls -l shows that all the other tests are executable but t9825 isn't.\n>> \n>> On Tue, Sep 22, 2015 at 9:02 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Lars Schneider <larsxschneider@gmail.com> writes:\n>>> \n>>>> This works.\n>>> \n>>> OK, and thanks; as I don't do perforce, the squash was without any\n>>> testing.\n>>> \n>>>> Do we need the “-e” option?\n>>> \n>>> In syntactic sense, no, but our codebase tends to prefer to have\n>>> one, because it is easier to spot which ones are the instructions if\n>>> you consistently have \"-e\" even when you give only one.\n>>> --\n>>> To unsubscribe from this list: send the line \"unsubscribe git\" in\n>>> the body of a message to majordomo@vger.kernel.org\n>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"}]}