* Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable
2024-01-03 7:58 ` [PATCH v2] " Chandra Pratap via GitGitGadget
@ 2024-01-03 8:22 ` Patrick Steinhardt
2024-01-03 17:15 ` Taylor Blau
2024-01-03 17:13 ` Junio C Hamano
2024-01-04 10:20 ` [PATCH v3] " Chandra Pratap via GitGitGadget
2 siblings, 1 reply; 13+ messages in thread
From: Patrick Steinhardt @ 2024-01-03 8:22 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]
On Wed, Jan 03, 2024 at 07:58:28AM +0000, Chandra Pratap via GitGitGadget wrote:
[snip]
> diff --git a/write-or-die.c b/write-or-die.c
> index 42a2dc73cd3..a6acabd329f 100644
> --- a/write-or-die.c
> +++ b/write-or-die.c
> @@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)
> {
> static int skip_stdout_flush = -1;
> struct stat st;
> - char *cp;
>
> if (f == stdout) {
> if (skip_stdout_flush < 0) {
> - /* NEEDSWORK: make this a normal Boolean */
> - cp = getenv("GIT_FLUSH");
> - if (cp)
> - skip_stdout_flush = (atoi(cp) == 0);
> - else if ((fstat(fileno(stdout), &st) == 0) &&
> + if (!git_env_bool("GIT_FLUSH", -1))
> + skip_stdout_flush = 1;
It's a bit surprising to pass `-1` as default value to `git_env_bool()`
here, as this value would hint that the caller wants to explicitly
handle the case where the "GIT_FLUSH" envvar is not set at all. We don't
though, and essentially fall back to "GIT_FLUSH=1", so passing `1` as
the fallback value would be less confusing.
Anyway, the resulting behaviour is the same regardless of whether we
pass `1` or `-1`, so I'm not sure whether this is worth a reroll.
Patrick
> + else if (!fstat(fileno(stdout), &st) &&
> S_ISREG(st.st_mode))
> skip_stdout_flush = 1;
> else
>
> base-commit: 1a87c842ece327d03d08096395969aca5e0a6996
> --
> gitgitgadget
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable
2024-01-03 8:22 ` Patrick Steinhardt
@ 2024-01-03 17:15 ` Taylor Blau
2024-01-03 18:42 ` Torsten Bögershausen
0 siblings, 1 reply; 13+ messages in thread
From: Taylor Blau @ 2024-01-03 17:15 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Chandra Pratap via GitGitGadget, git, Chandra Pratap,
Chandra Pratap
On Wed, Jan 03, 2024 at 09:22:13AM +0100, Patrick Steinhardt wrote:
> On Wed, Jan 03, 2024 at 07:58:28AM +0000, Chandra Pratap via GitGitGadget wrote:
> [snip]
> > diff --git a/write-or-die.c b/write-or-die.c
> > index 42a2dc73cd3..a6acabd329f 100644
> > --- a/write-or-die.c
> > +++ b/write-or-die.c
> > @@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)
> > {
> > static int skip_stdout_flush = -1;
> > struct stat st;
> > - char *cp;
> >
> > if (f == stdout) {
> > if (skip_stdout_flush < 0) {
> > - /* NEEDSWORK: make this a normal Boolean */
> > - cp = getenv("GIT_FLUSH");
> > - if (cp)
> > - skip_stdout_flush = (atoi(cp) == 0);
> > - else if ((fstat(fileno(stdout), &st) == 0) &&
> > + if (!git_env_bool("GIT_FLUSH", -1))
> > + skip_stdout_flush = 1;
>
> It's a bit surprising to pass `-1` as default value to `git_env_bool()`
> here, as this value would hint that the caller wants to explicitly
> handle the case where the "GIT_FLUSH" envvar is not set at all. We don't
> though, and essentially fall back to "GIT_FLUSH=1", so passing `1` as
> the fallback value would be less confusing.
>
> Anyway, the resulting behaviour is the same regardless of whether we
> pass `1` or `-1`, so I'm not sure whether this is worth a reroll.
Hmm. If we pass -1 as the default value in the call to git_env_bool(),
the only time we'll end up in the else branch is if the environment is
set to some false-y value.
I don't think that matches the existing behavior, since right now we'll
infer skip_stdout_flush based on whether or not stdout is a regular file
or something else.
I think you'd probably want something closer to:
--- 8< ---
diff --git a/write-or-die.c b/write-or-die.c
index 42a2dc73cd..f12e111688 100644
--- a/write-or-die.c
+++ b/write-or-die.c
@@ -19,20 +19,17 @@
void maybe_flush_or_die(FILE *f, const char *desc)
{
static int skip_stdout_flush = -1;
- struct stat st;
- char *cp;
if (f == stdout) {
if (skip_stdout_flush < 0) {
- /* NEEDSWORK: make this a normal Boolean */
- cp = getenv("GIT_FLUSH");
- if (cp)
- skip_stdout_flush = (atoi(cp) == 0);
- else if ((fstat(fileno(stdout), &st) == 0) &&
- S_ISREG(st.st_mode))
- skip_stdout_flush = 1;
- else
- skip_stdout_flush = 0;
+ skip_stdout_flush = git_env_bool("GIT_FLUSH", -1);
+ if (skip_stdout_flush < 0) {
+ struct stat st;
+ if (fstat(fileno(f), &st))
+ skip_stdout_flush = 0;
+ else
+ skip_stdout_flush = S_ISREG(st.st_mode);
+ }
}
if (skip_stdout_flush && !ferror(f))
return;
--- >8 ---
You could lose one level of indentation, but it costs an extra fstat()
call in the case where GIT_FLUSH is set to some explicit value. Doing
that would look like this ugly thing instead ;-):
--- 8< ---
diff --git a/write-or-die.c b/write-or-die.c
index 42a2dc73cd..b3275d7577 100644
--- a/write-or-die.c
+++ b/write-or-die.c
@@ -19,20 +19,11 @@
void maybe_flush_or_die(FILE *f, const char *desc)
{
static int skip_stdout_flush = -1;
- struct stat st;
- char *cp;
if (f == stdout) {
if (skip_stdout_flush < 0) {
- /* NEEDSWORK: make this a normal Boolean */
- cp = getenv("GIT_FLUSH");
- if (cp)
- skip_stdout_flush = (atoi(cp) == 0);
- else if ((fstat(fileno(stdout), &st) == 0) &&
- S_ISREG(st.st_mode))
- skip_stdout_flush = 1;
- else
- skip_stdout_flush = 0;
+ struct stat st;
+ skip_stdout_flush = git_env_bool("GIT_FLUSH", !fstat(fileno(f), &st) && S_ISREG(st.st_mode));
}
if (skip_stdout_flush && !ferror(f))
return;
--- >8 ---
Thanks,
Taylor
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable
2024-01-03 17:15 ` Taylor Blau
@ 2024-01-03 18:42 ` Torsten Bögershausen
2024-01-03 19:18 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: Torsten Bögershausen @ 2024-01-03 18:42 UTC (permalink / raw)
To: Taylor Blau
Cc: Patrick Steinhardt, Chandra Pratap via GitGitGadget, git,
Chandra Pratap, Chandra Pratap
On Wed, Jan 03, 2024 at 12:15:26PM -0500, Taylor Blau wrote:
> On Wed, Jan 03, 2024 at 09:22:13AM +0100, Patrick Steinhardt wrote:
> > On Wed, Jan 03, 2024 at 07:58:28AM +0000, Chandra Pratap via GitGitGadget wrote:
> > [snip]
> > > diff --git a/write-or-die.c b/write-or-die.c
> > > index 42a2dc73cd3..a6acabd329f 100644
> > > --- a/write-or-die.c
> > > +++ b/write-or-die.c
> > > @@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)
> > > {
> > > static int skip_stdout_flush = -1;
> > > struct stat st;
> > > - char *cp;
> > >
> > > if (f == stdout) {
> > > if (skip_stdout_flush < 0) {
> > > - /* NEEDSWORK: make this a normal Boolean */
> > > - cp = getenv("GIT_FLUSH");
> > > - if (cp)
> > > - skip_stdout_flush = (atoi(cp) == 0);
> > > - else if ((fstat(fileno(stdout), &st) == 0) &&
> > > + if (!git_env_bool("GIT_FLUSH", -1))
> > > + skip_stdout_flush = 1;
> >
> > It's a bit surprising to pass `-1` as default value to `git_env_bool()`
> > here, as this value would hint that the caller wants to explicitly
> > handle the case where the "GIT_FLUSH" envvar is not set at all. We don't
> > though, and essentially fall back to "GIT_FLUSH=1", so passing `1` as
> > the fallback value would be less confusing.
> >
> > Anyway, the resulting behaviour is the same regardless of whether we
> > pass `1` or `-1`, so I'm not sure whether this is worth a reroll.
>
> Hmm. If we pass -1 as the default value in the call to git_env_bool(),
> the only time we'll end up in the else branch is if the environment is
> set to some false-y value.
>
> I don't think that matches the existing behavior, since right now we'll
> infer skip_stdout_flush based on whether or not stdout is a regular file
> or something else.
>
> I think you'd probably want something closer to:
>
> --- 8< ---
> diff --git a/write-or-die.c b/write-or-die.c
> index 42a2dc73cd..f12e111688 100644
> --- a/write-or-die.c
> +++ b/write-or-die.c
> @@ -19,20 +19,17 @@
> void maybe_flush_or_die(FILE *f, const char *desc)
> {
> static int skip_stdout_flush = -1;
> - struct stat st;
> - char *cp;
>
> if (f == stdout) {
> if (skip_stdout_flush < 0) {
> - /* NEEDSWORK: make this a normal Boolean */
> - cp = getenv("GIT_FLUSH");
> - if (cp)
> - skip_stdout_flush = (atoi(cp) == 0);
> - else if ((fstat(fileno(stdout), &st) == 0) &&
> - S_ISREG(st.st_mode))
> - skip_stdout_flush = 1;
> - else
> - skip_stdout_flush = 0;
> + skip_stdout_flush = git_env_bool("GIT_FLUSH", -1);
> + if (skip_stdout_flush < 0) {
> + struct stat st;
> + if (fstat(fileno(f), &st))
> + skip_stdout_flush = 0;
> + else
> + skip_stdout_flush = S_ISREG(st.st_mode);
> + }
> }
> if (skip_stdout_flush && !ferror(f))
> return;
> --- >8 ---
Thanks for a nice reading - I can not imagine a better version.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable
2024-01-03 18:42 ` Torsten Bögershausen
@ 2024-01-03 19:18 ` Junio C Hamano
2024-01-04 17:35 ` Taylor Blau
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2024-01-03 19:18 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Taylor Blau, Patrick Steinhardt, Chandra Pratap via GitGitGadget,
git, Chandra Pratap, Chandra Pratap
Torsten Bögershausen <tboegi@web.de> writes:
>> - cp = getenv("GIT_FLUSH");
>> - if (cp)
>> - skip_stdout_flush = (atoi(cp) == 0);
>> - else if ((fstat(fileno(stdout), &st) == 0) &&
>> - S_ISREG(st.st_mode))
>> - skip_stdout_flush = 1;
>> - else
>> - skip_stdout_flush = 0;
>> + skip_stdout_flush = git_env_bool("GIT_FLUSH", -1);
>> + if (skip_stdout_flush < 0) {
>> + struct stat st;
>> + if (fstat(fileno(f), &st))
>> + skip_stdout_flush = 0;
>> + else
>> + skip_stdout_flush = S_ISREG(st.st_mode);
>> + }
>> }
>> if (skip_stdout_flush && !ferror(f))
>> return;
>> --- >8 ---
>
> Thanks for a nice reading - I can not imagine a better version.
Yup, the flow of the logic feels very natural with this version by
making it clear that the case that the default "-1" is returned to
us is where we need to figure out an appropriate value ourselves.
An added bonus is that the scope "struct stat" has is limited to the
absolute minimum. I like it, too.
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable
2024-01-03 19:18 ` Junio C Hamano
@ 2024-01-04 17:35 ` Taylor Blau
0 siblings, 0 replies; 13+ messages in thread
From: Taylor Blau @ 2024-01-04 17:35 UTC (permalink / raw)
To: Junio C Hamano
Cc: Torsten Bögershausen, Patrick Steinhardt,
Chandra Pratap via GitGitGadget, git, Chandra Pratap,
Chandra Pratap
On Wed, Jan 03, 2024 at 11:18:50AM -0800, Junio C Hamano wrote:
> > Thanks for a nice reading - I can not imagine a better version.
>
> Yup, the flow of the logic feels very natural with this version by
> making it clear that the case that the default "-1" is returned to
> us is where we need to figure out an appropriate value ourselves.
> An added bonus is that the scope "struct stat" has is limited to the
> absolute minimum. I like it, too.
Thanks, both.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable
2024-01-03 7:58 ` [PATCH v2] " Chandra Pratap via GitGitGadget
2024-01-03 8:22 ` Patrick Steinhardt
@ 2024-01-03 17:13 ` Junio C Hamano
2024-01-04 10:20 ` [PATCH v3] " Chandra Pratap via GitGitGadget
2 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2024-01-03 17:13 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
"Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Documentation/git.txt | 16 +++++++---------
> write-or-die.c | 9 +++------
> 2 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 2535a30194f..83fd60f2d11 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -724,16 +724,14 @@ for further details.
> waiting for someone with sufficient permissions to fix it.
>
> `GIT_FLUSH`::
> -// NEEDSWORK: make it into a usual Boolean environment variable
> - If this environment variable is set to "1", then commands such
> + If this Boolean environment variable is set to true, then commands such
> as 'git blame' (in incremental mode), 'git rev-list', 'git log',
> - 'git check-attr' and 'git check-ignore' will
> - force a flush of the output stream after each record have been
> - flushed. If this
> - variable is set to "0", the output of these commands will be done
> - using completely buffered I/O. If this environment variable is
> - not set, Git will choose buffered or record-oriented flushing
> - based on whether stdout appears to be redirected to a file or not.
> + 'git check-attr' and 'git check-ignore' will force a flush of the output
> + stream after each record have been flushed. If this variable is set to
> + false, the output of these commands will be done using completely buffered
> + I/O. If this environment variable is not set, Git will choose buffered or
> + record-oriented flushing based on whether stdout appears to be redirected
> + to a file or not.
It is somewhat irritating to see that we need to change this many
lines to just change "0" to "false" and "1" to "true". I wonder if
it becomes easier to grok if we changed the description into a sub
enumeration of three possibilities, but that would be outside the
scope of this change [*].
> diff --git a/write-or-die.c b/write-or-die.c
> index 42a2dc73cd3..a6acabd329f 100644
> --- a/write-or-die.c
> +++ b/write-or-die.c
> @@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)
> {
> static int skip_stdout_flush = -1;
> struct stat st;
> - char *cp;
>
> if (f == stdout) {
> if (skip_stdout_flush < 0) {
> - /* NEEDSWORK: make this a normal Boolean */
> - cp = getenv("GIT_FLUSH");
> - if (cp)
> - skip_stdout_flush = (atoi(cp) == 0);
> - else if ((fstat(fileno(stdout), &st) == 0) &&
> + if (!git_env_bool("GIT_FLUSH", -1))
> + skip_stdout_flush = 1;
> + else if (!fstat(fileno(stdout), &st) &&
> S_ISREG(st.st_mode))
> skip_stdout_flush = 1;
> else
The above logic does not look correct to me, primarily because the
return value of git_env_bool() is inspected only once to see if it
is zero, and does not differentiate the "unset" case from other
cases.
Since git_env_bool(k, def) returns
- "def" (-1 in this case) when k is not exported (in which case
you need to do the "fstat" dance).
- 0 when k is exported and has a string that is "false" (in
which case you would want to set skip_stdout_flush to true).
- 1 when k is exported and has a string that is "true" (in which
case you would want to set skip_stdout_flush to false).
- or dies if the string exported in k is bogus.
shouldn't it be more like
skip_stdout_flush = 0; /* assume flushing */
switch (git_env_bool("GIT_FLUSH", -1)) {
case 0: /* told not to flush */
skip_stdout_flush = 1;
;;
case -1: /* unspecified */
if (!fstat(...) && S_ISREG())
skip_stdout_flush = 1;
;;
default: /* told to flush */
;;
}
perhaps?
[Footnote]
* If I were to do this change, and if I were to also improve the
style of the documentation before I forget, the way I would do so
probably is with a two-patch series:
(1) update "0" and "1" in the documentation with "false" and
"true", without reflowing the text at all, and update the
code.
(2) rewrite the documentation to use 3-possibility
sub-enumeration for values (imitate the way how other
variables, like `diff.algorithm`, that can choose from a
set of handful possible values are described).
These two changes can be done in either order, but perhaps (1) is
much less controversial change than the other, so I'd probably do
so first.
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v3] write-or-die: make GIT_FLUSH a Boolean environment variable
2024-01-03 7:58 ` [PATCH v2] " Chandra Pratap via GitGitGadget
2024-01-03 8:22 ` Patrick Steinhardt
2024-01-03 17:13 ` Junio C Hamano
@ 2024-01-04 10:20 ` Chandra Pratap via GitGitGadget
2024-01-04 17:36 ` Taylor Blau
2024-01-04 18:35 ` Junio C Hamano
2 siblings, 2 replies; 13+ messages in thread
From: Chandra Pratap via GitGitGadget @ 2024-01-04 10:20 UTC (permalink / raw)
To: git; +Cc: Chandra Pratap, Chandra Pratap
From: Chandra Pratap <chandrapratap3519@gmail.com>
Among Git's environment variable, the ones marked as "Boolean"
accept values in a way similar to Boolean configuration variables,
i.e. values like 'yes', 'on', 'true' and positive numbers are
taken as "on" and values like 'no', 'off', 'false' are taken as
"off".
GIT_FLUSH can be used to force Git to use non-buffered I/O when
writing to stdout. It can only accept two values, '1' which causes
Git to flush more often and '0' which makes all output buffered.
Make GIT_FLUSH accept more values besides '0' and '1' by turning it
into a Boolean environment variable, modifying the required logic.
Update the related documentation.
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
write-or-die: make GIT_FLUSH a Boolean environment variable
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1628%2FChand-ra%2Fgit_flush-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1628/Chand-ra/git_flush-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1628
Range-diff vs v2:
1: 585c76fff68 ! 1: c98885c0ede write-or-die: make GIT_FLUSH a Boolean environment variable
@@ Documentation/git.txt: for further details.
- If this environment variable is set to "1", then commands such
+ If this Boolean environment variable is set to true, then commands such
as 'git blame' (in incremental mode), 'git rev-list', 'git log',
-- 'git check-attr' and 'git check-ignore' will
-- force a flush of the output stream after each record have been
-- flushed. If this
+ 'git check-attr' and 'git check-ignore' will
+ force a flush of the output stream after each record have been
+ flushed. If this
- variable is set to "0", the output of these commands will be done
-- using completely buffered I/O. If this environment variable is
-- not set, Git will choose buffered or record-oriented flushing
-- based on whether stdout appears to be redirected to a file or not.
-+ 'git check-attr' and 'git check-ignore' will force a flush of the output
-+ stream after each record have been flushed. If this variable is set to
-+ false, the output of these commands will be done using completely buffered
-+ I/O. If this environment variable is not set, Git will choose buffered or
-+ record-oriented flushing based on whether stdout appears to be redirected
-+ to a file or not.
-
- `GIT_TRACE`::
- Enables general trace messages, e.g. alias expansion, built-in
++ variable is set to false, the output of these commands will be done
+ using completely buffered I/O. If this environment variable is
+ not set, Git will choose buffered or record-oriented flushing
+ based on whether stdout appears to be redirected to a file or not.
## write-or-die.c ##
-@@ write-or-die.c: void maybe_flush_or_die(FILE *f, const char *desc)
+@@
+ void maybe_flush_or_die(FILE *f, const char *desc)
{
static int skip_stdout_flush = -1;
- struct stat st;
+- struct stat st;
- char *cp;
if (f == stdout) {
@@ write-or-die.c: void maybe_flush_or_die(FILE *f, const char *desc)
- if (cp)
- skip_stdout_flush = (atoi(cp) == 0);
- else if ((fstat(fileno(stdout), &st) == 0) &&
-+ if (!git_env_bool("GIT_FLUSH", -1))
-+ skip_stdout_flush = 1;
-+ else if (!fstat(fileno(stdout), &st) &&
- S_ISREG(st.st_mode))
- skip_stdout_flush = 1;
- else
+- S_ISREG(st.st_mode))
+- skip_stdout_flush = 1;
+- else
+- skip_stdout_flush = 0;
++ skip_stdout_flush = git_env_bool("GIT_FLUSH", -1);
++ if (skip_stdout_flush < 0) {
++ struct stat st;
++ if (fstat(fileno(stdout), &st))
++ skip_stdout_flush = 0;
++ else
++ skip_stdout_flush = S_ISREG(st.st_mode);
++ }
+ }
+ if (skip_stdout_flush && !ferror(f))
+ return;
Documentation/git.txt | 5 ++---
write-or-die.c | 19 ++++++++-----------
2 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 2535a30194f..d06eea024f7 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -724,13 +724,12 @@ for further details.
waiting for someone with sufficient permissions to fix it.
`GIT_FLUSH`::
-// NEEDSWORK: make it into a usual Boolean environment variable
- If this environment variable is set to "1", then commands such
+ If this Boolean environment variable is set to true, then commands such
as 'git blame' (in incremental mode), 'git rev-list', 'git log',
'git check-attr' and 'git check-ignore' will
force a flush of the output stream after each record have been
flushed. If this
- variable is set to "0", the output of these commands will be done
+ variable is set to false, the output of these commands will be done
using completely buffered I/O. If this environment variable is
not set, Git will choose buffered or record-oriented flushing
based on whether stdout appears to be redirected to a file or not.
diff --git a/write-or-die.c b/write-or-die.c
index 42a2dc73cd3..39421528653 100644
--- a/write-or-die.c
+++ b/write-or-die.c
@@ -19,20 +19,17 @@
void maybe_flush_or_die(FILE *f, const char *desc)
{
static int skip_stdout_flush = -1;
- struct stat st;
- char *cp;
if (f == stdout) {
if (skip_stdout_flush < 0) {
- /* NEEDSWORK: make this a normal Boolean */
- cp = getenv("GIT_FLUSH");
- if (cp)
- skip_stdout_flush = (atoi(cp) == 0);
- else if ((fstat(fileno(stdout), &st) == 0) &&
- S_ISREG(st.st_mode))
- skip_stdout_flush = 1;
- else
- skip_stdout_flush = 0;
+ skip_stdout_flush = git_env_bool("GIT_FLUSH", -1);
+ if (skip_stdout_flush < 0) {
+ struct stat st;
+ if (fstat(fileno(stdout), &st))
+ skip_stdout_flush = 0;
+ else
+ skip_stdout_flush = S_ISREG(st.st_mode);
+ }
}
if (skip_stdout_flush && !ferror(f))
return;
base-commit: 1a87c842ece327d03d08096395969aca5e0a6996
--
gitgitgadget
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v3] write-or-die: make GIT_FLUSH a Boolean environment variable
2024-01-04 10:20 ` [PATCH v3] " Chandra Pratap via GitGitGadget
@ 2024-01-04 17:36 ` Taylor Blau
2024-01-04 18:35 ` Junio C Hamano
1 sibling, 0 replies; 13+ messages in thread
From: Taylor Blau @ 2024-01-04 17:36 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
Hi Chandra,
On Thu, Jan 04, 2024 at 10:20:17AM +0000, Chandra Pratap via GitGitGadget wrote:
> ---
> Documentation/git.txt | 5 ++---
> write-or-die.c | 19 ++++++++-----------
> 2 files changed, 10 insertions(+), 14 deletions(-)
Thanks very much for working on this, and taking some of my suggestions
into account.
This version looks great to me.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] write-or-die: make GIT_FLUSH a Boolean environment variable
2024-01-04 10:20 ` [PATCH v3] " Chandra Pratap via GitGitGadget
2024-01-04 17:36 ` Taylor Blau
@ 2024-01-04 18:35 ` Junio C Hamano
1 sibling, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2024-01-04 18:35 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
"Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Chandra Pratap <chandrapratap3519@gmail.com>
>
> Among Git's environment variable, the ones marked as "Boolean"
> accept values in a way similar to Boolean configuration variables,
With "Among" you probably mean that there are many and some of them
are "marked as 'Boolean'", so I'd do "variable" -> "variables" while
queuing.
Other than that, looks great. Will queue. Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread