git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/5] Abide by our own rules regarding line endings
@ 2017-05-02 12:29 Johannes Schindelin
  2017-05-02 12:30 ` [PATCH 1/5] Fix build with core.autocrlf=true Johannes Schindelin
                   ` (7 more replies)
  0 siblings, 8 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-02 12:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Over the past decade, there have been a couple of attempts to remedy the
situation regarding line endings (Windows/DOS line endings are
traditionally CR/LF even if many current applications can handle LF,
too, Linux/MacOSX/*BSD/Unix uses LF-only line handlings, and old MacOS
software used to use CR-only line endings).

The current idea seems to be that the default line endings differ
depending on the platform, and for files that should be exempt from that
default, we strongly recommend using .gitattributes to define what the
source code requires.

It is time to heed our own recommendation and mark the files that
require LF-only line endings in our very own .gitattributes.

For starters, those files include shell scripts: the most prevalent
shell interpreter in use (and certainly used in Git for Windows) is
Bash, and Bash does not handle CR/LF line endings gracefully.

There are even two shell scripts that are used in the test suite even if
they are not technically supposed to be part of core Git, as indicated
by their habitat inside contrib/: git-new-workdir and
git-completion.bash.

Related to shell scripts: when generating common-cmds.h, we use tools
that generally operate on the assumption that input and output
deliminate their lines using LF-only line endings. Consequently, they
would happily copy the CR byte verbatim into the strings in
common-cmds.h, which in turn makes the C preprocessor barf (that
interprets them as MacOS-style line endings).

Further, the most obvious required fixes concern tests' support files
committed verbatim, to be compared to Git's output, which is always
LF-only.

There are a few SVN dump files, too, supporting the Subversion-related
tests, requiring LF-only line endings.

Finally, the test suite makes use of text files that are not obviously
supporting tests, such as t/README, comparing them to LF-only versions
(and consequently failing if the files have been checked out with CR/LF
line endings). Therefore we need to mark those as LF-only in the
.gitattributes, too.

Without these fixes, Git will fail to build and pass the test suite, as
can be verified even on Linux using this cadence:

	git config core.autocrlf true
	rm .git/index && git stash
	make DEVELOPER=1 -j15 test

Note: I separated out the change marking t/t4051/* as LF-only into an
individual commit for one reason: it would appear that the test passes
if checked out using core.autocrlf=true on Linux, but on Windows the
test fails. In that respect, this test is special, as the other changes
can be easily validated even without using Windows.


Johannes Schindelin (5):
  Fix build with core.autocrlf=true
  git-new-workdir: mark script as LF-only
  completion: mark bash script as LF-only
  Fix the remaining tests that failed with core.autocrlf=true
  t4051: mark supporting files as requiring LF-only line endings

 .gitattributes                    |  8 +++++++-
 contrib/completion/.gitattributes |  1 +
 contrib/workdir/.gitattributes    |  1 +
 git-gui/.gitattributes            |  1 +
 t/.gitattributes                  | 29 ++++++++++++++++++++++++++++-
 5 files changed, 38 insertions(+), 2 deletions(-)
 create mode 100644 contrib/completion/.gitattributes
 create mode 100644 contrib/workdir/.gitattributes


base-commit: d2bbb7c2bcf6e77ebfcabf4e12110fe6d5c91de6
Published-As: https://github.com/dscho/git/releases/tag/lf-attrs-v1
Fetch-It-Via: git fetch https://github.com/dscho/git lf-attrs-v1

-- 
2.12.2.windows.2.800.gede8f145e06


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH 1/5] Fix build with core.autocrlf=true
  2017-05-02 12:29 [PATCH 0/5] Abide by our own rules regarding line endings Johannes Schindelin
@ 2017-05-02 12:30 ` Johannes Schindelin
  2017-05-02 12:30 ` [PATCH 2/5] git-new-workdir: mark script as LF-only Johannes Schindelin
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-02 12:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

On Windows, the default line endings are denoted by a Carriage Return
byte followed by a Line Feed byte, while Linux and MacOSX use a single
Line Feed byte to denote a line ending.

To help with this situation, Git introduced several mechanisms over the
last decade, most prominently the `core.autocrlf` setting.

Sometimes, however, a single setting is incorrect, e.g. when certain
files in the source code are to be consumed by software that can handle
only LF line endings, while other files can use whatever is appropriate
for the current platform.

To allow for that, Git added the `eol` option to its .gitattributes
handling, expecting every user of Git to mark their source code
appropriately.

Bash assumes that line-endings of scripts are denoted by a single Line
Feed byte. Therefore, shell scripts in Git's source code are one example
where that `eol=lf` option is *required*.

When generating common-cmds.h, the Unix tools we use generally operate on
the assumption that input and output deliminate their lines using LF-only
line endings. Consequently, they would happily copy the CR byte verbatim
into the strings in common-cmds.h, which in turn makes the C preprocessor
barf (that interprets them as MacOS-style line endings). Therefore, we
have to mark the input files as LF-only: command-list.txt and
Documentation/git-*.txt.

Quite a bit belatedly, this patch brings Git's own source code in line
with those expectations by setting those attributes to allow for a
correct build even when core.autocrlf=true.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .gitattributes         | 8 +++++++-
 git-gui/.gitattributes | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/.gitattributes b/.gitattributes
index 320e33c327c..8ce9c6b8888 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,3 +1,9 @@
 * whitespace=!indent,trail,space
 *.[ch] whitespace=indent,trail,space diff=cpp
-*.sh whitespace=indent,trail,space
+*.sh whitespace=indent,trail,space eol=lf
+*.perl eol=lf
+*.pm eol=lf
+/Documentation/git-*.txt eol=lf
+/command-list.txt eol=lf
+/GIT-VERSION-GEN eol=lf
+/mergetools/* eol=lf
diff --git a/git-gui/.gitattributes b/git-gui/.gitattributes
index 33d07c06bd9..59cd41dbff7 100644
--- a/git-gui/.gitattributes
+++ b/git-gui/.gitattributes
@@ -2,3 +2,4 @@
 *           encoding=US-ASCII
 git-gui.sh  encoding=UTF-8
 /po/*.po    encoding=UTF-8
+/GIT-VERSION-GEN eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 2/5] git-new-workdir: mark script as LF-only
  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 ` Johannes Schindelin
  2017-05-02 12:30 ` [PATCH 3/5] completion: mark bash " Johannes Schindelin
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-02 12:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Bash does not handle scripts with CR/LF line endings correctly, therefore
they *have* to be forced to LF-only line endings.

Funnily enough, this fixes t3000-ls-files-others and
t1021-rerere-in-workdir when git.git was checked out with
core.autocrlf=true, as these test still use git-new-workdir (once `git
worktree` is no longer marked as experimental, both scripts probably
want to be ported to using that command instead).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/workdir/.gitattributes | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 contrib/workdir/.gitattributes

diff --git a/contrib/workdir/.gitattributes b/contrib/workdir/.gitattributes
new file mode 100644
index 00000000000..1f78c5d1bd3
--- /dev/null
+++ b/contrib/workdir/.gitattributes
@@ -0,0 +1 @@
+/git-new-workdir eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 3/5] completion: mark bash script as LF-only
  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 ` Johannes Schindelin
  2017-05-02 12:30 ` [PATCH 4/5] Fix the remaining tests that failed with core.autocrlf=true Johannes Schindelin
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-02 12:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Without this change, the completion script does not work, as Bash expects
its scripts to have line feeds as end-of-line markers (this is
particularly prominent in quoted multi-line strings, where carriage
returns would slip into the strings as verbatim characters otherwise).

This change is required to let t9902-completion pass when Git's source
code is checked out with `core.autocrlf = true`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/completion/.gitattributes | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 contrib/completion/.gitattributes

diff --git a/contrib/completion/.gitattributes b/contrib/completion/.gitattributes
new file mode 100644
index 00000000000..19116944c15
--- /dev/null
+++ b/contrib/completion/.gitattributes
@@ -0,0 +1 @@
+*.bash eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 4/5] Fix the remaining tests that failed with core.autocrlf=true
  2017-05-02 12:29 [PATCH 0/5] Abide by our own rules regarding line endings Johannes Schindelin
                   ` (2 preceding siblings ...)
  2017-05-02 12:30 ` [PATCH 3/5] completion: mark bash " Johannes Schindelin
@ 2017-05-02 12:30 ` Johannes Schindelin
  2017-05-02 12:30 ` [PATCH 5/5] t4051: mark supporting files as requiring LF-only line endings Johannes Schindelin
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-02 12:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

The test suite is mainly developed on Linux and MacOSX, which is the
reason that nobody thought to mark files as LF-only as needed.

The symptom is a test suite that fails left and right when being checked
out using Git for Windows (which defaults to core.autocrlf=true).

Mostly, the problems stem from Git's (LF-only) output being compared to
hard-coded files that are checked out with line endings according to
core.autocrlf (which is of course incorrect).

Note: the test suite also uses the t/README file as well as the COPYING
file in t/diff-lib/, expecting LF-only line endings explicitly and
failing if that assumption does not hold true. That is why we mark them
as LF-only in the .gitattributes, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/.gitattributes | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/t/.gitattributes b/t/.gitattributes
index 2d44088f56e..4064eba354d 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1,2 +1,28 @@
+README eol=lf
 t[0-9][0-9][0-9][0-9]/* -whitespace
-t0110/url-* binary
+/diff-lib/COPYING eol=lf
+/t0110/url-* binary
+/t3900/*.txt eol=lf
+/t3901*.txt eol=lf
+/t4034/*/* eol=lf
+/t4013/* eol=lf
+/t4018/* eol=lf
+/t4100/* eol=lf
+/t4101/* eol=lf
+/t4109/* eol=lf
+/t4110/* eol=lf
+/t4135/* eol=lf
+/t4211/* eol=lf
+/t4252/* eol=lf
+/t5100/* eol=lf
+/t5515/* eol=lf
+/t556x_common eol=lf
+/t7500/* eol=lf
+/t8005/*.txt eol=lf
+/t9110/svm.dump eol=lf
+/t9111/svnsync.dump eol=lf
+/t9115/funky-names.dump eol=lf
+/t9121/renamed-dir.dump eol=lf
+/t913[56]/svn.dump eol=lf
+/t915[0134]/*.dump eol=lf
+/t9161/branches.dump eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 5/5] t4051: mark supporting files as requiring LF-only line endings
  2017-05-02 12:29 [PATCH 0/5] Abide by our own rules regarding line endings Johannes Schindelin
                   ` (3 preceding siblings ...)
  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 ` Johannes Schindelin
  2017-05-02 21:56 ` [PATCH 0/5] Abide by our own rules regarding " Jonathan Nieder
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-02 12:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

The test t4051-diff-function-context.sh seems to pass 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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/.gitattributes | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/.gitattributes b/t/.gitattributes
index 4064eba354d..1bdc46a53f1 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -7,6 +7,7 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
 /t4034/*/* eol=lf
 /t4013/* eol=lf
 /t4018/* eol=lf
+/t4051/* eol=lf
 /t4100/* eol=lf
 /t4101/* eol=lf
 /t4109/* eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/5] Abide by our own rules regarding line endings
  2017-05-02 12:29 [PATCH 0/5] Abide by our own rules regarding line endings Johannes Schindelin
                   ` (4 preceding siblings ...)
  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 ` Jonathan Nieder
  2017-05-03 14:23   ` Johannes Schindelin
  2017-05-04  5:19 ` Junio C Hamano
  2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
  7 siblings, 1 reply; 39+ messages in thread
From: Jonathan Nieder @ 2017-05-02 21:56 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano

Hi,

Johannes Schindelin wrote:

> Over the past decade, there have been a couple of attempts to remedy the
[...]

I'm intentionally skimming this cover letter, since anything important
it says needs to also be in the commit messages.

[...]
> Without these fixes, Git will fail to build and pass the test suite, as
> can be verified even on Linux using this cadence:
>
> 	git config core.autocrlf true
> 	rm .git/index && git stash
> 	make DEVELOPER=1 -j15 test

This should go in a commit message (or perhaps in all of them).

[...]
> Johannes Schindelin (5):
>   Fix build with core.autocrlf=true
>   git-new-workdir: mark script as LF-only
>   completion: mark bash script as LF-only
>   Fix the remaining tests that failed with core.autocrlf=true
>   t4051: mark supporting files as requiring LF-only line endings

With or without that change,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

The t/README bit in patch 4/5 is sad (I want to be able to open
t/README using an old version of Notepad) but changing that test to
use another file seems out of scope for what this series does.

Thanks,
Jonathan

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/5] Abide by our own rules regarding line endings
  2017-05-02 21:56 ` [PATCH 0/5] Abide by our own rules regarding " Jonathan Nieder
@ 2017-05-03 14:23   ` Johannes Schindelin
  0 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-03 14:23 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Junio C Hamano

Hi Jonathan,

On Tue, 2 May 2017, Jonathan Nieder wrote:

> Johannes Schindelin wrote:
> 
> > Over the past decade, there have been a couple of attempts to remedy the
> [...]
> 
> I'm intentionally skimming this cover letter, since anything important
> it says needs to also be in the commit messages.

Sure, makes sense. I tried to do that, too, before sending out my patch
series.

> [...]
> > Without these fixes, Git will fail to build and pass the test suite, as
> > can be verified even on Linux using this cadence:
> >
> > 	git config core.autocrlf true
> > 	rm .git/index && git stash
> > 	make DEVELOPER=1 -j15 test
> 
> This should go in a commit message (or perhaps in all of them).

Hmm, okay. I wanted to keep it out of them, as commit messages are (in my
mind) more about the why?, and a little less about the how? when not
obvious from the diff.

I added a variation to the first patch (because the tests would still
fail, I skipped the `test` from the `make` invocation) and the unmodified
cadence to the "Fix the remaining tests..." patch.

> [...]
> > Johannes Schindelin (5):
> >   Fix build with core.autocrlf=true
> >   git-new-workdir: mark script as LF-only
> >   completion: mark bash script as LF-only
> >   Fix the remaining tests that failed with core.autocrlf=true
> >   t4051: mark supporting files as requiring LF-only line endings
> 
> With or without that change,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

I added that footer to the patches (not to the two new ones, though).

> The t/README bit in patch 4/5 is sad (I want to be able to open
> t/README using an old version of Notepad) but changing that test to
> use another file seems out of scope for what this series does.

Yes, it is sad. Maybe I should list the tests that do this (use files
outside any tNNNN/ directory):

t4003-diff-rename-1.sh (uses t/diff-lib/COPYING)
t4005-diff-rename-2.sh (uses t/diff-lib/COPYING)
t4007-rename-3.sh (uses t/diff-lib/COPYING)
t4008-diff-break-rewrite.sh (uses t/diff-lib/README and t/diff-lib/COPYING)
t4009-diff-rename-4.sh (uses t/diff-lib/COPYING)
t4022-diff-rewrite.sh (uses COPYING)
t4023-diff-rename-typechange.sh (uses COPYING)
t7001-mv.sh (uses README.md (!!!) and COPYING)
t7060-wtstatus.sh (uses t/README)
t7101-reset-empty-subdirs.sh (uses COPYING)

Note most of these tests may *use* those files, but do *not* assume that
they have Unix line endings! Only a couple test compare SHA-1s to
hardcoded values (which, if you ask me, is a bit fragile, given that files
outside the tests' control are used).

Interesting side note: t0022-crlf-rename.sh copies *itself* to the trash*
directory where it is then used to perform tests. So while this test uses
"an outside file", that file happens to be a .sh file which we already
have to mark LF-only for different reasons (Bash likes its input
CR/LF-free).

Another interesting side note: the convention appears to dictate that
supporting files should be either generated in the test script itself, or
be committed into t/tNNNN/ directories (where NNNN matches the respective
test script's number, or reuses another test script's supporting files). A
notable exception is t3901 which has the supporting files t3901-8859-1.txt
and t3901-utf8.txt. I would wageer that this is just a remnant of ancient
times before the current convention, judging by the date of the commit
that added these files: a731ec5eb82 (t3901: test "format-patch | am" pipe
with i18n, 2007-01-13). The scripts t0203-gettext-setlocale-sanity.sh,
t9350-fast-export.sh and t9500-gitweb-standalone-no-errors.sh merely reuse
those files, and when those scripts started using those files, they did
not change their location. I made this move a preparatory step in this
patch series.

Back to the question what to do about those tests that make improper
assumptions about line endings of files outside the tests' realm: I think
I can do this more cleverly, as it would appear that only four test
scripts make hard assumptions about the exact byte sequence of the COPYING
file, and I simply turned those `cp` (and even `cat`!) calls into `tr -d
"\015"` calls.

I will Cc: you on v2.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/5] Abide by our own rules regarding line endings
  2017-05-02 12:29 [PATCH 0/5] Abide by our own rules regarding line endings Johannes Schindelin
                   ` (5 preceding siblings ...)
  2017-05-02 21:56 ` [PATCH 0/5] Abide by our own rules regarding " Jonathan Nieder
@ 2017-05-04  5:19 ` Junio C Hamano
  2017-05-04  9:47   ` Johannes Schindelin
  2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
  7 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2017-05-04  5:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> For starters, those files include shell scripts: the most prevalent
> shell interpreter in use (and certainly used in Git for Windows) is
> Bash, and Bash does not handle CR/LF line endings gracefully.

Good to know.  I am not sure if it is OK for shell scripts not to
honor the platform convention, though.

Stated from the opposite angle, I would not be surprised if your
shell scripts do not work on Linux if you set core.autocrlf to true.
Git may honor it, but shells on Linux (or BSD for that matter) do
not pay attention to core.autocrlf and they are within their rights
to complain on an extra CR at the end of the line.  IOW, I would
doubt that it should be our goal to set core.autocrlf on a platform
whose native line endings is LF and make the tests to pass.

> Related to shell scripts: when generating common-cmds.h, we use tools
> that generally operate on the assumption that input and output
> deliminate their lines using LF-only line endings. Consequently, they
> would happily copy the CR byte verbatim into the strings in
> common-cmds.h, which in turn makes the C preprocessor barf (that
> interprets them as MacOS-style line endings).

This indeed is a problem.  "add\r" is not a name of a common
command, obviously, regardless of how the text file that lists the
names of the commands is encoded.  I am undecided if it is a problem
in the source text (i.e. command-list.txt is not a platform neutral
"text" but has to be encoded with LF endings) or the bug in the
tools used in the generate-cmdlist.sh script, though.  Shouldn't the
tools be aware of the platform convention of what text files are and
how their eol looks like?



^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/5] Abide by our own rules regarding line endings
  2017-05-04  5:19 ` Junio C Hamano
@ 2017-05-04  9:47   ` Johannes Schindelin
  2017-05-04 15:04     ` Junio C Hamano
  0 siblings, 1 reply; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04  9:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi Junio,

On Wed, 3 May 2017, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > For starters, those files include shell scripts: the most prevalent
> > shell interpreter in use (and certainly used in Git for Windows) is
> > Bash, and Bash does not handle CR/LF line endings gracefully.
> 
> Good to know.  I am not sure if it is OK for shell scripts not to honor
> the platform convention, though.

Well, a couple of comments about your comment:

- we say "shell scripts", but we're sloppy there: they are "Unix shell
  scripts", as they are executed by Unix shells. As such, it is pretty
  obvious that they favor Unix line endings, right? And that they do not
  really handle anything else well, right?

- You try to say that it is not okay for shell scripts to be checked
  out as LF-only when the platform convention for *text* files is CR/LF,
  right? Please note that if you follow through on this thought, you are
  very close to recommending to render shell scripts dysfunctional by
  checking them out with CR/LF endings.

That latter point, to recommend to break shell scripts, is something I
really fail to understand...

> Stated from the opposite angle, I would not be surprised if your
> shell scripts do not work on Linux if you set core.autocrlf to true.
> Git may honor it, but shells on Linux (or BSD for that matter) do
> not pay attention to core.autocrlf and they are within their rights
> to complain on an extra CR at the end of the line.  IOW, I would
> doubt that it should be our goal to set core.autocrlf on a platform
> whose native line endings is LF and make the tests to pass.

See? I *knew* it was a mistake to follow Jonathan's recommendation to make
this "you can reproduce this even on Linux" comment part of the commit
message.

I *never* asked to make core.autocrlf=true the default on Linux.

All I did was to point out that you do not need Windows to reproduce the
problems.

That is really a far cry from trying to convince anybody that it makes
sense to require Git to pass the build & tests with core.autocrlf=true *on
Linux*.

I want to make it pass on Windows, yes, and I do not want to force anybody
with a Linux setup to get a (free) Windows VM to test this. I want it to
pass on Windows, and to make it easier for you Linux-only folks, I tried
to give you a way to start validating my claims that core.autocrlf=true
was introduced by Git without even bothering to let Git itself build and
pass the test suite with that setting.

> > Related to shell scripts: when generating common-cmds.h, we use tools
> > that generally operate on the assumption that input and output
> > deliminate their lines using LF-only line endings. Consequently, they
> > would happily copy the CR byte verbatim into the strings in
> > common-cmds.h, which in turn makes the C preprocessor barf (that
> > interprets them as MacOS-style line endings).
> 
> This indeed is a problem.  "add\r" is not a name of a common
> command, obviously,

Please note that it is not "add\r" that is part of the common-cmds.h file
as generated by current git.git's `master` with core.autocrlf=true. I.e.
it is not the sequence containing a backslash followed by an `r`.

It is actually "add<CR>", which the GNU C preprocessor interprets as a
line break in the middle of the string constant (most likely for
backwards-compatibility with MacOS, where line breaks were indicated by
Carriage Returns *without* Line Feeds).

> regardless of how the text file that lists the names of the commands is
> encoded.  I am undecided if it is a problem in the source text (i.e.
> command-list.txt is not a platform neutral "text" but has to be encoded
> with LF endings) or the bug in the tools used in the generate-cmdlist.sh
> script, though.  Shouldn't the tools be aware of the platform convention
> of what text files are and how their eol looks like?

I wonder why we spend so much time on discussing this issue, really.

Clearly, command-list.txt is *intended* as input for scripts. We do not
ship the file verbatim to the end user, we only pass it through sed to
generate common-cmds.h, we pass it through sed to verify the completeness
of the docs, and we pass it through the Perl script
Documentation/cmd-list.perl to generate certain command lists intended for
inclusion in the man page.

In all cases, we expect to feed the contents of this file to Unix shell
scripts and/or Unix tools.

Is it so unobvious that the input should be crafted to fulfill that role
as best as it can by catering to Unix tools?

And if you truly think that we should use different tools based on the
platform, then you will have to swallow the rather large pill that Git's
own very heavy use of Unix shell scripting was a big, big mistake from the
beginning.

I doubt you are ready to accept that yet...

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH v2 0/7] Abide by our own rules regarding line endings
  2017-05-02 12:29 [PATCH 0/5] Abide by our own rules regarding line endings Johannes Schindelin
                   ` (6 preceding siblings ...)
  2017-05-04  5:19 ` Junio C Hamano
@ 2017-05-04  9:49 ` Johannes Schindelin
  2017-05-04  9:49   ` [PATCH v2 1/7] Fix build with core.autocrlf=true Johannes Schindelin
                     ` (7 more replies)
  7 siblings, 8 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04  9:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

[Cc:ing Jonathan Nieder, who reviewed the first iteration of this patch
series, and Jeff Hostetler who is guilty of prodding me into finishing
this patch series in the first place.]

Over the past decade, there have been a couple of attempts to remedy the
situation regarding line endings (Windows/DOS line endings are
traditionally CR/LF even if many current applications can handle LF,
too, Linux/MacOSX/*BSD/Unix uses LF-only line handlings, and old MacOS
software used to use CR-only line endings).

The current idea seems to be that the default line endings differ
depending on the platform, and for files that should be exempt from that
default, we strongly recommend using .gitattributes to define what the
source code requires.

It is time to heed our own recommendation and mark the files that
require LF-only line endings in our very own .gitattributes.

For starters, those files include shell scripts: the most prevalent
shell interpreter in use (and certainly used in Git for Windows) is
Bash, and Bash does not handle CR/LF line endings gracefully.

There are even two shell scripts that are used in the test suite even if
they are not technically supposed to be part of core Git, as indicated
by their habitat inside contrib/: git-new-workdir and
git-completion.bash.

Related to shell scripts: when generating common-cmds.h, we use tools
that generally operate on the assumption that input and output
deliminate their lines using LF-only line endings. Consequently, they
would happily copy the CR byte verbatim into the strings in
common-cmds.h, which in turn makes the C preprocessor barf (that
interprets them as MacOS-style line endings).

Further, the most obvious required fixes concern tests' support files
committed verbatim, to be compared to Git's output, which is always
LF-only.

There are a few SVN dump files, too, supporting the Subversion-related
tests, requiring LF-only line endings.

Finally, the test suite makes use of text files that are not obviously
supporting tests, such as t/README, comparing them to LF-only versions
(and consequently failing if the files have been checked out with CR/LF
line endings). Therefore we convert those to LF-only versions when
the test hard-codes that expectation.

Without these fixes, Git will fail to build and pass the test suite, as
can be verified even on Linux using this cadence:

	git config core.autocrlf true
	rm .git/index && git stash
	make DEVELOPER=1 -j15 test

Note: I separated out the change marking t/t4051/* as LF-only into an
individual commit for one reason: it would appear that the test passes
if checked out using core.autocrlf=true on Linux, but on Windows the
test fails. In that respect, this test is special, as the other changes
can be easily validated even without using Windows.

Changes since v1:

- the t/t3901-*.txt files have been moved into t/t3901/, in line with
  the de facto convention all other tests follow.

- instead of marking files outside of t/t[0-9]* as LF-only, the
  following tests are now adjusted *not* to expect LF-only line endings
  in files they do not control: t4003, t4005, t4007 & t4008.

- as a consequence, files used in the tests but living outside
  t/t[0-9]*, such as COPYING, etc, are no longer marked as LF-only.

- clarified in the commit message of the last patch why t4051 passes on
  Linux with core.autocrlf=true but not on Windows.


Johannes Schindelin (7):
  Fix build with core.autocrlf=true
  git-new-workdir: mark script as LF-only
  completion: mark bash script as LF-only
  t3901: move supporting files into t/t3901/
  t4003, t4005, t4007 & t4008: handle CR/LF in t/README &
    t/diff-lib/README
  Fix the remaining tests that failed with core.autocrlf=true
  t4051: mark supporting files as requiring LF-only line endings

 .gitattributes                           |  8 ++++++-
 contrib/completion/.gitattributes        |  1 +
 contrib/workdir/.gitattributes           |  1 +
 git-gui/.gitattributes                   |  1 +
 t/.gitattributes                         | 21 +++++++++++++++++-
 t/t0203-gettext-setlocale-sanity.sh      |  4 ++--
 t/t3901-i18n-patch.sh                    | 38 ++++++++++++++++----------------
 t/{t3901-8859-1.txt => t3901/8859-1.txt} |  0
 t/{t3901-utf8.txt => t3901/utf8.txt}     |  0
 t/t4003-diff-rename-1.sh                 |  4 ++--
 t/t4005-diff-rename-2.sh                 |  4 ++--
 t/t4007-rename-3.sh                      |  2 +-
 t/t4008-diff-break-rewrite.sh            |  4 ++--
 t/t9350-fast-export.sh                   |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh   |  4 ++--
 15 files changed, 61 insertions(+), 33 deletions(-)
 create mode 100644 contrib/completion/.gitattributes
 create mode 100644 contrib/workdir/.gitattributes
 rename t/{t3901-8859-1.txt => t3901/8859-1.txt} (100%)
 rename t/{t3901-utf8.txt => t3901/utf8.txt} (100%)


base-commit: d2bbb7c2bcf6e77ebfcabf4e12110fe6d5c91de6
Published-As: https://github.com/dscho/git/releases/tag/lf-attrs-v2
Fetch-It-Via: git fetch https://github.com/dscho/git lf-attrs-v2

Interdiff vs v1:

 diff --git a/t/.gitattributes b/t/.gitattributes
 index 1bdc46a53f1..bdd82cf31f7 100644
 --- a/t/.gitattributes
 +++ b/t/.gitattributes
 @@ -1,9 +1,7 @@
 -README eol=lf
  t[0-9][0-9][0-9][0-9]/* -whitespace
 -/diff-lib/COPYING eol=lf
  /t0110/url-* binary
  /t3900/*.txt eol=lf
 -/t3901*.txt eol=lf
 +/t3901/*.txt eol=lf
  /t4034/*/* eol=lf
  /t4013/* eol=lf
  /t4018/* eol=lf
 @@ -20,10 +18,4 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
  /t556x_common eol=lf
  /t7500/* eol=lf
  /t8005/*.txt eol=lf
 -/t9110/svm.dump eol=lf
 -/t9111/svnsync.dump eol=lf
 -/t9115/funky-names.dump eol=lf
 -/t9121/renamed-dir.dump eol=lf
 -/t913[56]/svn.dump eol=lf
 -/t915[0134]/*.dump eol=lf
 -/t9161/branches.dump eol=lf
 +/t9*/*.dump eol=lf
 diff --git a/t/t0203-gettext-setlocale-sanity.sh b/t/t0203-gettext-setlocale-sanity.sh
 index a2124600811..71b0d74b4dd 100755
 --- a/t/t0203-gettext-setlocale-sanity.sh
 +++ b/t/t0203-gettext-setlocale-sanity.sh
 @@ -8,7 +8,7 @@ test_description="The Git C functions aren't broken by setlocale(3)"
  . ./lib-gettext.sh
  
  test_expect_success 'git show a ISO-8859-1 commit under C locale' '
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  	test_commit "iso-c-commit" iso-under-c &&
  	git show >out 2>err &&
  	! test -s err &&
 @@ -16,7 +16,7 @@ test_expect_success 'git show a ISO-8859-1 commit under C locale' '
  '
  
  test_expect_success GETTEXT_LOCALE 'git show a ISO-8859-1 commit under a UTF-8 locale' '
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  	test_commit "iso-utf8-commit" iso-under-utf8 &&
  	LANGUAGE=is LC_ALL="$is_IS_locale" git show >out 2>err &&
  	! test -s err &&
 diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
 index f663d567c8a..923eb01f0ea 100755
 --- a/t/t3901-i18n-patch.sh
 +++ b/t/t3901-i18n-patch.sh
 @@ -31,7 +31,7 @@ test_expect_success setup '
  
  	# use UTF-8 in author and committer name to match the
  	# i18n.commitencoding settings
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  
  	test_tick &&
  	echo "$GIT_AUTHOR_NAME" >mine &&
 @@ -55,7 +55,7 @@ test_expect_success setup '
  		# the second one on the side branch is ISO-8859-1
  		git config i18n.commitencoding ISO8859-1 &&
  		# use author and committer name in ISO-8859-1 to match it.
 -		. "$TEST_DIRECTORY"/t3901-8859-1.txt
 +		. "$TEST_DIRECTORY"/t3901/8859-1.txt
  	fi &&
  	test_tick &&
  	echo Yet another >theirs &&
 @@ -100,7 +100,7 @@ test_expect_success 'rebase (U/U)' '
  
  	# The result will be committed by GIT_COMMITTER_NAME --
  	# we want UTF-8 encoded name.
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  	git checkout -b test &&
  	git rebase master &&
  
 @@ -110,7 +110,7 @@ test_expect_success 'rebase (U/U)' '
  test_expect_success 'rebase (U/L)' '
  	git config i18n.commitencoding UTF-8 &&
  	git config i18n.logoutputencoding ISO8859-1 &&
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  
  	git reset --hard side &&
  	git rebase master &&
 @@ -122,7 +122,7 @@ test_expect_success !MINGW 'rebase (L/L)' '
  	# In this test we want ISO-8859-1 encoded commits as the result
  	git config i18n.commitencoding ISO8859-1 &&
  	git config i18n.logoutputencoding ISO8859-1 &&
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  
  	git reset --hard side &&
  	git rebase master &&
 @@ -135,7 +135,7 @@ test_expect_success !MINGW 'rebase (L/U)' '
  	# to get ISO-8859-1 results.
  	git config i18n.commitencoding ISO8859-1 &&
  	git config i18n.logoutputencoding UTF-8 &&
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  
  	git reset --hard side &&
  	git rebase master &&
 @@ -148,7 +148,7 @@ test_expect_success 'cherry-pick(U/U)' '
  
  	git config i18n.commitencoding UTF-8 &&
  	git config i18n.logoutputencoding UTF-8 &&
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  
  	git reset --hard master &&
  	git cherry-pick side^ &&
 @@ -163,7 +163,7 @@ test_expect_success !MINGW 'cherry-pick(L/L)' '
  
  	git config i18n.commitencoding ISO8859-1 &&
  	git config i18n.logoutputencoding ISO8859-1 &&
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  
  	git reset --hard master &&
  	git cherry-pick side^ &&
 @@ -178,7 +178,7 @@ test_expect_success 'cherry-pick(U/L)' '
  
  	git config i18n.commitencoding UTF-8 &&
  	git config i18n.logoutputencoding ISO8859-1 &&
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  
  	git reset --hard master &&
  	git cherry-pick side^ &&
 @@ -194,7 +194,7 @@ test_expect_success !MINGW 'cherry-pick(L/U)' '
  
  	git config i18n.commitencoding ISO8859-1 &&
  	git config i18n.logoutputencoding UTF-8 &&
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  
  	git reset --hard master &&
  	git cherry-pick side^ &&
 @@ -207,7 +207,7 @@ test_expect_success !MINGW 'cherry-pick(L/U)' '
  test_expect_success 'rebase --merge (U/U)' '
  	git config i18n.commitencoding UTF-8 &&
  	git config i18n.logoutputencoding UTF-8 &&
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  
  	git reset --hard side &&
  	git rebase --merge master &&
 @@ -218,7 +218,7 @@ test_expect_success 'rebase --merge (U/U)' '
  test_expect_success 'rebase --merge (U/L)' '
  	git config i18n.commitencoding UTF-8 &&
  	git config i18n.logoutputencoding ISO8859-1 &&
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  
  	git reset --hard side &&
  	git rebase --merge master &&
 @@ -230,7 +230,7 @@ test_expect_success 'rebase --merge (L/L)' '
  	# In this test we want ISO-8859-1 encoded commits as the result
  	git config i18n.commitencoding ISO8859-1 &&
  	git config i18n.logoutputencoding ISO8859-1 &&
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  
  	git reset --hard side &&
  	git rebase --merge master &&
 @@ -243,7 +243,7 @@ test_expect_success 'rebase --merge (L/U)' '
  	# to get ISO-8859-1 results.
  	git config i18n.commitencoding ISO8859-1 &&
  	git config i18n.logoutputencoding UTF-8 &&
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  
  	git reset --hard side &&
  	git rebase --merge master &&
 @@ -254,7 +254,7 @@ test_expect_success 'rebase --merge (L/U)' '
  test_expect_success 'am (U/U)' '
  	# Apply UTF-8 patches with UTF-8 commitencoding
  	git config i18n.commitencoding UTF-8 &&
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  
  	git reset --hard master &&
  	git am out-u1 out-u2 &&
 @@ -265,7 +265,7 @@ test_expect_success 'am (U/U)' '
  test_expect_success !MINGW 'am (L/L)' '
  	# Apply ISO-8859-1 patches with ISO-8859-1 commitencoding
  	git config i18n.commitencoding ISO8859-1 &&
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  
  	git reset --hard master &&
  	git am out-l1 out-l2 &&
 @@ -276,7 +276,7 @@ test_expect_success !MINGW 'am (L/L)' '
  test_expect_success 'am (U/L)' '
  	# Apply ISO-8859-1 patches with UTF-8 commitencoding
  	git config i18n.commitencoding UTF-8 &&
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  	git reset --hard master &&
  
  	# am specifies --utf8 by default.
 @@ -288,7 +288,7 @@ test_expect_success 'am (U/L)' '
  test_expect_success 'am --no-utf8 (U/L)' '
  	# Apply ISO-8859-1 patches with UTF-8 commitencoding
  	git config i18n.commitencoding UTF-8 &&
 -	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  
  	git reset --hard master &&
  	git am --no-utf8 out-l1 out-l2 2>err &&
 @@ -303,7 +303,7 @@ test_expect_success 'am --no-utf8 (U/L)' '
  test_expect_success !MINGW 'am (L/U)' '
  	# Apply UTF-8 patches with ISO-8859-1 commitencoding
  	git config i18n.commitencoding ISO8859-1 &&
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  
  	git reset --hard master &&
  	# mailinfo will re-code the commit message to the charset specified by
 diff --git a/t/t3901-8859-1.txt b/t/t3901/8859-1.txt
 similarity index 100%
 rename from t/t3901-8859-1.txt
 rename to t/t3901/8859-1.txt
 diff --git a/t/t3901-utf8.txt b/t/t3901/utf8.txt
 similarity index 100%
 rename from t/t3901-utf8.txt
 rename to t/t3901/utf8.txt
 diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
 index df2accb6555..c3e0a3c3fc9 100755
 --- a/t/t4003-diff-rename-1.sh
 +++ b/t/t4003-diff-rename-1.sh
 @@ -11,7 +11,7 @@ test_description='More rename detection
  
  test_expect_success \
      'prepare reference tree' \
 -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
 +    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
       echo frotz >rezrov &&
      git update-index --add COPYING rezrov &&
      tree=$(git write-tree) &&
 @@ -99,7 +99,7 @@ test_expect_success \
  
  test_expect_success \
      'prepare work tree once again' \
 -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
 +    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
       git update-index --add --remove COPYING COPYING.1'
  
  # tree has COPYING and rezrov.  work tree has COPYING and COPYING.1,
 diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
 index 135addbfbda..f1641c35ee2 100755
 --- a/t/t4005-diff-rename-2.sh
 +++ b/t/t4005-diff-rename-2.sh
 @@ -11,7 +11,7 @@ test_description='Same rename detection as t4003 but testing diff-raw.
  
  test_expect_success \
      'prepare reference tree' \
 -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
 +    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
       echo frotz >rezrov &&
      git update-index --add COPYING rezrov &&
      tree=$(git write-tree) &&
 @@ -71,7 +71,7 @@ test_expect_success \
  
  test_expect_success \
      'prepare work tree once again' \
 -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
 +    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
       git update-index --add --remove COPYING COPYING.1'
  
  git diff-index -C --find-copies-harder $tree >current
 diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
 index dae327fabbf..0157fde5503 100755
 --- a/t/t4007-rename-3.sh
 +++ b/t/t4007-rename-3.sh
 @@ -11,7 +11,7 @@ test_description='Rename interaction with pathspec.
  
  test_expect_success 'prepare reference tree' '
  	mkdir path0 path1 &&
 -	cp "$TEST_DIRECTORY"/diff-lib/COPYING path0/COPYING &&
 +	tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >path0/COPYING &&
  	git update-index --add path0/COPYING &&
  	tree=$(git write-tree) &&
  	echo $tree
 diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh
 index 9dd1bc5e162..5af4fa6aadb 100755
 --- a/t/t4008-diff-break-rewrite.sh
 +++ b/t/t4008-diff-break-rewrite.sh
 @@ -25,8 +25,8 @@ Further, with -B and -M together, these should turn into two renames.
  . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
  
  test_expect_success setup '
 -	cat "$TEST_DIRECTORY"/diff-lib/README >file0 &&
 -	cat "$TEST_DIRECTORY"/diff-lib/COPYING >file1 &&
 +	tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/README >file0 &&
 +	tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >file1 &&
  	git update-index --add file0 file1 &&
  	git tag reference $(git write-tree)
  '
 diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
 index b5149fde6ec..8dcb05c4a57 100755
 --- a/t/t9350-fast-export.sh
 +++ b/t/t9350-fast-export.sh
 @@ -70,7 +70,7 @@ test_expect_success 'iso-8859-1' '
  
  	git config i18n.commitencoding ISO8859-1 &&
  	# use author and committer name in ISO-8859-1 to match it.
 -	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  	test_tick &&
  	echo rosten >file &&
  	git commit -s -m den file &&
 diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
 index 6d06ed96cbc..cc8d463e01a 100755
 --- a/t/t9500-gitweb-standalone-no-errors.sh
 +++ b/t/t9500-gitweb-standalone-no-errors.sh
 @@ -519,7 +519,7 @@ test_expect_success \
  
  test_expect_success \
  	'encode(commit): utf8' \
 -	'. "$TEST_DIRECTORY"/t3901-utf8.txt &&
 +	'. "$TEST_DIRECTORY"/t3901/utf8.txt &&
  	 test_when_finished "GIT_AUTHOR_NAME=\"A U Thor\"" &&
  	 test_when_finished "GIT_COMMITTER_NAME=\"C O Mitter\"" &&
  	 echo "UTF-8" >> file &&
 @@ -529,7 +529,7 @@ test_expect_success \
  
  test_expect_success \
  	'encode(commit): iso-8859-1' \
 -	'. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +	'. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
  	 test_when_finished "GIT_AUTHOR_NAME=\"A U Thor\"" &&
  	 test_when_finished "GIT_COMMITTER_NAME=\"C O Mitter\"" &&
  	 echo "ISO-8859-1" >> file &&

-- 
2.12.2.windows.2.800.gede8f145e06


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH v2 1/7] Fix build with core.autocrlf=true
  2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
