* Re: t0027 racy?
2016-08-09 11:49 ` Jeff King
@ 2016-08-09 12:59 ` Torsten Bögershausen
2016-08-09 13:27 ` Jeff King
2016-08-12 16:50 ` [PATCH v1 0/2] Fix conversion warnings tboegi
` (9 subsequent siblings)
10 siblings, 1 reply; 45+ messages in thread
From: Torsten Bögershausen @ 2016-08-09 12:59 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
On Tue, Aug 09, 2016 at 07:49:38AM -0400, Jeff King wrote:
> On Tue, Aug 09, 2016 at 11:33:37AM +0000, Torsten Bögershausen wrote:
>
> > > The second one seems plausible, given the history of issues with
> > > changing CRLF settings for an existing checkout. I'm not sure if it
> > > would be feasible to reset the index completely before each tested
> > > command, but that would probably solve it.
> > The content of the file has been changed (we appended the letter 'Z' to it,
> > so even if mtime is the same, st.st_size should differ.
> > And it seems as if the commit is triggered, see below.
>
> I don't think I made myself clear. It's not a question of whether there
> is something to commit. It's that when git asks the index "what is the
> sha1 of the content at this path?", the index may be able to answer
> directly (the file is up-to-date, so we return the index value), or it
> may have to go to the filesystem and read the file content. It is this
> latter which triggers convert_to_git(), which is what generates the
> message in question.
>
> For a more stripped-down example, try:
>
> git add foo
> git commit -m msg
>
> versus:
>
> git add foo
> sleep 1
> git commit -m msg
>
> In the latter case, we should not generally need convert_to_git() in the
> "commit" step. It was already done by "git add", and we reuse the cached
> result.
>
> Whereas in the first one, we may run into the racy-index problem and
> have to re-read the file to be on the safe side.
>
> -Peff
Thanks for the explanation, so there are 2 chances for a race.
I assume that the suggested "touch" will fix race#2 in most cases.
In my understanding, the change of the file size will be more reliable:
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 2860d2d..9933a9b 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -120,6 +120,7 @@ commit_chk_wrnNNO () {
cp $f $fname &&
printf Z >>"$fname" &&
git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
+ printf Z >>"$fname" &&
git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
done
-------------------
Does anybody agree ?
And, by the way, the convert warning may be issued twice, once in
"git add" and once in "git commit".
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: t0027 racy?
2016-08-09 12:59 ` Torsten Bögershausen
@ 2016-08-09 13:27 ` Jeff King
2016-08-09 21:28 ` Torsten Bögershausen
0 siblings, 1 reply; 45+ messages in thread
From: Jeff King @ 2016-08-09 13:27 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Johannes Schindelin, git
On Tue, Aug 09, 2016 at 12:59:58PM +0000, Torsten Bögershausen wrote:
> Thanks for the explanation, so there are 2 chances for a race.
>
> I assume that the suggested "touch" will fix race#2 in most cases.
> In my understanding, the change of the file size will be more reliable:
>
>
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index 2860d2d..9933a9b 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -120,6 +120,7 @@ commit_chk_wrnNNO () {
> cp $f $fname &&
> printf Z >>"$fname" &&
> git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
> + printf Z >>"$fname" &&
> git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
> done
> -------------------
> Does anybody agree ?
I think the mtime change is reliable. We know that the mtime on the file
will be greater than or equal to the index mtime (because it happened
afterwards), so git will always look at the on-disk contents.
With your change, "git commit" will also always re-read the file from
disk, because it actually has new content (and you provide the filename
on the command line, so it is stage-and-commit, not just
"commit-the-index").
So either is fine.
> And, by the way, the convert warning may be issued twice, once in
> "git add" and once in "git commit".
Yes, but you only save it from "git commit", so we can ignore what
happens from "add" here. But that's why I wondered if:
git -c core.autocrlf=$crlf add $fname >"${pfx}_$f.err" 2>&1
would make more sense. We _know_ that we have to do convert_to_git() in
that step because the content is changed. And then you can ignore the
warnings from "git commit" (which are racy), or you can simply commit as
a whole later, as some other loops do.
But like Dscho, I do not actually understand what this test is checking.
The function is called commit_chk_wrnNNO(), so perhaps you really are
interested in what "commit" has to say. But IMHO that is not an
interesting test. We know that if it has to read the content from disk,
it will call convert_to_git(), which is the exact same code path used by
"git add". So I do not understand what it is accomplishing to make a
commit at all here.
-Peff
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: t0027 racy?
2016-08-09 13:27 ` Jeff King
@ 2016-08-09 21:28 ` Torsten Bögershausen
2016-08-10 12:28 ` Johannes Schindelin
0 siblings, 1 reply; 45+ messages in thread
From: Torsten Bögershausen @ 2016-08-09 21:28 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
> git -c core.autocrlf=$crlf add $fname >"${pfx}_$f.err" 2>&1
>
> would make more sense. We _know_ that we have to do convert_to_git() in
> that step because the content is changed. And then you can ignore the
> warnings from "git commit" (which are racy), or you can simply commit as
> a whole later, as some other loops do.
>
> But like Dscho, I do not actually understand what this test is checking.
> The function is called commit_chk_wrnNNO(), so perhaps you really are
> interested in what "commit" has to say. But IMHO that is not an
> interesting test. We know that if it has to read the content from disk,
> it will call convert_to_git(), which is the exact same code path used by
> "git add". So I do not understand what it is accomplishing to make a
> commit at all here.
It seems as if the test has been written without understanding the raciness.
It should commit files with different line endings on top of
a file with mixed line endings.
The warning should be checked (and here "git add" can be used,
or the file can be commited directly).
I'm not sure why the test ended up in doing both.
However, doing it the right way triggers a bug in convert.c,
(some warnings are missing, so I need some days to come up
with a proper patch)
Thanks for reporting & digging.
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: t0027 racy?
2016-08-09 21:28 ` Torsten Bögershausen
@ 2016-08-10 12:28 ` Johannes Schindelin
2016-08-11 18:58 ` Torsten Bögershausen
0 siblings, 1 reply; 45+ messages in thread
From: Johannes Schindelin @ 2016-08-10 12:28 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jeff King, git
[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]
Hi Torsten,
On Tue, 9 Aug 2016, Torsten Bögershausen wrote:
> > git -c core.autocrlf=$crlf add $fname >"${pfx}_$f.err" 2>&1
> >
> > would make more sense. We _know_ that we have to do convert_to_git() in
> > that step because the content is changed. And then you can ignore the
> > warnings from "git commit" (which are racy), or you can simply commit as
> > a whole later, as some other loops do.
> >
> > But like Dscho, I do not actually understand what this test is checking.
> > The function is called commit_chk_wrnNNO(), so perhaps you really are
> > interested in what "commit" has to say. But IMHO that is not an
> > interesting test. We know that if it has to read the content from disk,
> > it will call convert_to_git(), which is the exact same code path used by
> > "git add". So I do not understand what it is accomplishing to make a
> > commit at all here.
>
> It seems as if the test has been written without understanding the raciness.
> It should commit files with different line endings on top of
> a file with mixed line endings.
> The warning should be checked (and here "git add" can be used,
> or the file can be commited directly).
> I'm not sure why the test ended up in doing both.
>
> However, doing it the right way triggers a bug in convert.c,
> (some warnings are missing, so I need some days to come up
> with a proper patch)
FWIW I would strongly prefer to use the warning of `git add` and not even
bother with `git commit`. What we are interested in is the warning
message, generated by convert_to_git(). Not using the first one and
triggering a second one merely adds unnecessary churn that increases the
CO2 budget of running the test.
On that matter, I wonder whether there would be a chance to revamp t0027
in a major way, with the following goals:
- to make it very obvious to the casual reader what is being tested
- to combine Git invocations when possible, e.g. running one big `git add`
on a couple of files and then verify the relevant parts of the output
- dramatically decreasing the time required to run the test, without
sacrificing correctness (I would wager a bet that not only a few of
those 1388 test cases essentially exercise identical code paths)
Ciao,
Dscho
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: t0027 racy?
2016-08-10 12:28 ` Johannes Schindelin
@ 2016-08-11 18:58 ` Torsten Bögershausen
2016-08-11 19:34 ` Junio C Hamano
2016-08-12 7:24 ` Jeff King
0 siblings, 2 replies; 45+ messages in thread
From: Torsten Bögershausen @ 2016-08-11 18:58 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, git
[]
> FWIW I would strongly prefer to use the warning of `git add` and not even
> bother with `git commit`. What we are interested in is the warning
> message, generated by convert_to_git().
The commit is needed, because we check the content of the commit later.
> Not using the first one and
[]
>
> On that matter, I wonder whether there would be a chance to revamp t0027
> in a major way, with the following goals:
>
> - to make it very obvious to the casual reader what is being tested
>
> - to combine Git invocations when possible, e.g. running one big `git add`
> on a couple of files and then verify the relevant parts of the output
>
> - dramatically decreasing the time required to run the test, without
> sacrificing correctness (I would wager a bet that not only a few of
> those 1388 test cases essentially exercise identical code paths)
Good ideas, I will work on a series that fixes bugs first, and then we
can see if there is room for optimization.
What do you think about this as a starting point,
more things will follow.
I like to here comments about the commit msg first ;-)
commit 3754404d3d1ea4a0cbbed4986cc4ac1b5fe6b66e
Author: Torsten Bögershausen <tboegi@web.de>
Date: Thu Aug 11 18:47:29 2016 +0200
t0027: Correct raciness in NNO test
When a non-reversible CRLF conversion is done in "git add",
a warning is printed on stderr.
The commit_chk_wrnNNO() function in t0027 was written to test this,
but did the wrong thing: Instead of looking at the warning
from "git add", it looked at the warning from "git commit".
Correct this and replace the commit for each and every file with a commit
of all files in one go.
The function commit_chk_wrnNNO() will to be renamed in a separate commit.
Thanks to Jeff King <peff@peff.net> for analizing t0027.
Reporyed-By: Johannes Schindelin <Johannes.Schindelin@gmx.de>
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 2860d2d..6e44382 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -119,8 +119,7 @@ commit_chk_wrnNNO () {
fname=${pfx}_$f.txt &&
cp $f $fname &&
printf Z >>"$fname" &&
- git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
- git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
+ git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
done
test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
@@ -394,11 +393,10 @@ test_expect_success 'commit files attr=crlf' '
# attr LF CRLF CRLFmixLF LF_mix_CR CRLFNUL
commit_chk_wrnNNO "" "" false "" "" "" "" ""
-commit_chk_wrnNNO "" "" true LF_CRLF "" "" "" ""
+commit_chk_wrnNNO "" "" true "" "" "" "" ""
commit_chk_wrnNNO "" "" input "" "" "" "" ""
-
-commit_chk_wrnNNO "auto" "" false "$WILC" "" "" "" ""
-commit_chk_wrnNNO "auto" "" true LF_CRLF "" "" "" ""
+commit_chk_wrnNNO "auto" "" false "" "" "" "" ""
+commit_chk_wrnNNO "auto" "" true "" "" "" "" ""
commit_chk_wrnNNO "auto" "" input "" "" "" "" ""
for crlf in true false input
do
@@ -408,7 +406,7 @@ do
commit_chk_wrnNNO "" lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
commit_chk_wrnNNO "" crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
commit_chk_wrnNNO auto lf $crlf "" "" "" "" ""
- commit_chk_wrnNNO auto crlf $crlf LF_CRLF "" "" "" ""
+ commit_chk_wrnNNO auto crlf $crlf "" "" "" "" ""
commit_chk_wrnNNO text lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
commit_chk_wrnNNO text crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
done
@@ -417,7 +415,8 @@ commit_chk_wrnNNO "text" "" false "$WILC" "$WICL" "$WAMIX" "$WILC
commit_chk_wrnNNO "text" "" true LF_CRLF "" LF_CRLF LF_CRLF ""
commit_chk_wrnNNO "text" "" input "" CRLF_LF CRLF_LF "" CRLF_LF
-test_expect_success 'create files cleanup' '
+test_expect_success 'commit NNO and cleanup' '
+ git commit -m "commit files on top of NNO" &&
rm -f *.txt &&
git -c core.autocrlf=false reset --hard
'
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: t0027 racy?
2016-08-11 18:58 ` Torsten Bögershausen
@ 2016-08-11 19:34 ` Junio C Hamano
2016-08-12 7:24 ` Jeff King
1 sibling, 0 replies; 45+ messages in thread
From: Junio C Hamano @ 2016-08-11 19:34 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Johannes Schindelin, Jeff King, git
Torsten Bögershausen <tboegi@web.de> writes:
> Good ideas, I will work on a series that fixes bugs first, and then we
> can see if there is room for optimization.
>
> What do you think about this as a starting point, more things will
> follow.
> I like to here comments about the commit msg first ;-)
Throughout t0027 there is no mention on what NNO stands for. Are
they about operations that result in un-normalized index entries?
> commit 3754404d3d1ea4a0cbbed4986cc4ac1b5fe6b66e
> Author: Torsten Bögershausen <tboegi@web.de>
> Date: Thu Aug 11 18:47:29 2016 +0200
>
> t0027: Correct raciness in NNO test
>
> When a non-reversible CRLF conversion is done in "git add",
> a warning is printed on stderr.
>
> The commit_chk_wrnNNO() function in t0027 was written to test this,
> but did the wrong thing: Instead of looking at the warning
> from "git add", it looked at the warning from "git commit".
>
> Correct this and replace the commit for each and every file with a commit
> of all files in one go.
>
> The function commit_chk_wrnNNO() will to be renamed in a separate commit.
> Thanks to Jeff King <peff@peff.net> for analizing t0027.
> Reporyed-By: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Reporyed?
An obligatory "comments-about-the-commit-msg";-)
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: t0027 racy?
2016-08-11 18:58 ` Torsten Bögershausen
2016-08-11 19:34 ` Junio C Hamano
@ 2016-08-12 7:24 ` Jeff King
1 sibling, 0 replies; 45+ messages in thread
From: Jeff King @ 2016-08-12 7:24 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Johannes Schindelin, git
On Thu, Aug 11, 2016 at 06:58:12PM +0000, Torsten Bögershausen wrote:
> commit 3754404d3d1ea4a0cbbed4986cc4ac1b5fe6b66e
> Author: Torsten Bögershausen <tboegi@web.de>
> Date: Thu Aug 11 18:47:29 2016 +0200
>
> t0027: Correct raciness in NNO test
>
> When a non-reversible CRLF conversion is done in "git add",
> a warning is printed on stderr.
>
> The commit_chk_wrnNNO() function in t0027 was written to test this,
> but did the wrong thing: Instead of looking at the warning
> from "git add", it looked at the warning from "git commit".
Maybe add:
This is racy because "git commit" may not have to do CRLF conversion
at all if it can use the sha1 value from the index (which depends on
whether "add" and "commit" run in a single second).
> Thanks to Jeff King <peff@peff.net> for analizing t0027.
> Reporyed-By: Johannes Schindelin <Johannes.Schindelin@gmx.de>
s/analizing/analyzing/; s/Reporyed/Reported/
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index 2860d2d..6e44382 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -119,8 +119,7 @@ commit_chk_wrnNNO () {
> fname=${pfx}_$f.txt &&
> cp $f $fname &&
> printf Z >>"$fname" &&
> - git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
> - git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
> + git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
> done
So this is the meat of the fix.
> @@ -394,11 +393,10 @@ test_expect_success 'commit files attr=crlf' '
>
> # attr LF CRLF CRLFmixLF LF_mix_CR CRLFNUL
> commit_chk_wrnNNO "" "" false "" "" "" "" ""
> -commit_chk_wrnNNO "" "" true LF_CRLF "" "" "" ""
> +commit_chk_wrnNNO "" "" true "" "" "" "" ""
> commit_chk_wrnNNO "" "" input "" "" "" "" ""
> -
> -commit_chk_wrnNNO "auto" "" false "$WILC" "" "" "" ""
> -commit_chk_wrnNNO "auto" "" true LF_CRLF "" "" "" ""
> +commit_chk_wrnNNO "auto" "" false "" "" "" "" ""
> +commit_chk_wrnNNO "auto" "" true "" "" "" "" ""
> commit_chk_wrnNNO "auto" "" input "" "" "" "" ""
I am not sure I understand why this reordering is necessary, but I guess
it's to group like things together in a single commit? Might be worth a
mention in the commit message.
-Peff
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v1 0/2] Fix conversion warnings
2016-08-09 11:49 ` Jeff King
2016-08-09 12:59 ` Torsten Bögershausen
@ 2016-08-12 16:50 ` tboegi
2016-08-12 16:51 ` [PATCH v1 1/2] t0027: Correct raciness in NNO test tboegi
` (8 subsequent siblings)
10 siblings, 0 replies; 45+ messages in thread
From: tboegi @ 2016-08-12 16:50 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
conversion warnings in convert.c and t0027
- Fix a raciness in t0027
- Fix an older problem with the conversion warnings when a file
has CRLF in the index, and is now commited with LF.
- Do not yet rename NNO to something different.
NNO means Not NOrmalized in the index.
The files are changed and content with different line endings
are added on top of the existing version.
A replacement for commit_chk_wrnNNO will be send after these bugfixes.
The term commit_chk_wrnNNO could be changed to
NNO_in_repo_add_file_check_warning.
Would NNO_addX__chk_wrn be more readable ?
Or CRLFindex_add_wrn ?
Changes against v0:
- Added raciness comment from Peff, and removed the removal of a line.
Torsten Bögershausen (2):
t0027: Correct raciness in NNO test
convert: missing `LF will be replaced by CRLF
convert.c | 97 ++++++++++++++++++++++++++++++----------------------
t/t0027-auto-crlf.sh | 10 +++---
2 files changed, 62 insertions(+), 45 deletions(-)
--
2.0.0.rc1.6318.g0c2c796
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH v1 1/2] t0027: Correct raciness in NNO test
2016-08-09 11:49 ` Jeff King
2016-08-09 12:59 ` Torsten Bögershausen
2016-08-12 16:50 ` [PATCH v1 0/2] Fix conversion warnings tboegi
@ 2016-08-12 16:51 ` tboegi
2016-08-12 17:56 ` Junio C Hamano
2016-08-13 16:50 ` Johannes Sixt
2016-08-12 16:51 ` [PATCH v1 2/2] convert: missing `LF will be replaced by CRLF tboegi
` (7 subsequent siblings)
10 siblings, 2 replies; 45+ messages in thread
From: tboegi @ 2016-08-12 16:51 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
When a non-reversible CRLF conversion is done in "git add",
a warning is printed on stderr (or Git dies, depending on checksafe)
The function commit_chk_wrnNNO() in t0027 was written to test this,
but did the wrong thing: Instead of looking at the warning
from "git add", it looked at the warning from "git commit".
This is racy because "git commit" may not have to do CRLF conversion
at all if it can use the sha1 value from the index (which depends on
whether "add" and "commit" run in a single second).
Correct this and replace the commit for each and every file with a commit
of all files in one go.
The function commit_chk_wrnNNO() will to be renamed in a separate commit.
Thanks to Jeff King <peff@peff.net> for analyzing t0027.
Reported-By: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
t/t0027-auto-crlf.sh | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 2860d2d..ab6e962 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -119,8 +119,7 @@ commit_chk_wrnNNO () {
fname=${pfx}_$f.txt &&
cp $f $fname &&
printf Z >>"$fname" &&
- git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
- git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
+ git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
done
test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
@@ -394,11 +393,11 @@ test_expect_success 'commit files attr=crlf' '
# attr LF CRLF CRLFmixLF LF_mix_CR CRLFNUL
commit_chk_wrnNNO "" "" false "" "" "" "" ""
-commit_chk_wrnNNO "" "" true LF_CRLF "" "" "" ""
+commit_chk_wrnNNO "" "" true "" "" "" "" ""
commit_chk_wrnNNO "" "" input "" "" "" "" ""
-commit_chk_wrnNNO "auto" "" false "$WILC" "" "" "" ""
-commit_chk_wrnNNO "auto" "" true LF_CRLF "" "" "" ""
+commit_chk_wrnNNO "auto" "" false "" "" "" "" ""
+commit_chk_wrnNNO "auto" "" true "" "" "" "" ""
commit_chk_wrnNNO "auto" "" input "" "" "" "" ""
for crlf in true false input
do
@@ -408,7 +407,7 @@ do
commit_chk_wrnNNO "" lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
commit_chk_wrnNNO "" crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
commit_chk_wrnNNO auto lf $crlf "" "" "" "" ""
- commit_chk_wrnNNO auto crlf $crlf LF_CRLF "" "" "" ""
+ commit_chk_wrnNNO auto crlf $crlf "" "" "" "" ""
commit_chk_wrnNNO text lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
commit_chk_wrnNNO text crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
done
@@ -417,7 +416,8 @@ commit_chk_wrnNNO "text" "" false "$WILC" "$WICL" "$WAMIX" "$WILC
commit_chk_wrnNNO "text" "" true LF_CRLF "" LF_CRLF LF_CRLF ""
commit_chk_wrnNNO "text" "" input "" CRLF_LF CRLF_LF "" CRLF_LF
-test_expect_success 'create files cleanup' '
+test_expect_success 'commit NNO and cleanup' '
+ git commit -m "commit files on top of NNO" &&
rm -f *.txt &&
git -c core.autocrlf=false reset --hard
'
--
2.0.0.rc1.6318.g0c2c796
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v1 1/2] t0027: Correct raciness in NNO test
2016-08-12 16:51 ` [PATCH v1 1/2] t0027: Correct raciness in NNO test tboegi
@ 2016-08-12 17:56 ` Junio C Hamano
2016-08-13 16:50 ` Johannes Sixt
1 sibling, 0 replies; 45+ messages in thread
From: Junio C Hamano @ 2016-08-12 17:56 UTC (permalink / raw)
To: tboegi; +Cc: git, peff, Johannes.Schindelin
tboegi@web.de writes:
> From: Torsten Bögershausen <tboegi@web.de>
>
> When a non-reversible CRLF conversion is done in "git add",
> a warning is printed on stderr (or Git dies, depending on checksafe)
>
> The function commit_chk_wrnNNO() in t0027 was written to test this,
> but did the wrong thing: Instead of looking at the warning
> from "git add", it looked at the warning from "git commit".
>
> This is racy because "git commit" may not have to do CRLF conversion
> at all if it can use the sha1 value from the index (which depends on
> whether "add" and "commit" run in a single second).
>
> Correct this and replace the commit for each and every file with a commit
> of all files in one go.
>
> The function commit_chk_wrnNNO() will to be renamed in a separate commit.
> Thanks to Jeff King <peff@peff.net> for analyzing t0027.
>
> Reported-By: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
Missing sign-off. The reasoning above looks very clearly explained.
Thanks.
> t/t0027-auto-crlf.sh | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index 2860d2d..ab6e962 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -119,8 +119,7 @@ commit_chk_wrnNNO () {
> fname=${pfx}_$f.txt &&
> cp $f $fname &&
> printf Z >>"$fname" &&
> - git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
> - git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
> + git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
> done
>
> test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
> @@ -394,11 +393,11 @@ test_expect_success 'commit files attr=crlf' '
>
> # attr LF CRLF CRLFmixLF LF_mix_CR CRLFNUL
> commit_chk_wrnNNO "" "" false "" "" "" "" ""
> -commit_chk_wrnNNO "" "" true LF_CRLF "" "" "" ""
> +commit_chk_wrnNNO "" "" true "" "" "" "" ""
> commit_chk_wrnNNO "" "" input "" "" "" "" ""
>
> -commit_chk_wrnNNO "auto" "" false "$WILC" "" "" "" ""
> -commit_chk_wrnNNO "auto" "" true LF_CRLF "" "" "" ""
> +commit_chk_wrnNNO "auto" "" false "" "" "" "" ""
> +commit_chk_wrnNNO "auto" "" true "" "" "" "" ""
> commit_chk_wrnNNO "auto" "" input "" "" "" "" ""
> for crlf in true false input
> do
> @@ -408,7 +407,7 @@ do
> commit_chk_wrnNNO "" lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
> commit_chk_wrnNNO "" crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
> commit_chk_wrnNNO auto lf $crlf "" "" "" "" ""
> - commit_chk_wrnNNO auto crlf $crlf LF_CRLF "" "" "" ""
> + commit_chk_wrnNNO auto crlf $crlf "" "" "" "" ""
> commit_chk_wrnNNO text lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
> commit_chk_wrnNNO text crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
> done
> @@ -417,7 +416,8 @@ commit_chk_wrnNNO "text" "" false "$WILC" "$WICL" "$WAMIX" "$WILC
> commit_chk_wrnNNO "text" "" true LF_CRLF "" LF_CRLF LF_CRLF ""
> commit_chk_wrnNNO "text" "" input "" CRLF_LF CRLF_LF "" CRLF_LF
>
> -test_expect_success 'create files cleanup' '
> +test_expect_success 'commit NNO and cleanup' '
> + git commit -m "commit files on top of NNO" &&
> rm -f *.txt &&
> git -c core.autocrlf=false reset --hard
> '
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v1 1/2] t0027: Correct raciness in NNO test
2016-08-12 16:51 ` [PATCH v1 1/2] t0027: Correct raciness in NNO test tboegi
2016-08-12 17:56 ` Junio C Hamano
@ 2016-08-13 16:50 ` Johannes Sixt
2016-08-13 21:18 ` Torsten Bögershausen
1 sibling, 1 reply; 45+ messages in thread
From: Johannes Sixt @ 2016-08-13 16:50 UTC (permalink / raw)
To: tboegi; +Cc: git, peff, Johannes.Schindelin
Am 12.08.2016 um 18:51 schrieb tboegi@web.de:
> From: Torsten Bögershausen <tboegi@web.de>
>
> When a non-reversible CRLF conversion is done in "git add",
> a warning is printed on stderr (or Git dies, depending on checksafe)
>
> The function commit_chk_wrnNNO() in t0027 was written to test this,
> but did the wrong thing: Instead of looking at the warning
> from "git add", it looked at the warning from "git commit".
>
> This is racy because "git commit" may not have to do CRLF conversion
> at all if it can use the sha1 value from the index (which depends on
> whether "add" and "commit" run in a single second).
>
> Correct this and replace the commit for each and every file with a commit
> of all files in one go.
The new test code does not only fix the race condition, but also tests
different things, or prepares test cases in a different manner. I would
have appreciated an explanation why this is necessary. Is it "on my
machine, the race condition was triggered consistently for a bunch of
tests, and so I recorded the wrong expected output in the test cases"?
>
> The function commit_chk_wrnNNO() will to be renamed in a separate commit.
> Thanks to Jeff King <peff@peff.net> for analyzing t0027.
>
> Reported-By: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
> t/t0027-auto-crlf.sh | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index 2860d2d..ab6e962 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -119,8 +119,7 @@ commit_chk_wrnNNO () {
> fname=${pfx}_$f.txt &&
> cp $f $fname &&
> printf Z >>"$fname" &&
> - git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
> - git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
> + git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
> done
>
> test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
> @@ -394,11 +393,11 @@ test_expect_success 'commit files attr=crlf' '
>
> # attr LF CRLF CRLFmixLF LF_mix_CR CRLFNUL
> commit_chk_wrnNNO "" "" false "" "" "" "" ""
> -commit_chk_wrnNNO "" "" true LF_CRLF "" "" "" ""
> +commit_chk_wrnNNO "" "" true "" "" "" "" ""
> commit_chk_wrnNNO "" "" input "" "" "" "" ""
>
> -commit_chk_wrnNNO "auto" "" false "$WILC" "" "" "" ""
> -commit_chk_wrnNNO "auto" "" true LF_CRLF "" "" "" ""
> +commit_chk_wrnNNO "auto" "" false "" "" "" "" ""
> +commit_chk_wrnNNO "auto" "" true "" "" "" "" ""
> commit_chk_wrnNNO "auto" "" input "" "" "" "" ""
> for crlf in true false input
> do
> @@ -408,7 +407,7 @@ do
> commit_chk_wrnNNO "" lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
> commit_chk_wrnNNO "" crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
> commit_chk_wrnNNO auto lf $crlf "" "" "" "" ""
> - commit_chk_wrnNNO auto crlf $crlf LF_CRLF "" "" "" ""
> + commit_chk_wrnNNO auto crlf $crlf "" "" "" "" ""
> commit_chk_wrnNNO text lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
> commit_chk_wrnNNO text crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
> done
> @@ -417,7 +416,8 @@ commit_chk_wrnNNO "text" "" false "$WILC" "$WICL" "$WAMIX" "$WILC
> commit_chk_wrnNNO "text" "" true LF_CRLF "" LF_CRLF LF_CRLF ""
> commit_chk_wrnNNO "text" "" input "" CRLF_LF CRLF_LF "" CRLF_LF
>
> -test_expect_success 'create files cleanup' '
> +test_expect_success 'commit NNO and cleanup' '
> + git commit -m "commit files on top of NNO" &&
> rm -f *.txt &&
> git -c core.autocrlf=false reset --hard
> '
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v1 1/2] t0027: Correct raciness in NNO test
2016-08-13 16:50 ` Johannes Sixt
@ 2016-08-13 21:18 ` Torsten Bögershausen
2016-08-14 20:37 ` Junio C Hamano
0 siblings, 1 reply; 45+ messages in thread
From: Torsten Bögershausen @ 2016-08-13 21:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, peff, Johannes.Schindelin
On Sat, Aug 13, 2016 at 06:50:19PM +0200, Johannes Sixt wrote:
> Am 12.08.2016 um 18:51 schrieb tboegi@web.de:
> >From: Torsten Bögershausen <tboegi@web.de>
> >
> >When a non-reversible CRLF conversion is done in "git add",
> >a warning is printed on stderr (or Git dies, depending on checksafe)
> >
> >The function commit_chk_wrnNNO() in t0027 was written to test this,
> >but did the wrong thing: Instead of looking at the warning
> >from "git add", it looked at the warning from "git commit".
> >
> >This is racy because "git commit" may not have to do CRLF conversion
> >at all if it can use the sha1 value from the index (which depends on
> >whether "add" and "commit" run in a single second).
> >
> >Correct this and replace the commit for each and every file with a commit
> >of all files in one go.
>
> The new test code does not only fix the race condition, but also
> tests different things, or prepares test cases in a different
> manner. I would have appreciated an explanation why this is
> necessary. Is it "on my machine, the race condition was triggered
> consistently for a bunch of tests, and so I recorded the wrong
> expected output in the test cases"?
>
Good point. The origanal intention was to let t0027 pass, and fix
the bug in convert.c in a later commit.
(and rename NNO in a 3rd commit, that is postponed until we figured this out).
Looking at t0027, it turns out that the changes in the test matrix done in 1/2
are reverted in 2/2.
This is not a good thing.
Either (a) they should be marked as test_expect_failure in 1/2 and
corrected in 2/2,
or (b) 1/2 and 2/2 are squashed together and the noise in t0027 is minimal.
I'll send a patch for (b) in a second.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 1/2] t0027: Correct raciness in NNO test
2016-08-13 21:18 ` Torsten Bögershausen
@ 2016-08-14 20:37 ` Junio C Hamano
0 siblings, 0 replies; 45+ messages in thread
From: Junio C Hamano @ 2016-08-14 20:37 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Johannes Sixt, git, peff, Johannes.Schindelin
Torsten Bögershausen <tboegi@web.de> writes:
> Looking at t0027, it turns out that the changes in the test matrix done in 1/2
> are reverted in 2/2.
> This is not a good thing.
> Either (a) they should be marked as test_expect_failure in 1/2 and
> corrected in 2/2,
> or (b) 1/2 and 2/2 are squashed together and the noise in t0027 is minimal.
Probably (a) is a bit more kosher and useful when somebody wants to
inspect the history of the test ("the test was flaky and that hid a
few bugs, but we fix the timing-dependency which exposes the bugs"
in 1/2, "this is the fix for the bug that the previous one exposed"
in 2/2). (b) might be a bit less work, though.
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v1 2/2] convert: missing `LF will be replaced by CRLF
2016-08-09 11:49 ` Jeff King
` (2 preceding siblings ...)
2016-08-12 16:51 ` [PATCH v1 1/2] t0027: Correct raciness in NNO test tboegi
@ 2016-08-12 16:51 ` tboegi
2016-08-12 17:52 ` Junio C Hamano
2016-08-13 21:29 ` [PATCH v2 0/1] convert: Correct NNO tests and missing `LF will be replaced by CRLF` tboegi
` (6 subsequent siblings)
10 siblings, 1 reply; 45+ messages in thread
From: tboegi @ 2016-08-12 16:51 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
When the "new safer autocrlf-handling" was introduced in c4805393, the logic
around check_safe_crlf() should have been changed:
Once a file has been commited with CRLF (and core.autocrlf is true),
line endings are no longer converted by "git add" or "git checkout".
The line endings will not normalized by Git.
The user may run e.g. dos2unix and commit the file.
Factor out will_convert_lf_to_crlf() which returns when LF are converted
into CRLF at checkout.
Update the logic around check_safe_crlf() and do the possible CRLF-LF
conversion in "git add".
Simulate the checkout (and a possible LF-CRLF conversion) with help of
will_convert_lf_to_crlf().
---
convert.c | 97 ++++++++++++++++++++++++++++++----------------------
t/t0027-auto-crlf.sh | 6 ++--
2 files changed, 60 insertions(+), 43 deletions(-)
diff --git a/convert.c b/convert.c
index 67d69b5..077f5e6 100644
--- a/convert.c
+++ b/convert.c
@@ -189,33 +189,25 @@ static enum eol output_eol(enum crlf_action crlf_action)
}
static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
- struct text_stat *stats, enum safe_crlf checksafe)
+ struct text_stat *old_stats, struct text_stat *new_stats,
+ enum safe_crlf checksafe)
{
- if (!checksafe)
- return;
-
- if (output_eol(crlf_action) == EOL_LF) {
+ if (old_stats->crlf && !new_stats->crlf ) {
/*
- * CRLFs would not be restored by checkout:
- * check if we'd remove CRLFs
+ * CRLFs would not be restored by checkout
*/
- if (stats->crlf) {
- if (checksafe == SAFE_CRLF_WARN)
- warning("CRLF will be replaced by LF in %s.\nThe file will have its original line endings in your working directory.", path);
- else /* i.e. SAFE_CRLF_FAIL */
- die("CRLF would be replaced by LF in %s.", path);
- }
- } else if (output_eol(crlf_action) == EOL_CRLF) {
+ if (checksafe == SAFE_CRLF_WARN)
+ warning("CRLF will be replaced by LF in %s.\nThe file will have its original line endings in your working directory.", path);
+ else /* i.e. SAFE_CRLF_FAIL */
+ die("CRLF would be replaced by LF in %s.", path);
+ } else if (old_stats->lonelf && !new_stats->lonelf ) {
/*
- * CRLFs would be added by checkout:
- * check if we have "naked" LFs
+ * CRLFs would be added by checkout
*/
- if (stats->lonelf) {
- if (checksafe == SAFE_CRLF_WARN)
- warning("LF will be replaced by CRLF in %s.\nThe file will have its original line endings in your working directory.", path);
- else /* i.e. SAFE_CRLF_FAIL */
- die("LF would be replaced by CRLF in %s", path);
- }
+ if (checksafe == SAFE_CRLF_WARN)
+ warning("LF will be replaced by CRLF in %s.\nThe file will have its original line endings in your working directory.", path);
+ else /* i.e. SAFE_CRLF_FAIL */
+ die("LF would be replaced by CRLF in %s", path);
}
}
@@ -233,12 +225,35 @@ static int has_cr_in_index(const char *path)
return has_cr;
}
+static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
+ enum crlf_action crlf_action)
+{
+ if (output_eol(crlf_action) != EOL_CRLF)
+ return 0;
+ /* No "naked" LF? Nothing to convert, regardless. */
+ if (!stats->lonelf)
+ return 0;
+
+ if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
+ /* If we have any CR or CRLF line endings, we do not touch it */
+ /* This is the new safer autocrlf-handling */
+ if (stats->lonecr || stats->crlf)
+ return 0;
+
+ if (convert_is_binary(len, stats))
+ return 0;
+ }
+ return 1;
+
+}
+
static int crlf_to_git(const char *path, const char *src, size_t len,
struct strbuf *buf,
enum crlf_action crlf_action, enum safe_crlf checksafe)
{
struct text_stat stats;
char *dst;
+ int convert_crlf_into_lf;
if (crlf_action == CRLF_BINARY ||
(src && !len))
@@ -252,6 +267,8 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
return 1;
gather_stats(src, len, &stats);
+ /* Optimization: No CRLF? Nothing to convert, regardless. */
+ convert_crlf_into_lf = !!stats.crlf;
if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
if (convert_is_binary(len, &stats))
@@ -263,12 +280,24 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
if (checksafe == SAFE_CRLF_RENORMALIZE)
checksafe = SAFE_CRLF_FALSE;
else if (has_cr_in_index(path))
- return 0;
+ convert_crlf_into_lf = 0;
}
- check_safe_crlf(path, crlf_action, &stats, checksafe);
-
- /* Optimization: No CRLF? Nothing to convert, regardless. */
- if (!stats.crlf)
+ if (checksafe && len) {
+ struct text_stat new_stats;
+ memcpy(&new_stats, &stats, sizeof(new_stats));
+ /* simulate "git add" */
+ if (convert_crlf_into_lf) {
+ new_stats.lonelf += new_stats.crlf;
+ new_stats.crlf = 0;
+ }
+ /* simulate "git checkout" */
+ if (will_convert_lf_to_crlf(len, &new_stats, crlf_action)) {
+ new_stats.crlf += new_stats.lonelf;
+ new_stats.lonelf = 0;
+ }
+ check_safe_crlf(path, crlf_action, &stats, &new_stats, checksafe);
+ }
+ if (!convert_crlf_into_lf)
return 0;
/*
@@ -314,21 +343,9 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
return 0;
gather_stats(src, len, &stats);
-
- /* No "naked" LF? Nothing to convert, regardless. */
- if (!stats.lonelf)
+ if (!will_convert_lf_to_crlf(len, &stats, crlf_action))
return 0;
- if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
- /* If we have any CR or CRLF line endings, we do not touch it */
- /* This is the new safer autocrlf-handling */
- if (stats.lonecr || stats.crlf )
- return 0;
-
- if (convert_is_binary(len, &stats))
- return 0;
- }
-
/* are we "faking" in place editing ? */
if (src == buf->buf)
to_free = strbuf_detach(buf, NULL);
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index ab6e962..7de8364 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -393,11 +393,11 @@ test_expect_success 'commit files attr=crlf' '
# attr LF CRLF CRLFmixLF LF_mix_CR CRLFNUL
commit_chk_wrnNNO "" "" false "" "" "" "" ""
-commit_chk_wrnNNO "" "" true "" "" "" "" ""
+commit_chk_wrnNNO "" "" true LF_CRLF "" "" "" ""
commit_chk_wrnNNO "" "" input "" "" "" "" ""
commit_chk_wrnNNO "auto" "" false "" "" "" "" ""
-commit_chk_wrnNNO "auto" "" true "" "" "" "" ""
+commit_chk_wrnNNO "auto" "" true LF_CRLF "" "" "" ""
commit_chk_wrnNNO "auto" "" input "" "" "" "" ""
for crlf in true false input
do
@@ -407,7 +407,7 @@ do
commit_chk_wrnNNO "" lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
commit_chk_wrnNNO "" crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
commit_chk_wrnNNO auto lf $crlf "" "" "" "" ""
- commit_chk_wrnNNO auto crlf $crlf "" "" "" "" ""
+ commit_chk_wrnNNO auto crlf $crlf LF_CRLF "" "" "" ""
commit_chk_wrnNNO text lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
commit_chk_wrnNNO text crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
done
--
2.0.0.rc1.6318.g0c2c796
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v1 2/2] convert: missing `LF will be replaced by CRLF
2016-08-12 16:51 ` [PATCH v1 2/2] convert: missing `LF will be replaced by CRLF tboegi
@ 2016-08-12 17:52 ` Junio C Hamano
0 siblings, 0 replies; 45+ messages in thread
From: Junio C Hamano @ 2016-08-12 17:52 UTC (permalink / raw)
To: tboegi; +Cc: git, peff, Johannes.Schindelin
tboegi@web.de writes:
> From: Torsten Bögershausen <tboegi@web.de>
>
> When the "new safer autocrlf-handling" was introduced in c4805393, the logic
> around check_safe_crlf() should have been changed:
Should have been changed from what to what? And instead it was
changed to which other logic? What is the difference between the
ideal this change tries to bring in and what c4805393 did?
> Once a file has been commited with CRLF (and core.autocrlf is true),
> line endings are no longer converted by "git add" or "git checkout".
> The line endings will not normalized by Git.
s/will not/& be/;
> The user may run e.g. dos2unix and commit the file.
It is unclear what this sentence wants to say. The user may overwrite
the file with random contents and commit it, too. So what?
What is missing from this statement is what problem is being worked
around. Do you mean: "Because Git does not normalize once CRLF is
in the index, if the user wants to have a normalized content in the
repository to correct that mistake, the user needs to do dos2unix to
fix it and commit the fixed result"?
> Factor out will_convert_lf_to_crlf() which returns when LF are converted
> into CRLF at checkout.
>
> Update the logic around check_safe_crlf() and do the possible CRLF-LF
> conversion in "git add".
> Simulate the checkout (and a possible LF-CRLF conversion) with help of
> will_convert_lf_to_crlf().
These two paragraphs are all "what I did to solve X", but what X is
is not very clear.
> ---
Missing sign-off.
> convert.c | 97 ++++++++++++++++++++++++++++++----------------------
> t/t0027-auto-crlf.sh | 6 ++--
> 2 files changed, 60 insertions(+), 43 deletions(-)
>
> diff --git a/convert.c b/convert.c
> index 67d69b5..077f5e6 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -189,33 +189,25 @@ static enum eol output_eol(enum crlf_action crlf_action)
> }
>
> static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
> - struct text_stat *stats, enum safe_crlf checksafe)
> + struct text_stat *old_stats, struct text_stat *new_stats,
> + enum safe_crlf checksafe)
> {
> - if (!checksafe)
> - return;
> -
> - if (output_eol(crlf_action) == EOL_LF) {
> + if (old_stats->crlf && !new_stats->crlf ) {
Space before ")"
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 0/1] convert: Correct NNO tests and missing `LF will be replaced by CRLF`
2016-08-09 11:49 ` Jeff King
` (3 preceding siblings ...)
2016-08-12 16:51 ` [PATCH v1 2/2] convert: missing `LF will be replaced by CRLF tboegi
@ 2016-08-13 21:29 ` tboegi
2016-08-17 12:46 ` Johannes Schindelin
2016-08-13 21:29 ` [PATCH v2 1/1] " tboegi
` (5 subsequent siblings)
10 siblings, 1 reply; 45+ messages in thread
From: tboegi @ 2016-08-13 21:29 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, j6t, Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
Change since v1:
- The changes done in 1/2 in t0027 needed to be reverted in 2/2.
Put both changes for convert.c and t0027 into a single commit
Thanks to Johannes Sixt.
Torsten Bögershausen (1):
convert: Correct NNO tests and missing `LF will be replaced by CRLF`
convert.c | 97 ++++++++++++++++++++++++++++++----------------------
t/t0027-auto-crlf.sh | 6 ++--
2 files changed, 60 insertions(+), 43 deletions(-)
--
2.9.3.599.g2376d31.dirty
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH v2 1/1] convert: Correct NNO tests and missing `LF will be replaced by CRLF`
2016-08-09 11:49 ` Jeff King
` (4 preceding siblings ...)
2016-08-13 21:29 ` [PATCH v2 0/1] convert: Correct NNO tests and missing `LF will be replaced by CRLF` tboegi
@ 2016-08-13 21:29 ` tboegi
2016-08-19 9:41 ` [PATCH v1 0/1] Rename NotNormalized (NNO) into CRLF in index tboegi
` (4 subsequent siblings)
10 siblings, 0 replies; 45+ messages in thread
From: tboegi @ 2016-08-13 21:29 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, j6t, Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
When a non-reversible CRLF conversion is done in "git add",
a warning is printed on stderr (or Git dies, depending on checksafe)
The function commit_chk_wrnNNO() in t0027 was written to test this,
but did the wrong thing: Instead of looking at the warning
from "git add", it looked at the warning from "git commit".
This is racy because "git commit" may not have to do CRLF conversion
at all if it can use the sha1 value from the index (which depends on
whether "add" and "commit" run in a single second).
Correct t0027 and replace the commit for each and every file with a commit
of all files in one go.
The function commit_chk_wrnNNO() should be renamed in a separate commit.
Now that t0027 does the right thing, it detects a bug in covert.c:
This sequence should generate the warning `LF will be replaced by CRLF`,
but does not:
$ git init
$ git config core.autocrlf false
$ printf "Line\r\n" >file
$ git add file
$ git commit -m "commit with CRLF"
$ git config core.autocrlf true
$ printf "Line\n" >file
$ git add file
"git add" calls crlf_to_git() in convert.c, which calls check_safe_crlf().
When has_cr_in_index(path) is true, crlf_to_git() returns too early and
check_safe_crlf() is not called at all.
Factor out the code which determines if "git checkout" converts LF->CRLF
into will_convert_lf_to_crlf().
Update the logic around check_safe_crlf() and "simulate" the possible
LF->CRLF conversion at "git checkout" with help of will_convert_lf_to_crlf().
Thanks to Jeff King <peff@peff.net> for analyzing t0027.
Reported-By: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
convert.c | 97 ++++++++++++++++++++++++++++++----------------------
t/t0027-auto-crlf.sh | 6 ++--
2 files changed, 60 insertions(+), 43 deletions(-)
diff --git a/convert.c b/convert.c
index 67d69b5..077f5e6 100644
--- a/convert.c
+++ b/convert.c
@@ -189,33 +189,25 @@ static enum eol output_eol(enum crlf_action crlf_action)
}
static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
- struct text_stat *stats, enum safe_crlf checksafe)
+ struct text_stat *old_stats, struct text_stat *new_stats,
+ enum safe_crlf checksafe)
{
- if (!checksafe)
- return;
-
- if (output_eol(crlf_action) == EOL_LF) {
+ if (old_stats->crlf && !new_stats->crlf ) {
/*
- * CRLFs would not be restored by checkout:
- * check if we'd remove CRLFs
+ * CRLFs would not be restored by checkout
*/
- if (stats->crlf) {
- if (checksafe == SAFE_CRLF_WARN)
- warning("CRLF will be replaced by LF in %s.\nThe file will have its original line endings in your working directory.", path);
- else /* i.e. SAFE_CRLF_FAIL */
- die("CRLF would be replaced by LF in %s.", path);
- }
- } else if (output_eol(crlf_action) == EOL_CRLF) {
+ if (checksafe == SAFE_CRLF_WARN)
+ warning("CRLF will be replaced by LF in %s.\nThe file will have its original line endings in your working directory.", path);
+ else /* i.e. SAFE_CRLF_FAIL */
+ die("CRLF would be replaced by LF in %s.", path);
+ } else if (old_stats->lonelf && !new_stats->lonelf ) {
/*
- * CRLFs would be added by checkout:
- * check if we have "naked" LFs
+ * CRLFs would be added by checkout
*/
- if (stats->lonelf) {
- if (checksafe == SAFE_CRLF_WARN)
- warning("LF will be replaced by CRLF in %s.\nThe file will have its original line endings in your working directory.", path);
- else /* i.e. SAFE_CRLF_FAIL */
- die("LF would be replaced by CRLF in %s", path);
- }
+ if (checksafe == SAFE_CRLF_WARN)
+ warning("LF will be replaced by CRLF in %s.\nThe file will have its original line endings in your working directory.", path);
+ else /* i.e. SAFE_CRLF_FAIL */
+ die("LF would be replaced by CRLF in %s", path);
}
}
@@ -233,12 +225,35 @@ static int has_cr_in_index(const char *path)
return has_cr;
}
+static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
+ enum crlf_action crlf_action)
+{
+ if (output_eol(crlf_action) != EOL_CRLF)
+ return 0;
+ /* No "naked" LF? Nothing to convert, regardless. */
+ if (!stats->lonelf)
+ return 0;
+
+ if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
+ /* If we have any CR or CRLF line endings, we do not touch it */
+ /* This is the new safer autocrlf-handling */
+ if (stats->lonecr || stats->crlf)
+ return 0;
+
+ if (convert_is_binary(len, stats))
+ return 0;
+ }
+ return 1;
+
+}
+
static int crlf_to_git(const char *path, const char *src, size_t len,
struct strbuf *buf,
enum crlf_action crlf_action, enum safe_crlf checksafe)
{
struct text_stat stats;
char *dst;
+ int convert_crlf_into_lf;
if (crlf_action == CRLF_BINARY ||
(src && !len))
@@ -252,6 +267,8 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
return 1;
gather_stats(src, len, &stats);
+ /* Optimization: No CRLF? Nothing to convert, regardless. */
+ convert_crlf_into_lf = !!stats.crlf;
if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
if (convert_is_binary(len, &stats))
@@ -263,12 +280,24 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
if (checksafe == SAFE_CRLF_RENORMALIZE)
checksafe = SAFE_CRLF_FALSE;
else if (has_cr_in_index(path))
- return 0;
+ convert_crlf_into_lf = 0;
}
- check_safe_crlf(path, crlf_action, &stats, checksafe);
-
- /* Optimization: No CRLF? Nothing to convert, regardless. */
- if (!stats.crlf)
+ if (checksafe && len) {
+ struct text_stat new_stats;
+ memcpy(&new_stats, &stats, sizeof(new_stats));
+ /* simulate "git add" */
+ if (convert_crlf_into_lf) {
+ new_stats.lonelf += new_stats.crlf;
+ new_stats.crlf = 0;
+ }
+ /* simulate "git checkout" */
+ if (will_convert_lf_to_crlf(len, &new_stats, crlf_action)) {
+ new_stats.crlf += new_stats.lonelf;
+ new_stats.lonelf = 0;
+ }
+ check_safe_crlf(path, crlf_action, &stats, &new_stats, checksafe);
+ }
+ if (!convert_crlf_into_lf)
return 0;
/*
@@ -314,21 +343,9 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
return 0;
gather_stats(src, len, &stats);
-
- /* No "naked" LF? Nothing to convert, regardless. */
- if (!stats.lonelf)
+ if (!will_convert_lf_to_crlf(len, &stats, crlf_action))
return 0;
- if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
- /* If we have any CR or CRLF line endings, we do not touch it */
- /* This is the new safer autocrlf-handling */
- if (stats.lonecr || stats.crlf )
- return 0;
-
- if (convert_is_binary(len, &stats))
- return 0;
- }
-
/* are we "faking" in place editing ? */
if (src == buf->buf)
to_free = strbuf_detach(buf, NULL);
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 2860d2d..90db54c 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -119,8 +119,7 @@ commit_chk_wrnNNO () {
fname=${pfx}_$f.txt &&
cp $f $fname &&
printf Z >>"$fname" &&
- git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
- git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
+ git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
done
test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
@@ -417,7 +416,8 @@ commit_chk_wrnNNO "text" "" false "$WILC" "$WICL" "$WAMIX" "$WILC
commit_chk_wrnNNO "text" "" true LF_CRLF "" LF_CRLF LF_CRLF ""
commit_chk_wrnNNO "text" "" input "" CRLF_LF CRLF_LF "" CRLF_LF
-test_expect_success 'create files cleanup' '
+test_expect_success 'commit NNO and cleanup' '
+ git commit -m "commit files on top of NNO" &&
rm -f *.txt &&
git -c core.autocrlf=false reset --hard
'
--
2.9.3.599.g2376d31.dirty
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v1 0/1] Rename NotNormalized (NNO) into CRLF in index
2016-08-09 11:49 ` Jeff King
` (5 preceding siblings ...)
2016-08-13 21:29 ` [PATCH v2 1/1] " tboegi
@ 2016-08-19 9:41 ` tboegi
2016-08-19 16:39 ` Junio C Hamano
2016-08-19 9:41 ` [PATCH v1 1/1] t0027: " tboegi
` (3 subsequent siblings)
10 siblings, 1 reply; 45+ messages in thread
From: tboegi @ 2016-08-19 9:41 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
Here comes the promised cleanup of t0027:
- The wording NNO is removed and replaced by CRI
- No code changes
- Needs to go on top of next or pu or tb/t0027-raciness-fix
Torsten Bögershausen (1):
t0027: Rename NotNormalized (NNO) into CRLF in index
t/t0027-auto-crlf.sh | 122 +++++++++++++++++++++++++--------------------------
1 file changed, 61 insertions(+), 61 deletions(-)
--
2.9.3.599.g2376d31.dirty
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v1 0/1] Rename NotNormalized (NNO) into CRLF in index
2016-08-19 9:41 ` [PATCH v1 0/1] Rename NotNormalized (NNO) into CRLF in index tboegi
@ 2016-08-19 16:39 ` Junio C Hamano
0 siblings, 0 replies; 45+ messages in thread
From: Junio C Hamano @ 2016-08-19 16:39 UTC (permalink / raw)
To: tboegi; +Cc: git
tboegi@web.de writes:
> From: Torsten Bögershausen <tboegi@web.de>
>
> Here comes the promised cleanup of t0027:
> - The wording NNO is removed and replaced by CRI
> - No code changes
> - Needs to go on top of next or pu or tb/t0027-raciness-fix
> Torsten Bögershausen (1):
> t0027: Rename NotNormalized (NNO) into CRLF in index
>
> t/t0027-auto-crlf.sh | 122 +++++++++++++++++++++++++--------------------------
> 1 file changed, 61 insertions(+), 61 deletions(-)
Until these acronyms are shown expanded upfront in the file (not in
the log message), this patch does not help the readers at all. They
used to say "What the heck is NNO?" Now they will be left wondering
"What the heck is CRI?".
Have a comment before the first use of the acronym, perhaps?
@@ -102,7 +84,7 @@ commit_check_warn () {
check_warning "$crlfnul" ${pfx}_CRLF_nul.err
}
-commit_chk_wrnNNO () {
+CRI_add_chk_wrn () {
attr=$1 ; shift
aeol=$1 ; shift
crlf=$1 ; shift
Incidentally it is a good place to describe what this humongous
helper function is meant to do.
# Having CR in the indexed contents (CRI in short) poses such and
# such tricky cases; this helper tests combinations of xyzzy, frotz
# and nitfol exhaustively by running xyzzy through (a, b, c),
# flipping frotz between (x, y), and ...
or something like that (of course the blanks need to be filled),
perhaps?
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v1 1/1] t0027: Rename NotNormalized (NNO) into CRLF in index
2016-08-09 11:49 ` Jeff King
` (6 preceding siblings ...)
2016-08-19 9:41 ` [PATCH v1 0/1] Rename NotNormalized (NNO) into CRLF in index tboegi
@ 2016-08-19 9:41 ` tboegi
2016-08-25 15:52 ` [PATCH v1 0/3] Update eol documentation tboegi
` (2 subsequent siblings)
10 siblings, 0 replies; 45+ messages in thread
From: tboegi @ 2016-08-19 9:41 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
Originally NNO stands for content, that had been commited
"Not NOrmalized", in other words files with CRLF in the index.
Make more clear what should be tested:
- commit a file with CRLF into the index
- Change the content in the working tree
- Run "git add" and check for the conversion warnings
- Repeat for different content (text, LF, CRLF, mixed) and
binary (LF and lone CR, CRLF with NUL)
Rename commit_chk_wrnNNO() into CRI_add_chk_wrn() and rename NNO into CRI.
Integrate create_NNO_files() into 'setup master'
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/t0027-auto-crlf.sh | 122 +++++++++++++++++++++++++--------------------------
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 90db54c..bfcf14b 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -49,24 +49,6 @@ create_gitattributes () {
} >.gitattributes
}
-create_NNO_files () {
- for crlf in false true input
- do
- for attr in "" auto text -text
- do
- for aeol in "" lf crlf
- do
- pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
- cp CRLF_mix_LF ${pfx}_LF.txt &&
- cp CRLF_mix_LF ${pfx}_CRLF.txt &&
- cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
- cp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&
- cp CRLF_mix_LF ${pfx}_CRLF_nul.txt
- done
- done
- done
-}
-
check_warning () {
case "$1" in
LF_CRLF) echo "warning: LF will be replaced by CRLF" >"$2".expect ;;
@@ -102,7 +84,7 @@ commit_check_warn () {
check_warning "$crlfnul" ${pfx}_CRLF_nul.err
}
-commit_chk_wrnNNO () {
+CRI_add_chk_wrn () {
attr=$1 ; shift
aeol=$1 ; shift
crlf=$1 ; shift
@@ -111,7 +93,7 @@ commit_chk_wrnNNO () {
lfmixcrlf=$1 ; shift
lfmixcr=$1 ; shift
crlfnul=$1 ; shift
- pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
+ pfx=CRI_attr_${attr}_aeol_${aeol}_${crlf}
#Commit files on top of existing file
create_gitattributes "$attr" $aeol &&
for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
@@ -122,22 +104,22 @@ commit_chk_wrnNNO () {
git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
done
- test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
+ test_expect_success "CRLF in index add file crlf=$crlf attr=$attr LF" '
check_warning "$lfwarn" ${pfx}_LF.err
'
- test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf CRLF" '
+ test_expect_success "CRLF in index add file attr=$attr aeol=$aeol crlf=$crlf CRLF" '
check_warning "$crlfwarn" ${pfx}_CRLF.err
'
- test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf CRLF_mix_LF" '
+ test_expect_success "CRLF in index add file attr=$attr aeol=$aeol crlf=$crlf CRLF_mix_LF" '
check_warning "$lfmixcrlf" ${pfx}_CRLF_mix_LF.err
'
- test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf LF_mix_cr" '
+ test_expect_success "CRLF in index add file attr=$attr aeol=$aeol crlf=$crlf LF_mix_cr" '
check_warning "$lfmixcr" ${pfx}_LF_mix_CR.err
'
- test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf CRLF_nul" '
+ test_expect_success "CRLF in index add file attr=$attr aeol=$aeol crlf=$crlf CRLF_nul" '
check_warning "$crlfnul" ${pfx}_CRLF_nul.err
'
}
@@ -199,7 +181,7 @@ check_files_in_repo () {
compare_files $crlfnul ${pfx}CRLF_nul.txt
}
-check_in_repo_NNO () {
+check_in_repo_CRI () {
attr=$1 ; shift
aeol=$1 ; shift
crlf=$1 ; shift
@@ -208,7 +190,7 @@ check_in_repo_NNO () {
lfmixcrlf=$1 ; shift
lfmixcr=$1 ; shift
crlfnul=$1 ; shift
- pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
+ pfx=CRI_attr_${attr}_aeol_${aeol}_${crlf}
test_expect_success "compare_files $lfname ${pfx}_LF.txt" '
compare_files $lfname ${pfx}_LF.txt
'
@@ -329,8 +311,22 @@ test_expect_success 'setup master' '
printf "\$Id: 0000000000000000000000000000000000000000 \$\r\nLINEONE\r\nLINETWO\rLINETHREE" >CRLF_mix_CR &&
printf "\$Id: 0000000000000000000000000000000000000000 \$\r\nLINEONEQ\r\nLINETWO\r\nLINETHREE" | q_to_nul >CRLF_nul &&
printf "\$Id: 0000000000000000000000000000000000000000 \$\nLINEONEQ\nLINETWO\nLINETHREE" | q_to_nul >LF_nul &&
- create_NNO_files CRLF_mix_LF CRLF_mix_LF CRLF_mix_LF CRLF_mix_LF CRLF_mix_LF &&
- git -c core.autocrlf=false add NNO_*.txt &&
+ for crlf in false true input
+ do
+ for attr in "" auto text -text
+ do
+ for aeol in "" lf crlf
+ do
+ pfx=CRI_attr_${attr}_aeol_${aeol}_${crlf}
+ cp CRLF_mix_LF ${pfx}_LF.txt &&
+ cp CRLF_mix_LF ${pfx}_CRLF.txt &&
+ cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
+ cp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&
+ cp CRLF_mix_LF ${pfx}_CRLF_nul.txt
+ done
+ done
+ done &&
+ git -c core.autocrlf=false add CRI_*.txt &&
git commit -m "mixed line endings" &&
test_tick
'
@@ -391,33 +387,37 @@ test_expect_success 'commit files attr=crlf' '
commit_check_warn input "crlf" "LF_CRLF" "" "LF_CRLF" "LF_CRLF" ""
'
-# attr LF CRLF CRLFmixLF LF_mix_CR CRLFNUL
-commit_chk_wrnNNO "" "" false "" "" "" "" ""
-commit_chk_wrnNNO "" "" true LF_CRLF "" "" "" ""
-commit_chk_wrnNNO "" "" input "" "" "" "" ""
-
-commit_chk_wrnNNO "auto" "" false "$WILC" "" "" "" ""
-commit_chk_wrnNNO "auto" "" true LF_CRLF "" "" "" ""
-commit_chk_wrnNNO "auto" "" input "" "" "" "" ""
+# Files had been commited with CRLF. run "git add" with different content and
+# check if
+# warning: LF will be replaced by CRLF or
+# warning: CRLF will be replaced by LF are written to stderr
+# attr LF CRLF CRLFmixLF LF_mix_CR CRLFNUL
+CRI_add_chk_wrn "" "" false "" "" "" "" ""
+CRI_add_chk_wrn "" "" true LF_CRLF "" "" "" ""
+CRI_add_chk_wrn "" "" input "" "" "" "" ""
+
+CRI_add_chk_wrn "auto" "" false "$WILC" "" "" "" ""
+CRI_add_chk_wrn "auto" "" true LF_CRLF "" "" "" ""
+CRI_add_chk_wrn "auto" "" input "" "" "" "" ""
for crlf in true false input
do
- commit_chk_wrnNNO -text "" $crlf "" "" "" "" ""
- commit_chk_wrnNNO -text lf $crlf "" "" "" "" ""
- commit_chk_wrnNNO -text crlf $crlf "" "" "" "" ""
- commit_chk_wrnNNO "" lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
- commit_chk_wrnNNO "" crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
- commit_chk_wrnNNO auto lf $crlf "" "" "" "" ""
- commit_chk_wrnNNO auto crlf $crlf LF_CRLF "" "" "" ""
- commit_chk_wrnNNO text lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
- commit_chk_wrnNNO text crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
+ CRI_add_chk_wrn -text "" $crlf "" "" "" "" ""
+ CRI_add_chk_wrn -text lf $crlf "" "" "" "" ""
+ CRI_add_chk_wrn -text crlf $crlf "" "" "" "" ""
+ CRI_add_chk_wrn "" lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
+ CRI_add_chk_wrn "" crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
+ CRI_add_chk_wrn auto lf $crlf "" "" "" "" ""
+ CRI_add_chk_wrn auto crlf $crlf LF_CRLF "" "" "" ""
+ CRI_add_chk_wrn text lf $crlf "" CRLF_LF CRLF_LF "" CRLF_LF
+ CRI_add_chk_wrn text crlf $crlf LF_CRLF "" LF_CRLF LF_CRLF ""
done
-commit_chk_wrnNNO "text" "" false "$WILC" "$WICL" "$WAMIX" "$WILC" "$WICL"
-commit_chk_wrnNNO "text" "" true LF_CRLF "" LF_CRLF LF_CRLF ""
-commit_chk_wrnNNO "text" "" input "" CRLF_LF CRLF_LF "" CRLF_LF
+CRI_add_chk_wrn text "" false "$WILC" "$WICL" "$WAMIX" "$WILC" "$WICL"
+CRI_add_chk_wrn text "" true LF_CRLF "" LF_CRLF LF_CRLF ""
+CRI_add_chk_wrn text "" input "" CRLF_LF CRLF_LF "" CRLF_LF
-test_expect_success 'commit NNO and cleanup' '
- git commit -m "commit files on top of NNO" &&
+test_expect_success 'commit files with CRLF in index CRI and cleanup' '
+ git commit -m "commit files on top of CRLF" &&
rm -f *.txt &&
git -c core.autocrlf=false reset --hard
'
@@ -449,16 +449,16 @@ test_expect_success 'commit -text' '
for crlf in true false input
do
# attr aeol LF CRLF CRLF_mix_LF LF_mix_CR CRLFNUL
- check_in_repo_NNO "" "" $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
- check_in_repo_NNO -text "" $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
- check_in_repo_NNO -text lf $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
- check_in_repo_NNO -text crlf $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
- check_in_repo_NNO auto "" $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
- check_in_repo_NNO auto lf $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
- check_in_repo_NNO auto crlf $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
- check_in_repo_NNO text "" $crlf LF LF LF LF_mix_CR LF_nul
- check_in_repo_NNO text lf $crlf LF LF LF LF_mix_CR LF_nul
- check_in_repo_NNO text crlf $crlf LF LF LF LF_mix_CR LF_nul
+ check_in_repo_CRI "" "" $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
+ check_in_repo_CRI -text "" $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
+ check_in_repo_CRI -text lf $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
+ check_in_repo_CRI -text crlf $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
+ check_in_repo_CRI auto "" $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
+ check_in_repo_CRI auto lf $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
+ check_in_repo_CRI auto crlf $crlf LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
+ check_in_repo_CRI text "" $crlf LF LF LF LF_mix_CR LF_nul
+ check_in_repo_CRI text lf $crlf LF LF LF LF_mix_CR LF_nul
+ check_in_repo_CRI text crlf $crlf LF LF LF LF_mix_CR LF_nul
done
################################################################################
# Check how files in the repo are changed when they are checked out
--
2.9.3.599.g2376d31.dirty
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v1 0/3] Update eol documentation
2016-08-09 11:49 ` Jeff King
` (7 preceding siblings ...)
2016-08-19 9:41 ` [PATCH v1 1/1] t0027: " tboegi
@ 2016-08-25 15:52 ` tboegi
2016-08-25 20:31 ` Junio C Hamano
2016-08-25 15:52 ` [PATCH v1 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10 tboegi
2016-08-25 15:52 ` [PATCH v1 2/2] gitattributes: Document the unified "auto" handling tboegi
10 siblings, 1 reply; 45+ messages in thread
From: tboegi @ 2016-08-25 15:52 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
Sorry for posting this so late:
While reviewing another patch I realized that the eol related
documentation was not updated as it should be.
Torsten Bögershausen (2):
git ls-files: text=auto eol=lf is supported in Git 2.10
gitattributes: Document the unified "auto" handling
Documentation/git-ls-files.txt | 3 +--
Documentation/gitattributes.txt | 24 ++++++++++++++++--------
2 files changed, 17 insertions(+), 10 deletions(-)
--
2.9.3.599.g2376d31.dirty
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v1 0/3] Update eol documentation
2016-08-25 15:52 ` [PATCH v1 0/3] Update eol documentation tboegi
@ 2016-08-25 20:31 ` Junio C Hamano
2016-08-26 1:00 ` Jacob Keller
2016-08-26 7:03 ` Torsten Bögershausen
0 siblings, 2 replies; 45+ messages in thread
From: Junio C Hamano @ 2016-08-25 20:31 UTC (permalink / raw)
To: tboegi; +Cc: git
tboegi@web.de writes:
> From: Torsten Bögershausen <tboegi@web.de>
>
> Sorry for posting this so late:
> While reviewing another patch I realized that the eol related
> documentation was not updated as it should be.
>
> Torsten Bögershausen (2):
> git ls-files: text=auto eol=lf is supported in Git 2.10
> gitattributes: Document the unified "auto" handling
>
> Documentation/git-ls-files.txt | 3 +--
> Documentation/gitattributes.txt | 24 ++++++++++++++++--------
> 2 files changed, 17 insertions(+), 10 deletions(-)
This [0/3] is meant to be a cover for [1/2] and [2/2]?
I am trying to see if we broke format-patch recently, or it is a
manual editing error. The latter I do not care about; the former I
do.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/3] Update eol documentation
2016-08-25 20:31 ` Junio C Hamano
@ 2016-08-26 1:00 ` Jacob Keller
2016-08-26 7:03 ` Torsten Bögershausen
1 sibling, 0 replies; 45+ messages in thread
From: Jacob Keller @ 2016-08-26 1:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, Git mailing list
On Thu, Aug 25, 2016 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> tboegi@web.de writes:
>
>> From: Torsten Bögershausen <tboegi@web.de>
>>
>> Sorry for posting this so late:
>> While reviewing another patch I realized that the eol related
>> documentation was not updated as it should be.
>>
>> Torsten Bögershausen (2):
>> git ls-files: text=auto eol=lf is supported in Git 2.10
>> gitattributes: Document the unified "auto" handling
>>
>> Documentation/git-ls-files.txt | 3 +--
>> Documentation/gitattributes.txt | 24 ++++++++++++++++--------
>> 2 files changed, 17 insertions(+), 10 deletions(-)
>
> This [0/3] is meant to be a cover for [1/2] and [2/2]?
>
> I am trying to see if we broke format-patch recently, or it is a
> manual editing error. The latter I do not care about; the former I
> do.
> --
Yes. I recently changed some of format patch and would like to make
sure I didn't break it...
Thanks,
Jake
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/3] Update eol documentation
2016-08-25 20:31 ` Junio C Hamano
2016-08-26 1:00 ` Jacob Keller
@ 2016-08-26 7:03 ` Torsten Bögershausen
1 sibling, 0 replies; 45+ messages in thread
From: Torsten Bögershausen @ 2016-08-26 7:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On 25/08/16 22:31, Junio C Hamano wrote:
> []
> This [0/3] is meant to be a cover for [1/2] and [2/2]?
>
> I am trying to see if we broke format-patch recently, or it is a
> manual editing error. The latter I do not care about; the former I
> do.
>
No worry, 0/3 is patched by me by hand, sorry for the confusion.
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v1 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10
2016-08-09 11:49 ` Jeff King
` (8 preceding siblings ...)
2016-08-25 15:52 ` [PATCH v1 0/3] Update eol documentation tboegi
@ 2016-08-25 15:52 ` tboegi
2016-08-25 20:38 ` Junio C Hamano
2016-08-25 15:52 ` [PATCH v1 2/2] gitattributes: Document the unified "auto" handling tboegi
10 siblings, 1 reply; 45+ messages in thread
From: tboegi @ 2016-08-25 15:52 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
The man page for `git ls-files --eol` mentions the combination
of text attributes "text=auto eol=lf" or "text=auto eol=crlf" as not
supported yet, but may be in the future.
Now they are supported
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Documentation/git-ls-files.txt | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 078b556..0d933ac 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -159,8 +159,7 @@ not accessible in the working tree.
+
<eolattr> is the attribute that is used when checking out or committing,
it is either "", "-text", "text", "text=auto", "text eol=lf", "text eol=crlf".
-Note: Currently Git does not support "text=auto eol=lf" or "text=auto eol=crlf",
-that may change in the future.
+Since Git 2.10 "text=auto eol=lf" and "text=auto eol=crlf" are supported.
+
Both the <eolinfo> in the index ("i/<eolinfo>")
and in the working tree ("w/<eolinfo>") are shown for regular files,
--
2.9.3.599.g2376d31.dirty
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v1 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10
2016-08-25 15:52 ` [PATCH v1 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10 tboegi
@ 2016-08-25 20:38 ` Junio C Hamano
0 siblings, 0 replies; 45+ messages in thread
From: Junio C Hamano @ 2016-08-25 20:38 UTC (permalink / raw)
To: tboegi; +Cc: git
tboegi@web.de writes:
> From: Torsten Bögershausen <tboegi@web.de>
>
> The man page for `git ls-files --eol` mentions the combination
> of text attributes "text=auto eol=lf" or "text=auto eol=crlf" as not
> supported yet, but may be in the future.
> Now they are supported
Thanks. I'll finish the sentence with a full-stop here ;-).
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> Documentation/git-ls-files.txt | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> index 078b556..0d933ac 100644
> --- a/Documentation/git-ls-files.txt
> +++ b/Documentation/git-ls-files.txt
> @@ -159,8 +159,7 @@ not accessible in the working tree.
> +
> <eolattr> is the attribute that is used when checking out or committing,
> it is either "", "-text", "text", "text=auto", "text eol=lf", "text eol=crlf".
> -Note: Currently Git does not support "text=auto eol=lf" or "text=auto eol=crlf",
> -that may change in the future.
> +Since Git 2.10 "text=auto eol=lf" and "text=auto eol=crlf" are supported.
It may be a good idea to have this for a while. Having this
sentence would only help those who have been dissuaded by the
existing Note by telling them that the limitation is no longer
there, but that will quickly become unnecessary.
We'd eventually want to remove this sentence. I wonder if it is a
better alternative to just remove the Note without adding new text,
though.
> Both the <eolinfo> in the index ("i/<eolinfo>")
> and in the working tree ("w/<eolinfo>") are shown for regular files,
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v1 2/2] gitattributes: Document the unified "auto" handling
2016-08-09 11:49 ` Jeff King
` (9 preceding siblings ...)
2016-08-25 15:52 ` [PATCH v1 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10 tboegi
@ 2016-08-25 15:52 ` tboegi
2016-08-25 20:46 ` Junio C Hamano
10 siblings, 1 reply; 45+ messages in thread
From: tboegi @ 2016-08-25 15:52 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
Update the documentation about text=auto:
text=auto now follows the core.autocrlf handling when files are not
normalized in the repository.
For a cross platform project recommend the usage of attributes for
line-ending conversions.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Documentation/gitattributes.txt | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 807577a..4012661 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -213,27 +213,35 @@ that text files that you introduce to the repository have their line
endings normalized to LF when they are added, and that files that are
already normalized in the repository stay normalized.
+If you want to ensure that text files that any contributor introduces to
+the repository have their line endings normalized, you could set the
+`text` attribute to "auto" for _all_ files.
+
+------------------------
+* text=auto
+------------------------
+
If you want to interoperate with a source code management system that
enforces end-of-line normalization, or you simply want all text files
in your repository to be normalized, you should instead set the `text`
-attribute to "auto" for _all_ files.
+attribute to "text" for text files.
------------------------
-* text=auto
+*.txt text
------------------------
-This ensures that all files that Git considers to be text will have
+This ensures that all files marked as text will have
normalized (LF) line endings in the repository. The `core.eol`
configuration variable controls which line endings Git will use for
normalized files in your working directory; the default is to use the
native line ending for your platform, or CRLF if `core.autocrlf` is
set.
-NOTE: When `text=auto` normalization is enabled in an existing
-repository, any text files containing CRLFs should be normalized. If
-they are not they will be normalized the next time someone tries to
-change them, causing unfortunate misattribution. From a clean working
-directory:
+NOTE: When you have a cross-platform project using push and pull
+to a central repository the text files containing CRLFs should be
+normalized. All text files should have a text attribute, either
+`text` or `text=auto`.
+From a clean working directory:
-------------------------------------------------
$ echo "* text=auto" >>.gitattributes
--
2.9.3.599.g2376d31.dirty
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v1 2/2] gitattributes: Document the unified "auto" handling
2016-08-25 15:52 ` [PATCH v1 2/2] gitattributes: Document the unified "auto" handling tboegi
@ 2016-08-25 20:46 ` Junio C Hamano
2016-08-26 20:18 ` [PATCH v2 0/2] Adjust the documentation to " tboegi
` (2 more replies)
0 siblings, 3 replies; 45+ messages in thread
From: Junio C Hamano @ 2016-08-25 20:46 UTC (permalink / raw)
To: tboegi; +Cc: git
tboegi@web.de writes:
> +If you want to ensure that text files that any contributor introduces to
> +the repository have their line endings normalized, you could set the
> +`text` attribute to "auto" for _all_ files.
> +
> +------------------------
> +* text=auto
> +------------------------
> +
That is very understandable, especially that the text before this
added paragraph is about "core.autocrlf" configuration that is about
"your" changes. It contrasts gitconfig vs gitattributes very well.
However, it is no longer clear what "you should instead" in the
existing paragraph attempts to contrast with. "If you want all text
files, then use '* text=auto'" is what is said previously. And your
updated example below says "If you do not want that, and instead you
want X, do '*.txt text'". But the value of X is reads the same as
the above one: "you want all text files to be normalized".
> If you want to interoperate with a source code management system that
> enforces end-of-line normalization, or you simply want all text files
> in your repository to be normalized, you should instead set the `text`
> -attribute to "auto" for _all_ files.
> +attribute to "text" for text files.
>
> ------------------------
> -* text=auto
> +*.txt text
> ------------------------
In short, the above is incoherent and not understandable, without
updating the three lines of introductory text you left untouched at
the beginning of the paragraph, when read in the (updated) context.
> -This ensures that all files that Git considers to be text will have
> +This ensures that all files marked as text will have
This is a good update of the description to match the updated example.
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH v2 0/2] Adjust the documentation to the unified "auto" handling
2016-08-25 20:46 ` Junio C Hamano
@ 2016-08-26 20:18 ` tboegi
2016-08-26 20:18 ` [PATCH v2 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10 tboegi
2016-08-26 20:18 ` [PATCH v2 2/2] gitattributes: Document the unified "auto" handling tboegi
2 siblings, 0 replies; 45+ messages in thread
From: tboegi @ 2016-08-26 20:18 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
Changes since v1:
- 1/2 is left unchanged
- 2/2 is re-written and should be more consistant to read.
Torsten Bögershausen (2):
git ls-files: text=auto eol=lf is supported in Git 2.10
gitattributes: Document the unified "auto" handling
Documentation/git-ls-files.txt | 3 +--
Documentation/gitattributes.txt | 58 +++++++++++++++++------------------------
2 files changed, 25 insertions(+), 36 deletions(-)
--
2.9.0.243.g5c589a7
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10
2016-08-25 20:46 ` Junio C Hamano
2016-08-26 20:18 ` [PATCH v2 0/2] Adjust the documentation to " tboegi
@ 2016-08-26 20:18 ` tboegi
2016-08-26 20:18 ` [PATCH v2 2/2] gitattributes: Document the unified "auto" handling tboegi
2 siblings, 0 replies; 45+ messages in thread
From: tboegi @ 2016-08-26 20:18 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
The man page for `git ls-files --eol` mentions the combination
of text attributes "text=auto eol=lf" or "text=auto eol=crlf" as not
supported yet, but may be in the future.
Now they are supported
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Documentation/git-ls-files.txt | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 078b556..0d933ac 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -159,8 +159,7 @@ not accessible in the working tree.
+
<eolattr> is the attribute that is used when checking out or committing,
it is either "", "-text", "text", "text=auto", "text eol=lf", "text eol=crlf".
-Note: Currently Git does not support "text=auto eol=lf" or "text=auto eol=crlf",
-that may change in the future.
+Since Git 2.10 "text=auto eol=lf" and "text=auto eol=crlf" are supported.
+
Both the <eolinfo> in the index ("i/<eolinfo>")
and in the working tree ("w/<eolinfo>") are shown for regular files,
--
2.9.0.243.g5c589a7
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v2 2/2] gitattributes: Document the unified "auto" handling
2016-08-25 20:46 ` Junio C Hamano
2016-08-26 20:18 ` [PATCH v2 0/2] Adjust the documentation to " tboegi
2016-08-26 20:18 ` [PATCH v2 1/2] git ls-files: text=auto eol=lf is supported in Git 2.10 tboegi
@ 2016-08-26 20:18 ` tboegi
2016-08-26 20:53 ` Junio C Hamano
2 siblings, 1 reply; 45+ messages in thread
From: tboegi @ 2016-08-26 20:18 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
From: Torsten Bögershausen <tboegi@web.de>
Update the documentation about text=auto:
text=auto now follows the core.autocrlf handling when files are not
normalized in the repository.
For a cross platform project recommend the usage of attributes for
line-ending conversions.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Documentation/gitattributes.txt | 58 +++++++++++++++++------------------------
1 file changed, 24 insertions(+), 34 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 807577a..7aff940 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -182,23 +182,6 @@ While Git normally leaves file contents alone, it can be configured to
normalize line endings to LF in the repository and, optionally, to
convert them to CRLF when files are checked out.
-Here is an example that will make Git normalize .txt, .vcproj and .sh
-files, ensure that .vcproj files have CRLF and .sh files have LF in
-the working directory, and prevent .jpg files from being normalized
-regardless of their content.
-
-------------------------
-* text=auto
-*.txt text
-*.vcproj text eol=crlf
-*.sh text eol=lf
-*.jpg -text
-------------------------
-
-Other source code management systems normalize all text files in their
-repositories, and there are two ways to enable similar automatic
-normalization in Git.
-
If you simply want to have CRLF line endings in your working directory
regardless of the repository you are working with, you can set the
config variable "core.autocrlf" without using any attributes.
@@ -208,35 +191,42 @@ config variable "core.autocrlf" without using any attributes.
autocrlf = true
------------------------
-This does not force normalization of all text files, but does ensure
+This does not force normalization of text files, but does ensure
that text files that you introduce to the repository have their line
endings normalized to LF when they are added, and that files that are
already normalized in the repository stay normalized.
-If you want to interoperate with a source code management system that
-enforces end-of-line normalization, or you simply want all text files
-in your repository to be normalized, you should instead set the `text`
-attribute to "auto" for _all_ files.
+If you want to ensure that text files that any contributor introduces to
+the repository have their line endings normalized, you can set the
+`text` attribute to "auto" for _all_ files.
------------------------
* text=auto
------------------------
-This ensures that all files that Git considers to be text will have
-normalized (LF) line endings in the repository. The `core.eol`
-configuration variable controls which line endings Git will use for
-normalized files in your working directory; the default is to use the
-native line ending for your platform, or CRLF if `core.autocrlf` is
-set.
+The attributes allow a fine-grained control, how the line endings
+are converted.
+Here is an example that will make Git normalize .txt, .vcproj and .sh
+files, ensure that .vcproj files have CRLF and .sh files have LF in
+the working directory, and prevent .jpg files from being normalized
+regardless of their content.
+
+------------------------
+* text=auto
+*.txt text
+*.vcproj text eol=crlf
+*.sh text eol=lf
+*.jpg -text
+------------------------
+
+NOTE: When `text=auto` conversion is enabled in a cross-platform
+project using push and pull to a central repository the text files
+containing CRLFs should be normalized.
-NOTE: When `text=auto` normalization is enabled in an existing
-repository, any text files containing CRLFs should be normalized. If
-they are not they will be normalized the next time someone tries to
-change them, causing unfortunate misattribution. From a clean working
-directory:
+From a clean working directory:
-------------------------------------------------
-$ echo "* text=auto" >>.gitattributes
+$ echo "* text=auto" >.gitattributes
$ rm .git/index # Remove the index to force Git to
$ git reset # re-scan the working directory
$ git status # Show files that will be normalized
--
2.9.0.243.g5c589a7
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH v2 2/2] gitattributes: Document the unified "auto" handling
2016-08-26 20:18 ` [PATCH v2 2/2] gitattributes: Document the unified "auto" handling tboegi
@ 2016-08-26 20:53 ` Junio C Hamano
0 siblings, 0 replies; 45+ messages in thread
From: Junio C Hamano @ 2016-08-26 20:53 UTC (permalink / raw)
To: tboegi; +Cc: git
tboegi@web.de writes:
> From: Torsten Bögershausen <tboegi@web.de>
>
> Update the documentation about text=auto:
> text=auto now follows the core.autocrlf handling when files are not
> normalized in the repository.
>
> For a cross platform project recommend the usage of attributes for
> line-ending conversions.
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> Documentation/gitattributes.txt | 58 +++++++++++++++++------------------------
> 1 file changed, 24 insertions(+), 34 deletions(-)
Looks good. Let me merge this to 'master' before cutting -rc2.
Thanks.
^ permalink raw reply [flat|nested] 45+ messages in thread