* t6023 broken under Mac OS @ 2016-01-01 15:36 Torsten Bögershausen 2016-01-01 17:14 ` Ramsay Jones 0 siblings, 1 reply; 7+ messages in thread From: Torsten Bögershausen @ 2016-01-01 15:36 UTC (permalink / raw) To: dev+git, Git Mailing List 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: t6023 broken under Mac OS 2016-01-01 15:36 t6023 broken under Mac OS Torsten Bögershausen @ 2016-01-01 17:14 ` Ramsay Jones 2016-01-01 17:49 ` Torsten Bögershausen 2016-01-02 19:35 ` Junio C Hamano 0 siblings, 2 replies; 7+ messages in thread From: Ramsay Jones @ 2016-01-01 17:14 UTC (permalink / raw) To: Torsten Bögershausen, dev+git, Git Mailing List Hi Torsten, On 01/01/16 15:36, Torsten Bögershausen wrote: > 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!): diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh index 245359a..68b306f 100755 --- a/t/t6023-merge-file.sh +++ b/t/t6023-merge-file.sh @@ -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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: t6023 broken under Mac OS 2016-01-01 17:14 ` Ramsay Jones @ 2016-01-01 17:49 ` Torsten Bögershausen 2016-01-01 21:14 ` Ramsay Jones 2016-01-02 19:35 ` Junio C Hamano 1 sibling, 1 reply; 7+ messages in thread From: Torsten Bögershausen @ 2016-01-01 17:49 UTC (permalink / raw) To: Ramsay Jones, Torsten Bögershausen, dev+git, Git Mailing List On 2016-01-01 18.14, Ramsay Jones wrote: > Hi Torsten, > > On 01/01/16 15:36, Torsten Bögershausen wrote: >> 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!): > > diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh > index 245359a..68b306f 100755 > --- a/t/t6023-merge-file.sh > +++ b/t/t6023-merge-file.sh > @@ -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. > > [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) diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh index 68b306f..b1f8e41 100755 --- a/t/t6023-merge-file.sh +++ b/t/t6023-merge-file.sh @@ -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 ' ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: t6023 broken under Mac OS 2016-01-01 17:49 ` Torsten Bögershausen @ 2016-01-01 21:14 ` Ramsay Jones 2016-01-01 22:23 ` Torsten Bögershausen 0 siblings, 1 reply; 7+ messages in thread From: Ramsay Jones @ 2016-01-01 21:14 UTC (permalink / raw) To: Torsten Bögershausen, dev+git, Git Mailing List On 01/01/16 17:49, Torsten Bögershausen wrote: > On 2016-01-01 18.14, Ramsay Jones wrote: >> Hi Torsten, >> >> On 01/01/16 15:36, Torsten Bögershausen wrote: >>> 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!): >> >> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh >> index 245359a..68b306f 100755 >> --- a/t/t6023-merge-file.sh >> +++ b/t/t6023-merge-file.sh >> @@ -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. > >> >> [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) > > diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh > index 68b306f..b1f8e41 100755 > --- a/t/t6023-merge-file.sh > +++ b/t/t6023-merge-file.sh > @@ -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 > ' > 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?): diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh index 245359a..0697b22 100755 --- a/t/t6023-merge-file.sh +++ b/t/t6023-merge-file.sh @@ -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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: t6023 broken under Mac OS 2016-01-01 21:14 ` Ramsay Jones @ 2016-01-01 22:23 ` Torsten Bögershausen 0 siblings, 0 replies; 7+ messages in thread From: Torsten Bögershausen @ 2016-01-01 22:23 UTC (permalink / raw) To: Ramsay Jones, Torsten Bögershausen, dev+git, Git Mailing List On 2016-01-01 22.14, Ramsay Jones wrote: > > > On 01/01/16 17:49, Torsten Bögershausen wrote: >> On 2016-01-01 18.14, Ramsay Jones wrote: >>> Hi Torsten, >>> >>> On 01/01/16 15:36, Torsten Bögershausen wrote: >>>> 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!): >>> >>> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh >>> index 245359a..68b306f 100755 >>> --- a/t/t6023-merge-file.sh >>> +++ b/t/t6023-merge-file.sh >>> @@ -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. >> >>> >>> [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) >> >> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh >> index 68b306f..b1f8e41 100755 >> --- a/t/t6023-merge-file.sh >> +++ b/t/t6023-merge-file.sh >> @@ -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 >> ' >> > > 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 > > diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh > index 245359a..0697b22 100755 > --- a/t/t6023-merge-file.sh > +++ b/t/t6023-merge-file.sh > @@ -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 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 && ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: t6023 broken under Mac OS 2016-01-01 17:14 ` Ramsay Jones 2016-01-01 17:49 ` Torsten Bögershausen @ 2016-01-02 19:35 ` Junio C Hamano 2016-01-02 20:06 ` Beat Bolli 1 sibling, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2016-01-02 19:35 UTC (permalink / raw) To: Ramsay Jones; +Cc: Torsten Bögershausen, dev+git, Git Mailing List Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > Hmm, I have never used a Mac, so I'm just guessing here, but > you could try something like (obviously untested!): > > diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh > index 245359a..68b306f 100755 > --- a/t/t6023-merge-file.sh > +++ b/t/t6023-merge-file.sh > @@ -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 ...] 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: t6023 broken under Mac OS 2016-01-02 19:35 ` Junio C Hamano @ 2016-01-02 20:06 ` Beat Bolli 0 siblings, 0 replies; 7+ messages in thread From: Beat Bolli @ 2016-01-02 20:06 UTC (permalink / raw) To: Junio C Hamano, Ramsay Jones; +Cc: Torsten Bögershausen, Git Mailing List On 02.01.16 20:35, Junio C Hamano wrote: > Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > >> Hmm, I have never used a Mac, so I'm just guessing here, but >> you could try something like (obviously untested!): >> >> diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh >> index 245359a..68b306f 100755 >> --- a/t/t6023-merge-file.sh >> +++ b/t/t6023-merge-file.sh >> @@ -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 ...] > > 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. I'll amend this topic once I find some time again... Thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-01-02 20:16 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-01-01 15:36 t6023 broken under Mac OS Torsten Bögershausen 2016-01-01 17:14 ` Ramsay Jones 2016-01-01 17:49 ` Torsten Bögershausen 2016-01-01 21:14 ` Ramsay Jones 2016-01-01 22:23 ` Torsten Bögershausen 2016-01-02 19:35 ` Junio C Hamano 2016-01-02 20:06 ` Beat Bolli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).