On Sat, Jun 9, 2018 at 8:10 PM Elijah Newren <newren@xxxxxxxxx> wrote:
>
> On Wed, Jun 6, 2018 at 10:02 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
> > diff --git a/merge-recursive.c b/merge-recursive.c
> > index b404ebac7c..4f054d6dbb 100644
> > --- a/merge-recursive.c
> > +++ b/merge-recursive.c
> > @@ -315,7 +315,7 @@ static int add_cacheinfo(struct merge_options *o,
> > struct cache_entry *ce;
> > int ret;
> >
> > - ce = make_cache_entry(mode, oid ? oid->hash : null_sha1, path, stage, 0);
> > + ce = make_index_entry(&the_index, mode, oid ? oid->hash : null_sha1, path, stage, 0);
> > if (!ce)
> > return err(o, _("add_cacheinfo failed for path '%s'; merge aborting."), path);
>
> There's also a refresh_cache_entry() call about ten lines after this;
> since you converted all other make_cache_entry() and
> refresh_cache_entry() calls in this patch, I'm curious if that one was
> left out for a reason or was just an oversight.
Ah I didn't mean to convert or kill refresh_cache_entry(), not outside
read-cache.c. I rely on NO_THE_INDEX_COMPATIBILITY_MACROS to catch
*cache* functions and if we set it in this file, we're going to have a
lot more work to do and plenty of the_index will show up.
> There are also a lot of add_cache_entry() calls in this function. I'm
> guessing we should either convert all of those too, or just change
> back this particular make_index_entry to make_cache_entry() as it was;
> it seems weird to have a mix of explicit the_index and implicit
> the_index usages in the same function.
Yes some files still have the mix of the_index and *cache*(). This one
and apply.c come to mind. There's more work to do to kill all
the_index outside builtin/
> If we convert them all,
> perhaps we should consider having merge_options store the index we're
> working on? If you want to punt this until later or leave it for me
> while I make all my ongoing merge-recursive changes, that's fine.
> Just thought I'd point it out.
Right you're updating merge-recursive.c, I'd love it if you could
define NO_THE_INDEX_COMPATIBILITY_MACROS here. Yes merge_options
sounds like a good place to tell merge-recursive where to get a struct
index_state.
--
Duy
