git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH] config.c: fix regression for core.safecrlf false
@ 2018-06-04 20:17 Anthony Sottile
  2018-06-06 15:53 ` Torsten Bögershausen
  2018-06-06 17:15 ` Eric Sunshine
  0 siblings, 2 replies; 9+ messages in thread
From: Anthony Sottile @ 2018-06-04 20:17 UTC (permalink / raw)
  To: git; +Cc: Anthony Sottile

A regression introduced in 8462ff43e42ab67cecd16fdfb59451a53cc8a945 caused
autocrlf rewrites to produce a warning message despite setting safecrlf=false.

Signed-off-by: Anthony Sottile <asottile@umich.edu>
---
 config.c        |  2 +-
 t/t0020-crlf.sh | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index fbbf0f8..de24e90 100644
--- a/config.c
+++ b/config.c
@@ -1233,7 +1233,7 @@ static int git_default_core_config(const char *var, const char *value)
 		}
 		eol_rndtrp_die = git_config_bool(var, value);
 		global_conv_flags_eol = eol_rndtrp_die ?
-			CONV_EOL_RNDTRP_DIE : CONV_EOL_RNDTRP_WARN;
+			CONV_EOL_RNDTRP_DIE : 0;
 		return 0;
 	}
 
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 71350e0..5f05698 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -98,6 +98,16 @@ test_expect_success 'safecrlf: git diff demotes safecrlf=true to warn' '
 '
 
 
+test_expect_success 'safecrlf: no warning with safecrlf=false' '
+	git config core.autocrlf input &&
+	git config core.safecrlf false &&
+
+	for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
+	git add allcrlf 2>err &&
+	test_must_be_empty err
+'
+
+
 test_expect_success 'switch off autocrlf, safecrlf, reset HEAD' '
 	git config core.autocrlf false &&
 	git config core.safecrlf false &&
-- 
2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] config.c: fix regression for core.safecrlf false
  2018-06-04 20:17 [PATCH] config.c: fix regression for core.safecrlf false Anthony Sottile
@ 2018-06-06 15:53 ` Torsten Bögershausen
  2018-06-06 17:15 ` Eric Sunshine
  1 sibling, 0 replies; 9+ messages in thread
From: Torsten Bögershausen @ 2018-06-06 15:53 UTC (permalink / raw)
  To: Anthony Sottile; +Cc: git

On Mon, Jun 04, 2018 at 01:17:42PM -0700, Anthony Sottile wrote:
> A regression introduced in 8462ff43e42ab67cecd16fdfb59451a53cc8a945 caused
> autocrlf rewrites to produce a warning message despite setting safecrlf=false.
> 
> Signed-off-by: Anthony Sottile <asottile@umich.edu>
> ---
>  config.c        |  2 +-
>  t/t0020-crlf.sh | 10 ++++++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/config.c b/config.c
> index fbbf0f8..de24e90 100644
> --- a/config.c
> +++ b/config.c
> @@ -1233,7 +1233,7 @@ static int git_default_core_config(const char *var, const char *value)
>  		}
>  		eol_rndtrp_die = git_config_bool(var, value);
>  		global_conv_flags_eol = eol_rndtrp_die ?
> -			CONV_EOL_RNDTRP_DIE : CONV_EOL_RNDTRP_WARN;
> +			CONV_EOL_RNDTRP_DIE : 0;
>  		return 0;
>  	}
>  
> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
> index 71350e0..5f05698 100755
> --- a/t/t0020-crlf.sh
> +++ b/t/t0020-crlf.sh
> @@ -98,6 +98,16 @@ test_expect_success 'safecrlf: git diff demotes safecrlf=true to warn' '
>  '
>  
>  
> +test_expect_success 'safecrlf: no warning with safecrlf=false' '
> +	git config core.autocrlf input &&
> +	git config core.safecrlf false &&
> +
> +	for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
> +	git add allcrlf 2>err &&
> +	test_must_be_empty err
> +'
> +
> +
>  test_expect_success 'switch off autocrlf, safecrlf, reset HEAD' '
>  	git config core.autocrlf false &&
>  	git config core.safecrlf false &&
> -- 
> 2.7.4
> 

