Re: [PATCH v2 19/24] submodule.c: remove implicit dependency on the_index
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- Oct 19, 2018, 16:33 UTC
- Message-ID
- <CACsJy8CGLRi64WzNUM-6NUm3i0JW7+ptfh7+NhiSb7J9Qb0u0w@mail.gmail.com>
- In-Reply-To
- <20181019162021.GA14778@sigill.intra.peff.net>
On Fri, Oct 19, 2018 at 6:20 PM Jeff King <peff@peff.net> wrote:
Show 22 quoted lines
>
> On Mon, Sep 03, 2018 at 08:09:27PM +0200, Nguyễn Thái Ngọc Duy wrote:
>
> > diff --git a/submodule.c b/submodule.c
> > index 50cbf5f13e..c0c1224760 100644
> > --- a/submodule.c
> > +++ b/submodule.c
> > @@ -766,7 +766,8 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,
> > * have a corresponding 'struct oid_array' (in the 'util' field) which lists
> > * what the submodule pointers were updated to during the change.
> > */
> > -static void collect_changed_submodules(struct string_list *changed,
> > +static void collect_changed_submodules(struct index_state *istate,
> > + struct string_list *changed,
> > struct argv_array *argv)
> > {
> > struct rev_info rev;
>
> This function doesn't actually make use of the newly added "istate" (nor
> does it use the_index, so there's not a spot that forgot to get
> converted). Is this in preparation for more refactoring, or is it just a
> mistake and can be dropped?It's possible that it was needed at some point when I converted diff api to pass index_state around. But in later iterations I think I passed "struct repository *" instead because diff code needed much more than the index, but did not clean this up. Sorry.
In this function, we still have the_repository for repo_init_revisions() and it should be gone eventually. "struct repository *r" could take the place of "struct index_state *istate" instead and we would need to reopen the path again.
So I'm not sure, if it's really bad, we could remove it now. Otherwise we could just leave it there, I don't think this "istate" will survive very long. I already started deleting some of the_repository in "kill the_index" series.
> I don't see anything in the "Kill the_index, final part" series that > would need it.
Yeah. Killing the_index is just the first small step (didn't look that small when I started). Now it's all about the_repository ;-) and we have like 400 references of it just in library code.
> If this can be dropped, then most of the other changes here can, too, > because they're just pushing the unused parameter down the stack. I.e.:
-- Duy