@ 2017-05-04  9:49   ` 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
                     ` (6 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04  9:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

On Windows, the default line endings are denoted by a Carriage Return
byte followed by a Line Feed byte, while Linux and MacOSX use a single
Line Feed byte to denote a line ending.

To help with this situation, Git introduced several mechanisms over the
last decade, most prominently the `core.autocrlf` setting.

Sometimes, however, a single setting is incorrect, e.g. when certain
files in the source code are to be consumed by software that can handle
only LF line endings, while other files can use whatever is appropriate
for the current platform.

To allow for that, Git added the `eol` option to its .gitattributes
handling, expecting every user of Git to mark their source code
appropriately.

Bash assumes that line-endings of scripts are denoted by a single Line
Feed byte. Therefore, shell scripts in Git's source code are one example
where that `eol=lf` option is *required*.

When generating common-cmds.h, the Unix tools we use generally operate on
the assumption that input and output deliminate their lines using LF-only
line endings. Consequently, they would happily copy the CR byte verbatim
into the strings in common-cmds.h, which in turn makes the C preprocessor
barf (that interprets them as MacOS-style line endings). Therefore, we
have to mark the input files as LF-only: command-list.txt and
Documentation/git-*.txt.

Quite a bit belatedly, this patch brings Git's own source code in line
with those expectations by setting those attributes to allow for a
correct build even when core.autocrlf=true.

This patch can be validated even on Linux, by using this cadence:

	git config core.autocrlf true
	rm .git/index && git stash
	make -j15 DEVELOPER=1

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .gitattributes         | 8 +++++++-
 git-gui/.gitattributes | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/.gitattributes b/.gitattributes
index 320e33c327c..8ce9c6b8888 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,3 +1,9 @@
 * whitespace=!indent,trail,space
 *.[ch] whitespace=indent,trail,space diff=cpp
-*.sh whitespace=indent,trail,space
+*.sh whitespace=indent,trail,space eol=lf
+*.perl eol=lf
+*.pm eol=lf
+/Documentation/git-*.txt eol=lf
+/command-list.txt eol=lf
+/GIT-VERSION-GEN eol=lf
+/mergetools/* eol=lf
diff --git a/git-gui/.gitattributes b/git-gui/.gitattributes
index 33d07c06bd9..59cd41dbff7 100644
--- a/git-gui/.gitattributes
+++ b/git-gui/.gitattributes
@@ -2,3 +2,4 @@
 *           encoding=US-ASCII
 git-gui.sh  encoding=UTF-8
 /po/*.po    encoding=UTF-8
+/GIT-VERSION-GEN eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 2/7] git-new-workdir: mark script as LF-only
  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-04  9:49   ` Johannes Schindelin
  2017-05-08  5:11     ` Junio C Hamano
  2017-05-04  9:49   ` [PATCH v2 3/7] completion: mark bash " Johannes Schindelin
                     ` (5 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04  9:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

Bash does not handle scripts with CR/LF line endings correctly, therefore
they *have* to be forced to LF-only line endings.

Funnily enough, this fixes t3000-ls-files-others and
t1021-rerere-in-workdir when git.git was checked out with
core.autocrlf=true, as these test still use git-new-workdir (once `git
worktree` is no longer marked as experimental, both scripts probably
want to be ported to using that command instead).

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/workdir/.gitattributes | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 contrib/workdir/.gitattributes

diff --git a/contrib/workdir/.gitattributes b/contrib/workdir/.gitattributes
new file mode 100644
index 00000000000..1f78c5d1bd3
--- /dev/null
+++ b/contrib/workdir/.gitattributes
@@ -0,0 +1 @@
+/git-new-workdir eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 3/7] completion: mark bash script as LF-only
  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-04  9:49   ` [PATCH v2 2/7] git-new-workdir: mark script as LF-only Johannes Schindelin
@ 2017-05-04  9:49   ` Johannes Schindelin
  2017-05-04  9:49   ` [PATCH v2 4/7] t3901: move supporting files into t/t3901/ Johannes Schindelin
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04  9:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

Without this change, the completion script does not work, as Bash expects
its scripts to have line feeds as end-of-line markers (this is
particularly prominent in quoted multi-line strings, where carriage
returns would slip into the strings as verbatim characters otherwise).

This change is required to let t9902-completion pass when Git's source
code is checked out with `core.autocrlf = true`.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/completion/.gitattributes | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 contrib/completion/.gitattributes

diff --git a/contrib/completion/.gitattributes b/contrib/completion/.gitattributes
new file mode 100644
index 00000000000..19116944c15
--- /dev/null
+++ b/contrib/completion/.gitattributes
@@ -0,0 +1 @@
+*.bash eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 4/7] t3901: move supporting files into t/t3901/
  2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
                     ` (2 preceding siblings ...)
  2017-05-04  9:49   ` [PATCH v2 3/7] completion: mark bash " Johannes Schindelin
@ 2017-05-04  9:49   ` 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
                     ` (3 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04  9:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

The current convention is to either generate files on the fly in tests,
or to use supporting files taken from a t/tNNNN/ directory (where NNNN
matches the test's number, or the number of the test from which we
borrow supporting files).

The test t3901-i18n-patch.sh was obviously introduced before that
convention was in full swing, hence its supporting files still lived in
t/t3901-8859-1.txt and t/t3901-utf8.txt, respectively.

Let's adjust to the current convention.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t0203-gettext-setlocale-sanity.sh      |  4 ++--
 t/t3901-i18n-patch.sh                    | 38 ++++++++++++++++----------------
 t/{t3901-8859-1.txt => t3901/8859-1.txt} |  0
 t/{t3901-utf8.txt => t3901/utf8.txt}     |  0
 t/t9350-fast-export.sh                   |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh   |  4 ++--
 6 files changed, 24 insertions(+), 24 deletions(-)
 rename t/{t3901-8859-1.txt => t3901/8859-1.txt} (100%)
 rename t/{t3901-utf8.txt => t3901/utf8.txt} (100%)

diff --git a/t/t0203-gettext-setlocale-sanity.sh b/t/t0203-gettext-setlocale-sanity.sh
index a2124600811..71b0d74b4dd 100755
--- a/t/t0203-gettext-setlocale-sanity.sh
+++ b/t/t0203-gettext-setlocale-sanity.sh
@@ -8,7 +8,7 @@ test_description="The Git C functions aren't broken by setlocale(3)"
 . ./lib-gettext.sh
 
 test_expect_success 'git show a ISO-8859-1 commit under C locale' '
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 	test_commit "iso-c-commit" iso-under-c &&
 	git show >out 2>err &&
 	! test -s err &&
@@ -16,7 +16,7 @@ test_expect_success 'git show a ISO-8859-1 commit under C locale' '
 '
 
 test_expect_success GETTEXT_LOCALE 'git show a ISO-8859-1 commit under a UTF-8 locale' '
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 	test_commit "iso-utf8-commit" iso-under-utf8 &&
 	LANGUAGE=is LC_ALL="$is_IS_locale" git show >out 2>err &&
 	! test -s err &&
diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
index f663d567c8a..923eb01f0ea 100755
--- a/t/t3901-i18n-patch.sh
+++ b/t/t3901-i18n-patch.sh
@@ -31,7 +31,7 @@ test_expect_success setup '
 
 	# use UTF-8 in author and committer name to match the
 	# i18n.commitencoding settings
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	test_tick &&
 	echo "$GIT_AUTHOR_NAME" >mine &&
@@ -55,7 +55,7 @@ test_expect_success setup '
 		# the second one on the side branch is ISO-8859-1
 		git config i18n.commitencoding ISO8859-1 &&
 		# use author and committer name in ISO-8859-1 to match it.
-		. "$TEST_DIRECTORY"/t3901-8859-1.txt
+		. "$TEST_DIRECTORY"/t3901/8859-1.txt
 	fi &&
 	test_tick &&
 	echo Yet another >theirs &&
@@ -100,7 +100,7 @@ test_expect_success 'rebase (U/U)' '
 
 	# The result will be committed by GIT_COMMITTER_NAME --
 	# we want UTF-8 encoded name.
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 	git checkout -b test &&
 	git rebase master &&
 
@@ -110,7 +110,7 @@ test_expect_success 'rebase (U/U)' '
 test_expect_success 'rebase (U/L)' '
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard side &&
 	git rebase master &&
@@ -122,7 +122,7 @@ test_expect_success !MINGW 'rebase (L/L)' '
 	# In this test we want ISO-8859-1 encoded commits as the result
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard side &&
 	git rebase master &&
@@ -135,7 +135,7 @@ test_expect_success !MINGW 'rebase (L/U)' '
 	# to get ISO-8859-1 results.
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard side &&
 	git rebase master &&
@@ -148,7 +148,7 @@ test_expect_success 'cherry-pick(U/U)' '
 
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard master &&
 	git cherry-pick side^ &&
@@ -163,7 +163,7 @@ test_expect_success !MINGW 'cherry-pick(L/L)' '
 
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard master &&
 	git cherry-pick side^ &&
@@ -178,7 +178,7 @@ test_expect_success 'cherry-pick(U/L)' '
 
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard master &&
 	git cherry-pick side^ &&
@@ -194,7 +194,7 @@ test_expect_success !MINGW 'cherry-pick(L/U)' '
 
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard master &&
 	git cherry-pick side^ &&
@@ -207,7 +207,7 @@ test_expect_success !MINGW 'cherry-pick(L/U)' '
 test_expect_success 'rebase --merge (U/U)' '
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard side &&
 	git rebase --merge master &&
@@ -218,7 +218,7 @@ test_expect_success 'rebase --merge (U/U)' '
 test_expect_success 'rebase --merge (U/L)' '
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard side &&
 	git rebase --merge master &&
@@ -230,7 +230,7 @@ test_expect_success 'rebase --merge (L/L)' '
 	# In this test we want ISO-8859-1 encoded commits as the result
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard side &&
 	git rebase --merge master &&
@@ -243,7 +243,7 @@ test_expect_success 'rebase --merge (L/U)' '
 	# to get ISO-8859-1 results.
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard side &&
 	git rebase --merge master &&
@@ -254,7 +254,7 @@ test_expect_success 'rebase --merge (L/U)' '
 test_expect_success 'am (U/U)' '
 	# Apply UTF-8 patches with UTF-8 commitencoding
 	git config i18n.commitencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard master &&
 	git am out-u1 out-u2 &&
@@ -265,7 +265,7 @@ test_expect_success 'am (U/U)' '
 test_expect_success !MINGW 'am (L/L)' '
 	# Apply ISO-8859-1 patches with ISO-8859-1 commitencoding
 	git config i18n.commitencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard master &&
 	git am out-l1 out-l2 &&
@@ -276,7 +276,7 @@ test_expect_success !MINGW 'am (L/L)' '
 test_expect_success 'am (U/L)' '
 	# Apply ISO-8859-1 patches with UTF-8 commitencoding
 	git config i18n.commitencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 	git reset --hard master &&
 
 	# am specifies --utf8 by default.
@@ -288,7 +288,7 @@ test_expect_success 'am (U/L)' '
 test_expect_success 'am --no-utf8 (U/L)' '
 	# Apply ISO-8859-1 patches with UTF-8 commitencoding
 	git config i18n.commitencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard master &&
 	git am --no-utf8 out-l1 out-l2 2>err &&
@@ -303,7 +303,7 @@ test_expect_success 'am --no-utf8 (U/L)' '
 test_expect_success !MINGW 'am (L/U)' '
 	# Apply UTF-8 patches with ISO-8859-1 commitencoding
 	git config i18n.commitencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard master &&
 	# mailinfo will re-code the commit message to the charset specified by
diff --git a/t/t3901-8859-1.txt b/t/t3901/8859-1.txt
similarity index 100%
rename from t/t3901-8859-1.txt
rename to t/t3901/8859-1.txt
diff --git a/t/t3901-utf8.txt b/t/t3901/utf8.txt
similarity index 100%
rename from t/t3901-utf8.txt
rename to t/t3901/utf8.txt
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index b5149fde6ec..8dcb05c4a57 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -70,7 +70,7 @@ test_expect_success 'iso-8859-1' '
 
 	git config i18n.commitencoding ISO8859-1 &&
 	# use author and committer name in ISO-8859-1 to match it.
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 	test_tick &&
 	echo rosten >file &&
 	git commit -s -m den file &&
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 6d06ed96cbc..cc8d463e01a 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -519,7 +519,7 @@ test_expect_success \
 
 test_expect_success \
 	'encode(commit): utf8' \
-	'. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	'. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 	 test_when_finished "GIT_AUTHOR_NAME=\"A U Thor\"" &&
 	 test_when_finished "GIT_COMMITTER_NAME=\"C O Mitter\"" &&
 	 echo "UTF-8" >> file &&
@@ -529,7 +529,7 @@ test_expect_success \
 
 test_expect_success \
 	'encode(commit): iso-8859-1' \
-	'. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	'. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 	 test_when_finished "GIT_AUTHOR_NAME=\"A U Thor\"" &&
 	 test_when_finished "GIT_COMMITTER_NAME=\"C O Mitter\"" &&
 	 echo "ISO-8859-1" >> file &&
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 5/7] t4003, t4005, t4007 & t4008: handle CR/LF in t/README & t/diff-lib/README
  2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
                     ` (3 preceding siblings ...)
  2017-05-04  9:49   ` [PATCH v2 4/7] t3901: move supporting files into t/t3901/ Johannes Schindelin
@ 2017-05-04  9:50   ` 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
                     ` (2 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04  9:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

Seeing as Git originates from the Linux ecosystem, it is understandable
that the assumption of Unix line endings is deeply ingrained in Git's
source code as well as its test suite.

However, we must not force files that are otherwise unrelated to tests
to have Unix line endings just to appease test scripts that may use
them. Instead, the test scripts should be indifferent what line endings
files outside their corresponding tNNNN/ directories have.

As t4003-diff-rename-1.sh, t4005-diff-rename-2.sh, t4007-rename-3.sh &
t4008-diff-break-rewrite.sh make hard-coded assumptions about the SHA-1
of the tested files, and as those files' contents originate from outside
this script's sphere of authority, it must handle CR/LF line endings in
those files gracefully. We do that by simply stripping out CR bytes.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t4003-diff-rename-1.sh      | 4 ++--
 t/t4005-diff-rename-2.sh      | 4 ++--
 t/t4007-rename-3.sh           | 2 +-
 t/t4008-diff-break-rewrite.sh | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
index df2accb6555..c3e0a3c3fc9 100755
--- a/t/t4003-diff-rename-1.sh
+++ b/t/t4003-diff-rename-1.sh
@@ -11,7 +11,7 @@ test_description='More rename detection
 
 test_expect_success \
     'prepare reference tree' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
      echo frotz >rezrov &&
     git update-index --add COPYING rezrov &&
     tree=$(git write-tree) &&
@@ -99,7 +99,7 @@ test_expect_success \
 
 test_expect_success \
     'prepare work tree once again' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
      git update-index --add --remove COPYING COPYING.1'
 
 # tree has COPYING and rezrov.  work tree has COPYING and COPYING.1,
diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
index 135addbfbda..f1641c35ee2 100755
--- a/t/t4005-diff-rename-2.sh
+++ b/t/t4005-diff-rename-2.sh
@@ -11,7 +11,7 @@ test_description='Same rename detection as t4003 but testing diff-raw.
 
 test_expect_success \
     'prepare reference tree' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
      echo frotz >rezrov &&
     git update-index --add COPYING rezrov &&
     tree=$(git write-tree) &&
@@ -71,7 +71,7 @@ test_expect_success \
 
 test_expect_success \
     'prepare work tree once again' \
-    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
+    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
      git update-index --add --remove COPYING COPYING.1'
 
 git diff-index -C --find-copies-harder $tree >current
diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
index dae327fabbf..0157fde5503 100755
--- a/t/t4007-rename-3.sh
+++ b/t/t4007-rename-3.sh
@@ -11,7 +11,7 @@ test_description='Rename interaction with pathspec.
 
 test_expect_success 'prepare reference tree' '
 	mkdir path0 path1 &&
-	cp "$TEST_DIRECTORY"/diff-lib/COPYING path0/COPYING &&
+	tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >path0/COPYING &&
 	git update-index --add path0/COPYING &&
 	tree=$(git write-tree) &&
 	echo $tree
diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh
index 9dd1bc5e162..5af4fa6aadb 100755
--- a/t/t4008-diff-break-rewrite.sh
+++ b/t/t4008-diff-break-rewrite.sh
@@ -25,8 +25,8 @@ Further, with -B and -M together, these should turn into two renames.
 . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
 
 test_expect_success setup '
-	cat "$TEST_DIRECTORY"/diff-lib/README >file0 &&
-	cat "$TEST_DIRECTORY"/diff-lib/COPYING >file1 &&
+	tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/README >file0 &&
+	tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >file1 &&
 	git update-index --add file0 file1 &&
 	git tag reference $(git write-tree)
 '
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 6/7] Fix the remaining tests that failed with core.autocrlf=true
  2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
                     ` (4 preceding siblings ...)
  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-04  9:50   ` 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-09 12:53   ` [PATCH v3 0/6] Abide by our own rules regarding " Johannes Schindelin
  7 siblings, 1 reply; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04  9:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

The test suite is mainly developed on Linux and MacOSX, which is the
reason that nobody thought to mark files as LF-only as needed.

The symptom is a test suite that fails left and right when being checked
out using Git for Windows (which defaults to core.autocrlf=true).

Mostly, the problems stem from Git's (LF-only) output being compared to
hard-coded files that are checked out with line endings according to
core.autocrlf (which is of course incorrect).

Note: the test suite also uses the t/README file as well as the COPYING
file in t/diff-lib/, expecting LF-only line endings explicitly and
failing if that assumption does not hold true. That is why we mark them
as LF-only in the .gitattributes, too.

This patch can be validated even on Linux by using this cadence:

	git config core.autocrlf true
	rm .git/index && git stash
	make -j15 DEVELOPER=1 test

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/.gitattributes | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/t/.gitattributes b/t/.gitattributes
index 2d44088f56e..3525ca43f30 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1,2 +1,20 @@
 t[0-9][0-9][0-9][0-9]/* -whitespace
-t0110/url-* binary
+/t0110/url-* binary
+/t3900/*.txt eol=lf
+/t3901/*.txt eol=lf
+/t4034/*/* eol=lf
+/t4013/* eol=lf
+/t4018/* eol=lf
+/t4100/* eol=lf
+/t4101/* eol=lf
+/t4109/* eol=lf
+/t4110/* eol=lf
+/t4135/* eol=lf
+/t4211/* eol=lf
+/t4252/* eol=lf
+/t5100/* eol=lf
+/t5515/* eol=lf
+/t556x_common eol=lf
+/t7500/* eol=lf
+/t8005/*.txt eol=lf
+/t9*/*.dump eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 7/7] t4051: mark supporting files as requiring LF-only line endings
  2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
                     ` (5 preceding siblings ...)
  2017-05-04  9:50   ` [PATCH v2 6/7] Fix the remaining tests that failed with core.autocrlf=true Johannes Schindelin
