Re: [PATCH] Respect crlf attribute in "git add" even if core.autocrlf has not been set

From: Steffen Prohaska <prohaska@zib.de>
Date: Jul 23, 2008, 05:49 UTC
Message-ID: <719E03C0-E8C3-4C35-AE9C-9BD5A7BCDF03@zib.de>
In-Reply-To: <alpine.DEB.1.00.0807230229410.8986@racer>

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.

Show 24 quoted lines

> This patch does not affect the case when the crlf attribute is unset,
> or when checking files out.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	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, const  
> char *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)

Show 18 quoted lines

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

Show 10 quoted lines

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