git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 7/7] t4051: mark supporting files as requiring LF-only line endings

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
May 9, 2017, 11:20 UTC
Message-ID
<alpine.DEB.2.21.1.1705091306480.146734@virtualbox>
In-Reply-To
<xmqqefvz9asb.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 8 May 2017, Junio C Hamano wrote:
Show 15 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > The test t4051-diff-function-context.sh passes on Linux when
> > core.autocrlf=true even without marking its support files as LF-only,
> > but they fail when core.autocrlf=true in Git for Windows' SDK.
> >
> > The reason is that `grep ... >file.c.new` will keep CR/LF line endings
> > on Linux (obviously treating CRs as if they were regular characters),
> > but will be converted to LF-only line endings with MSYS2's grep that
> > is used in Git for Windows.
> 
> Ahem.  
> 
> I thought that according to your claim a UNIX tool like "grep" would
> alway use LF endings?  ;-)

The maintainers of the MSYS2 grep package apparently disagree with me on that point. You should be familiar with that reaction.

Show 8 quoted lines
> > As we do not want to validate the way the available `grep` works,
> > let's just mark the input as LF-only and move on.
> 
> I agree with this conclusion; just like we do not want to worry about
> how `grep` works when given CRLF files in this patch, we do not want to
> worry about how other commands like `sh` works when given CRLF files.
> And that is consistent with the overall theme of this series that marked
> *.sh, *.perl and other files with eol=lf attribute.
Good. That agreement is something on which you and I can build.
Show 10 quoted lines
> The only question I still have with this series is about the
> README/COPYING thing.  I _think_ it was my ancient mistake to use the
> toplevel README and COPYING as test files, and that mistake was
> corrected by somebody else earlier by having a frozen copy in
> t/diff-lib/ and making tests use these files from that directory.  If we
> broke our tests to again use these files from outside t/diff-lib/, then
> we would need to fix the tests not to do so.  And if we are only looking
> at t/diff-lib/ copy, then I think it is more consistent with the rest of
> this series to mark them with eol=lf rather than passing them through
> "tr -d '\015'".

Thank you for pointing out that the README and COPYING files were reproduced in t/diff-lib/ specifically to serve as files for use in the tests. It had not really occurred to me, as I mistook this to be an extra anal licensing clarification for the diff-lib ;-)

I will "re-roll" the patch series, dropping the ugly tr calls and marking t/diff-lib/* as LF-only, as you suggested.

Ciao, Dscho

Previous: Junio C HamanoNext: Johannes Schindelin
Message 29 of 39 in “Abide by our own rules regarding line endings”
  1. 0/5 Abide by our own rules regarding line endingsJohannes Schindelin, May 2, 2017
  2. 2/5 git-new-workdir: mark script as LF-onlyJohannes Schindelin, May 2, 2017
  3. 1/5 Fix build with core.autocrlf=trueJohannes Schindelin, May 2, 2017
  4. 3/5 completion: mark bash script as LF-onlyJohannes Schindelin, May 2, 2017
  5. 5/5 t4051: mark supporting files as requiring LF-only line endingsJohannes Schindelin, May 2, 2017
  6. 4/5 Fix the remaining tests that failed with core.autocrlf=trueJohannes Schindelin, May 2, 2017
  7. Jonathan NiederMay 2, 2017
  8. Johannes SchindelinMay 3, 2017
  9. Junio C HamanoMay 4, 2017
  10. Johannes SchindelinMay 4, 2017
  11. Junio C HamanoMay 4, 2017
  12. Johannes SchindelinMay 4, 2017
  13. Junio C HamanoMay 7, 2017
  14. Johannes SchindelinMay 8, 2017
  15. 0/7 Abide by our own rules regarding line endingsJohannes Schindelin, May 4, 2017
  16. 1/7 Fix build with core.autocrlf=trueJohannes Schindelin, May 4, 2017
  17. Junio C HamanoMay 8, 2017
  18. 2/7 git-new-workdir: mark script as LF-onlyJohannes Schindelin, May 4, 2017
  19. Junio C HamanoMay 8, 2017
  20. 4/7 t3901: move supporting files into t/t3901/Johannes Schindelin, May 4, 2017
  21. Junio C HamanoMay 8, 2017
  22. 3/7 completion: mark bash script as LF-onlyJohannes Schindelin, May 4, 2017
  23. 5/7 t4003, t4005, t4007 & t4008: handle CR/LF in t/README & t/diff-lib/READMEJohannes Schindelin, May 4, 2017
  24. Junio C HamanoMay 8, 2017
  25. 6/7 Fix the remaining tests that failed with core.autocrlf=trueJohannes Schindelin, May 4, 2017
  26. Junio C HamanoMay 8, 2017
  27. 7/7 t4051: mark supporting files as requiring LF-only line endingsJohannes Schindelin, May 4, 2017
  28. Junio C HamanoMay 8, 2017
  29. Johannes SchindelinMay 9, 2017
  30. 0/6 Abide by our own rules regarding line endingsJohannes Schindelin, May 9, 2017
  31. 1/6 Fix build with core.autocrlf=trueJohannes Schindelin, May 9, 2017
  32. Jonathan NiederMay 22, 2017
  33. Junio C HamanoMay 23, 2017
  34. Johannes SchindelinMay 23, 2017
  35. 2/6 git-new-workdir: mark script as LF-onlyJohannes Schindelin, May 9, 2017
  36. 3/6 completion: mark bash script as LF-onlyJohannes Schindelin, May 9, 2017
  37. 4/6 t3901: move supporting files into t/t3901/Johannes Schindelin, May 9, 2017
  38. 5/6 Fix the remaining tests that failed with core.autocrlf=trueJohannes Schindelin, May 9, 2017
  39. 6/6 t4051: mark supporting files as requiring LF-only line endingsJohannes Schindelin, May 9, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.