@ 2017-05-04  9:50   ` Johannes Schindelin
  2017-05-08  5:29     ` Junio C Hamano
  2017-05-09 12:53   ` [PATCH v3 0/6] Abide by our own rules regarding " Johannes Schindelin
  7 siblings, 1 reply; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04  9:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

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.

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.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/.gitattributes | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/.gitattributes b/t/.gitattributes
index 3525ca43f30..bdd82cf31f7 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -5,6 +5,7 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
 /t4034/*/* eol=lf
 /t4013/* eol=lf
 /t4018/* eol=lf
+/t4051/* eol=lf
 /t4100/* eol=lf
 /t4101/* eol=lf
 /t4109/* eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/5] Abide by our own rules regarding line endings
  2017-05-04  9:47   ` Johannes Schindelin
@ 2017-05-04 15:04     ` Junio C Hamano
  2017-05-04 18:48       ` Johannes Schindelin
  0 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2017-05-04 15:04 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Well, a couple of comments about your comment:
>
> - we say "shell scripts", but we're sloppy there: they are "Unix shell
>   scripts", as they are executed by Unix shells. As such, it is pretty
>   obvious that they favor Unix line endings, right? And that they do not
>   really handle anything else well, right?

Not really.  I would expect that

	cat A B C

with CRLF line endings (i.e. "cat A B C<CR><LF>") on platforms whose
eol convention is LF to barf due to the missing file whose name is
"C<CR>", while on platforms whose eol convention is CRLF, I do
expect the contents of file C comes at the end of the output.

