From: Steffen Prohaska <prohaska@zib.de>
Date: Wed, 23 Jul 2008 05:49:20 GMT
Subject: Re: [PATCH] Respect crlf attribute in "git add" even if core.autocrlf has not been set
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.


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



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