{"thread":{"id":"60673","subject":"[PATCH] write-or-die: make GIT_FLUSH a Boolean environment variable","startedAt":"2023-12-30T16:54:10Z","lastAt":"2024-01-04T18:36:05Z","messageCount":13,"participants":["Chandra Pratap via GitGitGadget","Torsten Bögershausen","Junio C Hamano","Patrick Steinhardt","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"486171","messageId":"pull.1628.git.1703955246308.gitgitgadget@gmail.com","threadId":"60673","inReplyTo":null,"subject":"[PATCH] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Chandra Pratap via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-12-30T16:54:06Z","receivedAt":"2023-12-30T16:54:10Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"From: Chandra Pratap <chandrapratap3519@gmail.com>\n\nAmong Git's environment variable, the ones marked as \"Boolean\"\naccept values in a way similar to Boolean configuration variables,\ni.e. values like 'yes', 'on', 'true' and positive numbers are\ntaken as \"on\" and values like 'no', 'off', 'false' are taken as\n\"off\".\nGIT_FLUSH can be used to force Git to use non-buffered I/O when\nwriting to stdout. It can only accept two values, '1' which causes\nGit to flush more often and '0' which makes all output buffered.\nMake GIT_FLUSH accept more values besides '0' and '1' by turning it\ninto a Boolean environment variable, modifying the required logic.\nUpdate the related documentation.\n\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n    write-or-die: make GIT_FLUSH a Boolean environment variable\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1628%2FChand-ra%2Fgit_flush-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1628/Chand-ra/git_flush-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1628\n\n Documentation/git.txt | 16 +++++++---------\n write-or-die.c        |  9 ++++-----\n 2 files changed, 11 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 2535a30194f..83fd60f2d11 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -724,16 +724,14 @@ for further details.\n \twaiting for someone with sufficient permissions to fix it.\n \n `GIT_FLUSH`::\n-// NEEDSWORK: make it into a usual Boolean environment variable\n-\tIf this environment variable is set to \"1\", then commands such\n+\tIf this Boolean environment variable is set to true, then commands such\n \tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n-\t'git check-attr' and 'git check-ignore' will\n-\tforce a flush of the output stream after each record have been\n-\tflushed. If this\n-\tvariable is set to \"0\", the output of these commands will be done\n-\tusing completely buffered I/O.   If this environment variable is\n-\tnot set, Git will choose buffered or record-oriented flushing\n-\tbased on whether stdout appears to be redirected to a file or not.\n+\t'git check-attr' and 'git check-ignore' will force a flush of the output\n+\tstream after each record have been flushed. If this variable is set to\n+\tfalse, the output of these commands will be done using completely buffered\n+\tI/O. If this environment variable is not set, Git will choose buffered or\n+\trecord-oriented flushing based on whether stdout appears to be redirected\n+\tto a file or not.\n \n `GIT_TRACE`::\n \tEnables general trace messages, e.g. alias expansion, built-in\ndiff --git a/write-or-die.c b/write-or-die.c\nindex 42a2dc73cd3..f501a6e382a 100644\n--- a/write-or-die.c\n+++ b/write-or-die.c\n@@ -20,14 +20,13 @@ void maybe_flush_or_die(FILE *f, const char *desc)\n {\n \tstatic int skip_stdout_flush = -1;\n \tstruct stat st;\n-\tchar *cp;\n+\tint cp;\n \n \tif (f == stdout) {\n \t\tif (skip_stdout_flush < 0) {\n-\t\t\t/* NEEDSWORK: make this a normal Boolean */\n-\t\t\tcp = getenv(\"GIT_FLUSH\");\n-\t\t\tif (cp)\n-\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n+\t\t\tcp = git_env_bool(\"GIT_FLUSH\", -1);\n+\t\t\tif (cp >= 0)\n+\t\t\t\tskip_stdout_flush = (cp == 0);\n \t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n \t\t\t\t S_ISREG(st.st_mode))\n \t\t\t\tskip_stdout_flush = 1;\n\nbase-commit: 1a87c842ece327d03d08096395969aca5e0a6996\n-- \ngitgitgadget\n"},{"id":"486183","messageId":"20240101082446.GA21905@tb-raspi4","threadId":"60673","inReplyTo":"pull.1628.git.1703955246308.gitgitgadget@gmail.com","subject":"Re: [PATCH] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-01-01T08:24:46Z","receivedAt":"2024-01-01T08:24:55Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Sat, Dec 30, 2023 at 04:54:06PM +0000, Chandra Pratap via GitGitGadget wrote:\n> From: Chandra Pratap <chandrapratap3519@gmail.com>\n>\n> Among Git's environment variable, the ones marked as \"Boolean\"\n> accept values in a way similar to Boolean configuration variables,\n> i.e. values like 'yes', 'on', 'true' and positive numbers are\n> taken as \"on\" and values like 'no', 'off', 'false' are taken as\n> \"off\".\n> GIT_FLUSH can be used to force Git to use non-buffered I/O when\n> writing to stdout. It can only accept two values, '1' which causes\n> Git to flush more often and '0' which makes all output buffered.\n> Make GIT_FLUSH accept more values besides '0' and '1' by turning it\n> into a Boolean environment variable, modifying the required logic.\n> Update the related documentation.\n>\n> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n> ---\n>     write-or-die: make GIT_FLUSH a Boolean environment variable\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1628%2FChand-ra%2Fgit_flush-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1628/Chand-ra/git_flush-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1628\n>\n>  Documentation/git.txt | 16 +++++++---------\n>  write-or-die.c        |  9 ++++-----\n>  2 files changed, 11 insertions(+), 14 deletions(-)\n>\n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 2535a30194f..83fd60f2d11 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -724,16 +724,14 @@ for further details.\n>  \twaiting for someone with sufficient permissions to fix it.\n>\n>  `GIT_FLUSH`::\n> -// NEEDSWORK: make it into a usual Boolean environment variable\n> -\tIf this environment variable is set to \"1\", then commands such\n> +\tIf this Boolean environment variable is set to true, then commands such\n>  \tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n> -\t'git check-attr' and 'git check-ignore' will\n> -\tforce a flush of the output stream after each record have been\n> -\tflushed. If this\n> -\tvariable is set to \"0\", the output of these commands will be done\n> -\tusing completely buffered I/O.   If this environment variable is\n> -\tnot set, Git will choose buffered or record-oriented flushing\n> -\tbased on whether stdout appears to be redirected to a file or not.\n> +\t'git check-attr' and 'git check-ignore' will force a flush of the output\n> +\tstream after each record have been flushed. If this variable is set to\n> +\tfalse, the output of these commands will be done using completely buffered\n> +\tI/O. If this environment variable is not set, Git will choose buffered or\n> +\trecord-oriented flushing based on whether stdout appears to be redirected\n> +\tto a file or not.\n>\n>  `GIT_TRACE`::\n>  \tEnables general trace messages, e.g. alias expansion, built-in\n> diff --git a/write-or-die.c b/write-or-die.c\n> index 42a2dc73cd3..f501a6e382a 100644\n> --- a/write-or-die.c\n> +++ b/write-or-die.c\n> @@ -20,14 +20,13 @@ void maybe_flush_or_die(FILE *f, const char *desc)\n>  {\n>  \tstatic int skip_stdout_flush = -1;\n>  \tstruct stat st;\n> -\tchar *cp;\n> +\tint cp;\n>\n>  \tif (f == stdout) {\n>  \t\tif (skip_stdout_flush < 0) {\n> -\t\t\t/* NEEDSWORK: make this a normal Boolean */\n> -\t\t\tcp = getenv(\"GIT_FLUSH\");\n> -\t\t\tif (cp)\n> -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n> +\t\t\tcp = git_env_bool(\"GIT_FLUSH\", -1);\n> +\t\t\tif (cp >= 0)\n> +\t\t\t\tskip_stdout_flush = (cp == 0);\n>  \t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n>  \t\t\t\t S_ISREG(st.st_mode))\n>  \t\t\t\tskip_stdout_flush = 1;\n\nI think that the \"cp\" variable could be renamed,\ncp is not a \"char pointer\" any more.\nHowever, using the logic from git_env_bool(), it can go away anyway,\nwouldn't it ?\n\n\n  diff --git a/write-or-die.c b/write-or-die.c\n  index 42a2dc73cd..01b042bf12 100644\n  --- a/write-or-die.c\n  +++ b/write-or-die.c\n  @@ -13,21 +13,21 @@\n    * more. So we just ignore that case instead (and hope we get\n    * the right error code on the flush).\n    *\n  + * The flushing can be skipped like this:\n  + * GIT_FLUSH=0 git-rev-list HEAD\n  + *\n    * If the file handle is stdout, and stdout is a file, then skip the\n  - * flush entirely since it's not needed.\n  + * flush as well since it's not needed.\n    */\n   void maybe_flush_or_die(FILE *f, const char *desc)\n   {\n          static int skip_stdout_flush = -1;\n          struct stat st;\n  -       char *cp;\n\n          if (f == stdout) {\n                  if (skip_stdout_flush < 0) {\n  -                       /* NEEDSWORK: make this a normal Boolean */\n  -                       cp = getenv(\"GIT_FLUSH\");\n  -                       if (cp)\n  -                               skip_stdout_flush = (atoi(cp) == 0);\n  +                       if (git_env_bool(\"GIT_FLUSH\", -1) == 0)\n  +                               skip_stdout_flush = 1;\n                          else if ((fstat(fileno(stdout), &st) == 0) &&\n                                   S_ISREG(st.st_mode))\n                                  skip_stdout_flush = 1;\n"},{"id":"486198","messageId":"xmqqil4bg0me.fsf@gitster.g","threadId":"60673","inReplyTo":"20240101082446.GA21905@tb-raspi4","subject":"Re: [PATCH] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-02T16:29:13Z","receivedAt":"2024-01-02T16:29:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n>> -\tchar *cp;\n>> +\tint cp;\n>>\n>>  \tif (f == stdout) {\n>>  \t\tif (skip_stdout_flush < 0) {\n>> -\t\t\t/* NEEDSWORK: make this a normal Boolean */\n>> -\t\t\tcp = getenv(\"GIT_FLUSH\");\n>> -\t\t\tif (cp)\n>> -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n>> +\t\t\tcp = git_env_bool(\"GIT_FLUSH\", -1);\n>> +\t\t\tif (cp >= 0)\n>> +\t\t\t\tskip_stdout_flush = (cp == 0);\n>>  \t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n>>  \t\t\t\t S_ISREG(st.st_mode))\n>>  \t\t\t\tskip_stdout_flush = 1;\n>\n> I think that the \"cp\" variable could be renamed,\n> cp is not a \"char pointer\" any more.\n\nAbsolutely.  Very good point.\n\n> However, using the logic from git_env_bool(), it can go away anyway,\n> wouldn't it ?\n\nTrue.\n\nIf we are doing clean-ups in this area, we may want to also stop\ndoing \"== 0\" comparisons on lines the patch touches while at it.\n\n>   diff --git a/write-or-die.c b/write-or-die.c\n>   index 42a2dc73cd..01b042bf12 100644\n>   --- a/write-or-die.c\n>   +++ b/write-or-die.c\n>   @@ -13,21 +13,21 @@\n>     * more. So we just ignore that case instead (and hope we get\n>     * the right error code on the flush).\n>     *\n>   + * The flushing can be skipped like this:\n>   + * GIT_FLUSH=0 git-rev-list HEAD\n>   + *\n>     * If the file handle is stdout, and stdout is a file, then skip the\n>   - * flush entirely since it's not needed.\n>   + * flush as well since it's not needed.\n>     */\n>    void maybe_flush_or_die(FILE *f, const char *desc)\n>    {\n>           static int skip_stdout_flush = -1;\n>           struct stat st;\n>   -       char *cp;\n>\n>           if (f == stdout) {\n>                   if (skip_stdout_flush < 0) {\n>   -                       /* NEEDSWORK: make this a normal Boolean */\n>   -                       cp = getenv(\"GIT_FLUSH\");\n>   -                       if (cp)\n>   -                               skip_stdout_flush = (atoi(cp) == 0);\n>   +                       if (git_env_bool(\"GIT_FLUSH\", -1) == 0)\n>   +                               skip_stdout_flush = 1;\n>                           else if ((fstat(fileno(stdout), &st) == 0) &&\n>                                    S_ISREG(st.st_mode))\n>                                   skip_stdout_flush = 1;\n"},{"id":"486242","messageId":"pull.1628.v2.git.1704268708720.gitgitgadget@gmail.com","threadId":"60673","inReplyTo":"pull.1628.git.1703955246308.gitgitgadget@gmail.com","subject":"[PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Chandra Pratap via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-03T07:58:28Z","receivedAt":"2024-01-03T07:58:31Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"From: Chandra Pratap <chandrapratap3519@gmail.com>\n\nAmong Git's environment variable, the ones marked as \"Boolean\"\naccept values in a way similar to Boolean configuration variables,\ni.e. values like 'yes', 'on', 'true' and positive numbers are\ntaken as \"on\" and values like 'no', 'off', 'false' are taken as\n\"off\".\nGIT_FLUSH can be used to force Git to use non-buffered I/O when\nwriting to stdout. It can only accept two values, '1' which causes\nGit to flush more often and '0' which makes all output buffered.\nMake GIT_FLUSH accept more values besides '0' and '1' by turning it\ninto a Boolean environment variable, modifying the required logic.\nUpdate the related documentation.\n\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n    write-or-die: make GIT_FLUSH a Boolean environment variable\n    \n    Helped-by: Torsten Bögershausen <tboegi@web.de>\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1628%2FChand-ra%2Fgit_flush-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1628/Chand-ra/git_flush-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1628\n\nRange-diff vs v1:\n\n 1:  2123b43a080 ! 1:  585c76fff68 write-or-die: make GIT_FLUSH a Boolean environment variable\n     @@ write-or-die.c: void maybe_flush_or_die(FILE *f, const char *desc)\n       \tstatic int skip_stdout_flush = -1;\n       \tstruct stat st;\n      -\tchar *cp;\n     -+\tint cp;\n       \n       \tif (f == stdout) {\n       \t\tif (skip_stdout_flush < 0) {\n     @@ write-or-die.c: void maybe_flush_or_die(FILE *f, const char *desc)\n      -\t\t\tcp = getenv(\"GIT_FLUSH\");\n      -\t\t\tif (cp)\n      -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n     -+\t\t\tcp = git_env_bool(\"GIT_FLUSH\", -1);\n     -+\t\t\tif (cp >= 0)\n     -+\t\t\t\tskip_stdout_flush = (cp == 0);\n     - \t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n     +-\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n     ++\t\t\tif (!git_env_bool(\"GIT_FLUSH\", -1))\n     ++\t\t\t\tskip_stdout_flush = 1;\n     ++\t\t\telse if (!fstat(fileno(stdout), &st) &&\n       \t\t\t\t S_ISREG(st.st_mode))\n       \t\t\t\tskip_stdout_flush = 1;\n     + \t\t\telse\n\n\n Documentation/git.txt | 16 +++++++---------\n write-or-die.c        |  9 +++------\n 2 files changed, 10 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 2535a30194f..83fd60f2d11 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -724,16 +724,14 @@ for further details.\n \twaiting for someone with sufficient permissions to fix it.\n \n `GIT_FLUSH`::\n-// NEEDSWORK: make it into a usual Boolean environment variable\n-\tIf this environment variable is set to \"1\", then commands such\n+\tIf this Boolean environment variable is set to true, then commands such\n \tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n-\t'git check-attr' and 'git check-ignore' will\n-\tforce a flush of the output stream after each record have been\n-\tflushed. If this\n-\tvariable is set to \"0\", the output of these commands will be done\n-\tusing completely buffered I/O.   If this environment variable is\n-\tnot set, Git will choose buffered or record-oriented flushing\n-\tbased on whether stdout appears to be redirected to a file or not.\n+\t'git check-attr' and 'git check-ignore' will force a flush of the output\n+\tstream after each record have been flushed. If this variable is set to\n+\tfalse, the output of these commands will be done using completely buffered\n+\tI/O. If this environment variable is not set, Git will choose buffered or\n+\trecord-oriented flushing based on whether stdout appears to be redirected\n+\tto a file or not.\n \n `GIT_TRACE`::\n \tEnables general trace messages, e.g. alias expansion, built-in\ndiff --git a/write-or-die.c b/write-or-die.c\nindex 42a2dc73cd3..a6acabd329f 100644\n--- a/write-or-die.c\n+++ b/write-or-die.c\n@@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)\n {\n \tstatic int skip_stdout_flush = -1;\n \tstruct stat st;\n-\tchar *cp;\n \n \tif (f == stdout) {\n \t\tif (skip_stdout_flush < 0) {\n-\t\t\t/* NEEDSWORK: make this a normal Boolean */\n-\t\t\tcp = getenv(\"GIT_FLUSH\");\n-\t\t\tif (cp)\n-\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n-\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n+\t\t\tif (!git_env_bool(\"GIT_FLUSH\", -1))\n+\t\t\t\tskip_stdout_flush = 1;\n+\t\t\telse if (!fstat(fileno(stdout), &st) &&\n \t\t\t\t S_ISREG(st.st_mode))\n \t\t\t\tskip_stdout_flush = 1;\n \t\t\telse\n\nbase-commit: 1a87c842ece327d03d08096395969aca5e0a6996\n-- \ngitgitgadget\n"},{"id":"486245","messageId":"ZZUZNQqDTx3bnveJ@tanuki","threadId":"60673","inReplyTo":"pull.1628.v2.git.1704268708720.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-03T08:22:13Z","receivedAt":"2024-01-03T08:22:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 03, 2024 at 07:58:28AM +0000, Chandra Pratap via GitGitGadget wrote:\n[snip]\n> diff --git a/write-or-die.c b/write-or-die.c\n> index 42a2dc73cd3..a6acabd329f 100644\n> --- a/write-or-die.c\n> +++ b/write-or-die.c\n> @@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)\n>  {\n>  \tstatic int skip_stdout_flush = -1;\n>  \tstruct stat st;\n> -\tchar *cp;\n>  \n>  \tif (f == stdout) {\n>  \t\tif (skip_stdout_flush < 0) {\n> -\t\t\t/* NEEDSWORK: make this a normal Boolean */\n> -\t\t\tcp = getenv(\"GIT_FLUSH\");\n> -\t\t\tif (cp)\n> -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n> -\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n> +\t\t\tif (!git_env_bool(\"GIT_FLUSH\", -1))\n> +\t\t\t\tskip_stdout_flush = 1;\n\nIt's a bit surprising to pass `-1` as default value to `git_env_bool()`\nhere, as this value would hint that the caller wants to explicitly\nhandle the case where the \"GIT_FLUSH\" envvar is not set at all. We don't\nthough, and essentially fall back to \"GIT_FLUSH=1\", so passing `1` as\nthe fallback value would be less confusing.\n\nAnyway, the resulting behaviour is the same regardless of whether we\npass `1` or `-1`, so I'm not sure whether this is worth a reroll.\n\nPatrick\n\n> +\t\t\telse if (!fstat(fileno(stdout), &st) &&\n>  \t\t\t\t S_ISREG(st.st_mode))\n>  \t\t\t\tskip_stdout_flush = 1;\n>  \t\t\telse\n> \n> base-commit: 1a87c842ece327d03d08096395969aca5e0a6996\n> -- \n> gitgitgadget\n> \n"},{"id":"486264","messageId":"xmqqmstm9w6t.fsf@gitster.g","threadId":"60673","inReplyTo":"pull.1628.v2.git.1704268708720.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-03T17:13:46Z","receivedAt":"2024-01-03T17:13:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Chandra Pratap via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>  Documentation/git.txt | 16 +++++++---------\n>  write-or-die.c        |  9 +++------\n>  2 files changed, 10 insertions(+), 15 deletions(-)\n>\n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 2535a30194f..83fd60f2d11 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -724,16 +724,14 @@ for further details.\n>  \twaiting for someone with sufficient permissions to fix it.\n>  \n>  `GIT_FLUSH`::\n> -// NEEDSWORK: make it into a usual Boolean environment variable\n> -\tIf this environment variable is set to \"1\", then commands such\n> +\tIf this Boolean environment variable is set to true, then commands such\n>  \tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n> -\t'git check-attr' and 'git check-ignore' will\n> -\tforce a flush of the output stream after each record have been\n> -\tflushed. If this\n> -\tvariable is set to \"0\", the output of these commands will be done\n> -\tusing completely buffered I/O.   If this environment variable is\n> -\tnot set, Git will choose buffered or record-oriented flushing\n> -\tbased on whether stdout appears to be redirected to a file or not.\n> +\t'git check-attr' and 'git check-ignore' will force a flush of the output\n> +\tstream after each record have been flushed. If this variable is set to\n> +\tfalse, the output of these commands will be done using completely buffered\n> +\tI/O. If this environment variable is not set, Git will choose buffered or\n> +\trecord-oriented flushing based on whether stdout appears to be redirected\n> +\tto a file or not.\n\nIt is somewhat irritating to see that we need to change this many\nlines to just change \"0\" to \"false\" and \"1\" to \"true\".  I wonder if\nit becomes easier to grok if we changed the description into a sub\nenumeration of three possibilities, but that would be outside the\nscope of this change [*].\n\n> diff --git a/write-or-die.c b/write-or-die.c\n> index 42a2dc73cd3..a6acabd329f 100644\n> --- a/write-or-die.c\n> +++ b/write-or-die.c\n> @@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)\n>  {\n>  \tstatic int skip_stdout_flush = -1;\n>  \tstruct stat st;\n> -\tchar *cp;\n>  \n>  \tif (f == stdout) {\n>  \t\tif (skip_stdout_flush < 0) {\n> -\t\t\t/* NEEDSWORK: make this a normal Boolean */\n> -\t\t\tcp = getenv(\"GIT_FLUSH\");\n> -\t\t\tif (cp)\n> -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n> -\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n> +\t\t\tif (!git_env_bool(\"GIT_FLUSH\", -1))\n> +\t\t\t\tskip_stdout_flush = 1;\n> +\t\t\telse if (!fstat(fileno(stdout), &st) &&\n>  \t\t\t\t S_ISREG(st.st_mode))\n>  \t\t\t\tskip_stdout_flush = 1;\n>  \t\t\telse\n\nThe above logic does not look correct to me, primarily because the\nreturn value of git_env_bool() is inspected only once to see if it\nis zero, and does not differentiate the \"unset\" case from other\ncases.\n\nSince git_env_bool(k, def) returns\n\n    - \"def\" (-1 in this case) when k is not exported (in which case\n      you need to do the \"fstat\" dance).\n\n    - 0 when k is exported and has a string that is \"false\" (in\n      which case you would want to set skip_stdout_flush to true).\n\n    - 1 when k is exported and has a string that is \"true\" (in which\n      case you would want to set skip_stdout_flush to false).\n\n    - or dies if the string exported in k is bogus.\n\nshouldn't it be more like\n\n                        skip_stdout_flush = 0; /* assume flushing */\n                        switch (git_env_bool(\"GIT_FLUSH\", -1)) {\n                        case 0: /* told not to flush */\n                                skip_stdout_flush = 1;\n                                ;;\n                        case -1: /* unspecified */\n                                if (!fstat(...) && S_ISREG())\n                                        skip_stdout_flush = 1;\n                                ;;\n                        default: /* told to flush */\n                                ;;\n                        }\n\nperhaps?\n\n\n[Footnote]\n\n * If I were to do this change, and if I were to also improve the\n   style of the documentation before I forget, the way I would do so\n   probably is with a two-patch series:\n\n    (1) update \"0\" and \"1\" in the documentation with \"false\" and\n        \"true\", without reflowing the text at all, and update the\n        code.\n\n    (2) rewrite the documentation to use 3-possibility\n        sub-enumeration for values (imitate the way how other\n        variables, like `diff.algorithm`, that can choose from a\n        set of handful possible values are described).\n\n   These two changes can be done in either order, but perhaps (1) is\n   much less controversial change than the other, so I'd probably do\n   so first.\n"},{"id":"486266","messageId":"ZZWWLkY+ixg+OMM4@nand.local","threadId":"60673","inReplyTo":"ZZUZNQqDTx3bnveJ@tanuki","subject":"Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-01-03T17:15:26Z","receivedAt":"2024-01-03T17:15:28Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jan 03, 2024 at 09:22:13AM +0100, Patrick Steinhardt wrote:\n> On Wed, Jan 03, 2024 at 07:58:28AM +0000, Chandra Pratap via GitGitGadget wrote:\n> [snip]\n> > diff --git a/write-or-die.c b/write-or-die.c\n> > index 42a2dc73cd3..a6acabd329f 100644\n> > --- a/write-or-die.c\n> > +++ b/write-or-die.c\n> > @@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)\n> >  {\n> >  \tstatic int skip_stdout_flush = -1;\n> >  \tstruct stat st;\n> > -\tchar *cp;\n> >\n> >  \tif (f == stdout) {\n> >  \t\tif (skip_stdout_flush < 0) {\n> > -\t\t\t/* NEEDSWORK: make this a normal Boolean */\n> > -\t\t\tcp = getenv(\"GIT_FLUSH\");\n> > -\t\t\tif (cp)\n> > -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n> > -\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n> > +\t\t\tif (!git_env_bool(\"GIT_FLUSH\", -1))\n> > +\t\t\t\tskip_stdout_flush = 1;\n>\n> It's a bit surprising to pass `-1` as default value to `git_env_bool()`\n> here, as this value would hint that the caller wants to explicitly\n> handle the case where the \"GIT_FLUSH\" envvar is not set at all. We don't\n> though, and essentially fall back to \"GIT_FLUSH=1\", so passing `1` as\n> the fallback value would be less confusing.\n>\n> Anyway, the resulting behaviour is the same regardless of whether we\n> pass `1` or `-1`, so I'm not sure whether this is worth a reroll.\n\nHmm. If we pass -1 as the default value in the call to git_env_bool(),\nthe only time we'll end up in the else branch is if the environment is\nset to some false-y value.\n\nI don't think that matches the existing behavior, since right now we'll\ninfer skip_stdout_flush based on whether or not stdout is a regular file\nor something else.\n\nI think you'd probably want something closer to:\n\n--- 8< ---\ndiff --git a/write-or-die.c b/write-or-die.c\nindex 42a2dc73cd..f12e111688 100644\n--- a/write-or-die.c\n+++ b/write-or-die.c\n@@ -19,20 +19,17 @@\n void maybe_flush_or_die(FILE *f, const char *desc)\n {\n \tstatic int skip_stdout_flush = -1;\n-\tstruct stat st;\n-\tchar *cp;\n\n \tif (f == stdout) {\n \t\tif (skip_stdout_flush < 0) {\n-\t\t\t/* NEEDSWORK: make this a normal Boolean */\n-\t\t\tcp = getenv(\"GIT_FLUSH\");\n-\t\t\tif (cp)\n-\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n-\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n-\t\t\t\t S_ISREG(st.st_mode))\n-\t\t\t\tskip_stdout_flush = 1;\n-\t\t\telse\n-\t\t\t\tskip_stdout_flush = 0;\n+\t\t\tskip_stdout_flush = git_env_bool(\"GIT_FLUSH\", -1);\n+\t\t\tif (skip_stdout_flush < 0) {\n+\t\t\t\tstruct stat st;\n+\t\t\t\tif (fstat(fileno(f), &st))\n+\t\t\t\t\tskip_stdout_flush = 0;\n+\t\t\t\telse\n+\t\t\t\t\tskip_stdout_flush = S_ISREG(st.st_mode);\n+\t\t\t}\n \t\t}\n \t\tif (skip_stdout_flush && !ferror(f))\n \t\t\treturn;\n--- >8 ---\n\nYou could lose one level of indentation, but it costs an extra fstat()\ncall in the case where GIT_FLUSH is set to some explicit value. Doing\nthat would look like this ugly thing instead ;-):\n\n--- 8< ---\ndiff --git a/write-or-die.c b/write-or-die.c\nindex 42a2dc73cd..b3275d7577 100644\n--- a/write-or-die.c\n+++ b/write-or-die.c\n@@ -19,20 +19,11 @@\n void maybe_flush_or_die(FILE *f, const char *desc)\n {\n \tstatic int skip_stdout_flush = -1;\n-\tstruct stat st;\n-\tchar *cp;\n\n \tif (f == stdout) {\n \t\tif (skip_stdout_flush < 0) {\n-\t\t\t/* NEEDSWORK: make this a normal Boolean */\n-\t\t\tcp = getenv(\"GIT_FLUSH\");\n-\t\t\tif (cp)\n-\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n-\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n-\t\t\t\t S_ISREG(st.st_mode))\n-\t\t\t\tskip_stdout_flush = 1;\n-\t\t\telse\n-\t\t\t\tskip_stdout_flush = 0;\n+\t\t\tstruct stat st;\n+\t\t\tskip_stdout_flush = git_env_bool(\"GIT_FLUSH\", !fstat(fileno(f), &st) && S_ISREG(st.st_mode));\n \t\t}\n \t\tif (skip_stdout_flush && !ferror(f))\n \t\t\treturn;\n--- >8 ---\n\nThanks,\nTaylor\n"},{"id":"486274","messageId":"20240103184203.GA4334@tb-raspi4","threadId":"60673","inReplyTo":"ZZWWLkY+ixg+OMM4@nand.local","subject":"Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-01-03T18:42:03Z","receivedAt":"2024-01-03T18:42:10Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Wed, Jan 03, 2024 at 12:15:26PM -0500, Taylor Blau wrote:\n> On Wed, Jan 03, 2024 at 09:22:13AM +0100, Patrick Steinhardt wrote:\n> > On Wed, Jan 03, 2024 at 07:58:28AM +0000, Chandra Pratap via GitGitGadget wrote:\n> > [snip]\n> > > diff --git a/write-or-die.c b/write-or-die.c\n> > > index 42a2dc73cd3..a6acabd329f 100644\n> > > --- a/write-or-die.c\n> > > +++ b/write-or-die.c\n> > > @@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)\n> > >  {\n> > >  \tstatic int skip_stdout_flush = -1;\n> > >  \tstruct stat st;\n> > > -\tchar *cp;\n> > >\n> > >  \tif (f == stdout) {\n> > >  \t\tif (skip_stdout_flush < 0) {\n> > > -\t\t\t/* NEEDSWORK: make this a normal Boolean */\n> > > -\t\t\tcp = getenv(\"GIT_FLUSH\");\n> > > -\t\t\tif (cp)\n> > > -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n> > > -\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n> > > +\t\t\tif (!git_env_bool(\"GIT_FLUSH\", -1))\n> > > +\t\t\t\tskip_stdout_flush = 1;\n> >\n> > It's a bit surprising to pass `-1` as default value to `git_env_bool()`\n> > here, as this value would hint that the caller wants to explicitly\n> > handle the case where the \"GIT_FLUSH\" envvar is not set at all. We don't\n> > though, and essentially fall back to \"GIT_FLUSH=1\", so passing `1` as\n> > the fallback value would be less confusing.\n> >\n> > Anyway, the resulting behaviour is the same regardless of whether we\n> > pass `1` or `-1`, so I'm not sure whether this is worth a reroll.\n>\n> Hmm. If we pass -1 as the default value in the call to git_env_bool(),\n> the only time we'll end up in the else branch is if the environment is\n> set to some false-y value.\n>\n> I don't think that matches the existing behavior, since right now we'll\n> infer skip_stdout_flush based on whether or not stdout is a regular file\n> or something else.\n>\n> I think you'd probably want something closer to:\n>\n> --- 8< ---\n> diff --git a/write-or-die.c b/write-or-die.c\n> index 42a2dc73cd..f12e111688 100644\n> --- a/write-or-die.c\n> +++ b/write-or-die.c\n> @@ -19,20 +19,17 @@\n>  void maybe_flush_or_die(FILE *f, const char *desc)\n>  {\n>  \tstatic int skip_stdout_flush = -1;\n> -\tstruct stat st;\n> -\tchar *cp;\n>\n>  \tif (f == stdout) {\n>  \t\tif (skip_stdout_flush < 0) {\n> -\t\t\t/* NEEDSWORK: make this a normal Boolean */\n> -\t\t\tcp = getenv(\"GIT_FLUSH\");\n> -\t\t\tif (cp)\n> -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n> -\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n> -\t\t\t\t S_ISREG(st.st_mode))\n> -\t\t\t\tskip_stdout_flush = 1;\n> -\t\t\telse\n> -\t\t\t\tskip_stdout_flush = 0;\n> +\t\t\tskip_stdout_flush = git_env_bool(\"GIT_FLUSH\", -1);\n> +\t\t\tif (skip_stdout_flush < 0) {\n> +\t\t\t\tstruct stat st;\n> +\t\t\t\tif (fstat(fileno(f), &st))\n> +\t\t\t\t\tskip_stdout_flush = 0;\n> +\t\t\t\telse\n> +\t\t\t\t\tskip_stdout_flush = S_ISREG(st.st_mode);\n> +\t\t\t}\n>  \t\t}\n>  \t\tif (skip_stdout_flush && !ferror(f))\n>  \t\t\treturn;\n> --- >8 ---\n\nThanks for a nice reading - I can not imagine a better version.\n"},{"id":"486275","messageId":"xmqq5y0a9qed.fsf@gitster.g","threadId":"60673","inReplyTo":"20240103184203.GA4334@tb-raspi4","subject":"Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-03T19:18:50Z","receivedAt":"2024-01-03T19:19:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n>> -\t\t\tcp = getenv(\"GIT_FLUSH\");\n>> -\t\t\tif (cp)\n>> -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n>> -\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n>> -\t\t\t\t S_ISREG(st.st_mode))\n>> -\t\t\t\tskip_stdout_flush = 1;\n>> -\t\t\telse\n>> -\t\t\t\tskip_stdout_flush = 0;\n>> +\t\t\tskip_stdout_flush = git_env_bool(\"GIT_FLUSH\", -1);\n>> +\t\t\tif (skip_stdout_flush < 0) {\n>> +\t\t\t\tstruct stat st;\n>> +\t\t\t\tif (fstat(fileno(f), &st))\n>> +\t\t\t\t\tskip_stdout_flush = 0;\n>> +\t\t\t\telse\n>> +\t\t\t\t\tskip_stdout_flush = S_ISREG(st.st_mode);\n>> +\t\t\t}\n>>  \t\t}\n>>  \t\tif (skip_stdout_flush && !ferror(f))\n>>  \t\t\treturn;\n>> --- >8 ---\n>\n> Thanks for a nice reading - I can not imagine a better version.\n\nYup, the flow of the logic feels very natural with this version by\nmaking it clear that the case that the default \"-1\" is returned to\nus is where we need to figure out an appropriate value ourselves.\nAn added bonus is that the scope \"struct stat\" has is limited to the\nabsolute minimum.  I like it, too.\n\nThanks.\n"},{"id":"486285","messageId":"pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com","threadId":"60673","inReplyTo":"pull.1628.v2.git.1704268708720.gitgitgadget@gmail.com","subject":"[PATCH v3] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Chandra Pratap via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-04T10:20:17Z","receivedAt":"2024-01-04T10:20:21Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"From: Chandra Pratap <chandrapratap3519@gmail.com>\n\nAmong Git's environment variable, the ones marked as \"Boolean\"\naccept values in a way similar to Boolean configuration variables,\ni.e. values like 'yes', 'on', 'true' and positive numbers are\ntaken as \"on\" and values like 'no', 'off', 'false' are taken as\n\"off\".\nGIT_FLUSH can be used to force Git to use non-buffered I/O when\nwriting to stdout. It can only accept two values, '1' which causes\nGit to flush more often and '0' which makes all output buffered.\nMake GIT_FLUSH accept more values besides '0' and '1' by turning it\ninto a Boolean environment variable, modifying the required logic.\nUpdate the related documentation.\n\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n    write-or-die: make GIT_FLUSH a Boolean environment variable\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1628%2FChand-ra%2Fgit_flush-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1628/Chand-ra/git_flush-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1628\n\nRange-diff vs v2:\n\n 1:  585c76fff68 ! 1:  c98885c0ede write-or-die: make GIT_FLUSH a Boolean environment variable\n     @@ Documentation/git.txt: for further details.\n      -\tIf this environment variable is set to \"1\", then commands such\n      +\tIf this Boolean environment variable is set to true, then commands such\n       \tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n     --\t'git check-attr' and 'git check-ignore' will\n     --\tforce a flush of the output stream after each record have been\n     --\tflushed. If this\n     + \t'git check-attr' and 'git check-ignore' will\n     + \tforce a flush of the output stream after each record have been\n     + \tflushed. If this\n      -\tvariable is set to \"0\", the output of these commands will be done\n     --\tusing completely buffered I/O.   If this environment variable is\n     --\tnot set, Git will choose buffered or record-oriented flushing\n     --\tbased on whether stdout appears to be redirected to a file or not.\n     -+\t'git check-attr' and 'git check-ignore' will force a flush of the output\n     -+\tstream after each record have been flushed. If this variable is set to\n     -+\tfalse, the output of these commands will be done using completely buffered\n     -+\tI/O. If this environment variable is not set, Git will choose buffered or\n     -+\trecord-oriented flushing based on whether stdout appears to be redirected\n     -+\tto a file or not.\n     - \n     - `GIT_TRACE`::\n     - \tEnables general trace messages, e.g. alias expansion, built-in\n     ++\tvariable is set to false, the output of these commands will be done\n     + \tusing completely buffered I/O.   If this environment variable is\n     + \tnot set, Git will choose buffered or record-oriented flushing\n     + \tbased on whether stdout appears to be redirected to a file or not.\n      \n       ## write-or-die.c ##\n     -@@ write-or-die.c: void maybe_flush_or_die(FILE *f, const char *desc)\n     +@@\n     + void maybe_flush_or_die(FILE *f, const char *desc)\n       {\n       \tstatic int skip_stdout_flush = -1;\n     - \tstruct stat st;\n     +-\tstruct stat st;\n      -\tchar *cp;\n       \n       \tif (f == stdout) {\n     @@ write-or-die.c: void maybe_flush_or_die(FILE *f, const char *desc)\n      -\t\t\tif (cp)\n      -\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n      -\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n     -+\t\t\tif (!git_env_bool(\"GIT_FLUSH\", -1))\n     -+\t\t\t\tskip_stdout_flush = 1;\n     -+\t\t\telse if (!fstat(fileno(stdout), &st) &&\n     - \t\t\t\t S_ISREG(st.st_mode))\n     - \t\t\t\tskip_stdout_flush = 1;\n     - \t\t\telse\n     +-\t\t\t\t S_ISREG(st.st_mode))\n     +-\t\t\t\tskip_stdout_flush = 1;\n     +-\t\t\telse\n     +-\t\t\t\tskip_stdout_flush = 0;\n     ++\t\t\tskip_stdout_flush = git_env_bool(\"GIT_FLUSH\", -1);\n     ++\t\t\tif (skip_stdout_flush < 0) {\n     ++\t\t\t\tstruct stat st;\n     ++\t\t\t\tif (fstat(fileno(stdout), &st))\n     ++\t\t\t\t\tskip_stdout_flush = 0;\n     ++\t\t\t\telse\n     ++\t\t\t\t\tskip_stdout_flush = S_ISREG(st.st_mode);\n     ++\t\t\t}\n     + \t\t}\n     + \t\tif (skip_stdout_flush && !ferror(f))\n     + \t\t\treturn;\n\n\n Documentation/git.txt |  5 ++---\n write-or-die.c        | 19 ++++++++-----------\n 2 files changed, 10 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 2535a30194f..d06eea024f7 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -724,13 +724,12 @@ for further details.\n \twaiting for someone with sufficient permissions to fix it.\n \n `GIT_FLUSH`::\n-// NEEDSWORK: make it into a usual Boolean environment variable\n-\tIf this environment variable is set to \"1\", then commands such\n+\tIf this Boolean environment variable is set to true, then commands such\n \tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n \t'git check-attr' and 'git check-ignore' will\n \tforce a flush of the output stream after each record have been\n \tflushed. If this\n-\tvariable is set to \"0\", the output of these commands will be done\n+\tvariable is set to false, the output of these commands will be done\n \tusing completely buffered I/O.   If this environment variable is\n \tnot set, Git will choose buffered or record-oriented flushing\n \tbased on whether stdout appears to be redirected to a file or not.\ndiff --git a/write-or-die.c b/write-or-die.c\nindex 42a2dc73cd3..39421528653 100644\n--- a/write-or-die.c\n+++ b/write-or-die.c\n@@ -19,20 +19,17 @@\n void maybe_flush_or_die(FILE *f, const char *desc)\n {\n \tstatic int skip_stdout_flush = -1;\n-\tstruct stat st;\n-\tchar *cp;\n \n \tif (f == stdout) {\n \t\tif (skip_stdout_flush < 0) {\n-\t\t\t/* NEEDSWORK: make this a normal Boolean */\n-\t\t\tcp = getenv(\"GIT_FLUSH\");\n-\t\t\tif (cp)\n-\t\t\t\tskip_stdout_flush = (atoi(cp) == 0);\n-\t\t\telse if ((fstat(fileno(stdout), &st) == 0) &&\n-\t\t\t\t S_ISREG(st.st_mode))\n-\t\t\t\tskip_stdout_flush = 1;\n-\t\t\telse\n-\t\t\t\tskip_stdout_flush = 0;\n+\t\t\tskip_stdout_flush = git_env_bool(\"GIT_FLUSH\", -1);\n+\t\t\tif (skip_stdout_flush < 0) {\n+\t\t\t\tstruct stat st;\n+\t\t\t\tif (fstat(fileno(stdout), &st))\n+\t\t\t\t\tskip_stdout_flush = 0;\n+\t\t\t\telse\n+\t\t\t\t\tskip_stdout_flush = S_ISREG(st.st_mode);\n+\t\t\t}\n \t\t}\n \t\tif (skip_stdout_flush && !ferror(f))\n \t\t\treturn;\n\nbase-commit: 1a87c842ece327d03d08096395969aca5e0a6996\n-- \ngitgitgadget\n"},{"id":"486297","messageId":"ZZbsVfFpJNQHGGEn@nand.local","threadId":"60673","inReplyTo":"xmqq5y0a9qed.fsf@gitster.g","subject":"Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-01-04T17:35:17Z","receivedAt":"2024-01-04T17:35:19Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jan 03, 2024 at 11:18:50AM -0800, Junio C Hamano wrote:\n> > Thanks for a nice reading - I can not imagine a better version.\n>\n> Yup, the flow of the logic feels very natural with this version by\n> making it clear that the case that the default \"-1\" is returned to\n> us is where we need to figure out an appropriate value ourselves.\n> An added bonus is that the scope \"struct stat\" has is limited to the\n> absolute minimum.  I like it, too.\n\nThanks, both.\n\nThanks,\nTaylor\n"},{"id":"486298","messageId":"ZZbsg6HG36gjXnhc@nand.local","threadId":"60673","inReplyTo":"pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-01-04T17:36:03Z","receivedAt":"2024-01-04T17:36:05Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Chandra,\n\nOn Thu, Jan 04, 2024 at 10:20:17AM +0000, Chandra Pratap via GitGitGadget wrote:\n> ---\n>  Documentation/git.txt |  5 ++---\n>  write-or-die.c        | 19 ++++++++-----------\n>  2 files changed, 10 insertions(+), 14 deletions(-)\n\nThanks very much for working on this, and taking some of my suggestions\ninto account.\n\nThis version looks great to me.\n\nThanks,\nTaylor\n"},{"id":"486305","messageId":"xmqqh6jt6j5f.fsf@gitster.g","threadId":"60673","inReplyTo":"pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] write-or-die: make GIT_FLUSH a Boolean environment variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-04T18:35:56Z","receivedAt":"2024-01-04T18:36:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Chandra Pratap via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Chandra Pratap <chandrapratap3519@gmail.com>\n>\n> Among Git's environment variable, the ones marked as \"Boolean\"\n> accept values in a way similar to Boolean configuration variables,\n\nWith \"Among\" you probably mean that there are many and some of them\nare \"marked as 'Boolean'\", so I'd do \"variable\" -> \"variables\" while\nqueuing.\n\nOther than that, looks great.  Will queue.  Thanks.\n"}]}