Looks good to me, thanks for cleaning my mess.

Acked-By: Torsten Bögershausen <tboegi@web.de>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] config.c: fix regression for core.safecrlf false
  2018-06-04 20:17 [PATCH] config.c: fix regression for core.safecrlf false Anthony Sottile
  2018-06-06 15:53 ` Torsten Bögershausen
@ 2018-06-06 17:15 ` Eric Sunshine
  2018-06-06 17:17   ` Eric Sunshine
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Sunshine @ 2018-06-06 17:15 UTC (permalink / raw)
  To: Anthony Sottile; +Cc: Git List

On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile <asottile@umich.edu> wrote:
> A regression introduced in 8462ff43e42ab67cecd16fdfb59451a53cc8a945 caused
> autocrlf rewrites to produce a warning message despite setting safecrlf=false.
>
> Signed-off-by: Anthony Sottile <asottile@umich.edu>
> ---
> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
> @@ -98,6 +98,16 @@ test_expect_success 'safecrlf: git diff demotes safecrlf=true to warn' '
> +test_expect_success 'safecrlf: no warning with safecrlf=false' '
> +       git config core.autocrlf input &&
> +       git config core.safecrlf false &&

I was going to suggest test_config() for these rather than bare
git-config, but I see other tests in this file already use the bare
form, so this is following existing practice.

> +       for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&

Simpler: printf "%s\n" I am all CRLF | append_cr >allcrlf &&

(probably not worth a re-roll)

> +       git add allcrlf 2>err &&
> +       test_must_be_empty err
> +'

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] config.c: fix regression for core.safecrlf false
  2018-06-06 17:15 ` Eric Sunshine
@ 2018-06-06 17:17   ` Eric Sunshine
  2018-06-06 17:18     ` Anthony Sottile
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Sunshine @ 2018-06-06 17:17 UTC (permalink / raw)
  To: Anthony Sottile; +Cc: Git List

On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile <asottile@umich.edu> wrote:
>> +       for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
>
> Simpler: printf "%s\n" I am all CRLF | append_cr >allcrlf &&

Or even simpler:

printf "%s\r\n" I am all CRLF >allcrlf &&

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] config.c: fix regression for core.safecrlf false
  2018-06-06 17:17   ` Eric Sunshine
@ 2018-06-06 17:18     ` Anthony Sottile
  2018-06-06 17:22       ` Eric Sunshine
  0 siblings, 1 reply; 9+ messages in thread
From: Anthony Sottile @ 2018-06-06 17:18 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List

On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile <asottile@umich.edu> wrote:
>>> +       for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
>>
>> Simpler: printf "%s\n" I am all CRLF | append_cr >allcrlf &&
>
> Or even simpler:
>
> printf "%s\r\n" I am all CRLF >allcrlf &&

Yeah, I just copied the line in my test from another test in this file
which was doing a ~similar thing.  My original bug report actually
uses `echo -en ...` to accomplish the same thing.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] config.c: fix regression for core.safecrlf false
  2018-06-06 17:18     ` Anthony Sottile
@ 2018-06-06 17:22       ` Eric Sunshine
  2018-06-12  1:46         ` Anthony Sottile
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Sunshine @ 2018-06-06 17:22 UTC (permalink / raw)
  To: Anthony Sottile; +Cc: Git List

On Wed, Jun 6, 2018 at 1:18 PM, Anthony Sottile <asottile@umich.edu> wrote:
> On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile <asottile@umich.edu> wrote:
>>>> +       for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
>>>
>>> Simpler: printf "%s\n" I am all CRLF | append_cr >allcrlf &&
>>
>> Or even simpler:
>>
>> printf "%s\r\n" I am all CRLF >allcrlf &&
>
> Yeah, I just copied the line in my test from another test in this file
> which was doing a ~similar thing. [...]

Thanks for pointing that out. In that case, it's following existing
practice, thus certainly not worth a re-roll.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] config.c: fix regression for core.safecrlf false
  2018-06-06 17:22       ` Eric Sunshine
@ 2018-06-12  1:46         ` Anthony Sottile
  2018-06-12  4:34           ` Eric Sunshine
  2018-06-13  1:19           ` Torsten Bögershausen
  0 siblings, 2 replies; 9+ messages in thread
