Re: [PATCH v4 00/23] Fix incorrect use of the_index

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 11, 2018 at 6:11 PM Elijah Newren <newren@xxxxxxxxx> wrote:
>
> Hi Duy,
>
> On Mon, Jun 11, 2018 at 9:05 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
> > On Sat, Jun 9, 2018 at 9:58 PM Elijah Newren <newren@xxxxxxxxx> wrote:
> >> I read over the rest.  Found a small grammatical error in a commit
> >> message.  Found multiple places that still need conversion, from
> >> pushing up &the_index usages to callers of ll-merge.c and sha1-file.c
> >> instead of having them in those files, to mixes of _cache_ and _index_
> >> functions as in apply.c and merge-recursive.c.  However, Duy pointed
> >> out there was more work to do,
> >
> > Yes. This is just fyi, 40 patches later, i'm down to leaving the_index
> > in three files outside builtin/: merge-recursive, notes-merge.c and
> > transport.c. Even after the conversion we may need some more follow-up
> > patches because it now shows places where we should _not_ touch the
> > index at all, which may involve not simply passing NULL index_state to
> > some functions, but fixing them up to tolerate NULL index_state. So
> > it's going to be a few patch series until the_index is gone for good
> > [1].

And I forgot one thing. There are other hidden dependencies as well.
Like hold_locked_index() will assume $GIT_DIR/index, but when you take
an arbitrary 'struct index_state *' I don't think you can make that
kind of assumption. This is mostly a note to myself in case I forget
it again.

> >
> > [1] but like cheap horror movies, there's always a sequel:
> > the_repository is still spread in many places and hides dependencies
> > in the same way. We can't do anything about it though until struct
> > repository conversion is more or less complete.
>
> I was just about to send you an email to ask if you were continuing on
> with the series.  I need diff-lib.c converted in order to make the
> changes Junio suggested to index_has_changes at
> https://public-inbox.org/git/xmqqvaaz5jcv.fsf@xxxxxxxxxxxxxxxxxxxxxxxxx/.
> Since you're already working on that, I won't duplicate your effort.
> Thanks for tackling all of this.  :-)

I'm not sure if it's possible to cherry pick this patch to continue
your work (because of dependencies between patches) but it's
https://gitlab.com/pclouds/git/commits/really-kill-the-index, commit
"merge.c: remove...".

Or you just leave it to me, update has_index_changes() to always take
'struct index_state *' and just pass &the_index in all uninteresting
places. (Or not update at all if it's really not needed for your work)

What about merge-recursive.c? Given that this whole thing will take
many release cycles to finish, your work may get merged before mine
and I could do the conversion now (and resolve conflicts and resubmit
later). Of course if you like to keep merge-recursive.c the_index-free
now, I will not stop you ;-)
-- 
Duy



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux