Re: [PATCH v4 00/23] Fix incorrect use of the_index
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- Jun 11, 2018, 16:24 UTC
- Message-ID
- <CACsJy8AOzwu+GJLTsGtN0dCnwxoRBQYpM6gzVebN++zjpXNu7w@mail.gmail.com>
- In-Reply-To
- <CABPp-BHYm57n=ufg_SqmA-UnOEMivWJZytnPPSU3WSYNeEph0g@mail.gmail.com>
On Mon, Jun 11, 2018 at 6:11 PM Elijah Newren <newren@gmail.com> wrote:
Show 20 quoted lines
> > Hi Duy, > > On Mon, Jun 11, 2018 at 9:05 AM, Duy Nguyen <pclouds@gmail.com> wrote: > > On Sat, Jun 9, 2018 at 9:58 PM Elijah Newren <newren@gmail.com> 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.
Show 12 quoted lines
> > > > [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@gitster-ct.c.googlers.com/. > 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