From: Anthony Sottile @ 2018-06-12  1:46 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List

On Wed, Jun 6, 2018 at 10:22 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Wed, Jun 6, 2018 at 1:18 PM, Anthony Sottile <asottile@umich.edu> wrote:
>> On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile <asottile@umich.edu> wrote:
>>>>> +       for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
>>>>
>>>> Simpler: printf "%s\n" I am all CRLF | append_cr >allcrlf &&
>>>
>>> Or even simpler:
>>>
>>> printf "%s\r\n" I am all CRLF >allcrlf &&
>>
>> Yeah, I just copied the line in my test from another test in this file
>> which was doing a ~similar thing. [...]
>
> Thanks for pointing that out. In that case, it's following existing
> practice, thus certainly not worth a re-roll.

Anything else for me to do here? (sorry! not super familiar with the process)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] config.c: fix regression for core.safecrlf false
  2018-06-12  1:46         ` Anthony Sottile
@ 2018-06-12  4:34           ` Eric Sunshine
  2018-06-13  1:19           ` Torsten Bögershausen
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Sunshine @ 2018-06-12  4:34 UTC (permalink / raw)
  To: Anthony Sottile; +Cc: Git List, Torsten Bögershausen

[cc:+torsten]

On Mon, Jun 11, 2018 at 9:46 PM, Anthony Sottile <asottile@umich.edu> wrote:
> On Wed, Jun 6, 2018 at 10:22 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> Thanks for pointing that out. In that case, it's following existing
>> practice, thus certainly not worth a re-roll.
>
> Anything else for me to do here? (sorry! not super familiar with the process)

I don't think so. Nothing in the review warranted a re-roll, and
(importantly) Torsten gave his Acked-by:, so the next step is to wait
for Junio to pick up the patch. He's been offline for a bit, so it
might take a some time for him to catch up since the list has been
plenty busy in his absence.

It's a good idea to check the "What's Cooking" summaries Junio sends
to the mailing list once in a while to check the progress of your
patch. If you don't see it show up in the summary within a couple
weeks or so, it wouldn't hurt to ping again (as you did here).

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] config.c: fix regression for core.safecrlf false
  2018-06-12  1:46         ` Anthony Sottile
  2018-06-12  4:34           ` Eric Sunshine
@ 2018-06-13  1:19           ` Torsten Bögershausen
  1 sibling, 0 replies; 9+ messages in thread
From: Torsten Bögershausen @ 2018-06-13  1:19 UTC (permalink / raw)
  To: Anthony Sottile; +Cc: Eric Sunshine, Git List

On Mon, Jun 11, 2018 at 06:46:34PM -0700, Anthony Sottile wrote:
[]
> Anything else for me to do here? (sorry! not super familiar with the process)

Your patch has been picked up by Junio, and is currently merged into the
"pu" branch (proposed updates):

  commit bc8ff8aec33836af3fefe1bcd3f533a1486b793f
  Merge: e69b544a38 6cb09125be
  Author: Junio C Hamano <gitster@pobox.com>
  Date:   Tue Jun 12 10:15:13 2018 -0700

      Merge branch 'as/safecrlf-quiet-fix' into jch
      
      Fix for 2.17-era regression.
      
      * as/safecrlf-quiet-fix:
        config.c: fix regression for core.safecrlf false

From there, it will typically progress into next and master,
unless reviewers come with comments and improvements.
You can watch out for "What's cooking in git" messages here on the list
to follow the progress.

From my experience it will end up in a Git release, but I don't know,
if it will be 2.18 or a later one.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-04 20:17 [PATCH] config.c: fix regression for core.safecrlf false Anthony Sottile
2018-06-06 15:53 ` Torsten Bögershausen
2018-06-06 17:15 ` Eric Sunshine
2018-06-06 17:17   ` Eric Sunshine
2018-06-06 17:18     ` Anthony Sottile
2018-06-06 17:22       ` Eric Sunshine
2018-06-12  1:46         ` Anthony Sottile
2018-06-12  4:34           ` Eric Sunshine
2018-06-13  1:19           ` Torsten Bögershausen

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox