Re: [PATCH] config.c: fix regression for core.safecrlf false
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 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.
Re: [PATCH] config.c: fix regression for core.safecrlf false
[cc:+torsten] On Mon, Jun 11, 2018 at 9:46 PM, Anthony Sottile wrote: > On Wed, Jun 6, 2018 at 10:22 AM, Eric Sunshine > 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).
Re: [PATCH] config.c: fix regression for core.safecrlf false
On Wed, Jun 6, 2018 at 10:22 AM, Eric Sunshine wrote: > On Wed, Jun 6, 2018 at 1:18 PM, Anthony Sottile wrote: >> On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine >> wrote: >>> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine >>> wrote: On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile 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)
Re: [PATCH] config.c: fix regression for core.safecrlf false
On Wed, Jun 6, 2018 at 1:18 PM, Anthony Sottile wrote: > On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine > wrote: >> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine >> wrote: >>> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile 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.
Re: [PATCH] config.c: fix regression for core.safecrlf false
On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine wrote: > On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine wrote: >> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile 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.
Re: [PATCH] config.c: fix regression for core.safecrlf false
On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine wrote: > On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile 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 &&
Re: [PATCH] config.c: fix regression for core.safecrlf false
On Mon, Jun 4, 2018 at 4:17 PM, 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 > --- > 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 > +'
Re: [PATCH] config.c: fix regression for core.safecrlf false
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 > --- > 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
[PATCH] config.c: fix regression for core.safecrlf false
A regression introduced in 8462ff43e42ab67cecd16fdfb59451a53cc8a945 caused autocrlf rewrites to produce a warning message despite setting safecrlf=false. Signed-off-by: Anthony Sottile --- 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
