{"thread":{"id":"48649","subject":"[PATCH] config.c: fix regression for core.safecrlf false","startedAt":"2018-06-04T20:33:26Z","lastAt":"2018-06-13T01:16:10Z","messageCount":9,"participants":["Anthony Sottile","Torsten Bögershausen","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"349297","messageId":"20180604201742.18992-1-asottile@umich.edu","threadId":"48649","inReplyTo":null,"subject":"[PATCH] config.c: fix regression for core.safecrlf false","fromName":"Anthony Sottile","fromEmail":"asottile@umich.edu","sentAt":"2018-06-04T20:17:42Z","receivedAt":"2018-06-04T20:33:26Z","isPatch":true,"sender":{"key":"asottile@umich.edu","avatar":"https://avatars.githubusercontent.com/u/1810591?v=4"},"body":"A regression introduced in 8462ff43e42ab67cecd16fdfb59451a53cc8a945 caused\nautocrlf rewrites to produce a warning message despite setting safecrlf=false.\n\nSigned-off-by: Anthony Sottile \n---\n config.c | 2 +-\n t/t0020-crlf.sh | 10 ++++++++++\n 2 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/config.c b/config.c\nindex fbbf0f8..de24e90 100644\n--- a/config.c\n+++ b/config.c\n@@ -1233,7 +1233,7 @@ static int git_default_core_config(const char *var, const char *value)\n \t\t}\n \t\teol_rndtrp_die = git_config_bool(var, value);\n \t\tglobal_conv_flags_eol = eol_rndtrp_die ?\n-\t\t\tCONV_EOL_RNDTRP_DIE : CONV_EOL_RNDTRP_WARN;\n+\t\t\tCONV_EOL_RNDTRP_DIE : 0;\n \t\treturn 0;\n \t}\n \ndiff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\nindex 71350e0..5f05698 100755\n--- a/t/t0020-crlf.sh\n+++ b/t/t0020-crlf.sh\n@@ -98,6 +98,16 @@ test_expect_success 'safecrlf: git diff demotes safecrlf=true to warn' '\n '\n \n \n+test_expect_success 'safecrlf: no warning with safecrlf=false' '\n+\tgit config core.autocrlf input &&\n+\tgit config core.safecrlf false &&\n+\n+\tfor w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&\n+\tgit add allcrlf 2>err &&\n+\ttest_must_be_empty err\n+'\n+\n+\n test_expect_success 'switch off autocrlf, safecrlf, reset HEAD' '\n \tgit config core.autocrlf false &&\n \tgit config core.safecrlf false &&\n-- \n2.7.4\n\n"},{"id":"349505","messageId":"20180606155309.GA5624@atze2.lan","threadId":"48649","inReplyTo":"20180604201742.18992-1-asottile@umich.edu","subject":"Re: [PATCH] config.c: fix regression for core.safecrlf false","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-06-06T15:53:09Z","receivedAt":"2018-06-06T15:49:30Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Mon, Jun 04, 2018 at 01:17:42PM -0700, Anthony Sottile wrote:\n> A regression introduced in 8462ff43e42ab67cecd16fdfb59451a53cc8a945 caused\n> autocrlf rewrites to produce a warning message despite setting safecrlf=false.\n> \n> Signed-off-by: Anthony Sottile \n> ---\n> config.c | 2 +-\n> t/t0020-crlf.sh | 10 ++++++++++\n> 2 files changed, 11 insertions(+), 1 deletion(-)\n> \n> diff --git a/config.c b/config.c\n> index fbbf0f8..de24e90 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1233,7 +1233,7 @@ static int git_default_core_config(const char *var, const char *value)\n> \t\t}\n> \t\teol_rndtrp_die = git_config_bool(var, value);\n> \t\tglobal_conv_flags_eol = eol_rndtrp_die ?\n> -\t\t\tCONV_EOL_RNDTRP_DIE : CONV_EOL_RNDTRP_WARN;\n> +\t\t\tCONV_EOL_RNDTRP_DIE : 0;\n> \t\treturn 0;\n> \t}\n> \n> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\n> index 71350e0..5f05698 100755\n> --- a/t/t0020-crlf.sh\n> +++ b/t/t0020-crlf.sh\n> @@ -98,6 +98,16 @@ test_expect_success 'safecrlf: git diff demotes safecrlf=true to warn' '\n> '\n> \n> \n> +test_expect_success 'safecrlf: no warning with safecrlf=false' '\n> +\tgit config core.autocrlf input &&\n> +\tgit config core.safecrlf false &&\n> +\n> +\tfor w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&\n> +\tgit add allcrlf 2>err &&\n> +\ttest_must_be_empty err\n> +'\n> +\n> +\n> test_expect_success 'switch off autocrlf, safecrlf, reset HEAD' '\n> \tgit config core.autocrlf false &&\n> \tgit config core.safecrlf false &&\n> -- \n> 2.7.4\n> \n\nLooks good to me, thanks for cleaning my mess.\n\nAcked-By: Torsten Bögershausen \n"},{"id":"349537","messageId":"CAPig+cSzJ=2Zz7jRNB7sK7FyZ+YwdAFseCTSDbM_m4E8K9WxHA@mail.gmail.com","threadId":"48649","inReplyTo":"20180604201742.18992-1-asottile@umich.edu","subject":"Re: [PATCH] config.c: fix regression for core.safecrlf false","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-06T17:15:54Z","receivedAt":"2018-06-06T17:15:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile wrote:\n> A regression introduced in 8462ff43e42ab67cecd16fdfb59451a53cc8a945 caused\n> autocrlf rewrites to produce a warning message despite setting safecrlf=false.\n>\n> Signed-off-by: Anthony Sottile \n> ---\n> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\n> @@ -98,6 +98,16 @@ test_expect_success 'safecrlf: git diff demotes safecrlf=true to warn' '\n> +test_expect_success 'safecrlf: no warning with safecrlf=false' '\n> + git config core.autocrlf input &&\n> + git config core.safecrlf false &&\n\nI was going to suggest test_config() for these rather than bare\ngit-config, but I see other tests in this file already use the bare\nform, so this is following existing practice.\n\n> + for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&\n\nSimpler: printf \"%s\\n\" I am all CRLF | append_cr >allcrlf &&\n\n(probably not worth a re-roll)\n\n> + git add allcrlf 2>err &&\n> + test_must_be_empty err\n> +'\n"},{"id":"349538","messageId":"CAPig+cRyv=JuGo+OfULuvbLrqRxoYZyBZDrSJrt5F8YRwzNn6w@mail.gmail.com","threadId":"48649","inReplyTo":"CAPig+cSzJ=2Zz7jRNB7sK7FyZ+YwdAFseCTSDbM_m4E8K9WxHA@mail.gmail.com","subject":"Re: [PATCH] config.c: fix regression for core.safecrlf false","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-06T17:17:30Z","receivedAt":"2018-06-06T17:17:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine wrote:\n> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile wrote:\n>> + for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&\n>\n> Simpler: printf \"%s\\n\" I am all CRLF | append_cr >allcrlf &&\n\nOr even simpler:\n\nprintf \"%s\\r\\n\" I am all CRLF >allcrlf &&\n"},{"id":"349539","messageId":"CA+dzEB=7tGeXduxdKrJpDpXrmNbb_ZnYg=CmByJ7J-w-iiyxsQ@mail.gmail.com","threadId":"48649","inReplyTo":"CAPig+cRyv=JuGo+OfULuvbLrqRxoYZyBZDrSJrt5F8YRwzNn6w@mail.gmail.com","subject":"Re: [PATCH] config.c: fix regression for core.safecrlf false","fromName":"Anthony Sottile","fromEmail":"asottile@umich.edu","sentAt":"2018-06-06T17:18:54Z","receivedAt":"2018-06-06T17:18:58Z","isPatch":true,"sender":{"key":"asottile@umich.edu","avatar":"https://avatars.githubusercontent.com/u/1810591?v=4"},"body":"On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine wrote:\n> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine wrote:\n>> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile wrote:\n>>> + for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&\n>>\n>> Simpler: printf \"%s\\n\" I am all CRLF | append_cr >allcrlf &&\n>\n> Or even simpler:\n>\n> printf \"%s\\r\\n\" I am all CRLF >allcrlf &&\n\nYeah, I just copied the line in my test from another test in this file\nwhich was doing a ~similar thing. My original bug report actually\nuses `echo -en ...` to accomplish the same thing.\n"},{"id":"349540","messageId":"CAPig+cSm7My9r8KN1vNyssendf_v_nMARDAq6ALA=X7nZ+spkA@mail.gmail.com","threadId":"48649","inReplyTo":"CA+dzEB=7tGeXduxdKrJpDpXrmNbb_ZnYg=CmByJ7J-w-iiyxsQ@mail.gmail.com","subject":"Re: [PATCH] config.c: fix regression for core.safecrlf false","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-06T17:22:22Z","receivedAt":"2018-06-06T17:22:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jun 6, 2018 at 1:18 PM, Anthony Sottile wrote:\n> On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine wrote:\n>> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine wrote:\n>>> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile wrote:\n>>>> + for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&\n>>>\n>>> Simpler: printf \"%s\\n\" I am all CRLF | append_cr >allcrlf &&\n>>\n>> Or even simpler:\n>>\n>> printf \"%s\\r\\n\" I am all CRLF >allcrlf &&\n>\n> Yeah, I just copied the line in my test from another test in this file\n> which was doing a ~similar thing. [...]\n\nThanks for pointing that out. In that case, it's following existing\npractice, thus certainly not worth a re-roll.\n"},{"id":"349965","messageId":"CA+dzEBk8H_=a9k1DaFUK=JJBhd17bhS3+ngSUBcBV+7hD-RFMw@mail.gmail.com","threadId":"48649","inReplyTo":"CAPig+cSm7My9r8KN1vNyssendf_v_nMARDAq6ALA=X7nZ+spkA@mail.gmail.com","subject":"Re: [PATCH] config.c: fix regression for core.safecrlf false","fromName":"Anthony Sottile","fromEmail":"asottile@umich.edu","sentAt":"2018-06-12T01:46:34Z","receivedAt":"2018-06-12T01:46:39Z","isPatch":true,"sender":{"key":"asottile@umich.edu","avatar":"https://avatars.githubusercontent.com/u/1810591?v=4"},"body":"On Wed, Jun 6, 2018 at 10:22 AM, Eric Sunshine wrote:\n> On Wed, Jun 6, 2018 at 1:18 PM, Anthony Sottile wrote:\n>> On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine wrote:\n>>> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine wrote:\n>>>> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile wrote:\n>>>>> + for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&\n>>>>\n>>>> Simpler: printf \"%s\\n\" I am all CRLF | append_cr >allcrlf &&\n>>>\n>>> Or even simpler:\n>>>\n>>> printf \"%s\\r\\n\" I am all CRLF >allcrlf &&\n>>\n>> Yeah, I just copied the line in my test from another test in this file\n>> which was doing a ~similar thing. [...]\n>\n> Thanks for pointing that out. In that case, it's following existing\n> practice, thus certainly not worth a re-roll.\n\nAnything else for me to do here? (sorry! not super familiar with the process)\n"},{"id":"349968","messageId":"CAPig+cTgJbwatnZ+9eAvjQ1Mn7dszWr9Tgh5GTJNAH-jNjV=Cg@mail.gmail.com","threadId":"48649","inReplyTo":"CA+dzEBk8H_=a9k1DaFUK=JJBhd17bhS3+ngSUBcBV+7hD-RFMw@mail.gmail.com","subject":"Re: [PATCH] config.c: fix regression for core.safecrlf false","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-12T04:34:03Z","receivedAt":"2018-06-12T04:34:07Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"[cc:+torsten]\n\nOn Mon, Jun 11, 2018 at 9:46 PM, Anthony Sottile wrote:\n> On Wed, Jun 6, 2018 at 10:22 AM, Eric Sunshine wrote:\n>> Thanks for pointing that out. In that case, it's following existing\n>> practice, thus certainly not worth a re-roll.\n>\n> Anything else for me to do here? (sorry! not super familiar with the process)\n\nI don't think so. Nothing in the review warranted a re-roll, and\n(importantly) Torsten gave his Acked-by:, so the next step is to wait\nfor Junio to pick up the patch. He's been offline for a bit, so it\nmight take a some time for him to catch up since the list has been\nplenty busy in his absence.\n\nIt's a good idea to check the \"What's Cooking\" summaries Junio sends\nto the mailing list once in a while to check the progress of your\npatch. If you don't see it show up in the summary within a couple\nweeks or so, it wouldn't hurt to ping again (as you did here).\n"},{"id":"350038","messageId":"20180613011923.GA5772@atze2.lan","threadId":"48649","inReplyTo":"CA+dzEBk8H_=a9k1DaFUK=JJBhd17bhS3+ngSUBcBV+7hD-RFMw@mail.gmail.com","subject":"Re: [PATCH] config.c: fix regression for core.safecrlf false","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-06-13T01:19:23Z","receivedAt":"2018-06-13T01:16:10Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Mon, Jun 11, 2018 at 06:46:34PM -0700, Anthony Sottile wrote:\n[]\n> Anything else for me to do here? (sorry! not super familiar with the process)\n\nYour patch has been picked up by Junio, and is currently merged into the\n\"pu\" branch (proposed updates):\n\n commit bc8ff8aec33836af3fefe1bcd3f533a1486b793f\n Merge: e69b544a38 6cb09125be\n Author: Junio C Hamano \n Date: Tue Jun 12 10:15:13 2018 -0700\n\n Merge branch 'as/safecrlf-quiet-fix' into jch\n \n Fix for 2.17-era regression.\n \n * as/safecrlf-quiet-fix:\n config.c: fix regression for core.safecrlf false\n\nFrom there, it will typically progress into next and master,\nunless reviewers come with comments and improvements.\nYou can watch out for \"What's cooking in git\" messages here on the list\nto follow the progress.\n\nFrom my experience it will end up in a Git release, but I don't know,\nif it will be 2.18 or a later one.\n"}]}