Re: [PATCH 12/12] builtin/*: remove or amend "USE_THE_INDEX_COMPATIBILITY_MACROS"

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Nov 18 2022, Taylor Blau wrote:

> On Fri, Nov 18, 2022 at 12:31:06PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> Follow-up the preceding commit and change
>> "USE_THE_INDEX_COMPATIBILITY_MACROS" to the more narrow
>> "USE_THE_INDEX_VARIABLE" in cases where we still use "the_index".
>>
>> Then remove "USE_THE_INDEX_VARIABLE" entirely in the few cases where
>> we don't require any compatibility macros or variables anymore.
>
> Could this get squashed into the previous commit? IOW, is there any
> reason to keep them separate?

I figured it would be easier to review if I split out the changes that
are coccinelle-created v.s. the ones I made manually.

But I don't have a preference at all, whatever you think is easier.

This *could* also be a 3 patch series, which I'd prefer, but figured it
would get push-back, so I thought I'd spoon-feed the list.

I.e. just:

 1. Fix discard_index() return value
 2. Add and apply the *.cocci rule to *ALL THE THINGS*"
 3. Narrow down the macro for t/helper/*

At that point we wouldn't need "USE_THE_INDEX_VARIABLE", as we'd have
nothing except "the_index", so "USE_THE_INDEX_COMPATIBILITY_MACROS"
would do exactly what "USE_THE_INDEX_VARIABLE" does in this topic.

(We could still rename it, but I think probably the churn isn't worth it
if it's not a stop-gap to new macro use being re-introduced)

It would rip the band-aid off, there's conflicts, but the --remerge-diff
with "seen" is rather easy, or:

	diff --git a/builtin/commit.c b/builtin/commit.c
	remerge CONFLICT (content): Merge conflict in builtin/commit.c
	index 4fb7604cc24..c7dbdb2a866 100644
	--- a/builtin/commit.c
	+++ b/builtin/commit.c
	@@ -991,16 +991,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
	 		struct object_id oid;
	 		const char *parent = "HEAD";
	 
	-<<<<<<< c23106a1d9f (hi)
	-		if (!the_index.cache_nr && repo_read_index(the_repository) < 0)
	-			die(_("Cannot read index"));
	-=======
	-		if (!active_nr) {
	-			discard_cache();
	-			if (read_cache() < 0)
	+		if (!the_index.cache_nr) {
	+			discard_index(&the_index);
	+			if (repo_read_index(the_repository) < 0)
	 				die(_("Cannot read index"));
	 		}
	->>>>>>> 75ae5b855fb (Merge branch 'rj/branch-copy-and-rename' into seen)
	 
	 		if (amend)
	 			parent = "HEAD^1";
	diff --git a/builtin/read-tree.c b/builtin/read-tree.c
	remerge CONFLICT (content): Merge conflict in builtin/read-tree.c
	index 5dcf0d09ed1..5d6e638c4b1 100644
	--- a/builtin/read-tree.c
	+++ b/builtin/read-tree.c
	@@ -250,15 +250,11 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
	 	if (opts.debug_unpack)
	 		opts.fn = debug_merge;
	 
	-<<<<<<< c23106a1d9f (hi)
	-	cache_tree_free(&the_index.cache_tree);
	-=======
	 	/* If we're going to prime_cache_tree later, skip cache tree update */
	 	if (nr_trees == 1 && !opts.prefix)
	 		opts.skip_cache_tree_update = 1;
	 
	-	cache_tree_free(&active_cache_tree);
	->>>>>>> 75ae5b855fb (Merge branch 'rj/branch-copy-and-rename' into seen)
	+	cache_tree_free(&the_index.cache_tree);
	 	for (i = 0; i < nr_trees; i++) {
	 		struct tree *tree = trees[i];
	 		parse_tree(tree);
	diff --git a/t/helper/test-cache-tree.c b/t/helper/test-cache-tree.c
	index 93051b25f56..5514afdfe7a 100644
	--- a/t/helper/test-cache-tree.c
	+++ b/t/helper/test-cache-tree.c
	@@ -30,7 +30,7 @@ int cmd__cache_tree(int argc, const char **argv)
	 
	 	argc = parse_options(argc, argv, NULL, options, test_cache_tree_usage, 0);
	 
	-	if (read_cache() < 0)
	+	if (repo_read_index(the_repository) < 0)
	 		die(_("unable to read index file"));
	 
	 	oidcpy(&oid, &the_index.cache_tree->oid);
	
...and can also be applied by a "take theirs", re-run the cocci, apply
cocci patch, so no manual resolution needed. Any future conflicts would
be solved with the same procedure.

The diff would be just under ~2k lines, but pretty trivial in terms of
non-cocci changes.

Anyway, just let me know. Right now I'm assuming I'll just re-roll this
(pending any further feedback for a bit) with squashing & some other
minor changes...