Subject: Re: [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories

From: Brandon Williams <[email-masked]>
Date: Tue, 24 Apr 2018 11:42:33 -0700

On 04/24, Jonathan Tan wrote:
> On Mon, 23 Apr 2018 16:43:27 -0700
> Stefan Beller <[email-masked]> wrote:
> 
> > This involves also adapting sha1_object_info_extended and a some
> > internal functions that are used to implement these. It all has to
> > happen in one patch, because of a single recursive chain of calls visits
> > all these functions.
> 
> In packfile.c, unpack_entry() invokes get_delta_base_cache_entry(),
> which references a global (delta_base_cache). Does delta_base_cache need
> to be moved to the repo object (or object store object) first, or is
> this safe?

After looking at this, I think it should be safe for now since its a
cache that requires a packed_git pointer to access (and those would be
per repository).  We may want to move it in to the repository at some
point though.

> 
> Also, in sha1_file.c, oid_object_info_extended() invokes fetch_object(),
> which attempts to fetch a missing object. For this, I think that it's
> best to guard with a "r == the_repository" check, or if there's a better
> way to distinguish between the "default" repository and any repository
> that we newly create (I vaguely remember some distinction when parsing
> environment variables when determining repo paths - the envvars were
> only used for the "default" repository, but not for the others).

This is a little more difficult and I'm not sure I know what the best
course of action would be for this.  Mostly because then this puts a big
recursive dependency on the whole fetch mechanism to handle arbitrary
repositories at the same time these functions are converted.  So maybe
throwing in the runtime check would be the best way to break the
dependencies for now.

-- 
Brandon Williams