Re: [PATCH 0/5] global: drop external `the_index` variable

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Apr 15, 2024, 13:55 UTC
Message-ID: <28541f00-9054-4aa0-8e47-348043b5f863@gmail.com>
In-Reply-To: <cover.1713180749.git.ps@pks.im>

Hi Patrick

On 15/04/2024 12:42, Patrick Steinhardt wrote:

Show 6 quoted lines

> The motivation of these patches comes from the introduction of the ref
> format extension. I found it really hard to reason about the state of
> `the_repository` and would like to continue its deprecation in favor of
> explicitly passing down a `struct repository *`. Getting rid of
> `the_index` is a first easy step into that direction to make things more
> self-contained.

Maybe I've got the wrong end of the stick but my impression is that it is the use of "the_repository" in library code (i.e. the files outside builtin/) that causes most of the pain. With that in mind would be we better focusing contributor and reviewer effort on eliminating "the_repository" from those files instead? It would need to be done in carefully in stages but would bring real benefits.

Best Wishes

Phillip

Show 50 quoted lines

> Patrick
> 
> Patrick Steinhardt (5):
>    t/helper: stop using `the_index`
>    builtin: stop using `the_index`
>    repository: initialize index in `repo_init()`
>    builtin/clone: stop using `the_index`
>    repository: drop global `the_index` variable
> 
>   builtin/add.c                        |  48 +++++------
>   builtin/am.c                         |  36 ++++----
>   builtin/cat-file.c                   |   3 +-
>   builtin/check-attr.c                 |   5 +-
>   builtin/check-ignore.c               |   7 +-
>   builtin/checkout-index.c             |  22 ++---
>   builtin/checkout.c                   |  87 ++++++++++---------
>   builtin/clean.c                      |   7 +-
>   builtin/clone.c                      |   7 +-
>   builtin/commit.c                     |  81 +++++++++---------
>   builtin/describe.c                   |   3 +-
>   builtin/diff-tree.c                  |   3 +-
>   builtin/diff.c                       |   6 +-
>   builtin/difftool.c                   |   4 +-
>   builtin/merge-index.c                |  17 ++--
>   builtin/merge-tree.c                 |   3 +-
>   builtin/merge.c                      |  31 ++++---
>   builtin/mv.c                         |  68 +++++++--------
>   builtin/pull.c                       |   4 +-
>   builtin/read-tree.c                  |  15 ++--
>   builtin/rebase.c                     |   3 +-
>   builtin/replay.c                     |   1 -
>   builtin/reset.c                      |  32 +++----
>   builtin/rev-parse.c                  |   6 +-
>   builtin/rm.c                         |  40 ++++-----
>   builtin/stash.c                      |  45 +++++-----
>   builtin/submodule--helper.c          |  21 +++--
>   builtin/update-index.c               | 122 +++++++++++++--------------
>   builtin/write-tree.c                 |   6 +-
>   repository.c                         |  32 ++++---
>   repository.h                         |   3 -
>   t/helper/test-cache-tree.c           |  17 ++--
>   t/helper/test-dump-cache-tree.c      |   5 +-
>   t/helper/test-dump-split-index.c     |  11 ++-
>   t/helper/test-dump-untracked-cache.c |   3 +-
>   t/helper/test-lazy-init-name-hash.c  |  39 +++++----
>   t/helper/test-read-cache.c           |   9 +-
>   t/helper/test-scrap-cache-tree.c     |   7 +-
>   t/helper/test-write-cache.c          |   3 +-
>   39 files changed, 420 insertions(+), 442 deletions(-)
>