[PATCH 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[PATCH 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[Ævar Arnfjörð Bjarmason]

My recent now-landed topic[1] to remove most use of
"USE_THE_INDEX_COMPATIBILITY_MACROS" was merged in 041df69edd3 (Merge
branch 'ab/fewer-the-index-macros', 2022-11-28).

It left out use of the macros that would have conflicted with
in-flight changes, but as those topics have landed we can now complete
the migration.

As before this is almost entirely a matter of applying the existing
"pending" coccinelle rules, the exceptions being 1/6, and the *.h
changes where we remove the macro definitions (the macro users being
edited by coccinelle).

The 4-5/6 then handle some edge cases we had left (but the code change
itself is done by coccinelle).

1. https://lore.kernel.org/git/cover-v2-00.11-00000000000-20221119T125550Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (6):
  builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"
  cocci & cache.h: fully apply "active_nr" part of index-compatibility
  cocci & cache.h: apply pending "index_cache_pos" rule
  cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*"
  cache-tree API: remove redundant update_main_cache_tree()
  cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS"

 builtin/am.c                                  |  6 ++--
 builtin/commit.c                              | 18 +++++-----
 builtin/merge.c                               |  8 ++---
 builtin/mv.c                                  |  8 +++--
 builtin/rm.c                                  |  2 +-
 builtin/stash.c                               | 11 +++---
 builtin/update-index.c                        |  4 +--
 builtin/write-tree.c                          |  5 +--
 cache-tree.h                                  | 15 --------
 cache.h                                       | 12 +------
 contrib/coccinelle/index-compatibility.cocci  | 36 ++++++++++++++-----
 .../index-compatibility.pending.cocci         | 24 -------------
 12 files changed, 62 insertions(+), 87 deletions(-)
 delete mode 100644 contrib/coccinelle/index-compatibility.pending.cocci

-- 
2.39.0.rc2.1048.g0e5493b8d5b

[PATCH 1/6] builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"

[Ævar Arnfjörð Bjarmason]

Replace the "USE_THE_INDEX_COMPATIBILITY_MACROS" define with the
narrower "USE_THE_INDEX_VARIABLE". This could have been done in
07047d68294 (cocci: apply "pending" index-compatibility to some
"builtin/*.c", 2022-11-19), but I missed it at the time.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/rm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index d4989d4d863..2cddf921951 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) Linus Torvalds 2006
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "advice.h"
 #include "config.h"
-- 
2.39.0.rc2.1048.g0e5493b8d5b

[PATCH 2/6] cocci & cache.h: fully apply "active_nr" part of index-compatibility

[Ævar Arnfjörð Bjarmason]

Apply the "active_nr" part of "index-compatibility.pending.cocci",
which was left out in [1] due to an in-flight conflict. As of [2] the
topic we conflicted with has been merged to "master", so we can fully
apply this rule.

1. dc594180d9e (cocci & cache.h: apply variable section of "pending"
   index-compatibility, 2022-11-19)
2. 9ea1378d046 (Merge branch 'ab/various-leak-fixes', 2022-12-14)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/commit.c                             |  2 +-
 cache.h                                      |  2 --
 contrib/coccinelle/index-compatibility.cocci | 13 ++++---------
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 44b763d7cd0..57a95123dff 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -991,7 +991,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		struct object_id oid;
 		const char *parent = "HEAD";
 
-		if (!active_nr) {
+		if (!the_index.cache_nr) {
 			discard_cache();
 			if (read_cache() < 0)
 				die(_("Cannot read index"));
diff --git a/cache.h b/cache.h
index 07d40b0964b..9de988574e0 100644
--- a/cache.h
+++ b/cache.h
@@ -438,8 +438,6 @@ extern struct index_state the_index;
 
 #ifndef USE_THE_INDEX_VARIABLE
 #ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
-#define active_nr (the_index.cache_nr)
-
 #define read_cache() repo_read_index(the_repository)
 #define discard_cache() discard_index(&the_index)
 #define cache_name_pos(name, namelen) index_name_pos(&the_index,(name),(namelen))
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index 8520f03128a..028ff53354a 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -1,6 +1,7 @@
 // the_index.* variables
 @@
 identifier AC = active_cache;
+identifier AN = active_nr;
 identifier ACC = active_cache_changed;
 identifier ACT = active_cache_tree;
 @@
@@ -8,6 +9,9 @@ identifier ACT = active_cache_tree;
 - AC
 + the_index.cache
 |
+- AN
++ the_index.cache_nr
+|
 - ACC
 + the_index.cache_changed
 |
@@ -15,15 +19,6 @@ identifier ACT = active_cache_tree;
 + the_index.cache_tree
 )
 
-@@
-identifier AN = active_nr;
-identifier f != prepare_to_commit;
-@@
-  f(...) {<...
-- AN
-+ the_index.cache_nr
-  ...>}
-
 // "the_repository" simple cases
 @@
 @@
-- 
2.39.0.rc2.1048.g0e5493b8d5b

[PATCH 3/6] cocci & cache.h: apply pending "index_cache_pos" rule

[Ævar Arnfjörð Bjarmason]

Apply the rule added in [1] to change "cache_name_pos" to
"index_name_pos", which allows us to get rid of another
"USE_THE_INDEX_COMPATIBILITY_MACROS" macro.

The replacement of "USE_THE_INDEX_COMPATIBILITY_MACROS" here with
"USE_THE_INDEX_VARIABLE" is a manual change on top, now that these
files only use "&the_index", and don't need any compatibility
macros (or functions).

1. 0e6550a2c63 (cocci: add a index-compatibility.pending.cocci,
   2022-11-19)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/mv.c                                         | 8 +++++---
 builtin/update-index.c                               | 4 ++--
 cache.h                                              | 1 -
 contrib/coccinelle/index-compatibility.cocci         | 3 +++
 contrib/coccinelle/index-compatibility.pending.cocci | 3 ---
 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 19790ce38fa..edd7b931fdb 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) 2006 Johannes Schindelin
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "pathspec.h"
@@ -489,7 +489,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 			if ((mode & SPARSE) &&
 			    path_in_sparse_checkout(dst, &the_index)) {
 				/* from out-of-cone to in-cone */
-				int dst_pos = cache_name_pos(dst, strlen(dst));
+				int dst_pos = index_name_pos(&the_index, dst,
+							     strlen(dst));
 				struct cache_entry *dst_ce = the_index.cache[dst_pos];
 
 				dst_ce->ce_flags &= ~CE_SKIP_WORKTREE;
@@ -500,7 +501,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 				   !(mode & SPARSE) &&
 				   !path_in_sparse_checkout(dst, &the_index)) {
 				/* from in-cone to out-of-cone */
-				int dst_pos = cache_name_pos(dst, strlen(dst));
+				int dst_pos = index_name_pos(&the_index, dst,
+							     strlen(dst));
 				struct cache_entry *dst_ce = the_index.cache[dst_pos];
 
 				/*
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 82d5902cc8b..bf38885d546 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "cache.h"
 #include "bulk-checkin.h"
 #include "config.h"
@@ -381,7 +381,7 @@ static int process_path(const char *path, struct stat *st, int stat_errno)
 	if (has_symlink_leading_path(path, len))
 		return error("'%s' is beyond a symbolic link", path);
 
-	pos = cache_name_pos(path, len);
+	pos = index_name_pos(&the_index, path, len);
 	ce = pos < 0 ? NULL : the_index.cache[pos];
 	if (ce && ce_skip_worktree(ce)) {
 		/*
diff --git a/cache.h b/cache.h
index 9de988574e0..4c79e732e45 100644
--- a/cache.h
+++ b/cache.h
@@ -440,7 +440,6 @@ extern struct index_state the_index;
 #ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
 #define read_cache() repo_read_index(the_repository)
 #define discard_cache() discard_index(&the_index)
-#define cache_name_pos(name, namelen) index_name_pos(&the_index,(name),(namelen))
 #endif
 #endif
 #endif
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index 028ff53354a..1d37546fdbd 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -91,6 +91,9 @@ identifier ACT = active_cache_tree;
 |
 - resolve_undo_clear
 + resolve_undo_clear_index
+|
+- cache_name_pos
++ index_name_pos
 )
   (
 + &the_index,
diff --git a/contrib/coccinelle/index-compatibility.pending.cocci b/contrib/coccinelle/index-compatibility.pending.cocci
index 01f875d0060..ecf3b45deca 100644
--- a/contrib/coccinelle/index-compatibility.pending.cocci
+++ b/contrib/coccinelle/index-compatibility.pending.cocci
@@ -15,9 +15,6 @@
 (
 - discard_cache
 + discard_index
-|
-- cache_name_pos
-+ index_name_pos
 )
   (
 + &the_index,
-- 
2.39.0.rc2.1048.g0e5493b8d5b

[PATCH 4/6] cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*"

[Ævar Arnfjörð Bjarmason]

Add a trivial rule for "write_cache_as_tree" to
"index-compatibility.cocci", and apply it. This was left out of the
rules added in 0e6550a2c63 (cocci: add a
index-compatibility.pending.cocci, 2022-11-19) because this
compatibility wrapper lived in "cache-tree.h", not "cache.h"

But it's like the other "USE_THE_INDEX_COMPATIBILITY_MACROS", so let's
migrate it too.

The replacement of "USE_THE_INDEX_COMPATIBILITY_MACROS" here with
"USE_THE_INDEX_VARIABLE" is a manual change on top, now that these
files only use "&the_index", and don't need any compatibility
macros (or functions).

The wrapping of some argument lists is likewise manual, as coccinelle
would otherwise give us overly long argument lists.

The reason for putting the "O" in the cocci rule on the "-" and "+"
lines is because I couldn't get correct whitespacing otherwise,
i.e. I'd end up with "oid,&the_index", not "oid, &the_index".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/am.c                                 |  6 +++---
 builtin/merge.c                              |  2 +-
 builtin/stash.c                              | 11 +++++++----
 builtin/write-tree.c                         |  5 +++--
 cache-tree.h                                 |  5 -----
 contrib/coccinelle/index-compatibility.cocci | 11 +++++++++++
 6 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 30c9b3a9cd7..0e2e86fe3d1 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -3,7 +3,7 @@
  *
  * Based on git-am.sh by Junio C Hamano.
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "cache.h"
 #include "config.h"
 #include "builtin.h"
@@ -1643,7 +1643,7 @@ static void do_commit(const struct am_state *state)
 	if (run_hooks("pre-applypatch"))
 		exit(1);
 
-	if (write_cache_as_tree(&tree, 0, NULL))
+	if (write_index_as_tree(&tree, &the_index, get_index_file(), 0, NULL))
 		die(_("git write-tree failed to write a tree"));
 
 	if (!get_oid_commit("HEAD", &parent)) {
@@ -2051,7 +2051,7 @@ static int clean_index(const struct object_id *head, const struct object_id *rem
 	if (fast_forward_to(head_tree, head_tree, 1))
 		return -1;
 
-	if (write_cache_as_tree(&index, 0, NULL))
+	if (write_index_as_tree(&index, &the_index, get_index_file(), 0, NULL))
 		return -1;
 
 	index_tree = parse_tree_indirect(&index);
diff --git a/builtin/merge.c b/builtin/merge.c
index ecccd5e9119..ad2e4114617 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -706,7 +706,7 @@ static int read_tree_trivial(struct object_id *common, struct object_id *head,
 
 static void write_tree_trivial(struct object_id *oid)
 {
-	if (write_cache_as_tree(oid, 0, NULL))
+	if (write_index_as_tree(oid, &the_index, get_index_file(), 0, NULL))
 		die(_("git write-tree failed to write a tree"));
 }
 
diff --git a/builtin/stash.c b/builtin/stash.c
index bb0fd861434..ba5d4c6c516 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1,4 +1,4 @@
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "parse-options.h"
@@ -528,7 +528,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 					 NULL, NULL, NULL))
 		return -1;
 
-	if (write_cache_as_tree(&c_tree, 0, NULL))
+	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
+				NULL))
 		return error(_("cannot apply a stash in the middle of a merge"));
 
 	if (index) {
@@ -552,7 +553,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 
 			discard_index(&the_index);
 			repo_read_index(the_repository);
-			if (write_cache_as_tree(&index_tree, 0, NULL))
+			if (write_index_as_tree(&index_tree, &the_index,
+						get_index_file(), 0, NULL))
 				return error(_("could not save index tree"));
 
 			reset_head();
@@ -1377,7 +1379,8 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 
 	strbuf_addf(&commit_tree_label, "index on %s\n", msg.buf);
 	commit_list_insert(head_commit, &parents);
-	if (write_cache_as_tree(&info->i_tree, 0, NULL) ||
+	if (write_index_as_tree(&info->i_tree, &the_index, get_index_file(), 0,
+				NULL) ||
 	    commit_tree(commit_tree_label.buf, commit_tree_label.len,
 			&info->i_tree, parents, &info->i_commit, NULL, NULL)) {
 		if (!quiet)
diff --git a/builtin/write-tree.c b/builtin/write-tree.c
index 45d61707e7d..078010315f0 100644
--- a/builtin/write-tree.c
+++ b/builtin/write-tree.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "cache.h"
 #include "config.h"
@@ -38,7 +38,8 @@ int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)
 	argc = parse_options(argc, argv, cmd_prefix, write_tree_options,
 			     write_tree_usage, 0);
 
-	ret = write_cache_as_tree(&oid, flags, tree_prefix);
+	ret = write_index_as_tree(&oid, &the_index, get_index_file(), flags,
+				  tree_prefix);
 	switch (ret) {
 	case 0:
 		printf("%s\n", oid_to_hex(&oid));
diff --git a/cache-tree.h b/cache-tree.h
index 8efeccebfc9..84890c9ff32 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -55,11 +55,6 @@ void prime_cache_tree(struct repository *, struct index_state *, struct tree *);
 int cache_tree_matches_traversal(struct cache_tree *, struct name_entry *ent, struct traverse_info *info);
 
 #ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
-static inline int write_cache_as_tree(struct object_id *oid, int flags, const char *prefix)
-{
-	return write_index_as_tree(oid, &the_index, get_index_file(), flags, prefix);
-}
-
 static inline int update_main_cache_tree(int flags)
 {
 	if (!the_index.cache_tree)
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index 1d37546fdbd..e245d805dcd 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -135,3 +135,14 @@ identifier ACT = active_cache_tree;
   ...
 + , NULL, NULL, NULL
   )
+
+@@
+expression O;
+@@
+- write_cache_as_tree
++ write_index_as_tree
+  (
+- O,
++ O, &the_index, get_index_file(),
+  ...
+  )
-- 
2.39.0.rc2.1048.g0e5493b8d5b

[PATCH 5/6] cache-tree API: remove redundant update_main_cache_tree()

[Ævar Arnfjörð Bjarmason]

Remove the redundant update_main_cache_tree() function, and make its
users use cache_tree_update() instead.

The behavior of populating the "the_index.cache_tree" if it wasn't
present already was needed when this function was introduced in [1],
but it hasn't been needed since [2]; The "cache_tree_update()" will
now lazy-allocate, so there's no need for the wrapper.

1. 996277c5206 (Refactor cache_tree_update idiom from commit,
   2011-12-06)
2. fb0882648e0 (cache-tree: clean up cache_tree_update(), 2021-01-23)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/commit.c                             | 10 +++++-----
 cache-tree.h                                 | 10 ----------
 contrib/coccinelle/index-compatibility.cocci |  3 +++
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 57a95123dff..31fbbd73d16 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -414,7 +414,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 		discard_index(&the_index);
 		read_index_from(&the_index, get_lock_file_path(&index_lock),
 				get_git_dir());
-		if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
+		if (cache_tree_update(&the_index, WRITE_TREE_SILENT) == 0) {
 			if (reopen_lock_file(&index_lock) < 0)
 				die(_("unable to write index file"));
 			if (write_locked_index(&the_index, &index_lock, 0))
@@ -444,7 +444,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 				       LOCK_DIE_ON_ERROR);
 		add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
-		update_main_cache_tree(WRITE_TREE_SILENT);
+		cache_tree_update(&the_index, WRITE_TREE_SILENT);
 		if (write_locked_index(&the_index, &index_lock, 0))
 			die(_("unable to write new_index file"));
 		commit_style = COMMIT_NORMAL;
@@ -467,7 +467,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 		refresh_cache_or_die(refresh_flags);
 		if (the_index.cache_changed
 		    || !cache_tree_fully_valid(the_index.cache_tree))
-			update_main_cache_tree(WRITE_TREE_SILENT);
+			cache_tree_update(&the_index, WRITE_TREE_SILENT);
 		if (write_locked_index(&the_index, &index_lock,
 				       COMMIT_LOCK | SKIP_IF_UNCHANGED))
 			die(_("unable to write new_index file"));
@@ -516,7 +516,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 	repo_hold_locked_index(the_repository, &index_lock, LOCK_DIE_ON_ERROR);
 	add_remove_files(&partial);
 	refresh_index(&the_index, REFRESH_QUIET, NULL, NULL, NULL);
-	update_main_cache_tree(WRITE_TREE_SILENT);
+	cache_tree_update(&the_index, WRITE_TREE_SILENT);
 	if (write_locked_index(&the_index, &index_lock, 0))
 		die(_("unable to write new_index file"));
 
@@ -1079,7 +1079,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	}
 	read_index_from(&the_index, index_file, get_git_dir());
 
-	if (update_main_cache_tree(0)) {
+	if (cache_tree_update(&the_index, 0)) {
 		error(_("Error building trees"));
 		return 0;
 	}
diff --git a/cache-tree.h b/cache-tree.h
index 84890c9ff32..bd97caa07b0 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -53,14 +53,4 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
 void prime_cache_tree(struct repository *, struct index_state *, struct tree *);
 
 int cache_tree_matches_traversal(struct cache_tree *, struct name_entry *ent, struct traverse_info *info);
-
-#ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
-static inline int update_main_cache_tree(int flags)
-{
-	if (!the_index.cache_tree)
-		the_index.cache_tree = cache_tree();
-	return cache_tree_update(&the_index, flags);
-}
-#endif
-
 #endif
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index e245d805dcd..9fca870162d 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -94,6 +94,9 @@ identifier ACT = active_cache_tree;
 |
 - cache_name_pos
 + index_name_pos
+|
+- update_main_cache_tree
++ cache_tree_update
 )
   (
 + &the_index,
-- 
2.39.0.rc2.1048.g0e5493b8d5b

[PATCH 6/6] cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS"

[Ævar Arnfjörð Bjarmason]

Have the last users of "USE_THE_INDEX_COMPATIBILITY_MACROS" use the
underlying *_index() variants instead. Now all previous users of
"USE_THE_INDEX_COMPATIBILITY_MACROS" have been migrated away from the
wrapper macros, and if applicable to use the "USE_THE_INDEX_VARIABLE"
added in [1].

Let's leave the "index-compatibility.cocci" in place, even though it
won't be doing anything on "master". It will benefit any out-of-tree
code that need to use these compatibility macros. We can eventually
remove it.

1. bdafeae0b9c (cache.h & test-tool.h: add & use
   "USE_THE_INDEX_VARIABLE", 2022-11-19)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/commit.c                              |  6 +++---
 builtin/merge.c                               |  6 +++---
 cache.h                                       |  9 +-------
 contrib/coccinelle/index-compatibility.cocci  |  6 ++++++
 .../index-compatibility.pending.cocci         | 21 -------------------
 5 files changed, 13 insertions(+), 35 deletions(-)
 delete mode 100644 contrib/coccinelle/index-compatibility.pending.cocci

diff --git a/builtin/commit.c b/builtin/commit.c
index 31fbbd73d16..985a0445b78 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -5,7 +5,7 @@
  * Based on git-commit.sh by Junio C Hamano and Linus Torvalds
  */
 
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "cache.h"
 #include "config.h"
 #include "lockfile.h"
@@ -992,8 +992,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		const char *parent = "HEAD";
 
 		if (!the_index.cache_nr) {
-			discard_cache();
-			if (read_cache() < 0)
+			discard_index(&the_index);
+			if (repo_read_index(the_repository) < 0)
 				die(_("Cannot read index"));
 		}
 
diff --git a/builtin/merge.c b/builtin/merge.c
index ad2e4114617..659fa10481e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -6,7 +6,7 @@
  * Based on git-merge.sh by Junio C Hamano.
  */
 
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "cache.h"
 #include "config.h"
 #include "parse-options.h"
@@ -390,8 +390,8 @@ static void restore_state(const struct object_id *head,
 	run_command(&cmd);
 
 refresh_cache:
-	discard_cache();
-	if (read_cache() < 0)
+	discard_index(&the_index);
+	if (repo_read_index(the_repository) < 0)
 		die(_("could not read index"));
 }
 
diff --git a/cache.h b/cache.h
index 4c79e732e45..36bd5667334 100644
--- a/cache.h
+++ b/cache.h
@@ -433,15 +433,8 @@ typedef int (*must_prefetch_predicate)(const struct cache_entry *);
 void prefetch_cache_entries(const struct index_state *istate,
 			    must_prefetch_predicate must_prefetch);
 
-#if defined(USE_THE_INDEX_COMPATIBILITY_MACROS) || defined(USE_THE_INDEX_VARIABLE)
+#ifdef USE_THE_INDEX_VARIABLE
 extern struct index_state the_index;
-
-#ifndef USE_THE_INDEX_VARIABLE
-#ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
-#define read_cache() repo_read_index(the_repository)
-#define discard_cache() discard_index(&the_index)
-#endif
-#endif
 #endif
 
 #define TYPE_BITS 3
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index 9fca870162d..31e36cf3c41 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -23,6 +23,9 @@ identifier ACT = active_cache_tree;
 @@
 @@
 (
+- read_cache
++ repo_read_index
+|
 - read_cache_unmerged
 + repo_read_index_unmerged
 |
@@ -97,6 +100,9 @@ identifier ACT = active_cache_tree;
 |
 - update_main_cache_tree
 + cache_tree_update
+|
+- discard_cache
++ discard_index
 )
   (
 + &the_index,
diff --git a/contrib/coccinelle/index-compatibility.pending.cocci b/contrib/coccinelle/index-compatibility.pending.cocci
deleted file mode 100644
index ecf3b45deca..00000000000
--- a/contrib/coccinelle/index-compatibility.pending.cocci
+++ /dev/null
@@ -1,21 +0,0 @@
-// "the_repository" simple cases
-@@
-@@
-(
-- read_cache
-+ repo_read_index
-)
-  (
-+ the_repository,
-  ...)
-
-// "the_index" simple cases
-@@
-@@
-(
-- discard_cache
-+ discard_index
-)
-  (
-+ &the_index,
-  ...)
-- 
2.39.0.rc2.1048.g0e5493b8d5b

Re: [PATCH 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[Phillip Wood]

Hi Ævar

On 15/12/2022 09:58, Ævar Arnfjörð Bjarmason wrote:
> My recent now-landed topic[1] to remove most use of
> "USE_THE_INDEX_COMPATIBILITY_MACROS" was merged in 041df69edd3 (Merge
> branch 'ab/fewer-the-index-macros', 2022-11-28).
> 
> It left out use of the macros that would have conflicted with
> in-flight changes, but as those topics have landed we can now complete
> the migration.
> 
> As before this is almost entirely a matter of applying the existing
> "pending" coccinelle rules, the exceptions being 1/6, and the *.h
> changes where we remove the macro definitions (the macro users being
> edited by coccinelle).
> 
> The 4-5/6 then handle some edge cases we had left (but the code change
> itself is done by coccinelle).

I've only given these patches a quick scan, but I think they look good. 
None of the callers that are converted here are in library code so using 
the_index makes perfect sense.

Best Wishes

Phillip


> 1. https://lore.kernel.org/git/cover-v2-00.11-00000000000-20221119T125550Z-avarab@gmail.com/
> 
> Ævar Arnfjörð Bjarmason (6):
>    builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"
>    cocci & cache.h: fully apply "active_nr" part of index-compatibility
>    cocci & cache.h: apply pending "index_cache_pos" rule
>    cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*"
>    cache-tree API: remove redundant update_main_cache_tree()
>    cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS"
> 
>   builtin/am.c                                  |  6 ++--
>   builtin/commit.c                              | 18 +++++-----
>   builtin/merge.c                               |  8 ++---
>   builtin/mv.c                                  |  8 +++--
>   builtin/rm.c                                  |  2 +-
>   builtin/stash.c                               | 11 +++---
>   builtin/update-index.c                        |  4 +--
>   builtin/write-tree.c                          |  5 +--
>   cache-tree.h                                  | 15 --------
>   cache.h                                       | 12 +------
>   contrib/coccinelle/index-compatibility.cocci  | 36 ++++++++++++++-----
>   .../index-compatibility.pending.cocci         | 24 -------------
>   12 files changed, 62 insertions(+), 87 deletions(-)
>   delete mode 100644 contrib/coccinelle/index-compatibility.pending.cocci
> 

Re: [PATCH 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[Ævar Arnfjörð Bjarmason]

On Mon, Dec 19 2022, Phillip Wood wrote:

> Hi Ævar
>
> On 15/12/2022 09:58, Ævar Arnfjörð Bjarmason wrote:
>> My recent now-landed topic[1] to remove most use of
>> "USE_THE_INDEX_COMPATIBILITY_MACROS" was merged in 041df69edd3 (Merge
>> branch 'ab/fewer-the-index-macros', 2022-11-28).
>> It left out use of the macros that would have conflicted with
>> in-flight changes, but as those topics have landed we can now complete
>> the migration.
>> As before this is almost entirely a matter of applying the existing
>> "pending" coccinelle rules, the exceptions being 1/6, and the *.h
>> changes where we remove the macro definitions (the macro users being
>> edited by coccinelle).
>> The 4-5/6 then handle some edge cases we had left (but the code
>> change
>> itself is done by coccinelle).
>
> I've only given these patches a quick scan, but I think they look
> good. None of the callers that are converted here are in library code
> so using the_index makes perfect sense.

Thanks for the review.

That's correct, although even if that were the case that wouldn't be an
issue with this migration, as we'd have been using "the_index" before,
just indirectly through a macro.

That wasn't the case here, but I do I have another similar migration for
migrating "the_repository" wrappers.

In those cases there's surely instances where e.g. we really should be
using a "r" argument instead, but I've opted to leave that question out,
as it would make the coccinelle rules involved & diffs much harder to
deal with.

And because in the end the result is the same if viewed with "cc -E",
i.e. these are just the macro shims we've been meaning to stop using for
a while.

Re: [PATCH 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[Phillip Wood]

On 19/12/2022 15:11, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Dec 19 2022, Phillip Wood wrote:
> 
>> Hi Ævar
>>
>> On 15/12/2022 09:58, Ævar Arnfjörð Bjarmason wrote:
>>> My recent now-landed topic[1] to remove most use of
>>> "USE_THE_INDEX_COMPATIBILITY_MACROS" was merged in 041df69edd3 (Merge
>>> branch 'ab/fewer-the-index-macros', 2022-11-28).
>>> It left out use of the macros that would have conflicted with
>>> in-flight changes, but as those topics have landed we can now complete
>>> the migration.
>>> As before this is almost entirely a matter of applying the existing
>>> "pending" coccinelle rules, the exceptions being 1/6, and the *.h
>>> changes where we remove the macro definitions (the macro users being
>>> edited by coccinelle).
>>> The 4-5/6 then handle some edge cases we had left (but the code
>>> change
>>> itself is done by coccinelle).
>>
>> I've only given these patches a quick scan, but I think they look
>> good. None of the callers that are converted here are in library code
>> so using the_index makes perfect sense.
> 
> Thanks for the review.
> 
> That's correct, although even if that were the case that wouldn't be an
> issue with this migration, as we'd have been using "the_index" before,
> just indirectly through a macro.

Indeed, I'm just not convinced that it is worth removing the macro in 
library code without changing to take a struct istate (I don't see the 
existence of the macro itself as a problem as I think it is just a 
symptom of the real problem) but I seem to be in the minority on that point.

Best Wishes

Phillip

> That wasn't the case here, but I do I have another similar migration for
> migrating "the_repository" wrappers.
> 
> In those cases there's surely instances where e.g. we really should be
> using a "r" argument instead, but I've opted to leave that question out,
> as it would make the coccinelle rules involved & diffs much harder to
> deal with.
> 
> And because in the end the result is the same if viewed with "cc -E",
> i.e. these are just the macro shims we've been meaning to stop using for
> a while.
> 

Re: [PATCH 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

>> That's correct, although even if that were the case that wouldn't
>> be an issue with this migration, as we'd have been using
>> "the_index" before, just indirectly through a macro.
>
> Indeed, I'm just not convinced that it is worth removing the macro in
> library code without changing to take a struct istate (I don't see the
> existence of the macro itself as a problem as I think it is just a
> symptom of the real problem) but I seem to be in the minority on that
> point.

True.

Many subcommands need to deal only with the_index and no other
index, so for the implementations of the top-level subcommands that
work only in a single repository, the macros are not by themselves
problems.  The deeper parts of the system that we want to reuse by
libifying of course eventually need to learn to take an arbitrary
"istate" and NO_THE_INDEX_COMPATIBILITY_MACROS mechanism (and its
successor USE_THE_INDEX_COMPATIBILITY_MACROS, probably) was a great
approach for that goal.

Re: [PATCH 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[Ævar Arnfjörð Bjarmason]

On Tue, Dec 20 2022, Junio C Hamano wrote:

> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>>> That's correct, although even if that were the case that wouldn't
>>> be an issue with this migration, as we'd have been using
>>> "the_index" before, just indirectly through a macro.
>>
>> Indeed, I'm just not convinced that it is worth removing the macro in
>> library code without changing to take a struct istate (I don't see the
>> existence of the macro itself as a problem as I think it is just a
>> symptom of the real problem) but I seem to be in the minority on that
>> point.
>
> True.
>
> Many subcommands need to deal only with the_index and no other
> index, so for the implementations of the top-level subcommands that
> work only in a single repository, the macros are not by themselves
> problems.  The deeper parts of the system that we want to reuse by
> libifying of course eventually need to learn to take an arbitrary
> "istate" and NO_THE_INDEX_COMPATIBILITY_MACROS mechanism (and its
> successor USE_THE_INDEX_COMPATIBILITY_MACROS, probably) was a great
> approach for that goal.

I'm not sure what to make of this comment & this series not having been
picked up (perhaps I'm reading too much into that), that you'd like to
keep USE_THE_INDEX_COMPATIBILITY_MACROS?

This side-thread is discussing a theoretical issue.

Phillip was saying (if I understand him correctly) that if we were using
"the_index" in libraries we should fix that, I was agreeing, but saying
that if that were the case this series would still be a good step
forward, we could fix those issues later. It looks like we disagreed on
the "later" part of that.

But in any case, as noted at the start of this thread there are no such
library users, so it's a moot point.

[PATCH v2 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[Ævar Arnfjörð Bjarmason]

Most of our use of these compatibility macros went away with
041df69edd3 (Merge branch 'ab/fewer-the-index-macros', 2022-11-28) ,
which was part of v2.39.0.

That topic left out these stragglers, as some of this would have
conflicted with in-flight topics, and I'd skipped the cache-tree.h
cases altogether.

The update in v2 is trivial, just to rebase the series for changes on
"master". There are no semantic or textual conflicts with "seen"
either, so finishing this migration before we get another user of them
would be nice.

The v1 had a side discussion that didn't need resolving here. The
question was what a series like this might do if we needed to convert
library code to make new use of "the_index" (as opposed to converting
the functions themselves to take it from their callers).

That's an interesting question, but irrelevant to this topic, as
there's no such library users to deal with, and this migration closes
the door on that hypothetical question needing to be addressed in the
future.

Ævar Arnfjörð Bjarmason (6):
  builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"
  cocci & cache.h: fully apply "active_nr" part of index-compatibility
  cocci & cache.h: apply pending "index_cache_pos" rule
  cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*"
  cache-tree API: remove redundant update_main_cache_tree()
  cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS"

 builtin/am.c                                  |  6 ++--
 builtin/commit.c                              | 18 +++++-----
 builtin/merge.c                               |  8 ++---
 builtin/mv.c                                  |  8 +++--
 builtin/rm.c                                  |  2 +-
 builtin/stash.c                               | 11 +++---
 builtin/update-index.c                        |  4 +--
 builtin/write-tree.c                          |  5 +--
 cache-tree.h                                  | 15 --------
 cache.h                                       | 12 +------
 contrib/coccinelle/index-compatibility.cocci  | 36 ++++++++++++++-----
 .../index-compatibility.pending.cocci         | 24 -------------
 12 files changed, 62 insertions(+), 87 deletions(-)
 delete mode 100644 contrib/coccinelle/index-compatibility.pending.cocci

Range-diff against v1:
1:  3517389f732 = 1:  916761cb50f builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"
2:  03c6e404367 = 2:  6040edad622 cocci & cache.h: fully apply "active_nr" part of index-compatibility
3:  2dbe4f45363 = 3:  3e9d97dbff2 cocci & cache.h: apply pending "index_cache_pos" rule
4:  679ddc857c1 ! 4:  e36a0ae562f cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*"
    @@ builtin/am.c
      #include "config.h"
      #include "builtin.h"
     @@ builtin/am.c: static void do_commit(const struct am_state *state)
    - 	if (run_hooks("pre-applypatch"))
    + 	if (!state->no_verify && run_hooks("pre-applypatch"))
      		exit(1);
      
     -	if (write_cache_as_tree(&tree, 0, NULL))
5:  7f956fd8b75 = 5:  ab8794da29c cache-tree API: remove redundant update_main_cache_tree()
6:  4807a3fe8ff = 6:  77c30cfe455 cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS"
-- 
2.39.1.1475.gc2542cdc5ef

[PATCH v2 1/6] builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"

[Ævar Arnfjörð Bjarmason]

Replace the "USE_THE_INDEX_COMPATIBILITY_MACROS" define with the
narrower "USE_THE_INDEX_VARIABLE". This could have been done in
07047d68294 (cocci: apply "pending" index-compatibility to some
"builtin/*.c", 2022-11-19), but I missed it at the time.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/rm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 4a4aec0d00e..8844f906557 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) Linus Torvalds 2006
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "advice.h"
 #include "config.h"
-- 
2.39.1.1475.gc2542cdc5ef

[PATCH v2 2/6] cocci & cache.h: fully apply "active_nr" part of index-compatibility

[Ævar Arnfjörð Bjarmason]

Apply the "active_nr" part of "index-compatibility.pending.cocci",
which was left out in [1] due to an in-flight conflict. As of [2] the
topic we conflicted with has been merged to "master", so we can fully
apply this rule.

1. dc594180d9e (cocci & cache.h: apply variable section of "pending"
   index-compatibility, 2022-11-19)
2. 9ea1378d046 (Merge branch 'ab/various-leak-fixes', 2022-12-14)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/commit.c                             |  2 +-
 cache.h                                      |  2 --
 contrib/coccinelle/index-compatibility.cocci | 13 ++++---------
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 44b763d7cd0..57a95123dff 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -991,7 +991,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		struct object_id oid;
 		const char *parent = "HEAD";
 
-		if (!active_nr) {
+		if (!the_index.cache_nr) {
 			discard_cache();
 			if (read_cache() < 0)
 				die(_("Cannot read index"));
diff --git a/cache.h b/cache.h
index 4bf14e0bd94..b0bbecf35ef 100644
--- a/cache.h
+++ b/cache.h
@@ -454,8 +454,6 @@ extern struct index_state the_index;
 
 #ifndef USE_THE_INDEX_VARIABLE
 #ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
-#define active_nr (the_index.cache_nr)
-
 #define read_cache() repo_read_index(the_repository)
 #define discard_cache() discard_index(&the_index)
 #define cache_name_pos(name, namelen) index_name_pos(&the_index,(name),(namelen))
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index 8520f03128a..028ff53354a 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -1,6 +1,7 @@
 // the_index.* variables
 @@
 identifier AC = active_cache;
+identifier AN = active_nr;
 identifier ACC = active_cache_changed;
 identifier ACT = active_cache_tree;
 @@
@@ -8,6 +9,9 @@ identifier ACT = active_cache_tree;
 - AC
 + the_index.cache
 |
+- AN
++ the_index.cache_nr
+|
 - ACC
 + the_index.cache_changed
 |
@@ -15,15 +19,6 @@ identifier ACT = active_cache_tree;
 + the_index.cache_tree
 )
 
-@@
-identifier AN = active_nr;
-identifier f != prepare_to_commit;
-@@
-  f(...) {<...
-- AN
-+ the_index.cache_nr
-  ...>}
-
 // "the_repository" simple cases
 @@
 @@
-- 
2.39.1.1475.gc2542cdc5ef

[PATCH v2 3/6] cocci & cache.h: apply pending "index_cache_pos" rule

[Ævar Arnfjörð Bjarmason]

Apply the rule added in [1] to change "cache_name_pos" to
"index_name_pos", which allows us to get rid of another
"USE_THE_INDEX_COMPATIBILITY_MACROS" macro.

The replacement of "USE_THE_INDEX_COMPATIBILITY_MACROS" here with
"USE_THE_INDEX_VARIABLE" is a manual change on top, now that these
files only use "&the_index", and don't need any compatibility
macros (or functions).

1. 0e6550a2c63 (cocci: add a index-compatibility.pending.cocci,
   2022-11-19)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/mv.c                                         | 8 +++++---
 builtin/update-index.c                               | 4 ++--
 cache.h                                              | 1 -
 contrib/coccinelle/index-compatibility.cocci         | 3 +++
 contrib/coccinelle/index-compatibility.pending.cocci | 3 ---
 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 19790ce38fa..edd7b931fdb 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) 2006 Johannes Schindelin
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "pathspec.h"
@@ -489,7 +489,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 			if ((mode & SPARSE) &&
 			    path_in_sparse_checkout(dst, &the_index)) {
 				/* from out-of-cone to in-cone */
-				int dst_pos = cache_name_pos(dst, strlen(dst));
+				int dst_pos = index_name_pos(&the_index, dst,
+							     strlen(dst));
 				struct cache_entry *dst_ce = the_index.cache[dst_pos];
 
 				dst_ce->ce_flags &= ~CE_SKIP_WORKTREE;
@@ -500,7 +501,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 				   !(mode & SPARSE) &&
 				   !path_in_sparse_checkout(dst, &the_index)) {
 				/* from in-cone to out-of-cone */
-				int dst_pos = cache_name_pos(dst, strlen(dst));
+				int dst_pos = index_name_pos(&the_index, dst,
+							     strlen(dst));
 				struct cache_entry *dst_ce = the_index.cache[dst_pos];
 
 				/*
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 82d5902cc8b..bf38885d546 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "cache.h"
 #include "bulk-checkin.h"
 #include "config.h"
@@ -381,7 +381,7 @@ static int process_path(const char *path, struct stat *st, int stat_errno)
 	if (has_symlink_leading_path(path, len))
 		return error("'%s' is beyond a symbolic link", path);
 
-	pos = cache_name_pos(path, len);
+	pos = index_name_pos(&the_index, path, len);
 	ce = pos < 0 ? NULL : the_index.cache[pos];
 	if (ce && ce_skip_worktree(ce)) {
 		/*
diff --git a/cache.h b/cache.h
index b0bbecf35ef..c44aef1af7c 100644
--- a/cache.h
+++ b/cache.h
@@ -456,7 +456,6 @@ extern struct index_state the_index;
 #ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
 #define read_cache() repo_read_index(the_repository)
 #define discard_cache() discard_index(&the_index)
-#define cache_name_pos(name, namelen) index_name_pos(&the_index,(name),(namelen))
 #endif
 #endif
 #endif
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index 028ff53354a..1d37546fdbd 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -91,6 +91,9 @@ identifier ACT = active_cache_tree;
 |
 - resolve_undo_clear
 + resolve_undo_clear_index
+|
+- cache_name_pos
++ index_name_pos
 )
   (
 + &the_index,
diff --git a/contrib/coccinelle/index-compatibility.pending.cocci b/contrib/coccinelle/index-compatibility.pending.cocci
index 01f875d0060..ecf3b45deca 100644
--- a/contrib/coccinelle/index-compatibility.pending.cocci
+++ b/contrib/coccinelle/index-compatibility.pending.cocci
@@ -15,9 +15,6 @@
 (
 - discard_cache
 + discard_index
-|
-- cache_name_pos
-+ index_name_pos
 )
   (
 + &the_index,
-- 
2.39.1.1475.gc2542cdc5ef

[PATCH v2 4/6] cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*"

[Ævar Arnfjörð Bjarmason]

Add a trivial rule for "write_cache_as_tree" to
"index-compatibility.cocci", and apply it. This was left out of the
rules added in 0e6550a2c63 (cocci: add a
index-compatibility.pending.cocci, 2022-11-19) because this
compatibility wrapper lived in "cache-tree.h", not "cache.h"

But it's like the other "USE_THE_INDEX_COMPATIBILITY_MACROS", so let's
migrate it too.

The replacement of "USE_THE_INDEX_COMPATIBILITY_MACROS" here with
"USE_THE_INDEX_VARIABLE" is a manual change on top, now that these
files only use "&the_index", and don't need any compatibility
macros (or functions).

The wrapping of some argument lists is likewise manual, as coccinelle
would otherwise give us overly long argument lists.

The reason for putting the "O" in the cocci rule on the "-" and "+"
lines is because I couldn't get correct whitespacing otherwise,
i.e. I'd end up with "oid,&the_index", not "oid, &the_index".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/am.c                                 |  6 +++---
 builtin/merge.c                              |  2 +-
 builtin/stash.c                              | 11 +++++++----
 builtin/write-tree.c                         |  5 +++--
 cache-tree.h                                 |  5 -----
 contrib/coccinelle/index-compatibility.cocci | 11 +++++++++++
 6 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 82a41cbfc4e..8b3dcb66f08 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -3,7 +3,7 @@
  *
  * Based on git-am.sh by Junio C Hamano.
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "cache.h"
 #include "config.h"
 #include "builtin.h"
@@ -1655,7 +1655,7 @@ static void do_commit(const struct am_state *state)
 	if (!state->no_verify && run_hooks("pre-applypatch"))
 		exit(1);
 
-	if (write_cache_as_tree(&tree, 0, NULL))
+	if (write_index_as_tree(&tree, &the_index, get_index_file(), 0, NULL))
 		die(_("git write-tree failed to write a tree"));
 
 	if (!get_oid_commit("HEAD", &parent)) {
@@ -2063,7 +2063,7 @@ static int clean_index(const struct object_id *head, const struct object_id *rem
 	if (fast_forward_to(head_tree, head_tree, 1))
 		return -1;
 
-	if (write_cache_as_tree(&index, 0, NULL))
+	if (write_index_as_tree(&index, &the_index, get_index_file(), 0, NULL))
 		return -1;
 
 	index_tree = parse_tree_indirect(&index);
diff --git a/builtin/merge.c b/builtin/merge.c
index 74de2ebd2b3..d7cc8dc8aed 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -706,7 +706,7 @@ static int read_tree_trivial(struct object_id *common, struct object_id *head,
 
 static void write_tree_trivial(struct object_id *oid)
 {
-	if (write_cache_as_tree(oid, 0, NULL))
+	if (write_index_as_tree(oid, &the_index, get_index_file(), 0, NULL))
 		die(_("git write-tree failed to write a tree"));
 }
 
diff --git a/builtin/stash.c b/builtin/stash.c
index 839569a9803..78d69da8cf7 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1,4 +1,4 @@
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "parse-options.h"
@@ -528,7 +528,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 					 NULL, NULL, NULL))
 		return -1;
 
-	if (write_cache_as_tree(&c_tree, 0, NULL))
+	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
+				NULL))
 		return error(_("cannot apply a stash in the middle of a merge"));
 
 	if (index) {
@@ -552,7 +553,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 
 			discard_index(&the_index);
 			repo_read_index(the_repository);
-			if (write_cache_as_tree(&index_tree, 0, NULL))
+			if (write_index_as_tree(&index_tree, &the_index,
+						get_index_file(), 0, NULL))
 				return error(_("could not save index tree"));
 
 			reset_head();
@@ -1377,7 +1379,8 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 
 	strbuf_addf(&commit_tree_label, "index on %s\n", msg.buf);
 	commit_list_insert(head_commit, &parents);
-	if (write_cache_as_tree(&info->i_tree, 0, NULL) ||
+	if (write_index_as_tree(&info->i_tree, &the_index, get_index_file(), 0,
+				NULL) ||
 	    commit_tree(commit_tree_label.buf, commit_tree_label.len,
 			&info->i_tree, parents, &info->i_commit, NULL, NULL)) {
 		if (!quiet)
diff --git a/builtin/write-tree.c b/builtin/write-tree.c
index 45d61707e7d..078010315f0 100644
--- a/builtin/write-tree.c
+++ b/builtin/write-tree.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "cache.h"
 #include "config.h"
@@ -38,7 +38,8 @@ int cmd_write_tree(int argc, const char **argv, const char *cmd_prefix)
 	argc = parse_options(argc, argv, cmd_prefix, write_tree_options,
 			     write_tree_usage, 0);
 
-	ret = write_cache_as_tree(&oid, flags, tree_prefix);
+	ret = write_index_as_tree(&oid, &the_index, get_index_file(), flags,
+				  tree_prefix);
 	switch (ret) {
 	case 0:
 		printf("%s\n", oid_to_hex(&oid));
diff --git a/cache-tree.h b/cache-tree.h
index 8efeccebfc9..84890c9ff32 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -55,11 +55,6 @@ void prime_cache_tree(struct repository *, struct index_state *, struct tree *);
 int cache_tree_matches_traversal(struct cache_tree *, struct name_entry *ent, struct traverse_info *info);
 
 #ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
-static inline int write_cache_as_tree(struct object_id *oid, int flags, const char *prefix)
-{
-	return write_index_as_tree(oid, &the_index, get_index_file(), flags, prefix);
-}
-
 static inline int update_main_cache_tree(int flags)
 {
 	if (!the_index.cache_tree)
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index 1d37546fdbd..e245d805dcd 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -135,3 +135,14 @@ identifier ACT = active_cache_tree;
   ...
 + , NULL, NULL, NULL
   )
+
+@@
+expression O;
+@@
+- write_cache_as_tree
++ write_index_as_tree
+  (
+- O,
++ O, &the_index, get_index_file(),
+  ...
+  )
-- 
2.39.1.1475.gc2542cdc5ef

[PATCH v2 5/6] cache-tree API: remove redundant update_main_cache_tree()

[Ævar Arnfjörð Bjarmason]

Remove the redundant update_main_cache_tree() function, and make its
users use cache_tree_update() instead.

The behavior of populating the "the_index.cache_tree" if it wasn't
present already was needed when this function was introduced in [1],
but it hasn't been needed since [2]; The "cache_tree_update()" will
now lazy-allocate, so there's no need for the wrapper.

1. 996277c5206 (Refactor cache_tree_update idiom from commit,
   2011-12-06)
2. fb0882648e0 (cache-tree: clean up cache_tree_update(), 2021-01-23)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/commit.c                             | 10 +++++-----
 cache-tree.h                                 | 10 ----------
 contrib/coccinelle/index-compatibility.cocci |  3 +++
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 57a95123dff..31fbbd73d16 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -414,7 +414,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 		discard_index(&the_index);
 		read_index_from(&the_index, get_lock_file_path(&index_lock),
 				get_git_dir());
-		if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
+		if (cache_tree_update(&the_index, WRITE_TREE_SILENT) == 0) {
 			if (reopen_lock_file(&index_lock) < 0)
 				die(_("unable to write index file"));
 			if (write_locked_index(&the_index, &index_lock, 0))
@@ -444,7 +444,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 				       LOCK_DIE_ON_ERROR);
 		add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
-		update_main_cache_tree(WRITE_TREE_SILENT);
+		cache_tree_update(&the_index, WRITE_TREE_SILENT);
 		if (write_locked_index(&the_index, &index_lock, 0))
 			die(_("unable to write new_index file"));
 		commit_style = COMMIT_NORMAL;
@@ -467,7 +467,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 		refresh_cache_or_die(refresh_flags);
 		if (the_index.cache_changed
 		    || !cache_tree_fully_valid(the_index.cache_tree))
-			update_main_cache_tree(WRITE_TREE_SILENT);
+			cache_tree_update(&the_index, WRITE_TREE_SILENT);
 		if (write_locked_index(&the_index, &index_lock,
 				       COMMIT_LOCK | SKIP_IF_UNCHANGED))
 			die(_("unable to write new_index file"));
@@ -516,7 +516,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
 	repo_hold_locked_index(the_repository, &index_lock, LOCK_DIE_ON_ERROR);
 	add_remove_files(&partial);
 	refresh_index(&the_index, REFRESH_QUIET, NULL, NULL, NULL);
-	update_main_cache_tree(WRITE_TREE_SILENT);
+	cache_tree_update(&the_index, WRITE_TREE_SILENT);
 	if (write_locked_index(&the_index, &index_lock, 0))
 		die(_("unable to write new_index file"));
 
@@ -1079,7 +1079,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	}
 	read_index_from(&the_index, index_file, get_git_dir());
 
-	if (update_main_cache_tree(0)) {
+	if (cache_tree_update(&the_index, 0)) {
 		error(_("Error building trees"));
 		return 0;
 	}
diff --git a/cache-tree.h b/cache-tree.h
index 84890c9ff32..bd97caa07b0 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -53,14 +53,4 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
 void prime_cache_tree(struct repository *, struct index_state *, struct tree *);
 
 int cache_tree_matches_traversal(struct cache_tree *, struct name_entry *ent, struct traverse_info *info);
-
-#ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
-static inline int update_main_cache_tree(int flags)
-{
-	if (!the_index.cache_tree)
-		the_index.cache_tree = cache_tree();
-	return cache_tree_update(&the_index, flags);
-}
-#endif
-
 #endif
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index e245d805dcd..9fca870162d 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -94,6 +94,9 @@ identifier ACT = active_cache_tree;
 |
 - cache_name_pos
 + index_name_pos
+|
+- update_main_cache_tree
++ cache_tree_update
 )
   (
 + &the_index,
-- 
2.39.1.1475.gc2542cdc5ef

[PATCH v2 6/6] cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS"

[Ævar Arnfjörð Bjarmason]

Have the last users of "USE_THE_INDEX_COMPATIBILITY_MACROS" use the
underlying *_index() variants instead. Now all previous users of
"USE_THE_INDEX_COMPATIBILITY_MACROS" have been migrated away from the
wrapper macros, and if applicable to use the "USE_THE_INDEX_VARIABLE"
added in [1].

Let's leave the "index-compatibility.cocci" in place, even though it
won't be doing anything on "master". It will benefit any out-of-tree
code that need to use these compatibility macros. We can eventually
remove it.

1. bdafeae0b9c (cache.h & test-tool.h: add & use
   "USE_THE_INDEX_VARIABLE", 2022-11-19)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/commit.c                              |  6 +++---
 builtin/merge.c                               |  6 +++---
 cache.h                                       |  9 +-------
 contrib/coccinelle/index-compatibility.cocci  |  6 ++++++
 .../index-compatibility.pending.cocci         | 21 -------------------
 5 files changed, 13 insertions(+), 35 deletions(-)
 delete mode 100644 contrib/coccinelle/index-compatibility.pending.cocci

diff --git a/builtin/commit.c b/builtin/commit.c
index 31fbbd73d16..985a0445b78 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -5,7 +5,7 @@
  * Based on git-commit.sh by Junio C Hamano and Linus Torvalds
  */
 
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "cache.h"
 #include "config.h"
 #include "lockfile.h"
@@ -992,8 +992,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		const char *parent = "HEAD";
 
 		if (!the_index.cache_nr) {
-			discard_cache();
-			if (read_cache() < 0)
+			discard_index(&the_index);
+			if (repo_read_index(the_repository) < 0)
 				die(_("Cannot read index"));
 		}
 
diff --git a/builtin/merge.c b/builtin/merge.c
index d7cc8dc8aed..2aafbf67450 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -6,7 +6,7 @@
  * Based on git-merge.sh by Junio C Hamano.
  */
 
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
+#define USE_THE_INDEX_VARIABLE
 #include "cache.h"
 #include "config.h"
 #include "parse-options.h"
@@ -390,8 +390,8 @@ static void restore_state(const struct object_id *head,
 	run_command(&cmd);
 
 refresh_cache:
-	discard_cache();
-	if (read_cache() < 0)
+	discard_index(&the_index);
+	if (repo_read_index(the_repository) < 0)
 		die(_("could not read index"));
 }
 
diff --git a/cache.h b/cache.h
index c44aef1af7c..2f643cf5309 100644
--- a/cache.h
+++ b/cache.h
@@ -449,15 +449,8 @@ typedef int (*must_prefetch_predicate)(const struct cache_entry *);
 void prefetch_cache_entries(const struct index_state *istate,
 			    must_prefetch_predicate must_prefetch);
 
-#if defined(USE_THE_INDEX_COMPATIBILITY_MACROS) || defined(USE_THE_INDEX_VARIABLE)
+#ifdef USE_THE_INDEX_VARIABLE
 extern struct index_state the_index;
-
-#ifndef USE_THE_INDEX_VARIABLE
-#ifdef USE_THE_INDEX_COMPATIBILITY_MACROS
-#define read_cache() repo_read_index(the_repository)
-#define discard_cache() discard_index(&the_index)
-#endif
-#endif
 #endif
 
 #define TYPE_BITS 3
diff --git a/contrib/coccinelle/index-compatibility.cocci b/contrib/coccinelle/index-compatibility.cocci
index 9fca870162d..31e36cf3c41 100644
--- a/contrib/coccinelle/index-compatibility.cocci
+++ b/contrib/coccinelle/index-compatibility.cocci
@@ -23,6 +23,9 @@ identifier ACT = active_cache_tree;
 @@
 @@
 (
+- read_cache
++ repo_read_index
+|
 - read_cache_unmerged
 + repo_read_index_unmerged
 |
@@ -97,6 +100,9 @@ identifier ACT = active_cache_tree;
 |
 - update_main_cache_tree
 + cache_tree_update
+|
+- discard_cache
++ discard_index
 )
   (
 + &the_index,
diff --git a/contrib/coccinelle/index-compatibility.pending.cocci b/contrib/coccinelle/index-compatibility.pending.cocci
deleted file mode 100644
index ecf3b45deca..00000000000
--- a/contrib/coccinelle/index-compatibility.pending.cocci
+++ /dev/null
@@ -1,21 +0,0 @@
-// "the_repository" simple cases
-@@
-@@
-(
-- read_cache
-+ repo_read_index
-)
-  (
-+ the_repository,
-  ...)
-
-// "the_index" simple cases
-@@
-@@
-(
-- discard_cache
-+ discard_index
-)
-  (
-+ &the_index,
-  ...)
-- 
2.39.1.1475.gc2542cdc5ef

Re: [PATCH v2 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Most of our use of these compatibility macros went away with
> 041df69edd3 (Merge branch 'ab/fewer-the-index-macros', 2022-11-28) ,
> which was part of v2.39.0.
>
> That topic left out these stragglers, as some of this would have
> conflicted with in-flight topics, and I'd skipped the cache-tree.h
> cases altogether.
>
> The update in v2 is trivial, just to rebase the series for changes on
> "master". There are no semantic or textual conflicts with "seen"
> either, so finishing this migration before we get another user of them
> would be nice.

Yay.

Thanks.

Re: [PATCH v2 1/6] builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Replace the "USE_THE_INDEX_COMPATIBILITY_MACROS" define with the
> narrower "USE_THE_INDEX_VARIABLE". This could have been done in
> 07047d68294 (cocci: apply "pending" index-compatibility to some
> "builtin/*.c", 2022-11-19), but I missed it at the time.

It's a minor thing but can we stop saying "I did X" or "I didn't do
X".  It is not just your fault that this was missed.  Reviewers also
failed to spot it but we are not in the blame passing game.  "but it
was forgotten", "but it was missed", or "but nobody noticed it"
would suffice.

The patch is of course good.  For this kind of change, if it
compiles, it cannot be incorrect ;-)

Thanks.

Re: [PATCH v2 3/6] cocci & cache.h: apply pending "index_cache_pos" rule

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> -				int dst_pos = cache_name_pos(dst, strlen(dst));
> +				int dst_pos = index_name_pos(&the_index, dst,
> +							     strlen(dst));

This is not wrong per-se, but open coding what cache_name_pos() did
does not reduce the reliance of the singleton default instance of
the in-core index data structure.

What may have more value may be passing the pointer to an in-core
index structure through the codebase, which may help more parts to
be made callable as if they were library functions, but that would
be a much larger task and won't belong to this "finish the
conversion" topic.  But doing this change will not help us to get
closer to such a longer term clean-up, I am afraid.

Will queue.

Re: [PATCH v2 6/6] cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS"

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Have the last users of "USE_THE_INDEX_COMPATIBILITY_MACROS" use the
> underlying *_index() variants instead. Now all previous users of
> "USE_THE_INDEX_COMPATIBILITY_MACROS" have been migrated away from the
> wrapper macros, and if applicable to use the "USE_THE_INDEX_VARIABLE"
> added in [1].
>
> Let's leave the "index-compatibility.cocci" in place, even though it
> won't be doing anything on "master". It will benefit any out-of-tree
> code that need to use these compatibility macros. We can eventually
> remove it.

When I saw 2/6 I thought it funny that discard_cache() and
read_cache() was left there.  So three clean-ups (the other one is
the .active_nr member) were done across two patches, which looks a
bit funny.

Shouldn't 2/6 and 6/6 combined into one and/or split into three?

The end result is that we need to write our reliance to either
the_index or the_repository more explicitly, which may be what some
folks want.

Will queue.