[PATCH] read-cache: hide refresh_cache_entry()

[PATCH] read-cache: hide refresh_cache_entry()

[Junio C Hamano]

Once upon a time, there was a merge strategy called "recursive",
which was added in 6d297f8137 (Status update on merge-recursive in
C, 2006-07-08).  This implementation came with its own machinery
that creates an in-core index entry, and then refreshed the index
entry it created by calling the refresh_cache_entry() function
borrowed from the read-cache.c API.  refresh_cache_entry() used to
be file-scope static, but was made public with this change.

When ad45b327c0 (merge-recursive.[ch]: thoroughly debug these,
2025-04-08) finally removed the entire code for the "recursive"
merge strategy, the last external caller of refresh_cache_entry()
went with it.

Hide the low-level implementation detail again by making it a
file-scope static function.  The external callers would indirectly
call it when they call make_cache_entry() or rename_index_entry_at()
external interface, which "ort" (the successor of "recursive") does.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 read-cache-ll.h | 2 --
 read-cache.c    | 8 +++++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/read-cache-ll.h b/read-cache-ll.h
index 066dd8bc3b..dd30befbb3 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -475,8 +475,6 @@ int refresh_index(struct index_state *, unsigned int flags, const struct pathspe
  */
 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);
 
-struct cache_entry *refresh_cache_entry(struct index_state *, struct cache_entry *, unsigned int);
-
 void set_alternate_index_output(const char *);
 
 extern int verify_index_checksum;
diff --git a/read-cache.c b/read-cache.c
index c4cf08a3a3..94dff890a7 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -129,6 +129,8 @@ static struct mem_pool *find_mem_pool(struct index_state *istate)
 	return *pool_ptr;
 }
 
+static struct cache_entry *refresh_cache_entry(struct index_state *, struct cache_entry *, unsigned int);
+
 static const char *alternate_index_output;
 
 static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)
@@ -1624,9 +1626,9 @@ int refresh_index(struct index_state *istate, unsigned int flags,
 	return has_errors;
 }
 
-struct cache_entry *refresh_cache_entry(struct index_state *istate,
-					struct cache_entry *ce,
-					unsigned int options)
+static struct cache_entry *refresh_cache_entry(struct index_state *istate,
+					       struct cache_entry *ce,
+					       unsigned int options)
 {
 	return refresh_cache_ent(istate, ce, options, NULL, NULL, NULL, NULL);
 }
-- 
2.56.0-433-ga9209adf11

Re: [PATCH] read-cache: hide refresh_cache_entry()

[Patrick Steinhardt]

On Wed, Oct 28, 2026 at 01:34:42PM -0700, Junio C Hamano wrote:
> Once upon a time, there was a merge strategy called "recursive",
> which was added in 6d297f8137 (Status update on merge-recursive in
> C, 2006-07-08).  This implementation came with its own machinery
> that creates an in-core index entry, and then refreshed the index
> entry it created by calling the refresh_cache_entry() function
> borrowed from the read-cache.c API.  refresh_cache_entry() used to
> be file-scope static, but was made public with this change.
> 
> When ad45b327c0 (merge-recursive.[ch]: thoroughly debug these,
> 2025-04-08) finally removed the entire code for the "recursive"
> merge strategy, the last external caller of refresh_cache_entry()
> went with it.
> 
> Hide the low-level implementation detail again by making it a
> file-scope static function.  The external callers would indirectly
> call it when they call make_cache_entry() or rename_index_entry_at()
> external interface, which "ort" (the successor of "recursive") does.

Seeing all these patches dropping public declarations I wonder whether
they should have been part of the same patch series :)

> diff --git a/read-cache.c b/read-cache.c
> index c4cf08a3a3..94dff890a7 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -129,6 +129,8 @@ static struct mem_pool *find_mem_pool(struct index_state *istate)
>  	return *pool_ptr;
>  }
>  
> +static struct cache_entry *refresh_cache_entry(struct index_state *, struct cache_entry *, unsigned int);
> +
>  static const char *alternate_index_output;
>  
>  static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)

Could we avoid this forward declaration by moving the function around?

Patrick

Re: [PATCH] read-cache: hide refresh_cache_entry()

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> Hide the low-level implementation detail again by making it a
>> file-scope static function.  The external callers would indirectly
>> call it when they call make_cache_entry() or rename_index_entry_at()
>> external interface, which "ort" (the successor of "recursive") does.
>
> Seeing all these patches dropping public declarations I wonder whether
> they should have been part of the same patch series :)

Perhaps ;-).

>> +static struct cache_entry *refresh_cache_entry(struct index_state *, struct cache_entry *, unsigned int);
>> +
>>  static const char *alternate_index_output;
>>  
>>  static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)
>
> Could we avoid this forward declaration by moving the function around?

As refresh_cache_entry() that has mere one line in its body calls
refresh_cache_ent() that is a screenful and defined nearby, both of
them need to be moved upward, so it becomes a rather unsightly
change, even though nobody would care when they only look at the end
result X-<.

By the way, I tried "history fixup" to make this change, and noticed
that the subcommand ignores "[notes] rewriteref = refs/notes/amlog",
just like "git history reword" does.  Again, I wonder if this is a
known limitation, or a deliberate design.