{"thread":{"id":"41106","subject":"t6023 broken under Mac OS","startedAt":"2016-01-01T15:36:48Z","lastAt":"2016-01-02T20:06:55Z","messageCount":7,"participants":["Torsten Bögershausen","Ramsay Jones","Junio C Hamano","Beat Bolli"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"275257","messageId":"56869D10.9090105@web.de","threadId":"41106","inReplyTo":null,"subject":"t6023 broken under Mac OS","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-01-01T15:36:48Z","receivedAt":"2016-01-01T15:36:48Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"The (last) test case\n'conflict markers contain CRLF when core.eol=crlf'\n\ndoes not work as expected under Mac OS: \"wc -l\" is not portable and the line\ntest $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\nfails.\n\n(The other problem is that the usage of \"\\r\" in a sed expression is not portable\neither.)\n\nIn other words: I don't have a solution at hand.\n"},{"id":"275260","messageId":"5686B400.7000800@ramsayjones.plus.com","threadId":"41106","inReplyTo":"56869D10.9090105@web.de","subject":"Re: t6023 broken under Mac OS","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-01-01T17:14:40Z","receivedAt":"2016-01-01T17:14:40Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Hi Torsten,\n\nOn 01/01/16 15:36, Torsten Bögershausen wrote:\n> The (last) test case\n> 'conflict markers contain CRLF when core.eol=crlf'\n> \n> does not work as expected under Mac OS: \"wc -l\" is not portable and the line\n> test $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n> fails.\n\nHmm, I have never used a Mac, so I'm just guessing here, but\nyou could try something like (obviously untested!):\n\ndiff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\nindex 245359a..68b306f 100755\n--- a/t/t6023-merge-file.sh\n+++ b/t/t6023-merge-file.sh\n@@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \\\n test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n \ttest_must_fail git -c core.eol=crlf merge-file -p \\\n \t\tnolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n-\ttest $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n+\ttest $(tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" | wc -l) -eq 3\n '\n \n test_done\n\n[The 'wc -l' portability should only be a problem if you rely on the\nexact textual form of the output, rather than the integer count.\n'wc -l' is used in many many tests ...]\n\nNote that this test is not checking all conflict markers (the\n======= marker does not have a filename appended). Should that\nbe fixed also?\n\nATB,\nRamsay Jones\n"},{"id":"275261","messageId":"5686BC3B.5080302@web.de","threadId":"41106","inReplyTo":"5686B400.7000800@ramsayjones.plus.com","subject":"Re: t6023 broken under Mac OS","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-01-01T17:49:47Z","receivedAt":"2016-01-01T17:49:47Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2016-01-01 18.14, Ramsay Jones wrote:\n> Hi Torsten,\n> \n> On 01/01/16 15:36, Torsten Bögershausen wrote:\n>> The (last) test case\n>> 'conflict markers contain CRLF when core.eol=crlf'\n>>\n>> does not work as expected under Mac OS: \"wc -l\" is not portable and the line\n>> test $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n>> fails.\n> \n> Hmm, I have never used a Mac, so I'm just guessing here, but\n> you could try something like (obviously untested!):\n> \n> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\n> index 245359a..68b306f 100755\n> --- a/t/t6023-merge-file.sh\n> +++ b/t/t6023-merge-file.sh\n> @@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \\\n>  test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n>  \ttest_must_fail git -c core.eol=crlf merge-file -p \\\n>  \t\tnolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n> -\ttest $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n> +\ttest $(tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" | wc -l) -eq 3\n>  '\n>  \n>  test_done\nYes, this works.\n\n> \n> [The 'wc -l' portability should only be a problem if you rely on the\n> exact textual form of the output, rather than the integer count.\n> 'wc -l' is used in many many tests ...]\n> \n> Note that this test is not checking all conflict markers (the\n> ======= marker does not have a filename appended). Should that\n> be fixed also?\nThis is may attempt (against pu)\n\ndiff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\nindex 68b306f..b1f8e41 100755\n--- a/t/t6023-merge-file.sh\n+++ b/t/t6023-merge-file.sh\n@@ -350,7 +350,13 @@ test_expect_success 'conflict at EOF without LF resolved by\n--union' \\\n test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n        test_must_fail git -c core.eol=crlf merge-file -p \\\n                nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n-       test $(tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" | wc -l) -eq 3\n+       tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" >out &&\n+       cat >exp <<\\EOF  &&\n+<<<<<<< nolf-diff1.txtQ\n+||||||| nolf-orig.txtQ\n+>>>>>>> nolf-diff2.txtQ\n+EOF\n+        test_cmp exp out\n '\n"},{"id":"275262","messageId":"5686EC29.3080901@ramsayjones.plus.com","threadId":"41106","inReplyTo":"5686BC3B.5080302@web.de","subject":"Re: t6023 broken under Mac OS","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-01-01T21:14:17Z","receivedAt":"2016-01-01T21:14:17Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 01/01/16 17:49, Torsten Bögershausen wrote:\n> On 2016-01-01 18.14, Ramsay Jones wrote:\n>> Hi Torsten,\n>>\n>> On 01/01/16 15:36, Torsten Bögershausen wrote:\n>>> The (last) test case\n>>> 'conflict markers contain CRLF when core.eol=crlf'\n>>>\n>>> does not work as expected under Mac OS: \"wc -l\" is not portable and the line\n>>> test $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n>>> fails.\n>>\n>> Hmm, I have never used a Mac, so I'm just guessing here, but\n>> you could try something like (obviously untested!):\n>>\n>> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\n>> index 245359a..68b306f 100755\n>> --- a/t/t6023-merge-file.sh\n>> +++ b/t/t6023-merge-file.sh\n>> @@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \\\n>>  test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n>>  \ttest_must_fail git -c core.eol=crlf merge-file -p \\\n>>  \t\tnolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n>> -\ttest $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n>> +\ttest $(tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" | wc -l) -eq 3\n>>  '\n>>  \n>>  test_done\n> Yes, this works.\n> \n>>\n>> [The 'wc -l' portability should only be a problem if you rely on the\n>> exact textual form of the output, rather than the integer count.\n>> 'wc -l' is used in many many tests ...]\n>>\n>> Note that this test is not checking all conflict markers (the\n>> ======= marker does not have a filename appended). Should that\n>> be fixed also?\n> This is may attempt (against pu)\n> \n> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\n> index 68b306f..b1f8e41 100755\n> --- a/t/t6023-merge-file.sh\n> +++ b/t/t6023-merge-file.sh\n> @@ -350,7 +350,13 @@ test_expect_success 'conflict at EOF without LF resolved by\n> --union' \\\n>  test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n>         test_must_fail git -c core.eol=crlf merge-file -p \\\n>                 nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n> -       test $(tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" | wc -l) -eq 3\n> +       tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" >out &&\n> +       cat >exp <<\\EOF  &&\n> +<<<<<<< nolf-diff1.txtQ\n> +||||||| nolf-orig.txtQ\n> +>>>>>>> nolf-diff2.txtQ\n> +EOF\n> +        test_cmp exp out\n>  '\n> \n\nThis still doesn't test the '======= conflict marker', how about\nsomething like this (again not tested on Mac - is the re in the\nsed invocation OK on the Mac?):\n\ndiff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\nindex 245359a..0697b22 100755\n--- a/t/t6023-merge-file.sh\n+++ b/t/t6023-merge-file.sh\n@@ -350,7 +350,14 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \\\n test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n \ttest_must_fail git -c core.eol=crlf merge-file -p \\\n \t\tnolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n-\ttest $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n+\ttr \"\\015\" Q <output.txt | sed -n \"/^[<=>|].*Q$/p\" >out.txt &&\n+\tcat >expect.txt <<-\\EOF &&\n+\t<<<<<<< nolf-diff1.txtQ\n+\t||||||| nolf-orig.txtQ\n+\t=======Q\n+\t>>>>>>> nolf-diff2.txtQ\n+\tEOF\n+\ttest_cmp expect.txt out.txt\n '\n \n test_done\n\nATB,\nRamsay Jones\n"},{"id":"275263","messageId":"5686FC72.8030008@web.de","threadId":"41106","inReplyTo":"5686EC29.3080901@ramsayjones.plus.com","subject":"Re: t6023 broken under Mac OS","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-01-01T22:23:46Z","receivedAt":"2016-01-01T22:23:46Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2016-01-01 22.14, Ramsay Jones wrote:\n> \n> \n> On 01/01/16 17:49, Torsten Bögershausen wrote:\n>> On 2016-01-01 18.14, Ramsay Jones wrote:\n>>> Hi Torsten,\n>>>\n>>> On 01/01/16 15:36, Torsten Bögershausen wrote:\n>>>> The (last) test case\n>>>> 'conflict markers contain CRLF when core.eol=crlf'\n>>>>\n>>>> does not work as expected under Mac OS: \"wc -l\" is not portable and the line\n>>>> test $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n>>>> fails.\n>>>\n>>> Hmm, I have never used a Mac, so I'm just guessing here, but\n>>> you could try something like (obviously untested!):\n>>>\n>>> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\n>>> index 245359a..68b306f 100755\n>>> --- a/t/t6023-merge-file.sh\n>>> +++ b/t/t6023-merge-file.sh\n>>> @@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \\\n>>>  test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n>>>  \ttest_must_fail git -c core.eol=crlf merge-file -p \\\n>>>  \t\tnolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n>>> -\ttest $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n>>> +\ttest $(tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" | wc -l) -eq 3\n>>>  '\n>>>  \n>>>  test_done\n>> Yes, this works.\n>>\n>>>\n>>> [The 'wc -l' portability should only be a problem if you rely on the\n>>> exact textual form of the output, rather than the integer count.\n>>> 'wc -l' is used in many many tests ...]\n>>>\n>>> Note that this test is not checking all conflict markers (the\n>>> ======= marker does not have a filename appended). Should that\n>>> be fixed also?\n>> This is may attempt (against pu)\n>>\n>> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\n>> index 68b306f..b1f8e41 100755\n>> --- a/t/t6023-merge-file.sh\n>> +++ b/t/t6023-merge-file.sh\n>> @@ -350,7 +350,13 @@ test_expect_success 'conflict at EOF without LF resolved by\n>> --union' \\\n>>  test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n>>         test_must_fail git -c core.eol=crlf merge-file -p \\\n>>                 nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n>> -       test $(tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" | wc -l) -eq 3\n>> +       tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" >out &&\n>> +       cat >exp <<\\EOF  &&\n>> +<<<<<<< nolf-diff1.txtQ\n>> +||||||| nolf-orig.txtQ\n>> +>>>>>>> nolf-diff2.txtQ\n>> +EOF\n>> +        test_cmp exp out\n>>  '\n>>\n> \n> This still doesn't test the '======= conflict marker', how about\n> something like this (again not tested on Mac - is the re in the\n> sed invocation OK on the Mac?):\nsed is OK (The problem is the usage of \"\\r\" inside sed:)\n\nLinux:\nprintf \"AA\\r\\n\" | sed 's/\\r$//' | od -c\n0000000   A   A  \\n\n\nMac OS:\nprintf \"AA\\r\\n\" | sed 's/\\r$//' | od -c\n0000000    A   A  \\r  \\n\n0000004\n> \n> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\n> index 245359a..0697b22 100755\n> --- a/t/t6023-merge-file.sh\n> +++ b/t/t6023-merge-file.sh\n> @@ -350,7 +350,14 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \\\n>  test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n>  \ttest_must_fail git -c core.eol=crlf merge-file -p \\\n>  \t\tnolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n> -\ttest $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n> +\ttr \"\\015\" Q <output.txt | sed -n \"/^[<=>|].*Q$/p\" >out.txt &&\n> +\tcat >expect.txt <<-\\EOF &&\n> +\t<<<<<<< nolf-diff1.txtQ\n> +\t||||||| nolf-orig.txtQ\n> +\t=======Q\n> +\t>>>>>>> nolf-diff2.txtQ\n> +\tEOF\n> +\ttest_cmp expect.txt out.txt\n>  '\n>  \n>  test_done\nYour fix works under Mac OS.\n\nMicro-nit: should the sed expression use -e (is that more Git style ?)\ntr \"\\015\" Q <output.txt | sed -n -e \"/^[<=>|].*Q$/p\" >out.txt &&\nMicro.nit 2:\nWe can simplify and use grep instead:\ntr \"\\015\" Q <output.txt | grep \"^[<=>|]\" >out.txt &&\n"},{"id":"275267","messageId":"xmqqr3hzpyf5.fsf@gitster.mtv.corp.google.com","threadId":"41106","inReplyTo":"5686B400.7000800@ramsayjones.plus.com","subject":"Re: t6023 broken under Mac OS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-02T19:35:26Z","receivedAt":"2016-01-02T19:35:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsayjones.plus.com> writes:\n\n> Hmm, I have never used a Mac, so I'm just guessing here, but\n> you could try something like (obviously untested!):\n>\n> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\n> index 245359a..68b306f 100755\n> --- a/t/t6023-merge-file.sh\n> +++ b/t/t6023-merge-file.sh\n> @@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \\\n>  test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n>  \ttest_must_fail git -c core.eol=crlf merge-file -p \\\n>  \t\tnolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n> -\ttest $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n> +\ttest $(tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" | wc -l) -eq 3\n>  '\n>  \n>  test_done\n>\n> [The 'wc -l' portability should only be a problem if you rely on the\n> exact textual form of the output, rather than the integer count.\n> 'wc -l' is used in many many tests ...]\n\nLooks OK, thanks.\n\nThe use of the unportable '\\r' with sed exists only in a stale topic\nparked on 'pu', so I won't worry about it myself at this point, but\nwhen the topic is rerolled, reviewers please be careful to spot it\nand stop it from introducing this bug to our tree.\n\nThanks.\n"},{"id":"275268","messageId":"56882DDF.2030701@drbeat.li","threadId":"41106","inReplyTo":"xmqqr3hzpyf5.fsf@gitster.mtv.corp.google.com","subject":"Re: t6023 broken under Mac OS","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2016-01-02T20:06:55Z","receivedAt":"2016-01-02T20:06:55Z","isPatch":false,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 02.01.16 20:35, Junio C Hamano wrote:\n> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:\n> \n>> Hmm, I have never used a Mac, so I'm just guessing here, but\n>> you could try something like (obviously untested!):\n>>\n>> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\n>> index 245359a..68b306f 100755\n>> --- a/t/t6023-merge-file.sh\n>> +++ b/t/t6023-merge-file.sh\n>> @@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \\\n>>  test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '\n>>  \ttest_must_fail git -c core.eol=crlf merge-file -p \\\n>>  \t\tnolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&\n>> -\ttest $(sed -n \"/\\.txt\\r$/p\" output.txt | wc -l) = 3\n>> +\ttest $(tr \"\\015\" Q <output.txt | sed -n \"/\\.txtQ$/p\" | wc -l) -eq 3\n>>  '\n>>  \n>>  test_done\n>>\n>> [The 'wc -l' portability should only be a problem if you rely on the\n>> exact textual form of the output, rather than the integer count.\n>> 'wc -l' is used in many many tests ...]\n> \n> Looks OK, thanks.\n> \n> The use of the unportable '\\r' with sed exists only in a stale topic\n> parked on 'pu', so I won't worry about it myself at this point, but\n> when the topic is rerolled, reviewers please be careful to spot it\n> and stop it from introducing this bug to our tree.\n\nI'll amend this topic once I find some time again...\n\nThanks.\n"}]}