diff mbox series

[v3] write-or-die: make GIT_FLUSH a Boolean environment variable

Message ID pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 556e68032f8248c831e48207e5cb923c9fe0e42c
Headers show
Series [v3] write-or-die: make GIT_FLUSH a Boolean environment variable | expand

Commit Message

Chandra Pratap Jan. 4, 2024, 10:20 a.m. UTC 
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(-)


base-commit: 1a87c842ece327d03d08096395969aca5e0a6996

Comments

Taylor Blau Jan. 4, 2024, 5:36 p.m. UTC | #1 
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
Junio C Hamano Jan. 4, 2024, 6:35 p.m. UTC | #2 
"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.
diff mbox series

Patch

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;