Git development
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
	Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH v2 7/7] t4051: mark supporting files as requiring LF-only line endings
Date: Tue, 9 May 2017 13:20:11 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.1.1705091306480.146734@virtualbox> (raw)
In-Reply-To: <xmqqefvz9asb.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Mon, 8 May 2017, Junio C Hamano wrote:

> 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.

> > 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.

> 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

  reply	other threads:[~2017-05-09 11:20 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-02 12:29 [PATCH 0/5] Abide by our own rules regarding line endings Johannes Schindelin
2017-05-02 12:30 ` [PATCH 1/5] Fix build with core.autocrlf=true Johannes Schindelin
2017-05-02 12:30 ` [PATCH 2/5] git-new-workdir: mark script as LF-only Johannes Schindelin
2017-05-02 12:30 ` [PATCH 3/5] completion: mark bash " Johannes Schindelin
2017-05-02 12:30 ` [PATCH 4/5] Fix the remaining tests that failed with core.autocrlf=true Johannes Schindelin
2017-05-02 12:30 ` [PATCH 5/5] t4051: mark supporting files as requiring LF-only line endings Johannes Schindelin
2017-05-02 21:56 ` [PATCH 0/5] Abide by our own rules regarding " Jonathan Nieder
2017-05-03 14:23   ` Johannes Schindelin
2017-05-04  5:19 ` Junio C Hamano
2017-05-04  9:47   ` Johannes Schindelin
2017-05-04 15:04     ` Junio C Hamano
2017-05-04 18:48       ` Johannes Schindelin
2017-05-07  5:29         ` Junio C Hamano
2017-05-08 10:49           ` Johannes Schindelin
2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
2017-05-04  9:49   ` [PATCH v2 1/7] Fix build with core.autocrlf=true Johannes Schindelin
2017-05-08  5:08     ` Junio C Hamano
2017-05-04  9:49   ` [PATCH v2 2/7] git-new-workdir: mark script as LF-only Johannes Schindelin
2017-05-08  5:11     ` Junio C Hamano
2017-05-04  9:49   ` [PATCH v2 3/7] completion: mark bash " Johannes Schindelin
2017-05-04  9:49   ` [PATCH v2 4/7] t3901: move supporting files into t/t3901/ Johannes Schindelin
2017-05-08  5:12     ` Junio C Hamano
2017-05-04  9:50   ` [PATCH v2 5/7] t4003, t4005, t4007 & t4008: handle CR/LF in t/README & t/diff-lib/README Johannes Schindelin
2017-05-08  5:14     ` Junio C Hamano
2017-05-04  9:50   ` [PATCH v2 6/7] Fix the remaining tests that failed with core.autocrlf=true Johannes Schindelin
2017-05-08  5:22     ` Junio C Hamano
2017-05-04  9:50   ` [PATCH v2 7/7] t4051: mark supporting files as requiring LF-only line endings Johannes Schindelin
2017-05-08  5:29     ` Junio C Hamano
2017-05-09 11:20       ` Johannes Schindelin [this message]
2017-05-09 12:53   ` [PATCH v3 0/6] Abide by our own rules regarding " Johannes Schindelin
2017-05-09 12:53     ` [PATCH v3 1/6] Fix build with core.autocrlf=true Johannes Schindelin
2017-05-22 17:57       ` Jonathan Nieder
2017-05-23  3:35         ` Junio C Hamano
2017-05-23  9:01           ` Johannes Schindelin
2017-05-09 12:53     ` [PATCH v3 2/6] git-new-workdir: mark script as LF-only Johannes Schindelin
2017-05-09 12:54     ` [PATCH v3 3/6] completion: mark bash " Johannes Schindelin
2017-05-09 12:54     ` [PATCH v3 4/6] t3901: move supporting files into t/t3901/ Johannes Schindelin
2017-05-09 12:54     ` [PATCH v3 5/6] Fix the remaining tests that failed with core.autocrlf=true Johannes Schindelin
2017-05-09 12:54     ` [PATCH v3 6/6] t4051: mark supporting files as requiring LF-only line endings Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.1.1705091306480.146734@virtualbox \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox