threads / patch / 37402

v2, 3 partsPush the NATIVE_CRLF Makefile variable to C and added a test for native.

Subject: [PATCH v2 1/3] Push the NATIVE_CRLF Makefile variable to C and added a test for native.

## tl;dr

4 messages between Aug 22, 2014 and Aug 25, 2014. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Torsten Bögershausen· Aug 22, 2014, 19:18 UTC · lore

Commit 95f31e9a correctly points out that the NATIVE_CRLF setting is incorrectly set on Mingw git. However, the Makefile variable is not propagated to the C preprocessor and results in no change. This patch pushes the definition to the C code and adds a test to validate that when core.eol as native is crlf, we actually normalize text files to this line ending convention when core.autocrlf is false.

Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---

This mini series mainly updates git.git with patches from msysgit: Patch 1 is taken "as is", Patch 2 is taken "as is", and Patch 3 is the outcome of the code-review

Thanks for careful reading
 Makefile              |  3 +++
 t/t0026-eol-config.sh | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)
Show changes to 2 files +21 −0

Makefile, t/t0026-eol-config.sh

diff --git a/Makefile b/Makefile
index 2320de5..517036e 100644
--- a/Makefile
+++ b/Makefile
@@ -1479,6 +1479,9 @@ ifdef NO_REGEX
 	COMPAT_CFLAGS += -Icompat/regex
 	COMPAT_OBJS += compat/regex/regex.o
 endif
+ifdef NATIVE_CRLF
+	BASIC_CFLAGS += -DNATIVE_CRLF
+endif
 
 ifdef USE_NED_ALLOCATOR
        COMPAT_CFLAGS += -Icompat/nedmalloc
diff --git a/t/t0026-eol-config.sh b/t/t0026-eol-config.sh
index 4807b0f..43a580a 100755
--- a/t/t0026-eol-config.sh
+++ b/t/t0026-eol-config.sh
@@ -80,4 +80,22 @@ test_expect_success 'autocrlf=true overrides unset eol' '
 	test -z "$onediff" && test -z "$twodiff"
 '
 
+test_expect_success NATIVE_CRLF 'eol native is crlf' '
+
+	rm -rf native_eol && mkdir native_eol &&
+	( cd native_eol &&
+	printf "*.txt text\n" > .gitattributes
+	printf "one\r\ntwo\r\nthree\r\n" > filedos.txt
+	printf "one\ntwo\nthree\n" > fileunix.txt
+	git init &&
+	git config core.autocrlf false &&
+	git config core.eol native &&
+	git add filedos.txt fileunix.txt &&
+	git commit -m "first" &&
+	rm file*.txt &&
+	git reset --hard HEAD &&
+	has_cr filedos.txt && has_cr fileunix.txt
+	)
+'
+
 test_done
-- 
2.1.0.rc2.210.g636bceb
Junio C Hamano· Aug 22, 2014, 22:54 UTC · re: Torsten Bögershausen · lore

Re: [PATCH v2 1/3] Push the NATIVE_CRLF Makefile variable to C and added a test for native.

Torsten Bögershausen <tboegi@web.de> writes:
Show 11 quoted lines
> Commit 95f31e9a correctly points out that the NATIVE_CRLF setting is
> incorrectly set on Mingw git. However, the Makefile variable is not
> propagated to the C preprocessor and results in no change. This patch
> pushes the definition to the C code and adds a test to validate that
> when core.eol as native is crlf, we actually normalize text files to this
> line ending convention when core.autocrlf is false.
>
> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
> Signed-off-by: Stepan Kasal <kasal@ucw.cz>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
Who should I record as the author of this patch?
Show 42 quoted lines
>
> This mini series mainly updates git.git with patches from msysgit:
> Patch 1 is taken "as is",
> Patch 2 is taken "as is",
> and Patch 3 is the outcome of the code-review 
>
> Thanks for careful reading
>
>
>  Makefile              |  3 +++
>  t/t0026-eol-config.sh | 18 ++++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 2320de5..517036e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1479,6 +1479,9 @@ ifdef NO_REGEX
>  	COMPAT_CFLAGS += -Icompat/regex
>  	COMPAT_OBJS += compat/regex/regex.o
>  endif
> +ifdef NATIVE_CRLF
> +	BASIC_CFLAGS += -DNATIVE_CRLF
> +endif
>  
>  ifdef USE_NED_ALLOCATOR
>         COMPAT_CFLAGS += -Icompat/nedmalloc
> diff --git a/t/t0026-eol-config.sh b/t/t0026-eol-config.sh
> index 4807b0f..43a580a 100755
> --- a/t/t0026-eol-config.sh
> +++ b/t/t0026-eol-config.sh
> @@ -80,4 +80,22 @@ test_expect_success 'autocrlf=true overrides unset eol' '
>  	test -z "$onediff" && test -z "$twodiff"
>  '
>  
> +test_expect_success NATIVE_CRLF 'eol native is crlf' '
> +
> +	rm -rf native_eol && mkdir native_eol &&
> +	( cd native_eol &&
> +	printf "*.txt text\n" > .gitattributes
> +	printf "one\r\ntwo\r\nthree\r\n" > filedos.txt
> +	printf "one\ntwo\nthree\n" > fileunix.txt

I think I saw a few style fixes sent against the previous round of patches; have you missed them?

Show 12 quoted lines
> +	git init &&
> +	git config core.autocrlf false &&
> +	git config core.eol native &&
> +	git add filedos.txt fileunix.txt &&
> +	git commit -m "first" &&
> +	rm file*.txt &&
> +	git reset --hard HEAD &&
> +	has_cr filedos.txt && has_cr fileunix.txt
> +	)
> +'
> +
>  test_done
Torsten Bögershausen· Aug 23, 2014, 06:22 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 1/3] Push the NATIVE_CRLF Makefile variable to C and added a test for native.

On 2014-08-23 00.54, Junio C Hamano wrote:
Show 16 quoted lines
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> Commit 95f31e9a correctly points out that the NATIVE_CRLF setting is
>> incorrectly set on Mingw git. However, the Makefile variable is not
>> propagated to the C preprocessor and results in no change. This patch
>> pushes the definition to the C code and adds a test to validate that
>> when core.eol as native is crlf, we actually normalize text files to this
>> line ending convention when core.autocrlf is false.
>>
>> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
>> Signed-off-by: Stepan Kasal <kasal@ucw.cz>
>> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
>> ---
> 
> Who should I record as the author of this patch?
> 
Sorry for missing this, here is what Mingw says: 
commit 0caba2cacbb9d8e6a31783b45f1a13e52dec6ce8
Author: Pat Thoyts <patthoyts@users.sourceforge.net>
Date:   Mon Nov 26 00:24:00 2012 +0000
    Push the NATIVE_CRLF Makefile variable to C and added a test for native.
    
    Commit 95f31e9a correctly points out that the NATIVE_CRLF setting is
[]
Junio C Hamano· Aug 25, 2014, 17:36 UTC · re: Torsten Bögershausen · lore

Re: [PATCH v2 1/3] Push the NATIVE_CRLF Makefile variable to C and added a test for native.

Torsten Bögershausen <tboegi@web.de> writes:
Show 28 quoted lines
> On 2014-08-23 00.54, Junio C Hamano wrote:
>> Torsten Bögershausen <tboegi@web.de> writes:
>> 
>>> Commit 95f31e9a correctly points out that the NATIVE_CRLF setting is
>>> incorrectly set on Mingw git. However, the Makefile variable is not
>>> propagated to the C preprocessor and results in no change. This patch
>>> pushes the definition to the C code and adds a test to validate that
>>> when core.eol as native is crlf, we actually normalize text files to this
>>> line ending convention when core.autocrlf is false.
>>>
>>> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
>>> Signed-off-by: Stepan Kasal <kasal@ucw.cz>
>>> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
>>> ---
>> 
>> Who should I record as the author of this patch?
>> 
>
> Sorry for missing this, here is what Mingw says: 
>
> commit 0caba2cacbb9d8e6a31783b45f1a13e52dec6ce8
> Author: Pat Thoyts <patthoyts@users.sourceforge.net>
> Date:   Mon Nov 26 00:24:00 2012 +0000
>
>     Push the NATIVE_CRLF Makefile variable to C and added a test for native.
>     
>     Commit 95f31e9a correctly points out that the NATIVE_CRLF setting is
> []

When forwarding somebody else's patch, please start the *body* of your message with the in-body header to force the author, followed by a blank line and then the message, i.e.

 	From: Pat Thoyts <patthoyts@users.sourceforge.net>
	Commit 95f31e9a correctly points out that the NATIVE_CRLF setting is
	incorrectly set on Mingw git. However, the Makefile variable is not...
	...

The request applies to other patches in the series as well. I suspect that using send-email on format-patch output may do the right thing automatically.

Thanks.

← back to recent threads