Re: [PATCH] Respect crlf attribute in "git add" even if core.autocrlf has not been set
From: Steffen Prohaska <hidden>
Date: 2016-06-15 22:44:59
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Jul 23, 2008, at 3:31 AM, Johannes Schindelin wrote:
When a file's crlf attribute is explicitely set, it does not make sense to ignore it when adding the file, just because the config variable core.autocrlf has not been set.
Your patch is not about core.autocrlf unset, but about core.autocrlf=false. On Unix, the two cases seem to be identical, but on Windows they are not. (see below).
The alternative would be risking to get CR/LF files into the repository just because one user forgot to set core.autocrlf.
Git could help the user *and* the maintainer of the repository if we chose core.autocrlf=input as the default on Unix. We would never let CR/LF enter the repository unless explicitly requested to do so by core.autocrlf=false. This setting however would not be recommended to the average user. But with Unix' default core.autocrlf=false, it makes sense to let the maintainer of a repository enforce stripping CR from all files if not explicitly configured otherwise for specific paths. Setting "crlf=input" in .gitattribute seems to be the documented way to do so --- although the documentation in gitattributes.txt is a bit complex to read.
quoted hunk ↗ jump to hunk
This patch does not affect the case when the crlf attribute is unset, or when checking files out. Signed-off-by: Johannes Schindelin <redacted> --- Okay, so I lied and did not go to sleep (but that part comes now). Only the wording in the commit message has changed. convert.c | 2 +- t/t0020-crlf.sh | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletions(-)diff --git a/convert.c b/convert.c index 78efed8..d038d2f 100644 --- a/convert.c +++ b/convert.c@@ -126,7 +126,7 @@ static int crlf_to_git(const char *path, constchar *src, size_t len, struct text_stat stats; char *dst; - if ((action == CRLF_BINARY) || !auto_crlf || !len) + if ((action == CRLF_BINARY) || (!auto_crlf && action < 0) || !len)
I think we should strictly follow the documentation, so this should read + if ((action == CRLF_BINARY) || (!auto_crlf && action != CRLF_INPUT) || !len)
quoted hunk ↗ jump to hunk
return 0; gather_stats(src, len, &stats);diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh index 1be7446..0bb3e6f 100755 --- a/t/t0020-crlf.sh +++ b/t/t0020-crlf.sh@@ -436,4 +436,14 @@ test_expect_success 'invalid .gitattributes(must not crash)' ' ' +test_expect_success 'attribute crlf is heeded even without core.autocrlf' ' + + echo "allcrlf crlf=input" > .gitattributes && + git config --unset core.autocrlf &&
You should set core.autocrlf explicitly to false:
git config core.autocrlf false
Otherwise the test would pick up the user's global default.
+ git add allcrlf && + git show :allcrlf | append_cr > expect && + test_cmp allcrlf expect + +' + test_done -- 1.6.0.rc0.22.gf2096d.dirty
... and we should verify that we only treat crlf=input specially, but not "crlf". The following changes could be applied on top of yours. (Apologies if lines are wrapped. I am composing this mail with the wrong email client.)
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index b37059b..e51e810 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh@@ -436,7 +436,7 @@ test_expect_success 'invalid .gitattributes (must not crash)' '
'
-test_expect_success 'attribute crlf is heeded even without
core.autocrlf' '
+test_expect_success 'attribute crlf=input is heeded even with
core.autocrlf=false' '
echo "allcrlf crlf=input" > .gitattributes &&
git config core.autocrlf false &&@@ -446,4 +446,15 @@ test_expect_success 'attribute crlf is heeded even without core.autocrlf' ' ' +test_expect_success 'attribute crlf is ignored with core.autocrlf=false' ' + + echo "allcrlf crlf" > .gitattributes && + git config core.autocrlf false && + git read-tree --reset HEAD && + git add allcrlf && + git show :allcrlf > expect && + test_cmp allcrlf expect + +' + test_done