git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] clean: flush after each line
@ 2023-02-01 10:09 Orgad Shaneh via GitGitGadget
  2023-02-01 17:45 ` Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: Orgad Shaneh via GitGitGadget @ 2023-02-01 10:09 UTC (permalink / raw)
  To: git; +Cc: Orgad Shaneh, Orgad Shaneh

From: Orgad Shaneh <orgads@gmail.com>

Some platforms don't automatically flush after \n, and this causes delay
of the output, and also sometimes incomplete file names appear until the
next chunk is flushed.

Reported here: https://github.com/git-for-windows/git/issues/3706

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
    clean: flush after each line

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1447%2Forgads%2Fclean-flush-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1447/orgads/clean-flush-v1
Pull-Request: https://github.com/git/git/pull/1447

 builtin/clean.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index b2701a28158..f3de8170f9a 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -270,8 +270,10 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 
 	if (!*dir_gone && !quiet) {
 		int i;
-		for (i = 0; i < dels.nr; i++)
+		for (i = 0; i < dels.nr; i++) {
 			printf(dry_run ?  _(msg_would_remove) : _(msg_remove), dels.items[i].string);
+			fflush(stdout);
+		}
 	}
 out:
 	strbuf_release(&realpath);
@@ -544,6 +546,7 @@ static int parse_choice(struct menu_stuff *menu_stuff,
 			clean_print_color(CLEAN_COLOR_ERROR);
 			printf(_("Huh (%s)?\n"), (*ptr)->buf);
 			clean_print_color(CLEAN_COLOR_RESET);
+			fflush(stdout);
 			continue;
 		}
 

base-commit: 2fc9e9ca3c7505bc60069f11e7ef09b1aeeee473
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] clean: flush after each line
  2023-02-01 10:09 [PATCH] clean: flush after each line Orgad Shaneh via GitGitGadget
@ 2023-02-01 17:45 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2023-02-01 17:45 UTC (permalink / raw)
  To: Orgad Shaneh via GitGitGadget; +Cc: git, Orgad Shaneh

"Orgad Shaneh via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Orgad Shaneh <orgads@gmail.com>
>
> Some platforms don't automatically flush after \n, and this causes delay
> of the output, and also sometimes incomplete file names appear until the
> next chunk is flushed.
>
> Reported here: https://github.com/git-for-windows/git/issues/3706
>
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>
> ---
>     clean: flush after each line

We do not flush after every line when producing output from "git
diff", "git status".  I do not want to see "git clean" special
cased, as such a solution will not scale.

> diff --git a/builtin/clean.c b/builtin/clean.c
> index b2701a28158..f3de8170f9a 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -270,8 +270,10 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
>  
>  	if (!*dir_gone && !quiet) {
>  		int i;
> -		for (i = 0; i < dels.nr; i++)
> +		for (i = 0; i < dels.nr; i++) {
>  			printf(dry_run ?  _(msg_would_remove) : _(msg_remove), dels.items[i].string);
> +			fflush(stdout);
> +		}

If the standard output is connected to an interactive terminal, this
might make sense (but then that equally applies to "git status",
"git diff" and all other commands), but shouldn't the stdout follow
the simple "Output streams that refer to terminal devices are always
line buffered by default" rule?

I think this should be fixed at the platform level, either by
talking to the platform maintainers.  An acceptable workaround may
be to have an #ifdef'ed hack early in our start-up code, e.g.

	void sanitize_stdfds(void)
	{
		int fd = ...;
		if (fd > 2)
			close(fd);
	#ifdef BUGGY_STDOUT_FULLY_BUFFERED
		if (isatty(1))
			setlinebuf(stdout);
	#endif
	}

somewhere that is reached early from common-main.c::main().

That way, we do not have to carry a special-case in builtin/clean.c
and watch out for other commands that produce multiple lines of
output start needing a workaround on platforms with such a buffering
behaviour.

> @@ -544,6 +546,7 @@ static int parse_choice(struct menu_stuff *menu_stuff,
>  			clean_print_color(CLEAN_COLOR_ERROR);
>  			printf(_("Huh (%s)?\n"), (*ptr)->buf);
>  			clean_print_color(CLEAN_COLOR_RESET);
> +			fflush(stdout);
>  			continue;

This is clearly interactive codepath, and I do not think we mind an
extra fflush().  But it would be redundant if we fix the stdio on
such a platform.

>  		}
>  
>
> base-commit: 2fc9e9ca3c7505bc60069f11e7ef09b1aeeee473

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-02-01 17:45 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-01 10:09 [PATCH] clean: flush after each line Orgad Shaneh via GitGitGadget
2023-02-01 17:45 ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).