> - You try to say that it is not okay for shell scripts to be checked
>   out as LF-only when the platform convention for *text* files is CR/LF,
>   right? 

I didn't try to and I didn't say that, I hope.  I think it is not OK
for shell scripts to be checked out with <CR><LF> when the platform
convention for text is <LF>, though.  It may be possible for an
implementation on CRLF platform to tolerate missing <CR> (i.e. a
file in LF convention--instead of treating a lone <LF> as a non-eol
but just a regular "funny" byte, treat it as a normal eol), but that
would be a quality-of-implementation thing.  So when the platform
convention for text files is <CR><LF>, I would have thought that it
was more natural to check out shell scripts as such.

However, I did not think things through when I said "I am not sure
if it is OK for shell scripts not to honor the platform convention,
though."  In a sense, the "cat A B C" example does not have to worry
about the platform convention issue very much.  In real scripts, we
have things like a string literal that spans lines (i.e. do we have
a LF, or a CRLF pair, in between these two lines?) and here
documents (ditto).  To handle these "correctly" while treating shell
scripts mere "text" files, a shell implementation on CRLF platform
has to accept CRLF, pretend as if it saw only LF, do all the
processing as if the eol convention were LF, and convert LF in the
output from the script to CRLF.  I think that is _too_ much to ask
for an implementation, and such an attempt would get corner cases
wrong.  IOW, I was not sure if it is OK for scripts not to honor the
platorm convention when I wrote the message you are responding to,
but I think it probably is not just OK but is the simplest/cleanest
for shell implementations not to honor the platform convention and
instead pretend that they always live in LF-only world.

