{"thread":{"id":"59172","subject":"[PATCH] clean: flush after each line","startedAt":"2023-02-01T10:09:24Z","lastAt":"2023-02-01T17:45:33Z","messageCount":2,"participants":["Orgad Shaneh via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"471222","messageId":"pull.1447.git.git.1675246158282.gitgitgadget@gmail.com","threadId":"59172","inReplyTo":null,"subject":"[PATCH] clean: flush after each line","fromName":"Orgad Shaneh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-02-01T10:09:18Z","receivedAt":"2023-02-01T10:09:24Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"From: Orgad Shaneh <orgads@gmail.com>\n\nSome platforms don't automatically flush after \\n, and this causes delay\nof the output, and also sometimes incomplete file names appear until the\nnext chunk is flushed.\n\nReported here: https://github.com/git-for-windows/git/issues/3706\n\nSigned-off-by: Orgad Shaneh <orgads@gmail.com>\n---\n    clean: flush after each line\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1447%2Forgads%2Fclean-flush-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1447/orgads/clean-flush-v1\nPull-Request: https://github.com/git/git/pull/1447\n\n builtin/clean.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex b2701a28158..f3de8170f9a 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -270,8 +270,10 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \n \tif (!*dir_gone && !quiet) {\n \t\tint i;\n-\t\tfor (i = 0; i < dels.nr; i++)\n+\t\tfor (i = 0; i < dels.nr; i++) {\n \t\t\tprintf(dry_run ?  _(msg_would_remove) : _(msg_remove), dels.items[i].string);\n+\t\t\tfflush(stdout);\n+\t\t}\n \t}\n out:\n \tstrbuf_release(&realpath);\n@@ -544,6 +546,7 @@ static int parse_choice(struct menu_stuff *menu_stuff,\n \t\t\tclean_print_color(CLEAN_COLOR_ERROR);\n \t\t\tprintf(_(\"Huh (%s)?\\n\"), (*ptr)->buf);\n \t\t\tclean_print_color(CLEAN_COLOR_RESET);\n+\t\t\tfflush(stdout);\n \t\t\tcontinue;\n \t\t}\n \n\nbase-commit: 2fc9e9ca3c7505bc60069f11e7ef09b1aeeee473\n-- \ngitgitgadget\n"},{"id":"471255","messageId":"xmqqedr91dqf.fsf@gitster.g","threadId":"59172","inReplyTo":"pull.1447.git.git.1675246158282.gitgitgadget@gmail.com","subject":"Re: [PATCH] clean: flush after each line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-01T17:45:28Z","receivedAt":"2023-02-01T17:45:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Orgad Shaneh via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Orgad Shaneh <orgads@gmail.com>\n>\n> Some platforms don't automatically flush after \\n, and this causes delay\n> of the output, and also sometimes incomplete file names appear until the\n> next chunk is flushed.\n>\n> Reported here: https://github.com/git-for-windows/git/issues/3706\n>\n> Signed-off-by: Orgad Shaneh <orgads@gmail.com>\n> ---\n>     clean: flush after each line\n\nWe do not flush after every line when producing output from \"git\ndiff\", \"git status\".  I do not want to see \"git clean\" special\ncased, as such a solution will not scale.\n\n> diff --git a/builtin/clean.c b/builtin/clean.c\n> index b2701a28158..f3de8170f9a 100644\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n> @@ -270,8 +270,10 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n>  \n>  \tif (!*dir_gone && !quiet) {\n>  \t\tint i;\n> -\t\tfor (i = 0; i < dels.nr; i++)\n> +\t\tfor (i = 0; i < dels.nr; i++) {\n>  \t\t\tprintf(dry_run ?  _(msg_would_remove) : _(msg_remove), dels.items[i].string);\n> +\t\t\tfflush(stdout);\n> +\t\t}\n\nIf the standard output is connected to an interactive terminal, this\nmight make sense (but then that equally applies to \"git status\",\n\"git diff\" and all other commands), but shouldn't the stdout follow\nthe simple \"Output streams that refer to terminal devices are always\nline buffered by default\" rule?\n\nI think this should be fixed at the platform level, either by\ntalking to the platform maintainers.  An acceptable workaround may\nbe to have an #ifdef'ed hack early in our start-up code, e.g.\n\n\tvoid sanitize_stdfds(void)\n\t{\n\t\tint fd = ...;\n\t\tif (fd > 2)\n\t\t\tclose(fd);\n\t#ifdef BUGGY_STDOUT_FULLY_BUFFERED\n\t\tif (isatty(1))\n\t\t\tsetlinebuf(stdout);\n\t#endif\n\t}\n\nsomewhere that is reached early from common-main.c::main().\n\nThat way, we do not have to carry a special-case in builtin/clean.c\nand watch out for other commands that produce multiple lines of\noutput start needing a workaround on platforms with such a buffering\nbehaviour.\n\n> @@ -544,6 +546,7 @@ static int parse_choice(struct menu_stuff *menu_stuff,\n>  \t\t\tclean_print_color(CLEAN_COLOR_ERROR);\n>  \t\t\tprintf(_(\"Huh (%s)?\\n\"), (*ptr)->buf);\n>  \t\t\tclean_print_color(CLEAN_COLOR_RESET);\n> +\t\t\tfflush(stdout);\n>  \t\t\tcontinue;\n\nThis is clearly interactive codepath, and I do not think we mind an\nextra fflush().  But it would be redundant if we fix the stdio on\nsuch a platform.\n\n>  \t\t}\n>  \n>\n> base-commit: 2fc9e9ca3c7505bc60069f11e7ef09b1aeeee473\n"}]}