{"thread":{"id":"46293","subject":"Bug with automated processing of git status results","startedAt":"2017-06-30T06:00:22Z","lastAt":"2017-07-05T18:53:40Z","messageCount":9,"participants":["Сергей Шестаков","Konstantin Khomoutov","Matthieu Moy","Torsten Bögershausen","Stefan Beller","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"323547","messageId":"CAM1TZMGXiDpzt3uhpDaE41KR8GWyjMq=+Mcvz5Zrj0EffNrGrw@mail.gmail.com","threadId":"46293","inReplyTo":null,"subject":"Bug with automated processing of git status results","fromName":"Сергей Шестаков","fromEmail":"s_shestakov@playrix.com","sentAt":"2017-06-30T06:00:14Z","receivedAt":"2017-06-30T06:00:22Z","isPatch":false,"sender":{"key":"s_shestakov@playrix.com","avatar":null},"body":"Hi!\n\nI am trying to make an automated processing of \"git status\" results.\nI execute the command\n\ngit status -z -uno\n\nI expect that it has stable output format. However, it still can print\nwarnings like\n\nwarning: CRLF will be replaced by LF in somefile.xml\n\nI understand that we can turn off core.safecrlf, but it's\ninconvinient. It would be better if \"git status\" command had an\noptional parameter that disables any other output besides changed\nfiles.\n\nThanks!\nSergey Shestakov\nPlayrix\n"},{"id":"323549","messageId":"20170630071219.m757vvyccrp3afli@tigra","threadId":"46293","inReplyTo":"CAM1TZMGXiDpzt3uhpDaE41KR8GWyjMq=+Mcvz5Zrj0EffNrGrw@mail.gmail.com","subject":"Re: Bug with automated processing of git status results","fromName":"Konstantin Khomoutov","fromEmail":"kostix+git@007spb.ru","sentAt":"2017-06-30T07:12:19Z","receivedAt":"2017-06-30T07:12:26Z","isPatch":false,"sender":{"key":"kostix+git@007spb.ru","avatar":null},"body":"On Fri, Jun 30, 2017 at 09:00:14AM +0300, Сергей Шестаков wrote:\n\n> I am trying to make an automated processing of \"git status\" results.\n> I execute the command\n> \n> git status -z -uno\n> \n> I expect that it has stable output format. However, it still can print\n> warnings like\n> \n> warning: CRLF will be replaced by LF in somefile.xml\n> \n> I understand that we can turn off core.safecrlf, but it's\n> inconvinient. It would be better if \"git status\" command had an\n> optional parameter that disables any other output besides changed\n> files.\n\nThe `git status` command supposedly writes their \"regular\" data to its\nstandard output while warnings go to its standard error stream.\nIs this not the case?\n\n"},{"id":"323554","messageId":"vpqr2y1u9j2.fsf@anie.imag.fr","threadId":"46293","inReplyTo":"CAM1TZMGXiDpzt3uhpDaE41KR8GWyjMq=+Mcvz5Zrj0EffNrGrw@mail.gmail.com","subject":"Re: Bug with automated processing of git status results","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2017-06-30T09:09:21Z","receivedAt":"2017-06-30T09:09:28Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Сергей Шестаков writes:\n\n> I understand that we can turn off core.safecrlf, but it's\n> inconvinient.\n\nNote that you can do that without actually changing the config file:\n\n git -c core.safecrlf=false status ...\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"323555","messageId":"70c9a162-ac2f-c347-d13b-f24ac24d1133@web.de","threadId":"46293","inReplyTo":"vpqr2y1u9j2.fsf@anie.imag.fr","subject":"Re: Bug with automated processing of git status results","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-06-30T09:22:18Z","receivedAt":"2017-06-30T09:22:29Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\nOn 30/06/17 11:09, Matthieu Moy wrote:\n> Сергей Шестаков writes:\n> \n>> I understand that we can turn off core.safecrlf, but it's\n>> inconvinient.\n> \n> Note that you can do that without actually changing the config file:\n> \n> git -c core.safecrlf=false status ...\n> \nBeside that, I would recommend to set up a .gitattributes file:\n\n$ echo \"*.xml text eol=lf\" >>.gitattributes\n$ git add .gitattributes\n$ git commit -m \"xml files are text with LF line endings\"\n"},{"id":"323573","messageId":"20170630162826.27711-1-sbeller@google.com","threadId":"46293","inReplyTo":"70c9a162-ac2f-c347-d13b-f24ac24d1133@web.de","subject":"[PATCH] status: suppress additional warning output in plumbing modes","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-06-30T16:28:26Z","receivedAt":"2017-06-30T16:28:33Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When status is called with '--porcelain' (as implied by '-z'), we promise\nto output only messages as described in the man page.\n\nSuppress CRLF warnings.\n\nSigned-off-by: Stefan Beller \n---\n\nMaybe something like this?\n\n builtin/commit.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 00a01f07c3..3705d5ec6f 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1126,6 +1126,11 @@ static void finalize_deferred_config(struct wt_status *s)\n \t\t\tdie(_(\"--long and -z are incompatible\"));\n \t}\n \n+\t/* suppress all additional output in porcelain mode */\n+\tif (status_format == STATUS_FORMAT_PORCELAIN ||\n+\t status_format == STATUS_FORMAT_PORCELAIN_V2)\n+\t\tsafe_crlf = SAFE_CRLF_FALSE;\n+\n \tif (use_deferred_config && status_format == STATUS_FORMAT_UNSPECIFIED)\n \t\tstatus_format = status_deferred_config.status_format;\n \tif (status_format == STATUS_FORMAT_UNSPECIFIED)\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323672","messageId":"d401dcc7-cb4c-e895-e52a-cd54c8130a65@web.de","threadId":"46293","inReplyTo":"20170630162826.27711-1-sbeller@google.com","subject":"Re: [PATCH] status: suppress additional warning output in plumbing modes","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-07-01T12:49:29Z","receivedAt":"2017-07-01T12:49:50Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\n >On 30/06/17 18:28, Stefan Beller wrote:\n\nThe patch makes a lot of sense - thanks for the fast reply.\nA question: does the header correspond to the patch ?\n\n< [PATCH] status: suppress additional warning output in plumbing modes\n > [PATCH] status: suppress CRLF warnings in porcelain modes\n\n(And may be the comment in the code:)\n\n< / * suppress all additional output in porcelain mode */\n > / * suppress CRLF conversion warnings in porcelain mode */\n\n> When status is called with '--porcelain' (as implied by '-z'), we promise\n> to output only messages as described in the man page.\n> \n> Suppress CRLF warnings.\n> \n> Signed-off-by: Stefan Beller \n> ---\n> \n> Maybe something like this?\n> \n> builtin/commit.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 00a01f07c3..3705d5ec6f 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1126,6 +1126,11 @@ static void finalize_deferred_config(struct wt_status *s)\n> \t\t\tdie(_(\"--long and -z are incompatible\"));\n> \t}\n> \n> +\t/* suppress all additional output in porcelain mode */\n> +\tif (status_format == STATUS_FORMAT_PORCELAIN ||\n> +\t status_format == STATUS_FORMAT_PORCELAIN_V2)\n> +\t\tsafe_crlf = SAFE_CRLF_FALSE;\n> +\n> \tif (use_deferred_config && status_format == STATUS_FORMAT_UNSPECIFIED)\n> \t\tstatus_format = status_deferred_config.status_format;\n> \tif (status_format == STATUS_FORMAT_UNSPECIFIED)\n> \n"},{"id":"323681","messageId":"87h8ywe03e.fsf@gmail.com","threadId":"46293","inReplyTo":"20170630162826.27711-1-sbeller@google.com","subject":"Re: [PATCH] status: suppress additional warning output in plumbing modes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-07-01T13:52:05Z","receivedAt":"2017-07-01T13:52:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jun 30 2017, Stefan Beller jotted:\n\n> When status is called with '--porcelain' (as implied by '-z'), we promise\n> to output only messages as described in the man page.\n>\n> Suppress CRLF warnings.\n>\n> Signed-off-by: Stefan Beller \n> ---\n>\n> Maybe something like this?\n\nIt looks sensibly implemented, but as for the approach I think we should\njust document that you might get errors on stderr, but stable output on\nstdout.\n\nMany consumers of --porcelain, such as magit, will run arbitrary git\noutput and expect that if they get something on stderr they should be\nshowing it in some special buffer to the user as associated error\ninformation.\n\nI think it makes sense to do this & document it in the man page, if you\ndon't care about possible error messages 2>/dev/null is trivial, but\nit's not trivial to discover in some non-expensive way (parsing the\nnon-porcelain output) that there *are* errors.\n\n> builtin/commit.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 00a01f07c3..3705d5ec6f 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1126,6 +1126,11 @@ static void finalize_deferred_config(struct wt_status *s)\n> \t\t\tdie(_(\"--long and -z are incompatible\"));\n> \t}\n>\n> +\t/* suppress all additional output in porcelain mode */\n> +\tif (status_format == STATUS_FORMAT_PORCELAIN ||\n> +\t status_format == STATUS_FORMAT_PORCELAIN_V2)\n> +\t\tsafe_crlf = SAFE_CRLF_FALSE;\n> +\n> \tif (use_deferred_config && status_format == STATUS_FORMAT_UNSPECIFIED)\n> \t\tstatus_format = status_deferred_config.status_format;\n> \tif (status_format == STATUS_FORMAT_UNSPECIFIED)\n"},{"id":"323689","messageId":"xmqqk23snjpm.fsf@gitster.mtv.corp.google.com","threadId":"46293","inReplyTo":"20170630162826.27711-1-sbeller@google.com","subject":"Re: [PATCH] status: suppress additional warning output in plumbing modes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-01T17:35:49Z","receivedAt":"2017-07-01T17:35:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> When status is called with '--porcelain' (as implied by '-z'), we promise\n> to output only messages as described in the man page.\n>\n> Suppress CRLF warnings.\n>\n> Signed-off-by: Stefan Beller \n> ---\n>\n> Maybe something like this?\n\nThis looks to me like a stimulus having enough time to go to the\nspinal cord to induce a knee-jerk reaction, without giving a chance\nto the brain to think things through.\n\nSurely the reported symptom may have only been about CRLF, but who\nsays that would be the only kind of warning that would be seen\nduring \"status --porcelain\" codepath?\n\nI tend to agree with Ævar's \"output for the script can be read from\nour standard output\" should probably be our first response.\n\nThe patch _is_ a good start to document that we may want to do\nsomething differently under _PORCELAIN output modes and one location\nin the code that may be a good place to make that decision, but if\nwe are to squelch the warnings, we should make sure we do not give\nany warning, not limited to squelching the safe-crlf warning, to the\nstandard error, but still diagnose errors and show error messages,\nor something like that, I would think.\n\n>\n> builtin/commit.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 00a01f07c3..3705d5ec6f 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1126,6 +1126,11 @@ static void finalize_deferred_config(struct wt_status *s)\n> \t\t\tdie(_(\"--long and -z are incompatible\"));\n> \t}\n> \n> +\t/* suppress all additional output in porcelain mode */\n> +\tif (status_format == STATUS_FORMAT_PORCELAIN ||\n> +\t status_format == STATUS_FORMAT_PORCELAIN_V2)\n> +\t\tsafe_crlf = SAFE_CRLF_FALSE;\n> +\n> \tif (use_deferred_config && status_format == STATUS_FORMAT_UNSPECIFIED)\n> \t\tstatus_format = status_deferred_config.status_format;\n> \tif (status_format == STATUS_FORMAT_UNSPECIFIED)\n"},{"id":"323861","messageId":"CAGZ79kZedckynRcBTGVOAJO_iNrrBR0eAo3XvBaW+LUbOJXQXA@mail.gmail.com","threadId":"46293","inReplyTo":"xmqqk23snjpm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] status: suppress additional warning output in plumbing modes","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-05T18:53:20Z","receivedAt":"2017-07-05T18:53:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Jul 1, 2017 at 10:35 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> When status is called with '--porcelain' (as implied by '-z'), we promise\n>> to output only messages as described in the man page.\n>>\n>> Suppress CRLF warnings.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> Maybe something like this?\n>\n> This looks to me like a stimulus having enough time to go to the\n> spinal cord to induce a knee-jerk reaction, without giving a chance\n> to the brain to think things through.\n>\n\nsort of.\n\n> Surely the reported symptom may have only been about CRLF, but who\n> says that would be the only kind of warning that would be seen\n> during \"status --porcelain\" codepath?\n\nI was slightly worried about this, too.\n\n>\n> I tend to agree with Ævar's \"output for the script can be read from\n> our standard output\" should probably be our first response.\n>\n> The patch _is_ a good start to document that we may want to do\n> something differently under _PORCELAIN output modes and one location\n> in the code that may be a good place to make that decision, but if\n> we are to squelch the warnings, we should make sure we do not give\n> any warning, not limited to squelching the safe-crlf warning, to the\n> standard error, but still diagnose errors and show error messages,\n> or something like that, I would think.\n\nSo for now we'd rather want to go with a documentation patch first\nand then the refinement of the porcelain mode of potentially\nsuppressing more warnings?\n\nNote that this patch was a one-off by me, so I no longer pursue\nfixing the problem here, someone else is kindly asked to step up.\n\nThanks,\nStefan\n"}]}