And all of the above ultimately does not matter.  If the shell you
have on Windows cannot take CRLF files, then our shell script must
be checked out as LF files even on Windows.  My "I am not sure" was
mostly from not knowing how ingrained that "cannot take CRLF files"
was (i.e. I didn't know if there may be some knobs similar to
core.crlf we have that you can toggle for your shell to honor the
platform convention).



^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/5] Abide by our own rules regarding line endings
  2017-05-04 15:04     ` Junio C Hamano
@ 2017-05-04 18:48       ` Johannes Schindelin
  2017-05-07  5:29         ` Junio C Hamano
  0 siblings, 1 reply; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-04 18:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi Junio,

On Thu, 4 May 2017, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Well, a couple of comments about your comment:
> >
> > - we say "shell scripts", but we're sloppy there: they are "Unix shell
> >   scripts", as they are executed by Unix shells. As such, it is pretty
> >   obvious that they favor Unix line endings, right? And that they do not
> >   really handle anything else well, right?
> 
> Not really.  I would expect that
> 
> 	cat A B C
> 
> with CRLF line endings (i.e. "cat A B C<CR><LF>") on platforms whose
> eol convention is LF to barf due to the missing file whose name is
> "C<CR>", while on platforms whose eol convention is CRLF, I do
> expect the contents of file C comes at the end of the output.

Let me repeat myself: `cat` is a Unix utility. Unix line endings are
LF-only.

So when you say "platform whose eol convention is CRLF", you speak about a
platform that is not Unix.

If you want to run `cat` on Linux, you have to have a Unix-y environment.
Ergo: LF-only line endings.

The real problem arises only because I decided to ship Git for Windows
with a POSIX emulating environment to execute the Unix shell and Perl
scripts.

The real solution would have been to push harder for "builtinification",
but you know yourself how hard of an uphill effort that is, as the idea is
still prevalent that having a large part of Git be implemented as
shell/Perl scripts is not only okay, but even a Good Thing (and
portability is then a matter of making every contributor know what
constructs are portable and to avoid, say, Bashisms or GNU sed's options
that are incompatible with BSD sed's options).

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/5] Abide by our own rules regarding line endings
  2017-05-04 18:48       ` Johannes Schindelin
