This is the follow up to [1], but actually completed now.
The reasoning why this series is a good idea is in the commit message of the
last patch, quoted here:
The NO_THE_INDEX_COMPATIBILITY_MACROS pre processor setting was introduced
in 4aab5b46f4 (Make read-cache.c "the_index" free., 2007-04-01), stating:
This makes all low-level functions defined in read-cache.c to
take an explicit index_state structure as their first parameter,
to specify which index to work on.
The reasoning is very vague, maybe indicating that having the index
specified to work on is easier to for the new reader to understand what
is going on.
All preceding patches worked on retiring functions that do not take an
explicit index as to where to perform its work. Spelling out the reasons
why we want to specify the index at each call:
1) Ease of understanding:
The recent patches dropped a lot of macros that may confuse new people
diving into the code base.
2a) Spelling out global state explicitly:
Anything you do in submodule land today needs to spawn new processes in
the submodule. This is cumbersome and not performant. So we may want to
have an abstraction of a repo, i.e. all repository state in one struct.
That way we can open a submodule in-process and perform the required
actions without spawning a process. The road to this future is a long
road, and we have to start somewhere. Exposing the global state seems
like a good starter point.
2b) Spelling out global state explicitly (II): (Jeff Hostetler wrote)
In addition to (eventually) allowing multiple repos be open at the same
time for submodules, it would also help with various multi-threading
efforts. For example, we have loops that do a
"for (k = 0, k < active_nr; k++) {...}" There is no visual clue in that
code that it references "the_index" and therefore should be subject to
the same locking. Granted, this is a trivial example, but goes to the
argument that the code has lots of subtle global variables and macros
that make it difficult to reason about the code.
This step would help un-hide this.
Review as well as critique if this is the right approach to an endgame with
less globals splattered throughout the codebase is welcome.
Thanks,
Stefan
[1] https://public-inbox.org/git/[email protected]/
Stefan Beller (24):
cache.h: drop read_cache()
cache.h: drop active_* macros
cache.h: drop read_cache_from
cache.h: drop read_cache_preload(pathspec)
cache.h: drop read_cache_unmerged()
unpack-trees.c: rename parameter 'the_index'
cache.h: drop read_blob_data_from_cache
cache.h: drop unmerge_cache[_entry_at]
cache.h: drop resolve_undo_clear
cache.h: drop cache_name_is_other
cache.h: drop cache_file_exists
cache.h: drop cache_dir_exists
cache.h: drop is_cache_unborn(), discard_cache(), unmerged_cache()
cache.h: drop cache_name_pos
cache.h: drop add_cache_entry
cache.h: drop rename_cache_entry_at
cache.h: drop remove_file_from_cache
cache.h: drop add_to_cache
cache.h: drop add_file_to_cache
cache.h: drop chmod_cache_entry
cache.h: drop refresh_cache
cache.h: drop ce_modified
cache.h: drop ce_match_stat
cache.h: retire NO_THE_INDEX_COMPATIBILITY_MACROS
apply.c | 32 +++++++------
attr.c | 1 -
builtin/add.c | 12 ++---
builtin/am.c | 26 +++++------
builtin/blame.c | 19 ++++----
builtin/check-attr.c | 2 +-
builtin/check-ignore.c | 4 +-
builtin/checkout-index.c | 12 ++---
builtin/checkout.c | 66 +++++++++++++-------------
builtin/clean.c | 4 +-
builtin/commit.c | 52 ++++++++++-----------
builtin/describe.c | 2 +-
builtin/diff-files.c | 4 +-
builtin/diff-index.c | 6 +--
builtin/diff.c | 19 ++++----
builtin/fsck.c | 14 +++---
builtin/grep.c | 10 ++--
builtin/ls-files.c | 49 +++++++++----------
builtin/merge-index.c | 14 +++---
builtin/merge.c | 24 +++++-----
builtin/mv.c | 22 ++++-----
builtin/pull.c | 4 +-
builtin/read-tree.c | 8 ++--
builtin/reset.c | 11 +++--
builtin/rev-parse.c | 2 +-
builtin/rm.c | 26 +++++------
builtin/submodule--helper.c | 10 ++--
builtin/update-index.c | 91 +++++++++++++++++++-----------------
cache.h | 35 --------------
check-racy.c | 10 ++--
convert.c | 4 +-
diff-lib.c | 8 ++--
diff.c | 14 +++---
dir.c | 41 ++++++++--------
entry.c | 3 +-
merge-recursive.c | 65 +++++++++++++-------------
merge.c | 8 ++--
name-hash.c | 1 -
pathspec.c | 16 +++----
read-cache.c | 5 +-
rerere.c | 38 +++++++--------
revision.c | 22 ++++-----
sequencer.c | 29 ++++++------
sha1_name.c | 22 ++++-----
submodule.c | 20 ++++----
t/helper/test-dump-cache-tree.c | 4 +-
t/helper/test-dump-untracked-cache.c | 2 +-
t/helper/test-lazy-init-name-hash.c | 20 ++++----
t/helper/test-read-cache.c | 4 +-
t/helper/test-scrap-cache-tree.c | 4 +-
t/t2107-update-index-basic.sh | 2 +-
tree.c | 10 ++--
unpack-trees.c | 9 ++--
wt-status.c | 24 +++++-----
54 files changed, 471 insertions(+), 495 deletions(-)
--
2.13.0.rc1.39.ga6db8bfa24
