Re: [PATCH 2/4] promisor-remote: support per-repository config

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jun 5, 2021, 01:54 UTC
Message-ID: <20210605015402.607304-1-jonathantanmy@google.com>
In-Reply-To: <CABPp-BHZ0OrADfP3V-hZ1YYf3fQYW3fU-tCJWgs976amG-T1Dg@mail.gmail.com>

Show 12 quoted lines

> > +int repo_has_promisor_remote(struct repository *r);
> > +static inline int has_promisor_remote(void)
> > +{
> > +       return repo_has_promisor_remote(the_repository);
> > +}
> 
> Is part of the plan for supporting partial clones within submodules to
> audit the code for use of these inline wrappers and convert them over
> to the repo_* variants?  I'm particularly interested in the
> has_promisor_remote() function, since there are calls in
> diffcore-rename at least that protect that call with a check against r
> == the_repository.

Good point. Yes, that should be part of the plan - at least, we should see what those invocations are for.

Show 16 quoted lines

> > @@ -139,6 +140,9 @@ struct repository {
> >         /* True if commit-graph has been disabled within this process. */
> >         int commit_graph_disabled;
> >
> > +       /* Configurations related to promisor remotes. */
> > +       struct promisor_remote_config *promisor_remote_config;
> > +
> >         /* Configurations */
> >
> >         /* Indicate if a repository has a different 'commondir' from 'gitdir' */
> > --
> > 2.32.0.rc0.204.g9fa02ecfa5-goog
> 
> Looks like a reasonable step in moving away from globals and have
> repository-specific variants of these functions; I didn't spot any
> problems, just one question about additional plans.

Thanks for taking a look.