{"thread":{"id":"66718","subject":"[PATCH 0/4] mark symbols that are unnecessarily public static","startedAt":"2026-10-30T17:39:52Z","lastAt":"2026-11-04T06:41:16Z","messageCount":14,"participants":["Junio C Hamano","Jeff King","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"556941","messageId":"20261030173956.585250-1-gitster@pobox.com","threadId":"66718","inReplyTo":null,"subject":"[PATCH 0/4] mark symbols that are unnecessarily public static","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-30T17:39:52Z","receivedAt":"2026-10-30T17:39:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Over the years, refactoring various subsystems, adding new callers,\nand rewriting them from scratch, made some symbols that started as\nstatic into global, and some other symbols that started as global,\nno longer need to be visible outside a file that use them. Here is\na small patch series that turns them back into file-scope static.\n\nHonestly, I suspect that [3/4] was probably better with a forward\ndeclaration in the preview I sent earlier, but in the longer term,\nreaders would not care if this conversion ended up in a large code\nmovement, as they do not have to even know it.\n\n 1/4: ident: remove unused author_ident_sufficiently_given()\n 2/4: add -i: hide init/clear_add_i_state() functions\n 3/4: read-cache: hide refresh_cache_entry()\n 4/4: unpack-trees: hide verify_uptodate()\n\n add-interactive.c | 11 +-\n add-interactive.h | 9 --\n ident.c | 5 -\n ident.h | 1 -\n read-cache-ll.h | 2 -\n read-cache.c | 258 +++++++++++++++++++++++-----------------------\n unpack-trees.c | 4 +-\n unpack-trees.h | 3 -\n 8 files changed, 139 insertions(+), 154 deletions(-)\n\n-- \n2.56.0-486-gd49ba3b80a\n\n\n"},{"id":"556942","messageId":"20261030173956.585250-2-gitster@pobox.com","threadId":"66718","inReplyTo":"20261030173956.585250-1-gitster@pobox.com","subject":"[PATCH 1/4] ident: remove unused author_ident_sufficiently_given()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-30T17:39:53Z","receivedAt":"2026-10-30T17:39:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A pair of functions {author,committer}_ident_sufficiently_given()\nwere created to replace user_ident_explicitly_given() in d6991ceedc\n(ident: keep separate \"explicit\" flags for author and committer,\n2012-11-14).\n\nBut author_ident_sufficiently_given() is never used in our history.\nWe only check if committer ident is sufficiently given and use it to\nnudge the user to set user.{name,email} configuration variables.\n\nRemove the unused author_ident_sufficiently_given() function.\n\nSigned-off-by: Junio C Hamano \n---\n ident.c | 5 -----\n ident.h | 1 -\n 2 files changed, 6 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 0b7aacecd7..eb73d073d8 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -602,11 +602,6 @@ int committer_ident_sufficiently_given(void)\n \treturn ident_is_sufficient(committer_ident_explicitly_given);\n }\n \n-int author_ident_sufficiently_given(void)\n-{\n-\treturn ident_is_sufficient(author_ident_explicitly_given);\n-}\n-\n static int set_ident(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"author.name\")) {\ndiff --git a/ident.h b/ident.h\nindex 3c03403879..7e60e36415 100644\n--- a/ident.h\n+++ b/ident.h\n@@ -60,7 +60,6 @@ const char *fmt_ident(const char *name, const char *email,\n const char *fmt_name(enum want_ident);\n \n int committer_ident_sufficiently_given(void);\n-int author_ident_sufficiently_given(void);\n \n struct config_context;\n int git_ident_config(const char *, const char *, const struct config_context *,\n-- \n2.56.0-486-gd49ba3b80a\n\n\n"},{"id":"556943","messageId":"20261030173956.585250-3-gitster@pobox.com","threadId":"66718","inReplyTo":"20261030173956.585250-1-gitster@pobox.com","subject":"[PATCH 2/4] add -i: hide init/clear_add_i_state() functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-30T17:39:54Z","receivedAt":"2026-10-30T17:39:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"add -i\" machinery is encapsulated in run_add_i() function,\nwhich uses init_add_i_state() to initialize the machinery, does its\nthing, and clear_add_i_state() to tear it down. External callers\nhave no need to call anything other than run_add_i(), starting at\nd51b61f5da (add-patch: remove dependency on \"add-interactive\"\nsubsystem, 2026-03-02) refactored the code paths involved.\n\nMake init/clear_add_i_state() file-scope static functions, and\n\"struct add_i_state\" type as well.\n\nSigned-off-by: Junio C Hamano \n---\n add-interactive.c | 11 ++++++++---\n add-interactive.h | 9 ---------\n 2 files changed, 8 insertions(+), 12 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 3cf8a1dbf8..9a00daf9f9 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -19,14 +19,19 @@\n #include \"prompt.h\"\n #include \"tree.h\"\n \n-void init_add_i_state(struct add_i_state *s, struct repository *r,\n-\t\t struct interactive_options *opts)\n+struct add_i_state {\n+\tstruct repository *r;\n+\tstruct interactive_config cfg;\n+};\n+\n+static void init_add_i_state(struct add_i_state *s, struct repository *r,\n+\t\t\t struct interactive_options *opts)\n {\n \ts->r = r;\n \tinteractive_config_init(&s->cfg, r, opts);\n }\n \n-void clear_add_i_state(struct add_i_state *s)\n+static void clear_add_i_state(struct add_i_state *s)\n {\n \tinteractive_config_clear(&s->cfg);\n \tmemset(s, 0, sizeof(*s));\ndiff --git a/add-interactive.h b/add-interactive.h\nindex eefa2edc7c..47f67ede82 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -6,15 +6,6 @@\n struct pathspec;\n struct repository;\n \n-struct add_i_state {\n-\tstruct repository *r;\n-\tstruct interactive_config cfg;\n-};\n-\n-void init_add_i_state(struct add_i_state *s, struct repository *r,\n-\t\t struct interactive_options *opts);\n-void clear_add_i_state(struct add_i_state *s);\n-\n int run_add_i(struct repository *r, const struct pathspec *ps,\n \t struct interactive_options *opts);\n \n-- \n2.56.0-486-gd49ba3b80a\n\n\n"},{"id":"556944","messageId":"20261030173956.585250-4-gitster@pobox.com","threadId":"66718","inReplyTo":"20261030173956.585250-1-gitster@pobox.com","subject":"[PATCH 3/4] read-cache: hide refresh_cache_entry()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-30T17:39:55Z","receivedAt":"2026-10-30T17:39:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Once upon a time, there was a merge strategy called \"recursive\",\nwhich was added in 6d297f8137 (Status update on merge-recursive in\nC, 2006-07-08). This implementation came with its own machinery\nthat creates an in-core index entry, and then refreshed the index\nentry it created by calling the refresh_cache_entry() function\nborrowed from the read-cache.c API. refresh_cache_entry() used to\nbe file-scope static, but was made public with this change.\n\nWhen ad45b327c0 (merge-recursive.[ch]: thoroughly debug these,\n2025-04-08) finally removed the entire code for the \"recursive\"\nmerge strategy, the last external caller of refresh_cache_entry()\nwent with it.\n\nHide the low-level implementation detail again by making it a\nfile-scope static function. The external callers would indirectly\ncall it when they call make_cache_entry() or rename_index_entry_at()\nexternal interface, which \"ort\" (the successor of \"recursive\") does.\n\nSigned-off-by: Junio C Hamano \n---\n read-cache-ll.h | 2 -\n read-cache.c | 258 ++++++++++++++++++++++++------------------------\n 2 files changed, 129 insertions(+), 131 deletions(-)\n\ndiff --git a/read-cache-ll.h b/read-cache-ll.h\nindex 066dd8bc3b..dd30befbb3 100644\n--- a/read-cache-ll.h\n+++ b/read-cache-ll.h\n@@ -475,8 +475,6 @@ int refresh_index(struct index_state *, unsigned int flags, const struct pathspe\n */\n int repo_refresh_and_write_index(struct repository*, unsigned int refresh_flags, unsigned int write_flags, int gentle, const struct pathspec *, char *seen, const char *header_msg);\n \n-struct cache_entry *refresh_cache_entry(struct index_state *, struct cache_entry *, unsigned int);\n-\n void set_alternate_index_output(const char *);\n \n extern int verify_index_checksum;\ndiff --git a/read-cache.c b/read-cache.c\nindex e40f290bb3..555bf55132 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -129,6 +129,135 @@ static struct mem_pool *find_mem_pool(struct index_state *istate)\n \treturn *pool_ptr;\n }\n \n+/*\n+ * \"refresh\" does not calculate a new sha1 file or bring the\n+ * cache up-to-date for mode/content changes. But what it\n+ * _does_ do is to \"re-match\" the stat information of a file\n+ * with the cache, so that you can refresh the cache for a\n+ * file that hasn't been changed but where the stat entry is\n+ * out of date.\n+ *\n+ * For example, you'd want to do this after doing a \"git-read-tree\",\n+ * to link up the stat cache details with the proper files.\n+ */\n+static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n+\t\t\t\t\t struct cache_entry *ce,\n+\t\t\t\t\t unsigned int options, int *err,\n+\t\t\t\t\t int *changed_ret,\n+\t\t\t\t\t int *t2_did_lstat,\n+\t\t\t\t\t int *t2_did_scan)\n+{\n+\tstruct stat st;\n+\tstruct cache_entry *updated;\n+\tint changed;\n+\tint refresh = options & CE_MATCH_REFRESH;\n+\tint ignore_valid = options & CE_MATCH_IGNORE_VALID;\n+\tint ignore_skip_worktree = options & CE_MATCH_IGNORE_SKIP_WORKTREE;\n+\tint ignore_missing = options & CE_MATCH_IGNORE_MISSING;\n+\tint ignore_fsmonitor = options & CE_MATCH_IGNORE_FSMONITOR;\n+\n+\tif (!refresh || ce_uptodate(ce))\n+\t\treturn ce;\n+\n+\tif (!ignore_fsmonitor)\n+\t\trefresh_fsmonitor(istate);\n+\t/*\n+\t * CE_VALID or CE_SKIP_WORKTREE means the user promised us\n+\t * that the change to the work tree does not matter and told\n+\t * us not to worry.\n+\t */\n+\tif (!ignore_skip_worktree && ce_skip_worktree(ce)) {\n+\t\tce_mark_uptodate(ce);\n+\t\treturn ce;\n+\t}\n+\tif (!ignore_valid && (ce->ce_flags & CE_VALID)) {\n+\t\tce_mark_uptodate(ce);\n+\t\treturn ce;\n+\t}\n+\tif (!ignore_fsmonitor && (ce->ce_flags & CE_FSMONITOR_VALID)) {\n+\t\tce_mark_uptodate(ce);\n+\t\treturn ce;\n+\t}\n+\n+\tif (has_symlink_leading_path(ce->name, ce_namelen(ce))) {\n+\t\tif (ignore_missing)\n+\t\t\treturn ce;\n+\t\tif (err)\n+\t\t\t*err = ENOENT;\n+\t\treturn NULL;\n+\t}\n+\n+\tif (t2_did_lstat)\n+\t\t*t2_did_lstat = 1;\n+\tif (lstat(ce->name, &st) < 0) {\n+\t\tif (ignore_missing && errno == ENOENT)\n+\t\t\treturn ce;\n+\t\tif (err)\n+\t\t\t*err = errno;\n+\t\treturn NULL;\n+\t}\n+\n+\tchanged = ie_match_stat(istate, ce, &st, options);\n+\tif (changed_ret)\n+\t\t*changed_ret = changed;\n+\tif (!changed) {\n+\t\t/*\n+\t\t * The path is unchanged. If we were told to ignore\n+\t\t * valid bit, then we did the actual stat check and\n+\t\t * found that the entry is unmodified. If the entry\n+\t\t * is not marked VALID, this is the place to mark it\n+\t\t * valid again, under \"assume unchanged\" mode.\n+\t\t */\n+\t\tif (ignore_valid && assume_unchanged &&\n+\t\t !(ce->ce_flags & CE_VALID))\n+\t\t\t; /* mark this one VALID again */\n+\t\telse {\n+\t\t\t/*\n+\t\t\t * We do not mark the index itself \"modified\"\n+\t\t\t * because CE_UPTODATE flag is in-core only;\n+\t\t\t * we are not going to write this change out.\n+\t\t\t */\n+\t\t\tif (!S_ISGITLINK(ce->ce_mode)) {\n+\t\t\t\tce_mark_uptodate(ce);\n+\t\t\t\tmark_fsmonitor_valid(istate, ce);\n+\t\t\t}\n+\t\t\treturn ce;\n+\t\t}\n+\t}\n+\n+\tif (t2_did_scan)\n+\t\t*t2_did_scan = 1;\n+\tif (ie_modified(istate, ce, &st, options)) {\n+\t\tif (err)\n+\t\t\t*err = EINVAL;\n+\t\treturn NULL;\n+\t}\n+\n+\tupdated = make_empty_cache_entry(istate, ce_namelen(ce));\n+\tcopy_cache_entry(updated, ce);\n+\tmemcpy(updated->name, ce->name, ce->ce_namelen + 1);\n+\tfill_stat_cache_info(istate, updated, &st);\n+\t/*\n+\t * If ignore_valid is not set, we should leave CE_VALID bit\n+\t * alone. Otherwise, paths marked with --no-assume-unchanged\n+\t * (i.e. things to be edited) will reacquire CE_VALID bit\n+\t * automatically, which is not really what we want.\n+\t */\n+\tif (!ignore_valid && assume_unchanged &&\n+\t !(ce->ce_flags & CE_VALID))\n+\t\tupdated->ce_flags &= ~CE_VALID;\n+\n+\t/* istate->cache_changed is updated in the caller */\n+\treturn updated;\n+}\n+\n+static struct cache_entry *refresh_cache_entry(struct index_state *istate,\n+\t\t\t\t\t struct cache_entry *ce,\n+\t\t\t\t\t unsigned int options)\n+{\n+\treturn refresh_cache_ent(istate, ce, options, NULL, NULL, NULL, NULL);\n+}\n+\n static const char *alternate_index_output;\n \n static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)\n@@ -1339,128 +1468,6 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti\n \treturn 0;\n }\n \n-/*\n- * \"refresh\" does not calculate a new sha1 file or bring the\n- * cache up-to-date for mode/content changes. But what it\n- * _does_ do is to \"re-match\" the stat information of a file\n- * with the cache, so that you can refresh the cache for a\n- * file that hasn't been changed but where the stat entry is\n- * out of date.\n- *\n- * For example, you'd want to do this after doing a \"git-read-tree\",\n- * to link up the stat cache details with the proper files.\n- */\n-static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n-\t\t\t\t\t struct cache_entry *ce,\n-\t\t\t\t\t unsigned int options, int *err,\n-\t\t\t\t\t int *changed_ret,\n-\t\t\t\t\t int *t2_did_lstat,\n-\t\t\t\t\t int *t2_did_scan)\n-{\n-\tstruct stat st;\n-\tstruct cache_entry *updated;\n-\tint changed;\n-\tint refresh = options & CE_MATCH_REFRESH;\n-\tint ignore_valid = options & CE_MATCH_IGNORE_VALID;\n-\tint ignore_skip_worktree = options & CE_MATCH_IGNORE_SKIP_WORKTREE;\n-\tint ignore_missing = options & CE_MATCH_IGNORE_MISSING;\n-\tint ignore_fsmonitor = options & CE_MATCH_IGNORE_FSMONITOR;\n-\n-\tif (!refresh || ce_uptodate(ce))\n-\t\treturn ce;\n-\n-\tif (!ignore_fsmonitor)\n-\t\trefresh_fsmonitor(istate);\n-\t/*\n-\t * CE_VALID or CE_SKIP_WORKTREE means the user promised us\n-\t * that the change to the work tree does not matter and told\n-\t * us not to worry.\n-\t */\n-\tif (!ignore_skip_worktree && ce_skip_worktree(ce)) {\n-\t\tce_mark_uptodate(ce);\n-\t\treturn ce;\n-\t}\n-\tif (!ignore_valid && (ce->ce_flags & CE_VALID)) {\n-\t\tce_mark_uptodate(ce);\n-\t\treturn ce;\n-\t}\n-\tif (!ignore_fsmonitor && (ce->ce_flags & CE_FSMONITOR_VALID)) {\n-\t\tce_mark_uptodate(ce);\n-\t\treturn ce;\n-\t}\n-\n-\tif (has_symlink_leading_path(ce->name, ce_namelen(ce))) {\n-\t\tif (ignore_missing)\n-\t\t\treturn ce;\n-\t\tif (err)\n-\t\t\t*err = ENOENT;\n-\t\treturn NULL;\n-\t}\n-\n-\tif (t2_did_lstat)\n-\t\t*t2_did_lstat = 1;\n-\tif (lstat(ce->name, &st) < 0) {\n-\t\tif (ignore_missing && errno == ENOENT)\n-\t\t\treturn ce;\n-\t\tif (err)\n-\t\t\t*err = errno;\n-\t\treturn NULL;\n-\t}\n-\n-\tchanged = ie_match_stat(istate, ce, &st, options);\n-\tif (changed_ret)\n-\t\t*changed_ret = changed;\n-\tif (!changed) {\n-\t\t/*\n-\t\t * The path is unchanged. If we were told to ignore\n-\t\t * valid bit, then we did the actual stat check and\n-\t\t * found that the entry is unmodified. If the entry\n-\t\t * is not marked VALID, this is the place to mark it\n-\t\t * valid again, under \"assume unchanged\" mode.\n-\t\t */\n-\t\tif (ignore_valid && assume_unchanged &&\n-\t\t !(ce->ce_flags & CE_VALID))\n-\t\t\t; /* mark this one VALID again */\n-\t\telse {\n-\t\t\t/*\n-\t\t\t * We do not mark the index itself \"modified\"\n-\t\t\t * because CE_UPTODATE flag is in-core only;\n-\t\t\t * we are not going to write this change out.\n-\t\t\t */\n-\t\t\tif (!S_ISGITLINK(ce->ce_mode)) {\n-\t\t\t\tce_mark_uptodate(ce);\n-\t\t\t\tmark_fsmonitor_valid(istate, ce);\n-\t\t\t}\n-\t\t\treturn ce;\n-\t\t}\n-\t}\n-\n-\tif (t2_did_scan)\n-\t\t*t2_did_scan = 1;\n-\tif (ie_modified(istate, ce, &st, options)) {\n-\t\tif (err)\n-\t\t\t*err = EINVAL;\n-\t\treturn NULL;\n-\t}\n-\n-\tupdated = make_empty_cache_entry(istate, ce_namelen(ce));\n-\tcopy_cache_entry(updated, ce);\n-\tmemcpy(updated->name, ce->name, ce->ce_namelen + 1);\n-\tfill_stat_cache_info(istate, updated, &st);\n-\t/*\n-\t * If ignore_valid is not set, we should leave CE_VALID bit\n-\t * alone. Otherwise, paths marked with --no-assume-unchanged\n-\t * (i.e. things to be edited) will reacquire CE_VALID bit\n-\t * automatically, which is not really what we want.\n-\t */\n-\tif (!ignore_valid && assume_unchanged &&\n-\t !(ce->ce_flags & CE_VALID))\n-\t\tupdated->ce_flags &= ~CE_VALID;\n-\n-\t/* istate->cache_changed is updated in the caller */\n-\treturn updated;\n-}\n-\n static void show_file(const char * fmt, const char * name, int in_porcelain,\n \t\t int * first, const char *header_msg)\n {\n@@ -1627,13 +1634,6 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \treturn has_errors;\n }\n \n-struct cache_entry *refresh_cache_entry(struct index_state *istate,\n-\t\t\t\t\tstruct cache_entry *ce,\n-\t\t\t\t\tunsigned int options)\n-{\n-\treturn refresh_cache_ent(istate, ce, options, NULL, NULL, NULL, NULL);\n-}\n-\n \n /*****************************************************************\n * Index File I/O\n-- \n2.56.0-486-gd49ba3b80a\n\n\n"},{"id":"556945","messageId":"20261030173956.585250-5-gitster@pobox.com","threadId":"66718","inReplyTo":"20261030173956.585250-1-gitster@pobox.com","subject":"[PATCH 4/4] unpack-trees: hide verify_uptodate()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-30T17:39:56Z","receivedAt":"2026-10-30T17:39:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"64b1abe962 (merge-recursive: fix overwriting dirty files involved in\nrenames, 2018-04-19) made this function available to external callers.\n\nWith the removal of the \"recursive\" strategy in ad45b327c0\n(merge-recursive.[ch]: thoroughly debug these, 2025-04-08), the last\nexternal caller of this function want away.\n\nMake it file-scope static again.\n\nSigned-off-by: Junio C Hamano \n---\n unpack-trees.c | 4 ++--\n unpack-trees.h | 3 ---\n 2 files changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 1802809ad3..3298773490 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -2269,8 +2269,8 @@ static int verify_uptodate_1(const struct cache_entry *ce,\n \treturn add_rejected_path(o, error_type, ce->name);\n }\n \n-int verify_uptodate(const struct cache_entry *ce,\n-\t\t struct unpack_trees_options *o)\n+static int verify_uptodate(const struct cache_entry *ce,\n+\t\t\t struct unpack_trees_options *o)\n {\n \tif (!o->skip_sparse_checkout &&\n \t (ce->ce_flags & CE_SKIP_WORKTREE) &&\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 5867e26e17..638a6b076d 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -123,9 +123,6 @@ enum update_sparsity_result {\n enum update_sparsity_result update_sparsity(struct unpack_trees_options *options,\n \t\t\t\t\t struct pattern_list *pl);\n \n-int verify_uptodate(const struct cache_entry *ce,\n-\t\t struct unpack_trees_options *o);\n-\n int threeway_merge(const struct cache_entry * const *stages,\n \t\t struct unpack_trees_options *o);\n int twoway_merge(const struct cache_entry * const *src,\n-- \n2.56.0-486-gd49ba3b80a\n\n\n"},{"id":"556949","messageId":"20261030182211.GC7582@coredump.intra.peff.net","threadId":"66718","inReplyTo":"20261030173956.585250-2-gitster@pobox.com","subject":"Re: [PATCH 1/4] ident: remove unused author_ident_sufficiently_given()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-10-30T18:22:11Z","receivedAt":"2026-10-30T18:22:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 30, 2026 at 10:39:53AM -0700, Junio C Hamano wrote:\n\n> A pair of functions {author,committer}_ident_sufficiently_given()\n> were created to replace user_ident_explicitly_given() in d6991ceedc\n> (ident: keep separate \"explicit\" flags for author and committer,\n> 2012-11-14).\n> \n> But author_ident_sufficiently_given() is never used in our history.\n> We only check if committer ident is sufficiently given and use it to\n> nudge the user to set user.{name,email} configuration variables.\n> \n> Remove the unused author_ident_sufficiently_given() function.\n\nMakes sense. I think I must have just added it for symmetry with the\ncommitter one, but I agree it was never used. It almost looks like we\ncould drop author_ident_explicitly_given, too, which appears to be\nwrite-only after this patch. But it is somewhat subtly used via\nset_env_if() and prepare_fallback_ident(). So this seems like an obvious\ncleanup and a good stopping point.\n\n-Peff\n\n"},{"id":"556950","messageId":"20261030182519.GD7582@coredump.intra.peff.net","threadId":"66718","inReplyTo":"20261030173956.585250-4-gitster@pobox.com","subject":"Re: [PATCH 3/4] read-cache: hide refresh_cache_entry()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-10-30T18:25:19Z","receivedAt":"2026-10-30T18:25:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 30, 2026 at 10:39:55AM -0700, Junio C Hamano wrote:\n\n> Once upon a time, there was a merge strategy called \"recursive\",\n> which was added in 6d297f8137 (Status update on merge-recursive in\n> C, 2006-07-08).\n\n $ git log --grep='Once upon' --format=%an | sort | uniq -c | sort -rn\n 22 Jeff King\n 4 SZEDER Gábor\n 2 Johannes Schindelin\n 1 Jonas Fonseca\n 1 Elijah Newren\n\nWelcome to the club. :)\n\nThe patch itself seems obviously correct, and the commit message is a\nnice overview of how we got here.\n\n-Peff\n\n"},{"id":"556951","messageId":"20261030182648.GE7582@coredump.intra.peff.net","threadId":"66718","inReplyTo":"20261030173956.585250-1-gitster@pobox.com","subject":"Re: [PATCH 0/4] mark symbols that are unnecessarily public static","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-10-30T18:26:48Z","receivedAt":"2026-10-30T18:26:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 30, 2026 at 10:39:52AM -0700, Junio C Hamano wrote:\n\n> Over the years, refactoring various subsystems, adding new callers,\n> and rewriting them from scratch, made some symbols that started as\n> static into global, and some other symbols that started as global,\n> no longer need to be visible outside a file that use them. Here is\n> a small patch series that turns them back into file-scope static.\n> \n> Honestly, I suspect that [3/4] was probably better with a forward\n> declaration in the preview I sent earlier, but in the longer term,\n> readers would not care if this conversion ended up in a large code\n> movement, as they do not have to even know it.\n\nThe subject for patch 1 caught my eye, but I eyeballed the whole series\nand they all look reasonable to me. I agree that big code movement is\nsometimes annoying in a diff, but the flip side is that the resulting\nfile is nicer to read and maintain.\n\n-Peff\n\n"},{"id":"557288","messageId":"aum2GVDloMarZvwi@pks.im","threadId":"66718","inReplyTo":"20261030173956.585250-2-gitster@pobox.com","subject":"Re: [PATCH 1/4] ident: remove unused author_ident_sufficiently_given()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-03T10:23:21Z","receivedAt":"2026-11-03T10:23:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 30, 2026 at 10:39:53AM -0700, Junio C Hamano wrote:\n> A pair of functions {author,committer}_ident_sufficiently_given()\n> were created to replace user_ident_explicitly_given() in d6991ceedc\n\nNit, not worth a reroll, but isn't it \"was created\" as it is a singular\npair? Might just be me not being a native speaker though.\n\nPatrick\n\n"},{"id":"557289","messageId":"aum2HcP3BlSDQCGJ@pks.im","threadId":"66718","inReplyTo":"20261030173956.585250-3-gitster@pobox.com","subject":"Re: [PATCH 2/4] add -i: hide init/clear_add_i_state() functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-03T10:23:25Z","receivedAt":"2026-11-03T10:23:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 30, 2026 at 10:39:54AM -0700, Junio C Hamano wrote:\n> The \"add -i\" machinery is encapsulated in run_add_i() function,\n> which uses init_add_i_state() to initialize the machinery, does its\n> thing, and clear_add_i_state() to tear it down. External callers\n> have no need to call anything other than run_add_i(), starting at\n> d51b61f5da (add-patch: remove dependency on \"add-interactive\"\n> subsystem, 2026-03-02) refactored the code paths involved.\n\nThis sentence reads a bit odd. Probably missing a \"which\"?\n\nPatrick\n\n"},{"id":"557290","messageId":"aum2IssjPCGMySLm@pks.im","threadId":"66718","inReplyTo":"20261030182648.GE7582@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] mark symbols that are unnecessarily public static","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-03T10:23:30Z","receivedAt":"2026-11-03T10:23:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 30, 2026 at 02:26:48PM -0400, Jeff King wrote:\n> On Fri, Oct 30, 2026 at 10:39:52AM -0700, Junio C Hamano wrote:\n> \n> > Over the years, refactoring various subsystems, adding new callers,\n> > and rewriting them from scratch, made some symbols that started as\n> > static into global, and some other symbols that started as global,\n> > no longer need to be visible outside a file that use them. Here is\n> > a small patch series that turns them back into file-scope static.\n> > \n> > Honestly, I suspect that [3/4] was probably better with a forward\n> > declaration in the preview I sent earlier, but in the longer term,\n> > readers would not care if this conversion ended up in a large code\n> > movement, as they do not have to even know it.\n> \n> The subject for patch 1 caught my eye, but I eyeballed the whole series\n> and they all look reasonable to me. I agree that big code movement is\n> sometimes annoying in a diff, but the flip side is that the resulting\n> file is nicer to read and maintain.\n\nYup, I'm happy, as well. There's two tiny nits on the commit messages,\nbut nothing that'd really need a reroll.\n\nThanks!\n\nPatrick\n\n"},{"id":"557356","messageId":"xmqqtslx7nam.fsf@gitster.g","threadId":"66718","inReplyTo":"aum2GVDloMarZvwi@pks.im","subject":"Re: [PATCH 1/4] ident: remove unused author_ident_sufficiently_given()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-11-03T19:00:33Z","receivedAt":"2026-11-03T19:00:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Fri, Oct 30, 2026 at 10:39:53AM -0700, Junio C Hamano wrote:\n>> A pair of functions {author,committer}_ident_sufficiently_given()\n>> were created to replace user_ident_explicitly_given() in d6991ceedc\n>\n> Nit, not worth a reroll, but isn't it \"was created\" as it is a singular\n> pair? Might just be me not being a native speaker though.\n\nNeither am I a native speaker, but \"Syntactic vs Notional Agreement\"\nis what makes both valid.\n\nTreating the \"pair\" as a singular subject is the strictly formal\nchoice preferred by most academic and traditional style guides.\n\nEven though \"pair\" is grammatically singular, it represents two\ndistinct things, and in this particular case, we are retiring one of\nthem while keeping the other, treating the \"pair\" as a separable\ntwo.\n\n"},{"id":"557357","messageId":"xmqqpkwl7n9i.fsf@gitster.g","threadId":"66718","inReplyTo":"aum2HcP3BlSDQCGJ@pks.im","subject":"Re: [PATCH 2/4] add -i: hide init/clear_add_i_state() functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-11-03T19:01:13Z","receivedAt":"2026-11-03T19:01:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Fri, Oct 30, 2026 at 10:39:54AM -0700, Junio C Hamano wrote:\n>> The \"add -i\" machinery is encapsulated in run_add_i() function,\n>> which uses init_add_i_state() to initialize the machinery, does its\n>> thing, and clear_add_i_state() to tear it down. External callers\n>> have no need to call anything other than run_add_i(), starting at\n>> d51b61f5da (add-patch: remove dependency on \"add-interactive\"\n>> subsystem, 2026-03-02) refactored the code paths involved.\n>\n> This sentence reads a bit odd. Probably missing a \"which\"?\n>\n> Patrick\n\nThanks. \", which refactored the code paths...\" is what it should\nhave said.\n\n"},{"id":"557409","messageId":"aurTjEgKc5yw_vwD@pks.im","threadId":"66718","inReplyTo":"xmqqtslx7nam.fsf@gitster.g","subject":"Re: [PATCH 1/4] ident: remove unused author_ident_sufficiently_given()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-04T06:41:16Z","receivedAt":"2026-11-04T06:41:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Nov 03, 2026 at 11:00:33AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > On Fri, Oct 30, 2026 at 10:39:53AM -0700, Junio C Hamano wrote:\n> >> A pair of functions {author,committer}_ident_sufficiently_given()\n> >> were created to replace user_ident_explicitly_given() in d6991ceedc\n> >\n> > Nit, not worth a reroll, but isn't it \"was created\" as it is a singular\n> > pair? Might just be me not being a native speaker though.\n> \n> Neither am I a native speaker, but \"Syntactic vs Notional Agreement\"\n> is what makes both valid.\n> \n> Treating the \"pair\" as a singular subject is the strictly formal\n> choice preferred by most academic and traditional style guides.\n> \n> Even though \"pair\" is grammatically singular, it represents two\n> distinct things, and in this particular case, we are retiring one of\n> them while keeping the other, treating the \"pair\" as a separable\n> two.\n\nThanks, today I learned :)\n\nPatrick\n\n"}]}