t6023 broken under Mac OS
The (last) test case
'conflict markers contain CRLF when core.eol=crlf'
does not work as expected under Mac OS: "wc -l" is not portable and the line test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3
fails.
(The other problem is that the usage of "\r" in a sed expression is not portable either.)
In other words: I don't have a solution at hand.
Hi Torsten,
Quoted 6 lines · Torsten Bögershausen
The (last) test case
'conflict markers contain CRLF when core.eol=crlf'does not work as expected under Mac OS: "wc -l" is not portable and the line test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3
fails.
Hmm, I have never used a Mac, so I'm just guessing here, but you could try something like (obviously untested!):
t/t6023-merge-file.sh +1 -1
@@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3 + test $(tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" | wc -l) -eq 3 ' test_done
[The 'wc -l' portability should only be a problem if you rely on the exact textual form of the output, rather than the integer count. 'wc -l' is used in many many tests ...]
Note that this test is not checking all conflict markers (the ======= marker does not have a filename appended). Should that be fixed also?
ATB,
Ramsay Jones
Quoted 18 lines · Ramsay Jones
Hi Torsten,
Quoted 6 lines · Torsten Bögershausen
The (last) test case
'conflict markers contain CRLF when core.eol=crlf'does not work as expected under Mac OS: "wc -l" is not portable and the line test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3
fails.Hmm, I have never used a Mac, so I'm just guessing here, but you could try something like (obviously untested!):
t/t6023-merge-file.sh +0
-0
t/t6023-merge-file.sh test_expect_success 'conflict at EOF without LF resolved by --union' \ · line 356
Quoted 8 lines
t/t6023-merge-file.sh +1
-1@@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3 + test $(tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" | wc -l) -eq 3 ' test_done
Yes, this works.
Quoted 8 lines
[The 'wc -l' portability should only be a problem if you rely on the exact textual form of the output, rather than the integer count. 'wc -l' is used in many many tests ...]
Note that this test is not checking all conflict markers (the ======= marker does not have a filename appended). Should that be fixed also?
This is may attempt (against pu)
t/t6023-merge-file.sh +7 -2
@@ -350,7 +350,13 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" | wc -l) -eq 3 + tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" >out && + cat >exp <<\EOF && +<<<<<<< nolf-diff1.txtQ +||||||| nolf-orig.txtQ +>>>>>>> nolf-diff2.txtQ +EOF + test_cmp exp out '
Quoted 48 lines · Torsten Bögershausen
Quoted 18 lines · Ramsay Jones
Hi Torsten,
Quoted 6 lines · Torsten Bögershausen
The (last) test case
'conflict markers contain CRLF when core.eol=crlf'does not work as expected under Mac OS: "wc -l" is not portable and the line test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3
fails.Hmm, I have never used a Mac, so I'm just guessing here, but you could try something like (obviously untested!):
t/t6023-merge-file.sh +0
-0t/t6023-merge-file.sh test_expect_success 'conflict at EOF without LF resolved by --union' \ · line 356
Quoted 8 lines
t/t6023-merge-file.sh +1
-1@@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3 + test $(tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" | wc -l) -eq 3 ' test_doneYes, this works.
Quoted 8 lines
[The 'wc -l' portability should only be a problem if you rely on the exact textual form of the output, rather than the integer count. 'wc -l' is used in many many tests ...]
Note that this test is not checking all conflict markers (the ======= marker does not have a filename appended). Should that be fixed also?
This is may attempt (against pu)
t/t6023-merge-file.sh +0
-1@@ -350,7 +350,13 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&
t/t6023-merge-file.sh test_expect_success 'conflict at EOF without LF resolved by · line 361
Quoted 10 lines
t/t6023-merge-file.sh +7
-1- test $(tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" | wc -l) -eq 3 + tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" >out && + cat >exp <<\EOF && +<<<<<<< nolf-diff1.txtQ +||||||| nolf-orig.txtQ +>>>>>>> nolf-diff2.txtQ +EOF + test_cmp exp out '
This still doesn't test the '======= conflict marker', how about something like this (again not tested on Mac - is the re in the sed invocation OK on the Mac?):
t/t6023-merge-file.sh +8 -1
@@ -350,7 +350,14 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3 + tr "\015" Q <output.txt | sed -n "/^[<=>|].*Q$/p" >out.txt && + cat >expect.txt <<-\EOF && + <<<<<<< nolf-diff1.txtQ + ||||||| nolf-orig.txtQ + =======Q + >>>>>>> nolf-diff2.txtQ + EOF + test_cmp expect.txt out.txt ' test_done
ATB,
Ramsay Jones
Quoted 57 lines · Ramsay Jones
Quoted 54 lines · Torsten Bögershausen
Quoted 18 lines · Ramsay Jones
Hi Torsten,
Quoted 6 lines · Torsten Bögershausen
The (last) test case
'conflict markers contain CRLF when core.eol=crlf'does not work as expected under Mac OS: "wc -l" is not portable and the line test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3
fails.Hmm, I have never used a Mac, so I'm just guessing here, but you could try something like (obviously untested!):
t/t6023-merge-file.sh +0
-0t/t6023-merge-file.sh test_expect_success 'conflict at EOF without LF resolved by --union' \ · line 356
Quoted 8 lines
t/t6023-merge-file.sh +1
-1@@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3 + test $(tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" | wc -l) -eq 3 ' test_doneYes, this works.
Quoted 8 lines
[The 'wc -l' portability should only be a problem if you rely on the exact textual form of the output, rather than the integer count. 'wc -l' is used in many many tests ...]
Note that this test is not checking all conflict markers (the ======= marker does not have a filename appended). Should that be fixed also?
This is may attempt (against pu)
t/t6023-merge-file.sh +5
-2@@ -350,7 +350,13 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" | wc -l) -eq 3 + tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" >out && + cat >exp <<\EOF && +<<<<<<< nolf-diff1.txtQ +||||||| nolf-orig.txtQ +>>>>>>> nolf-diff2.txtQ
Quoted 8 lines
t/t6023-merge-file.sh test_expect_success 'conflict at EOF without LF resolved by --union' \ · line 362
Quoted 4 lines
t/t6023-merge-file.sh +2
-0+EOF + test_cmp exp out 'This still doesn't test the '======= conflict marker', how about something like this (again not tested on Mac - is the re in the sed invocation OK on the Mac?):
sed is OK (The problem is the usage of "\r" inside sed:)
Linux:
printf "AA\r\n" | sed 's/\r$//' | od -c
0000000 A A \n
Mac OS:
printf "AA\r\n" | sed 's/\r$//' | od -c
0000000 A A \r \n
0000004
Quoted 11 lines
t/t6023-merge-file.sh +1
-1@@ -350,7 +350,14 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3 + tr "\015" Q <output.txt | sed -n "/^[<=>|].*Q$/p" >out.txt &&
t/t6023-merge-file.sh test_expect_success 'conflict at EOF without LF resolved by --union' \ · line 363
Quoted 10 lines
t/t6023-merge-file.sh +7
-0+ cat >expect.txt <<-\EOF && + <<<<<<< nolf-diff1.txtQ + ||||||| nolf-orig.txtQ + =======Q + >>>>>>> nolf-diff2.txtQ + EOF + test_cmp expect.txt out.txt ' test_done
Your fix works under Mac OS.
Micro-nit: should the sed expression use -e (is that more Git style ?) tr "\015" Q <output.txt | sed -n -e "/^[<=>|].*Q$/p" >out.txt && Micro.nit 2:
We can simplify and use grep instead:
tr "\015" Q <output.txt | grep "^[<=>|]" >out.txt &&
Quoted 17 lines · Ramsay Jones
Hmm, I have never used a Mac, so I'm just guessing here, but you could try something like (obviously untested!):
t/t6023-merge-file.sh +1
-1@@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3 + test $(tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" | wc -l) -eq 3 ' test_done
Quoted 3 lines
[The 'wc -l' portability should only be a problem if you rely on the exact textual form of the output, rather than the integer count. 'wc -l' is used in many many tests ...]
Looks OK, thanks.
The use of the unportable '\r' with sed exists only in a stale topic parked on 'pu', so I won't worry about it myself at this point, but when the topic is rerolled, reviewers please be careful to spot it and stop it from introducing this bug to our tree.
Thanks.
Quoted 25 lines · Junio C Hamano
Quoted 17 lines · Ramsay Jones
Hmm, I have never used a Mac, so I'm just guessing here, but you could try something like (obviously untested!):
t/t6023-merge-file.sh +1
-1@@ -350,7 +350,7 @@ test_expect_success 'conflict at EOF without LF resolved by --union' \ test_expect_success 'conflict markers contain CRLF when core.eol=crlf' ' test_must_fail git -c core.eol=crlf merge-file -p \ nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - test $(sed -n "/\.txt\r$/p" output.txt | wc -l) = 3 + test $(tr "\015" Q <output.txt | sed -n "/\.txtQ$/p" | wc -l) -eq 3 ' test_doneQuoted 3 lines
[The 'wc -l' portability should only be a problem if you rely on the exact textual form of the output, rather than the integer count. 'wc -l' is used in many many tests ...]
Looks OK, thanks.
Quoted 4 lines
The use of the unportable '\r' with sed exists only in a stale topic parked on 'pu', so I won't worry about it myself at this point, but when the topic is rerolled, reviewers please be careful to spot it and stop it from introducing this bug to our tree.
I'll amend this topic once I find some time again...
Thanks.