@ 2017-05-07  5:29         ` Junio C Hamano
  2017-05-08 10:49           ` Johannes Schindelin
  0 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2017-05-07  5:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Let me repeat myself:

Don't. Instead, read through what you are responding to the end
before start typing a byte.  In case you didn't do that, in the end
I agree with the direction of the series ;-).

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 1/7] Fix build with core.autocrlf=true
  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
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-08  5:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jonathan Nieder, Jeff Hostetler

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> When generating common-cmds.h, the Unix tools we use generally operate on
> the assumption that input and output deliminate their lines using LF-only
> line endings. Consequently, they would happily copy the CR byte verbatim
> into the strings in common-cmds.h, which in turn makes the C preprocessor
> barf (that interprets them as MacOS-style line endings). Therefore, we
> have to mark the input files as LF-only: command-list.txt and
> Documentation/git-*.txt.

I guess nobody in the Windows land opens the doc source with
Notepad, like experienced Git developers runs "less" on them as a
faster way than accessing HTMLized (or manified) versions, and
marking Documentation/git-*.txt explicitly as eol=lf is OK.  That
was the only thing I had brief worry about this patch.

Thanks, will queue.

> diff --git a/.gitattributes b/.gitattributes
> index 320e33c327c..8ce9c6b8888 100644
> --- a/.gitattributes
> +++ b/.gitattributes
> @@ -1,3 +1,9 @@
>  * whitespace=!indent,trail,space
>  *.[ch] whitespace=indent,trail,space diff=cpp
> -*.sh whitespace=indent,trail,space
> +*.sh whitespace=indent,trail,space eol=lf
> +*.perl eol=lf
> +*.pm eol=lf
> +/Documentation/git-*.txt eol=lf
> +/command-list.txt eol=lf
> +/GIT-VERSION-GEN eol=lf
> +/mergetools/* eol=lf
> diff --git a/git-gui/.gitattributes b/git-gui/.gitattributes
> index 33d07c06bd9..59cd41dbff7 100644
> --- a/git-gui/.gitattributes
> +++ b/git-gui/.gitattributes
> @@ -2,3 +2,4 @@
>  *           encoding=US-ASCII
>  git-gui.sh  encoding=UTF-8
>  /po/*.po    encoding=UTF-8
> +/GIT-VERSION-GEN eol=lf

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 2/7] git-new-workdir: mark script as LF-only
  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
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-08  5:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jonathan Nieder, Jeff Hostetler

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Bash does not handle scripts with CR/LF line endings correctly, therefore
> they *have* to be forced to LF-only line endings.
>
> Funnily enough, this fixes t3000-ls-files-others and
> t1021-rerere-in-workdir when git.git was checked out with
> core.autocrlf=true, as these test still use git-new-workdir (once `git
> worktree` is no longer marked as experimental, both scripts probably
> want to be ported to using that command instead).
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

I wouldn't bother fixing these myself, but the above two credit
lines are swapped.  You wrote, then Jonathan reviewed (to which I'll
append my own as the 'editor' of the history when I commit).

>  contrib/workdir/.gitattributes | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 contrib/workdir/.gitattributes
>
> diff --git a/contrib/workdir/.gitattributes b/contrib/workdir/.gitattributes
> new file mode 100644
> index 00000000000..1f78c5d1bd3
> --- /dev/null
> +++ b/contrib/workdir/.gitattributes
> @@ -0,0 +1 @@
> +/git-new-workdir eol=lf

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 4/7] t3901: move supporting files into t/t3901/
  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
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-08  5:12 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jonathan Nieder, Jeff Hostetler

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> The current convention is to either generate files on the fly in tests,
> or to use supporting files taken from a t/tNNNN/ directory (where NNNN
> matches the test's number, or the number of the test from which we
> borrow supporting files).
>
> The test t3901-i18n-patch.sh was obviously introduced before that
> convention was in full swing, hence its supporting files still lived in
> t/t3901-8859-1.txt and t/t3901-utf8.txt, respectively.
>
> Let's adjust to the current convention.

Thanks for a clean-up.  This obviously is a good change ;-)

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 5/7] t4003, t4005, t4007 & t4008: handle CR/LF in t/README & t/diff-lib/README
  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
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-08  5:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jonathan Nieder, Jeff Hostetler

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
> index df2accb6555..c3e0a3c3fc9 100755
> --- a/t/t4003-diff-rename-1.sh
> +++ b/t/t4003-diff-rename-1.sh
> @@ -11,7 +11,7 @@ test_description='More rename detection
>  
>  test_expect_success \
>      'prepare reference tree' \
> -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&

Good ;-).  Thanks.


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 6/7] Fix the remaining tests that failed with core.autocrlf=true
  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
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-08  5:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jonathan Nieder, Jeff Hostetler

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Note: the test suite also uses the t/README file as well as the COPYING
> file in t/diff-lib/, expecting LF-only line endings explicitly and
> failing if that assumption does not hold true. That is why we mark them
> as LF-only in the .gitattributes, too.

I said the previous step that used COPYING was good because it
didn't force the file to be checked out with lf (and instead fixed
the test to strip CR if/as necessary), but come to think of it,
these COPYING/README files are in t/diff-lib/ that are shipped as
test vector, and meant to stay constant even when the end-user
facing COPYING and README at the top level changed.

I do not see t/diff-lib/* marked as eol=lf in this patch, but
shouldn't it be done here, just like all these test vector files?

I also wonder if that makes the previous step unnecessary.


>
> This patch can be validated even on Linux by using this cadence:
>
> 	git config core.autocrlf true
> 	rm .git/index && git stash
> 	make -j15 DEVELOPER=1 test
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/.gitattributes | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/t/.gitattributes b/t/.gitattributes
> index 2d44088f56e..3525ca43f30 100644
> --- a/t/.gitattributes
> +++ b/t/.gitattributes
> @@ -1,2 +1,20 @@
>  t[0-9][0-9][0-9][0-9]/* -whitespace
> -t0110/url-* binary
> +/t0110/url-* binary
> +/t3900/*.txt eol=lf
> +/t3901/*.txt eol=lf
> +/t4034/*/* eol=lf
> +/t4013/* eol=lf
> +/t4018/* eol=lf
> +/t4100/* eol=lf
> +/t4101/* eol=lf
> +/t4109/* eol=lf
> +/t4110/* eol=lf
> +/t4135/* eol=lf
> +/t4211/* eol=lf
> +/t4252/* eol=lf
> +/t5100/* eol=lf
> +/t5515/* eol=lf
> +/t556x_common eol=lf
> +/t7500/* eol=lf
> +/t8005/*.txt eol=lf
> +/t9*/*.dump eol=lf

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 7/7] t4051: mark supporting files as requiring LF-only line endings
  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
  0 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2017-05-08  5:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jonathan Nieder, Jeff Hostetler

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?  ;-)

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

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

Thanks.

