Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
> There is no need to call read_ref_full() or resolve_gitlink_ref() from
> read_loose_refs(), because we already have a ref_store object in hand.
> So we can call resolve_ref_recursively() ourselves. Happily, this
> unifies the code for the submodule vs. non-submodule cases.
>
> This requires resolve_ref_recursively() to be exposed to the refs
> subsystem, though not to non-refs code.
>
> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
> ---
> refs.c | 50 +++++++++++++++++++++++++-------------------------
> refs/files-backend.c | 18 ++++--------------
> refs/refs-internal.h | 5 +++++
> 3 files changed, 34 insertions(+), 39 deletions(-)
OK, but one thing puzzles me...
> @@ -1390,27 +1390,6 @@ static struct ref_store *main_ref_store;
> static struct hashmap submodule_ref_stores;
>
> /*
> - * Return the ref_store instance for the specified submodule (or the
> - * main repository if submodule is NULL). If that ref_store hasn't
> - * been initialized yet, return NULL.
> - */
> -static struct ref_store *lookup_ref_store(const char *submodule)
> -{
> - struct submodule_hash_entry *entry;
> -
> - if (!submodule)
> - return main_ref_store;
> -
> - if (!submodule_ref_stores.tablesize)
> - /* It's initialized on demand in register_ref_store(). */
> - return NULL;
> -
> - entry = hashmap_get_from_hash(&submodule_ref_stores,
> - strhash(submodule), submodule);
> - return entry ? entry->refs : NULL;
> -}
> -
> -/*
> * Register the specified ref_store to be the one that should be used
> * for submodule (or the main repository if submodule is NULL). It is
> * a fatal error to call this function twice for the same submodule.
> @@ -1451,6 +1430,27 @@ static struct ref_store *ref_store_init(const char *submodule)
> return refs;
> }
>
> +/*
> + * Return the ref_store instance for the specified submodule (or the
> + * main repository if submodule is NULL). If that ref_store hasn't
> + * been initialized yet, return NULL.
> + */
> +static struct ref_store *lookup_ref_store(const char *submodule)
> +{
> + struct submodule_hash_entry *entry;
> +
> + if (!submodule)
> + return main_ref_store;
> +
> + if (!submodule_ref_stores.tablesize)
> + /* It's initialized on demand in register_ref_store(). */
> + return NULL;
> +
> + entry = hashmap_get_from_hash(&submodule_ref_stores,
> + strhash(submodule), submodule);
> + return entry ? entry->refs : NULL;
> +}
> +
I somehow thought that we had an early "reorder the code" step to
avoid hunks like these? Am I missing some subtle changes made while
moving the function down?