On 12/02/2024 17:11, Junio C Hamano wrote:
Xiaoguang WANG <wxiaoguang@xxxxxxxxx> writes:
If GIT_FLUSH=true, it should mean to "do the flush". But that commit
made skip_stdout_flush=true when GIT_FLUSH=true.
Thanks for reporting. I am surprised that this flipping of polarity
slipped through.
And by the way, only accepting GIT_FLUSH=true is quite breaking, it
drops the compatibility of GIT_FLUSH=1
I do not think so. If the polarity is corrected, git_env_bool()
would say "that's affirmative" when any one of the "1", "true",
"yes", "on", etc. is given. If you have been passing "1", you
should get the "always flush" behaviour.
Perhaps something like this would fix it?
write-or-die.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git c/write-or-die.c w/write-or-die.c
index 3942152865..3ecb9e2af5 100644
--- c/write-or-die.c
+++ w/write-or-die.c
@@ -22,8 +22,11 @@ void maybe_flush_or_die(FILE *f, const char *desc)
if (f == stdout) {
if (skip_stdout_flush < 0) {
- skip_stdout_flush = git_env_bool("GIT_FLUSH", -1);
- if (skip_stdout_flush < 0) {
+ int flush_setting = git_env_bool("GIT_FLUSH", -1);
+
+ if (0 <= flush_setting)
+ skip_stdout_flush = !flush_setting;
+ else {
struct stat st;
if (fstat(fileno(stdout), &st))
skip_stdout_flush = 0;
Given we're in a rc-period a minimal fix like this looks appropriate
(though it is missing some braces according to our coding
guidelines). The interaction of "skip_stdout_flush" and git_env_bool()
is unfortunate, It might be clearer if we changed to having
"force_stdout_flush" instead but that would be a more invasive change.
Best Wishes
Phillip
diff --git a/write-or-die.c b/write-or-die.c
index 39421528653..e68265a94a6 100644
--- a/write-or-die.c
+++ b/write-or-die.c
@@ -18,20 +18,20 @@
*/
void maybe_flush_or_die(FILE *f, const char *desc)
{
- static int skip_stdout_flush = -1;
+ static int force_stdout_flush = -1;
if (f == stdout) {
- if (skip_stdout_flush < 0) {
- skip_stdout_flush = git_env_bool("GIT_FLUSH", -1);
- if (skip_stdout_flush < 0) {
+ if (force_stdout_flush < 0) {
+ force_stdout_flush = git_env_bool("GIT_FLUSH", -1);
+ if (force_stdout_flush < 0) {
struct stat st;
if (fstat(fileno(stdout), &st))
- skip_stdout_flush = 0;
+ force_stdout_flush = 1;
else
- skip_stdout_flush = S_ISREG(st.st_mode);
+ force_stdout_flush = !S_ISREG(st.st_mode);
}
}
- if (skip_stdout_flush && !ferror(f))
+ if (!force_stdout_flush && !ferror(f))
return;
}
if (fflush(f)) {