>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/.gitattributes | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/t/.gitattributes b/t/.gitattributes
> index 3525ca43f30..bdd82cf31f7 100644
> --- a/t/.gitattributes
> +++ b/t/.gitattributes
> @@ -5,6 +5,7 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
>  /t4034/*/* eol=lf
>  /t4013/* eol=lf
>  /t4018/* eol=lf
> +/t4051/* eol=lf
>  /t4100/* eol=lf
>  /t4101/* eol=lf
>  /t4109/* eol=lf

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/5] Abide by our own rules regarding line endings
  2017-05-07  5:29         ` Junio C Hamano
@ 2017-05-08 10:49           ` Johannes Schindelin
  0 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-08 10:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi Junio,

On Sun, 7 May 2017, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Let me repeat myself:
> 
> Don't.

I had to. You did not understand me.

> Instead, read through what you are responding to the end before start
> typing a byte.  In case you didn't do that, in the end I agree with the
> direction of the series ;-).

And I am still convinced that you do not understand me. At least on the
point that I tried to repeat in a different way, in an attempt to help you
to understand me.

What makes me so convinced that you do not understand me? The part of your
mail that you assumed I did not read. (Disclaimer: I did read it.)

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 7/7] t4051: mark supporting files as requiring LF-only line endings
  2017-05-08  5:29     ` Junio C Hamano
@ 2017-05-09 11:20       ` Johannes Schindelin
  0 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-09 11:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Nieder, Jeff Hostetler

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH v3 0/6] Abide by our own rules regarding line endings
  2017-05-04  9:49 ` [PATCH v2 0/7] " Johannes Schindelin
                     ` (6 preceding siblings ...)
  2017-05-04  9:50   ` [PATCH v2 7/7] t4051: mark supporting files as requiring LF-only line endings Johannes Schindelin
@ 2017-05-09 12:53   ` Johannes Schindelin
  2017-05-09 12:53     ` [PATCH v3 1/6] Fix build with core.autocrlf=true Johannes Schindelin
                       ` (5 more replies)
  7 siblings, 6 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-09 12:53 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

Over the past decade, there have been a couple of attempts to remedy the
situation regarding line endings (Windows/DOS line endings are
traditionally CR/LF even if many current applications can handle LF,
too, Linux/MacOSX/*BSD/Unix uses LF-only line handlings, and old MacOS
software used to use CR-only line endings).

The current idea seems to be that the default line endings differ
depending on the platform, and for files that should be exempt from that
default, we strongly recommend using .gitattributes to define what the
source code requires.

It is time to heed our own recommendation and mark the files that
require LF-only line endings in our very own .gitattributes.

For starters, those files include shell scripts: the most prevalent
shell interpreter in use (and certainly used in Git for Windows) is
Bash, and Bash does not handle CR/LF line endings gracefully.

There are even two shell scripts that are used in the test suite even if
they are not technically supposed to be part of core Git, as indicated
by their habitat inside contrib/: git-new-workdir and
git-completion.bash.

Related to shell scripts: when generating common-cmds.h, we use tools
that generally operate on the assumption that input and output
deliminate their lines using LF-only line endings. Consequently, they
would happily copy the CR byte verbatim into the strings in
common-cmds.h, which in turn makes the C preprocessor barf (that
interprets them as MacOS-style line endings).

Further, the most obvious required fixes concern tests' support files
committed verbatim, to be compared to Git's output, which is always
LF-only. This includes the two files in t/diff-lib/ (which does not, in
fact contain library functions as suggested by its name, but a copy of
the README and the COPYING files, specifically for use in the tests).

There are a few SVN dump files, too, supporting the Subversion-related
tests, requiring LF-only line endings.

Without these fixes, Git will fail to build and pass the test suite, as
can be verified even on Linux using this cadence:

	git config core.autocrlf true
	rm .git/index && git stash
	make DEVELOPER=1 -j15 test

Note: I separated out the change marking t/t4051/* as LF-only into an
individual commit for one reason: it would appear that some grep builds
on Windows automagically convert CR/LF input into LF-only output. I went
the easy route to work around this issue, as I do not want to bother
changing MSYS2 grep's behavior.

Changes since v2:

- marked t/diff-lib/* as LF-only, dropping the ugly `tr -d "\015"` patch.


Johannes Schindelin (6):
  Fix build with core.autocrlf=true
  git-new-workdir: mark script as LF-only
  completion: mark bash script as LF-only
  t3901: move supporting files into t/t3901/
  Fix the remaining tests that failed with core.autocrlf=true
  t4051: mark supporting files as requiring LF-only line endings

 .gitattributes                           |  8 ++++++-
 contrib/completion/.gitattributes        |  1 +
 contrib/workdir/.gitattributes           |  1 +
 git-gui/.gitattributes                   |  1 +
 t/.gitattributes                         | 22 +++++++++++++++++-
 t/t0203-gettext-setlocale-sanity.sh      |  4 ++--
 t/t3901-i18n-patch.sh                    | 38 ++++++++++++++++----------------
 t/{t3901-8859-1.txt => t3901/8859-1.txt} |  0
 t/{t3901-utf8.txt => t3901/utf8.txt}     |  0
 t/t9350-fast-export.sh                   |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh   |  4 ++--
 11 files changed, 55 insertions(+), 26 deletions(-)
 create mode 100644 contrib/completion/.gitattributes
 create mode 100644 contrib/workdir/.gitattributes
 rename t/{t3901-8859-1.txt => t3901/8859-1.txt} (100%)
 rename t/{t3901-utf8.txt => t3901/utf8.txt} (100%)


base-commit: 9b669787fc6ebc527df9ad058c4bcaf46bacc267
Published-As: https://github.com/dscho/git/releases/tag/lf-attrs-v3
Fetch-It-Via: git fetch https://github.com/dscho/git lf-attrs-v3

Interdiff vs v2:

 diff --git a/t/.gitattributes b/t/.gitattributes
 index bdd82cf31f7..3bd959ae523 100644
 --- a/t/.gitattributes
 +++ b/t/.gitattributes
 @@ -1,4 +1,5 @@
  t[0-9][0-9][0-9][0-9]/* -whitespace
 +/diff-lib/* eol=lf
  /t0110/url-* binary
  /t3900/*.txt eol=lf
  /t3901/*.txt eol=lf
 diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
 index c3e0a3c3fc9..df2accb6555 100755
 --- a/t/t4003-diff-rename-1.sh
 +++ b/t/t4003-diff-rename-1.sh
 @@ -11,7 +11,7 @@ test_description='More rename detection
  
  test_expect_success \
      'prepare reference tree' \
 -    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
 +    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
       echo frotz >rezrov &&
      git update-index --add COPYING rezrov &&
      tree=$(git write-tree) &&
 @@ -99,7 +99,7 @@ test_expect_success \
  
  test_expect_success \
      'prepare work tree once again' \
 -    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
 +    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
       git update-index --add --remove COPYING COPYING.1'
  
  # tree has COPYING and rezrov.  work tree has COPYING and COPYING.1,
 diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
 index f1641c35ee2..135addbfbda 100755
 --- a/t/t4005-diff-rename-2.sh
 +++ b/t/t4005-diff-rename-2.sh
 @@ -11,7 +11,7 @@ test_description='Same rename detection as t4003 but testing diff-raw.
  
  test_expect_success \
      'prepare reference tree' \
 -    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
 +    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
       echo frotz >rezrov &&
      git update-index --add COPYING rezrov &&
      tree=$(git write-tree) &&
 @@ -71,7 +71,7 @@ test_expect_success \
  
  test_expect_success \
      'prepare work tree once again' \
 -    'tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
 +    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
       git update-index --add --remove COPYING COPYING.1'
  
  git diff-index -C --find-copies-harder $tree >current
 diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
 index 0157fde5503..dae327fabbf 100755
 --- a/t/t4007-rename-3.sh
 +++ b/t/t4007-rename-3.sh
 @@ -11,7 +11,7 @@ test_description='Rename interaction with pathspec.
  
  test_expect_success 'prepare reference tree' '
  	mkdir path0 path1 &&
 -	tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >path0/COPYING &&
 +	cp "$TEST_DIRECTORY"/diff-lib/COPYING path0/COPYING &&
  	git update-index --add path0/COPYING &&
  	tree=$(git write-tree) &&
  	echo $tree
 diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh
 index 5af4fa6aadb..9dd1bc5e162 100755
 --- a/t/t4008-diff-break-rewrite.sh
 +++ b/t/t4008-diff-break-rewrite.sh
 @@ -25,8 +25,8 @@ Further, with -B and -M together, these should turn into two renames.
  . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
  
  test_expect_success setup '
 -	tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/README >file0 &&
 -	tr -d "\015" <"$TEST_DIRECTORY"/diff-lib/COPYING >file1 &&
 +	cat "$TEST_DIRECTORY"/diff-lib/README >file0 &&
 +	cat "$TEST_DIRECTORY"/diff-lib/COPYING >file1 &&
  	git update-index --add file0 file1 &&
  	git tag reference $(git write-tree)
  '

-- 
2.12.2.windows.2.800.gede8f145e06


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH v3 1/6] Fix build with core.autocrlf=true
  2017-05-09 12:53   ` [PATCH v3 0/6] Abide by our own rules regarding " Johannes Schindelin
@ 2017-05-09 12:53     ` Johannes Schindelin
  2017-05-22 17:57       ` Jonathan Nieder
  2017-05-09 12:53     ` [PATCH v3 2/6] git-new-workdir: mark script as LF-only Johannes Schindelin
                       ` (4 subsequent siblings)
  5 siblings, 1 reply; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-09 12:53 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

On Windows, the default line endings are denoted by a Carriage Return
byte followed by a Line Feed byte, while Linux and MacOSX use a single
Line Feed byte to denote a line ending.

To help with this situation, Git introduced several mechanisms over the
last decade, most prominently the `core.autocrlf` setting.

Sometimes, however, a single setting is incorrect, e.g. when certain
files in the source code are to be consumed by software that can handle
only LF line endings, while other files can use whatever is appropriate
for the current platform.

To allow for that, Git added the `eol` option to its .gitattributes
handling, expecting every user of Git to mark their source code
appropriately.

Bash assumes that line-endings of scripts are denoted by a single Line
Feed byte. Therefore, shell scripts in Git's source code are one example
where that `eol=lf` option is *required*.

When generating common-cmds.h, the Unix tools we use generally operate on
the assumption that input and output deliminate their lines using LF-only
line endings. Consequently, they would happily copy the CR byte verbatim
into the strings in common-cmds.h, which in turn makes the C preprocessor
barf (that interprets them as MacOS-style line endings). Therefore, we
have to mark the input files as LF-only: command-list.txt and
Documentation/git-*.txt.

Quite a bit belatedly, this patch brings Git's own source code in line
with those expectations by setting those attributes to allow for a
correct build even when core.autocrlf=true.

This patch can be validated even on Linux, by using this cadence:

	git config core.autocrlf true
	rm .git/index && git stash
	make -j15 DEVELOPER=1

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
 .gitattributes         | 8 +++++++-
 git-gui/.gitattributes | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/.gitattributes b/.gitattributes
index 320e33c327c..8ce9c6b8888 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,3 +1,9 @@
 * whitespace=!indent,trail,space
 *.[ch] whitespace=indent,trail,space diff=cpp
-*.sh whitespace=indent,trail,space
+*.sh whitespace=indent,trail,space eol=lf
+*.perl eol=lf
+*.pm eol=lf
+/Documentation/git-*.txt eol=lf
+/command-list.txt eol=lf
+/GIT-VERSION-GEN eol=lf
+/mergetools/* eol=lf
diff --git a/git-gui/.gitattributes b/git-gui/.gitattributes
index 33d07c06bd9..59cd41dbff7 100644
--- a/git-gui/.gitattributes
+++ b/git-gui/.gitattributes
@@ -2,3 +2,4 @@
 *           encoding=US-ASCII
 git-gui.sh  encoding=UTF-8
 /po/*.po    encoding=UTF-8
+/GIT-VERSION-GEN eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v3 2/6] git-new-workdir: mark script as LF-only
  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-09 12:53     ` Johannes Schindelin
  2017-05-09 12:54     ` [PATCH v3 3/6] completion: mark bash " Johannes Schindelin
                       ` (3 subsequent siblings)
  5 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-09 12:53 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

Bash does not handle scripts with CR/LF line endings correctly, therefore
they *have* to be forced to LF-only line endings.

Funnily enough, this fixes t3000-ls-files-others and
t1021-rerere-in-workdir when git.git was checked out with
core.autocrlf=true, as these test still use git-new-workdir (once `git
worktree` is no longer marked as experimental, both scripts probably
want to be ported to using that command instead).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
 contrib/workdir/.gitattributes | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 contrib/workdir/.gitattributes

diff --git a/contrib/workdir/.gitattributes b/contrib/workdir/.gitattributes
new file mode 100644
index 00000000000..1f78c5d1bd3
--- /dev/null
+++ b/contrib/workdir/.gitattributes
@@ -0,0 +1 @@
+/git-new-workdir eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v3 3/6] completion: mark bash script as LF-only
  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-09 12:53     ` [PATCH v3 2/6] git-new-workdir: mark script as LF-only Johannes Schindelin
@ 2017-05-09 12:54     ` Johannes Schindelin
  2017-05-09 12:54     ` [PATCH v3 4/6] t3901: move supporting files into t/t3901/ Johannes Schindelin
                       ` (2 subsequent siblings)
  5 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-09 12:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

Without this change, the completion script does not work, as Bash expects
its scripts to have line feeds as end-of-line markers (this is
particularly prominent in quoted multi-line strings, where carriage
returns would slip into the strings as verbatim characters otherwise).

This change is required to let t9902-completion pass when Git's source
code is checked out with `core.autocrlf = true`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
 contrib/completion/.gitattributes | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 contrib/completion/.gitattributes

diff --git a/contrib/completion/.gitattributes b/contrib/completion/.gitattributes
new file mode 100644
index 00000000000..19116944c15
--- /dev/null
+++ b/contrib/completion/.gitattributes
@@ -0,0 +1 @@
+*.bash eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v3 4/6] t3901: move supporting files into t/t3901/
  2017-05-09 12:53   ` [PATCH v3 0/6] Abide by our own rules regarding " Johannes Schindelin
                       ` (2 preceding siblings ...)
  2017-05-09 12:54     ` [PATCH v3 3/6] completion: mark bash " Johannes Schindelin
@ 2017-05-09 12:54     ` 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
  5 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-09 12:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

The current convention is to either generate files on the fly in tests,
or to use supporting files taken from a t/tNNNN/ directory (where NNNN
matches the test's number, or the number of the test from which we
borrow supporting files).

The test t3901-i18n-patch.sh was obviously introduced before that
convention was in full swing, hence its supporting files still lived in
t/t3901-8859-1.txt and t/t3901-utf8.txt, respectively.

Let's adjust to the current convention.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t0203-gettext-setlocale-sanity.sh      |  4 ++--
 t/t3901-i18n-patch.sh                    | 38 ++++++++++++++++----------------
 t/{t3901-8859-1.txt => t3901/8859-1.txt} |  0
 t/{t3901-utf8.txt => t3901/utf8.txt}     |  0
 t/t9350-fast-export.sh                   |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh   |  4 ++--
 6 files changed, 24 insertions(+), 24 deletions(-)
 rename t/{t3901-8859-1.txt => t3901/8859-1.txt} (100%)
 rename t/{t3901-utf8.txt => t3901/utf8.txt} (100%)

diff --git a/t/t0203-gettext-setlocale-sanity.sh b/t/t0203-gettext-setlocale-sanity.sh
index a2124600811..71b0d74b4dd 100755
--- a/t/t0203-gettext-setlocale-sanity.sh
+++ b/t/t0203-gettext-setlocale-sanity.sh
@@ -8,7 +8,7 @@ test_description="The Git C functions aren't broken by setlocale(3)"
 . ./lib-gettext.sh
 
 test_expect_success 'git show a ISO-8859-1 commit under C locale' '
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 	test_commit "iso-c-commit" iso-under-c &&
 	git show >out 2>err &&
 	! test -s err &&
@@ -16,7 +16,7 @@ test_expect_success 'git show a ISO-8859-1 commit under C locale' '
 '
 
 test_expect_success GETTEXT_LOCALE 'git show a ISO-8859-1 commit under a UTF-8 locale' '
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 	test_commit "iso-utf8-commit" iso-under-utf8 &&
 	LANGUAGE=is LC_ALL="$is_IS_locale" git show >out 2>err &&
 	! test -s err &&
diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
index f663d567c8a..923eb01f0ea 100755
--- a/t/t3901-i18n-patch.sh
+++ b/t/t3901-i18n-patch.sh
@@ -31,7 +31,7 @@ test_expect_success setup '
 
 	# use UTF-8 in author and committer name to match the
 	# i18n.commitencoding settings
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	test_tick &&
 	echo "$GIT_AUTHOR_NAME" >mine &&
@@ -55,7 +55,7 @@ test_expect_success setup '
 		# the second one on the side branch is ISO-8859-1
 		git config i18n.commitencoding ISO8859-1 &&
 		# use author and committer name in ISO-8859-1 to match it.
-		. "$TEST_DIRECTORY"/t3901-8859-1.txt
+		. "$TEST_DIRECTORY"/t3901/8859-1.txt
 	fi &&
 	test_tick &&
 	echo Yet another >theirs &&
@@ -100,7 +100,7 @@ test_expect_success 'rebase (U/U)' '
 
 	# The result will be committed by GIT_COMMITTER_NAME --
 	# we want UTF-8 encoded name.
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 	git checkout -b test &&
 	git rebase master &&
 
@@ -110,7 +110,7 @@ test_expect_success 'rebase (U/U)' '
 test_expect_success 'rebase (U/L)' '
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard side &&
 	git rebase master &&
@@ -122,7 +122,7 @@ test_expect_success !MINGW 'rebase (L/L)' '
 	# In this test we want ISO-8859-1 encoded commits as the result
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard side &&
 	git rebase master &&
@@ -135,7 +135,7 @@ test_expect_success !MINGW 'rebase (L/U)' '
 	# to get ISO-8859-1 results.
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard side &&
 	git rebase master &&
@@ -148,7 +148,7 @@ test_expect_success 'cherry-pick(U/U)' '
 
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard master &&
 	git cherry-pick side^ &&
@@ -163,7 +163,7 @@ test_expect_success !MINGW 'cherry-pick(L/L)' '
 
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard master &&
 	git cherry-pick side^ &&
@@ -178,7 +178,7 @@ test_expect_success 'cherry-pick(U/L)' '
 
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard master &&
 	git cherry-pick side^ &&
@@ -194,7 +194,7 @@ test_expect_success !MINGW 'cherry-pick(L/U)' '
 
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard master &&
 	git cherry-pick side^ &&
@@ -207,7 +207,7 @@ test_expect_success !MINGW 'cherry-pick(L/U)' '
 test_expect_success 'rebase --merge (U/U)' '
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard side &&
 	git rebase --merge master &&
@@ -218,7 +218,7 @@ test_expect_success 'rebase --merge (U/U)' '
 test_expect_success 'rebase --merge (U/L)' '
 	git config i18n.commitencoding UTF-8 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard side &&
 	git rebase --merge master &&
@@ -230,7 +230,7 @@ test_expect_success 'rebase --merge (L/L)' '
 	# In this test we want ISO-8859-1 encoded commits as the result
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard side &&
 	git rebase --merge master &&
@@ -243,7 +243,7 @@ test_expect_success 'rebase --merge (L/U)' '
 	# to get ISO-8859-1 results.
 	git config i18n.commitencoding ISO8859-1 &&
 	git config i18n.logoutputencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard side &&
 	git rebase --merge master &&
@@ -254,7 +254,7 @@ test_expect_success 'rebase --merge (L/U)' '
 test_expect_success 'am (U/U)' '
 	# Apply UTF-8 patches with UTF-8 commitencoding
 	git config i18n.commitencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard master &&
 	git am out-u1 out-u2 &&
@@ -265,7 +265,7 @@ test_expect_success 'am (U/U)' '
 test_expect_success !MINGW 'am (L/L)' '
 	# Apply ISO-8859-1 patches with ISO-8859-1 commitencoding
 	git config i18n.commitencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard master &&
 	git am out-l1 out-l2 &&
@@ -276,7 +276,7 @@ test_expect_success !MINGW 'am (L/L)' '
 test_expect_success 'am (U/L)' '
 	# Apply ISO-8859-1 patches with UTF-8 commitencoding
 	git config i18n.commitencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 	git reset --hard master &&
 
 	# am specifies --utf8 by default.
@@ -288,7 +288,7 @@ test_expect_success 'am (U/L)' '
 test_expect_success 'am --no-utf8 (U/L)' '
 	# Apply ISO-8859-1 patches with UTF-8 commitencoding
 	git config i18n.commitencoding UTF-8 &&
-	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 
 	git reset --hard master &&
 	git am --no-utf8 out-l1 out-l2 2>err &&
@@ -303,7 +303,7 @@ test_expect_success 'am --no-utf8 (U/L)' '
 test_expect_success !MINGW 'am (L/U)' '
 	# Apply UTF-8 patches with ISO-8859-1 commitencoding
 	git config i18n.commitencoding ISO8859-1 &&
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 
 	git reset --hard master &&
 	# mailinfo will re-code the commit message to the charset specified by
diff --git a/t/t3901-8859-1.txt b/t/t3901/8859-1.txt
similarity index 100%
rename from t/t3901-8859-1.txt
rename to t/t3901/8859-1.txt
diff --git a/t/t3901-utf8.txt b/t/t3901/utf8.txt
similarity index 100%
rename from t/t3901-utf8.txt
rename to t/t3901/utf8.txt
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index b5149fde6ec..8dcb05c4a57 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -70,7 +70,7 @@ test_expect_success 'iso-8859-1' '
 
 	git config i18n.commitencoding ISO8859-1 &&
 	# use author and committer name in ISO-8859-1 to match it.
-	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 	test_tick &&
 	echo rosten >file &&
 	git commit -s -m den file &&
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 6d06ed96cbc..cc8d463e01a 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -519,7 +519,7 @@ test_expect_success \
 
 test_expect_success \
 	'encode(commit): utf8' \
-	'. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	'. "$TEST_DIRECTORY"/t3901/utf8.txt &&
 	 test_when_finished "GIT_AUTHOR_NAME=\"A U Thor\"" &&
 	 test_when_finished "GIT_COMMITTER_NAME=\"C O Mitter\"" &&
 	 echo "UTF-8" >> file &&
@@ -529,7 +529,7 @@ test_expect_success \
 
 test_expect_success \
 	'encode(commit): iso-8859-1' \
-	'. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+	'. "$TEST_DIRECTORY"/t3901/8859-1.txt &&
 	 test_when_finished "GIT_AUTHOR_NAME=\"A U Thor\"" &&
 	 test_when_finished "GIT_COMMITTER_NAME=\"C O Mitter\"" &&
 	 echo "ISO-8859-1" >> file &&
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v3 5/6] Fix the remaining tests that failed with core.autocrlf=true
  2017-05-09 12:53   ` [PATCH v3 0/6] Abide by our own rules regarding " Johannes Schindelin
                       ` (3 preceding siblings ...)
  2017-05-09 12:54     ` [PATCH v3 4/6] t3901: move supporting files into t/t3901/ Johannes Schindelin
@ 2017-05-09 12:54     ` Johannes Schindelin
  2017-05-09 12:54     ` [PATCH v3 6/6] t4051: mark supporting files as requiring LF-only line endings Johannes Schindelin
  5 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-09 12:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

The test suite is mainly developed on Linux and MacOSX, which is the
reason that nobody thought to mark files as LF-only as needed.

The symptom is a test suite that fails left and right when being checked
out using Git for Windows (which defaults to core.autocrlf=true).

Mostly, the problems stem from Git's (LF-only) output being compared to
hard-coded files that are checked out with line endings according to
core.autocrlf (which is of course incorrect). This includes the two test
files in t/diff-lib/, README and COPYING.

This patch can be validated even on Linux by using this cadence:

	git config core.autocrlf true
	rm .git/index && git stash
	make -j15 DEVELOPER=1 test

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/.gitattributes | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/t/.gitattributes b/t/.gitattributes
index 2d44088f56e..11e5fe37281 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1,2 +1,21 @@
 t[0-9][0-9][0-9][0-9]/* -whitespace
-t0110/url-* binary
+/diff-lib/* eol=lf
+/t0110/url-* binary
+/t3900/*.txt eol=lf
+/t3901/*.txt eol=lf
+/t4034/*/* eol=lf
+/t4013/* eol=lf
+/t4018/* eol=lf
+/t4100/* eol=lf
+/t4101/* eol=lf
+/t4109/* eol=lf
+/t4110/* eol=lf
+/t4135/* eol=lf
+/t4211/* eol=lf
+/t4252/* eol=lf
+/t5100/* eol=lf
+/t5515/* eol=lf
+/t556x_common eol=lf
+/t7500/* eol=lf
+/t8005/*.txt eol=lf
+/t9*/*.dump eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06



