Michael Haggerty <mhag...@alum.mit.edu> 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 <mhag...@alum.mit.edu>
> ---
>  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?

Reply via email to