{"thread":{"id":"63808","subject":"[PATCH] revision: drop early output option","startedAt":"2025-07-19T07:08:14Z","lastAt":"2025-07-21T16:28:10Z","messageCount":2,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"522312","messageId":"20250719070813.GA706382@coredump.intra.peff.net","threadId":"63808","inReplyTo":null,"subject":"[PATCH] revision: drop early output option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-07-19T07:08:13Z","receivedAt":"2025-07-19T07:08:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We added the --early-output feature long ago in cdcefbc971 (Add\n\"--early-output\" log flag for interactive GUI use, 2007-11-03). The idea\nwas that GUIs could use it to progressively render a history view,\nshowing something quick-and-inaccurate at first and then enhancing it\nlater.\n\nBut we never documented it, and it appears never to have been used, even\nby the projects which initially expressed interest. There was an RFC\npatch for gitk to use it:\n\n  http://public-inbox.org/git/18221.2285.259487.655684@cargo.ozlabs.ibm.com/\n\nbut it was never merged. Likewise QGit had a patch in:\n\n  https://lore.kernel.org/git/e5bfff550711040225ne67c907r2023b1354c35f35@mail.gmail.com/\n\nbut it was never fully merged (to this day, QGit has a commented-out line to\nadd \"--early-output\" to the \"log\" invocation). Searching for other\nmentions on the web or forges like github.com turns up nothing.\n\nMeanwhile, the feature has been broken off and on over the years without\nanybody noticing (and naturally, there are no tests, either). From 2011\nto 2017 the option didn't even turn on via \"--early-output\"; this was\nfixed in e35b6ac56f (revision.h: turn rev_info.early_output back into an\nunsigned int, 2017-06-10).\n\nIt worked for a while then, but it does not interact well at all with\ncommit-graphs (which are turned on by default these days). The main\nlogic to count early commits is triggered by limit_list(), which we\ntraditionally invoked when showing output in topo-order (and\n--early-output always enables --topo-order). But that changed in\nf0d9cc4196 (revision.c: begin refactoring --topo-order logic,\n2018-11-01). Now when we have generation numbers, we skip limit_list()\nentirely, and the early-output code shows no commits, and just the final\nheader \"Final output: 1 done\". Which is syntactically OK, but\nsemantically wrong: that message should give the total number of commits\nwe're about to show.\n\nSo let's drop the feature. It is extra code that is untested and\nundocumented, and makes working on the revision machinery more brittle.\n\nGiven the history above, it seems unlikely that anybody is using it (or\nhas used it), and we can drop it without the usual deprecation period.\n\nA gentler option might be to \"soft\" drop it: keep accepting the option,\nhave it imply --topo-order as it does now, print \"Final output: 1 done\",\nand then do our regular traversal. That would keep any hypothetical\ncaller working. But it doesn't seem worth the hassle to me.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nMy ulterior motive is to make changes to the revision code, and I do not\nwant to drag this (untested!) code along. A lengthy deprecation period\nwould make that more annoying. I feel like the arguments above are\nreasonable, or am I just being lazy and impatient?\n\nThe \"soft\" version would not be too hard to do, if we find that more\npalatable.\n\n builtin/log.c | 129 --------------------------------------------------\n revision.c    |  17 -------\n revision.h    |   8 ----\n 3 files changed, 154 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 24a57c20a4..fb42e094af 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -391,145 +391,16 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \tcmd_log_init_finish(argc, argv, prefix, rev, opt, cfg);\n }\n \n-/*\n- * This gives a rough estimate for how many commits we\n- * will print out in the list.\n- */\n-static int estimate_commit_count(struct commit_list *list)\n-{\n-\tint n = 0;\n-\n-\twhile (list) {\n-\t\tstruct commit *commit = list->item;\n-\t\tunsigned int flags = commit->object.flags;\n-\t\tlist = list->next;\n-\t\tif (!(flags & (TREESAME | UNINTERESTING)))\n-\t\t\tn++;\n-\t}\n-\treturn n;\n-}\n-\n-static void show_early_header(struct rev_info *rev, const char *stage, int nr)\n-{\n-\tif (rev->shown_one) {\n-\t\trev->shown_one = 0;\n-\t\tif (rev->commit_format != CMIT_FMT_ONELINE)\n-\t\t\tputchar(rev->diffopt.line_termination);\n-\t}\n-\tfprintf(rev->diffopt.file, _(\"Final output: %d %s\\n\"), nr, stage);\n-}\n-\n-static struct itimerval early_output_timer;\n-\n-static void log_show_early(struct rev_info *revs, struct commit_list *list)\n-{\n-\tint i = revs->early_output;\n-\tint show_header = 1;\n-\tint no_free = revs->diffopt.no_free;\n-\n-\trevs->diffopt.no_free = 0;\n-\tsort_in_topological_order(&list, revs->sort_order);\n-\twhile (list && i) {\n-\t\tstruct commit *commit = list->item;\n-\t\tswitch (simplify_commit(revs, commit)) {\n-\t\tcase commit_show:\n-\t\t\tif (show_header) {\n-\t\t\t\tint n = estimate_commit_count(list);\n-\t\t\t\tshow_early_header(revs, \"incomplete\", n);\n-\t\t\t\tshow_header = 0;\n-\t\t\t}\n-\t\t\tlog_tree_commit(revs, commit);\n-\t\t\ti--;\n-\t\t\tbreak;\n-\t\tcase commit_ignore:\n-\t\t\tbreak;\n-\t\tcase commit_error:\n-\t\t\trevs->diffopt.no_free = no_free;\n-\t\t\tdiff_free(&revs->diffopt);\n-\t\t\treturn;\n-\t\t}\n-\t\tlist = list->next;\n-\t}\n-\n-\t/* Did we already get enough commits for the early output? */\n-\tif (!i) {\n-\t\trevs->diffopt.no_free = 0;\n-\t\tdiff_free(&revs->diffopt);\n-\t\treturn;\n-\t}\n-\n-\t/*\n-\t * ..if no, then repeat it twice a second until we\n-\t * do.\n-\t *\n-\t * NOTE! We don't use \"it_interval\", because if the\n-\t * reader isn't listening, we want our output to be\n-\t * throttled by the writing, and not have the timer\n-\t * trigger every second even if we're blocked on a\n-\t * reader!\n-\t */\n-\tearly_output_timer.it_value.tv_sec = 0;\n-\tearly_output_timer.it_value.tv_usec = 500000;\n-\tsetitimer(ITIMER_REAL, &early_output_timer, NULL);\n-}\n-\n-static void early_output(int signal UNUSED)\n-{\n-\tshow_early_output = log_show_early;\n-}\n-\n-static void setup_early_output(void)\n-{\n-\tstruct sigaction sa;\n-\n-\t/*\n-\t * Set up the signal handler, minimally intrusively:\n-\t * we only set a single volatile integer word (not\n-\t * using sigatomic_t - trying to avoid unnecessary\n-\t * system dependencies and headers), and using\n-\t * SA_RESTART.\n-\t */\n-\tmemset(&sa, 0, sizeof(sa));\n-\tsa.sa_handler = early_output;\n-\tsigemptyset(&sa.sa_mask);\n-\tsa.sa_flags = SA_RESTART;\n-\tsigaction(SIGALRM, &sa, NULL);\n-\n-\t/*\n-\t * If we can get the whole output in less than a\n-\t * tenth of a second, don't even bother doing the\n-\t * early-output thing..\n-\t *\n-\t * This is a one-time-only trigger.\n-\t */\n-\tearly_output_timer.it_value.tv_sec = 0;\n-\tearly_output_timer.it_value.tv_usec = 100000;\n-\tsetitimer(ITIMER_REAL, &early_output_timer, NULL);\n-}\n-\n-static void finish_early_output(struct rev_info *rev)\n-{\n-\tint n = estimate_commit_count(rev->commits);\n-\tsignal(SIGALRM, SIG_IGN);\n-\tshow_early_header(rev, \"done\", n);\n-}\n-\n static int cmd_log_walk_no_free(struct rev_info *rev)\n {\n \tstruct commit *commit;\n \tint saved_nrl = 0;\n \tint saved_dcctc = 0;\n \tint result;\n \n-\tif (rev->early_output)\n-\t\tsetup_early_output();\n-\n \tif (prepare_revision_walk(rev))\n \t\tdie(_(\"revision walk setup failed\"));\n \n-\tif (rev->early_output)\n-\t\tfinish_early_output(rev);\n-\n \t/*\n \t * For --check and --exit-code, the exit code is based on CHECK_FAILED\n \t * and HAS_CHANGES being accumulated in rev->diffopt, so be careful to\ndiff --git a/revision.c b/revision.c\nindex 0ca6a297a6..1ba8eb8a1c 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -50,8 +50,6 @@\n #include \"parse-options.h\"\n #include \"wildmatch.h\"\n \n-volatile show_early_output_fn_t show_early_output;\n-\n static char *term_bad;\n static char *term_good;\n \n@@ -1479,7 +1477,6 @@ static int limit_list(struct rev_info *revs)\n \twhile (original_list) {\n \t\tstruct commit *commit = pop_commit(&original_list);\n \t\tstruct object *obj = &commit->object;\n-\t\tshow_early_output_fn_t show;\n \n \t\tif (commit == interesting_cache)\n \t\t\tinteresting_cache = NULL;\n@@ -1503,13 +1500,6 @@ static int limit_list(struct rev_info *revs)\n \t\t\tcontinue;\n \t\tdate = commit->date;\n \t\tp = &commit_list_insert(commit, p)->next;\n-\n-\t\tshow = show_early_output;\n-\t\tif (!show)\n-\t\t\tcontinue;\n-\n-\t\tshow(revs, newlist);\n-\t\tshow_early_output = NULL;\n \t}\n \tif (revs->cherry_pick || revs->cherry_mark)\n \t\tcherry_pick_list(newlist, revs);\n@@ -2443,13 +2433,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--author-date-order\")) {\n \t\trevs->sort_order = REV_SORT_BY_AUTHOR_DATE;\n \t\trevs->topo_order = 1;\n-\t} else if (!strcmp(arg, \"--early-output\")) {\n-\t\trevs->early_output = 100;\n-\t\trevs->topo_order = 1;\n-\t} else if (skip_prefix(arg, \"--early-output=\", &optarg)) {\n-\t\tif (strtoul_ui(optarg, 10, &revs->early_output) < 0)\n-\t\t\tdie(\"'%s': not a non-negative integer\", optarg);\n-\t\trevs->topo_order = 1;\n \t} else if (!strcmp(arg, \"--parents\")) {\n \t\trevs->rewrite_parents = 1;\n \t\trevs->print_parents = 1;\ndiff --git a/revision.h b/revision.h\nindex 6d369cdad6..fe93567e89 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -160,8 +160,6 @@ struct rev_info {\n \t/* topo-sort */\n \tenum rev_sort_order sort_order;\n \n-\tunsigned int early_output;\n-\n \tunsigned int\tignore_missing:1,\n \t\t\tignore_missing_links:1;\n \n@@ -553,10 +551,4 @@ int rewrite_parents(struct rev_info *revs,\n  */\n struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);\n \n-/**\n- * Global for the (undocumented) \"--early-output\" flag for \"git log\".\n- */\n-typedef void (*show_early_output_fn_t)(struct rev_info *, struct commit_list *);\n-extern volatile show_early_output_fn_t show_early_output;\n-\n #endif\n-- \n2.50.1.589.g6e88b11be3\n"},{"id":"522361","messageId":"xmqqjz41zey1.fsf@gitster.g","threadId":"63808","inReplyTo":"20250719070813.GA706382@coredump.intra.peff.net","subject":"Re: [PATCH] revision: drop early output option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-21T16:28:06Z","receivedAt":"2025-07-21T16:28:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> We added the --early-output feature long ago in cdcefbc971 (Add\n> \"--early-output\" log flag for interactive GUI use, 2007-11-03). The idea\n> was that GUIs could use it to progressively render a history view,\n> showing something quick-and-inaccurate at first and then enhancing it\n> later.\n>\n> But we never documented it, and it appears never to have been used, even\n> by the projects which initially expressed interest. There was an RFC\n> patch for gitk to use it:\n>\n>   http://public-inbox.org/git/18221.2285.259487.655684@cargo.ozlabs.ibm.com/\n>\n> but it was never merged.\n\nAh, that one I remember.\n\n> So let's drop the feature. It is extra code that is untested and\n> undocumented, and makes working on the revision machinery more brittle.\n>\n> Given the history above, it seems unlikely that anybody is using it (or\n> has used it), and we can drop it without the usual deprecation period.\n>\n> A gentler option might be to \"soft\" drop it: keep accepting the option,\n> have it imply --topo-order as it does now, print \"Final output: 1 done\",\n> and then do our regular traversal. That would keep any hypothetical\n> caller working. But it doesn't seem worth the hassle to me.\n\nTrue.\n\nThe safety of dropping of it (instead of fixing) relies on it not\nbeing used at all, so using that same assumption that nobody even\nattempts to use it by passing the option, would be a sensible thing\nto do.\n\nWill queue.  Thanks.\n"}]}