^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v3 6/6] t4051: mark supporting files as requiring LF-only line endings
  2017-05-09 12:53   ` [PATCH v3 0/6] Abide by our own rules regarding " Johannes Schindelin
                       ` (4 preceding siblings ...)
  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     ` Johannes Schindelin
  5 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-09 12:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Jeff Hostetler

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.

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/.gitattributes | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/.gitattributes b/t/.gitattributes
index 11e5fe37281..3bd959ae523 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -6,6 +6,7 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
 /t4034/*/* eol=lf
 /t4013/* eol=lf
 /t4018/* eol=lf
+/t4051/* eol=lf
 /t4100/* eol=lf
 /t4101/* eol=lf
 /t4109/* eol=lf
-- 
2.12.2.windows.2.800.gede8f145e06

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v3 1/6] Fix build with core.autocrlf=true
  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
  0 siblings, 1 reply; 39+ messages in thread
From: Jonathan Nieder @ 2017-05-22 17:57 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, Jeff Hostetler, Pat Thoyts

(+cc: Pat Thoyts, git-gui maintainer)
Hi,

Johannes Schindelin wrote:

> On Windows, the default line endings are denoted by a Carriage Return
> byte followed by a Line Feed byte, while Linux and MacOSX use a single
> Line Feed byte to denote a line ending.
>
> To help with this situation, Git introduced several mechanisms over the
> last decade, most prominently the `core.autocrlf` setting.
>
> Sometimes, however, a single setting is incorrect, e.g. when certain
> files in the source code are to be consumed by software that can handle
> only LF line endings, while other files can use whatever is appropriate
> for the current platform.
>
> To allow for that, Git added the `eol` option to its .gitattributes
> handling, expecting every user of Git to mark their source code
> appropriately.
>
> Bash assumes that line-endings of scripts are denoted by a single Line
> Feed byte. Therefore, shell scripts in Git's source code are one example
> where that `eol=lf` option is *required*.
>
> When generating common-cmds.h, the Unix tools we use generally operate on
> the assumption that input and output deliminate their lines using LF-only
> line endings. Consequently, they would happily copy the CR byte verbatim
> into the strings in common-cmds.h, which in turn makes the C preprocessor
> barf (that interprets them as MacOS-style line endings). Therefore, we
> have to mark the input files as LF-only: command-list.txt and
> Documentation/git-*.txt.
>
> Quite a bit belatedly, this patch brings Git's own source code in line
> with those expectations by setting those attributes to allow for a
> correct build even when core.autocrlf=true.
>
> This patch can be validated even on Linux, by using this cadence:
>
> 	git config core.autocrlf true
> 	rm .git/index && git stash
> 	make -j15 DEVELOPER=1
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> ---

Thanks again for writing this patch.  One bit of administrivia I
missed before:

Pat, would you mind picking up this .gitattributes change for git-gui?

Junio, how do you prefer to handle this in git.git?  Would you need to
amend the patch to remove the git-gui/.gitattributes change and wait
to get it from Pat, or is getting the same change twice okay?  If I
understand git-gui/GIT-VERSION-GEN correctly, then either way, once
you perform a subtree merge from git://repo.or.cz/git-gui the
"version" file will be okay.

Patch left unsnipped for reference.

Thanks,
Jonathan

>  .gitattributes         | 8 +++++++-
>  git-gui/.gitattributes | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/.gitattributes b/.gitattributes
> index 320e33c327c..8ce9c6b8888 100644
> --- a/.gitattributes
> +++ b/.gitattributes
> @@ -1,3 +1,9 @@
>  * whitespace=!indent,trail,space
>  *.[ch] whitespace=indent,trail,space diff=cpp
> -*.sh whitespace=indent,trail,space
> +*.sh whitespace=indent,trail,space eol=lf
> +*.perl eol=lf
> +*.pm eol=lf
> +/Documentation/git-*.txt eol=lf
> +/command-list.txt eol=lf
> +/GIT-VERSION-GEN eol=lf
> +/mergetools/* eol=lf
> diff --git a/git-gui/.gitattributes b/git-gui/.gitattributes
> index 33d07c06bd9..59cd41dbff7 100644
> --- a/git-gui/.gitattributes
> +++ b/git-gui/.gitattributes
> @@ -2,3 +2,4 @@
>  *           encoding=US-ASCII
>  git-gui.sh  encoding=UTF-8
>  /po/*.po    encoding=UTF-8
> +/GIT-VERSION-GEN eol=lf

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v3 1/6] Fix build with core.autocrlf=true
  2017-05-22 17:57       ` Jonathan Nieder
@ 2017-05-23  3:35         ` Junio C Hamano
  2017-05-23  9:01           ` Johannes Schindelin
  0 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2017-05-23  3:35 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Johannes Schindelin, git, Jeff Hostetler, Pat Thoyts

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio, how do you prefer to handle this in git.git?  Would you need to
> amend the patch to remove the git-gui/.gitattributes change and wait
> to get it from Pat, or is getting the same change twice okay?

Yes, getting the same change twice should be fine.  I'll see
conflicts when I update from Pat when it happens next time, but we
know what the resolution should be already.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v3 1/6] Fix build with core.autocrlf=true
  2017-05-23  3:35         ` Junio C Hamano
@ 2017-05-23  9:01           ` Johannes Schindelin
  0 siblings, 0 replies; 39+ messages in thread
From: Johannes Schindelin @ 2017-05-23  9:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jonathan Nieder, git, Jeff Hostetler, Pat Thoyts

Hi,

On Tue, 23 May 2017, Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
> > Junio, how do you prefer to handle this in git.git?  Would you need to
> > amend the patch to remove the git-gui/.gitattributes change and wait
> > to get it from Pat, or is getting the same change twice okay?
> 
> Yes, getting the same change twice should be fine.  I'll see
> conflicts when I update from Pat when it happens next time, but we
> know what the resolution should be already.

FWIW the reason why I did not open a Pull Request in
https://github.com/patthoyts/git-gui for this change (which I had
originally considered) is that I already have a couple of Pull Requests
open in that repository, and they are rotting for over half a year
already (and will soon compete with certain Icelandic delicacies):

https://github.com/patthoyts/git-gui/pulls/dscho

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2017-05-23  9:01 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).