Re: [PATCH v3 3/8] update-index: add --force-full-index option for expand/collapse test

Junio C Hamano wrote:

"Victoria Dye via GitGitGadget" [off-list ref] writes:

quoted

+	/*
+	 * If --force-full-index is set, the command should skip manually
+	 * setting `command_requires_full_index`.
+	 */

Hmph, doesn't that feel unnaturally backwards, though?

The settings.command_requires_full_index bit forces read-cache to
call ensure_full_index() immediately after the in-core index is read
from the disk.  If we are forcing operating on the full index, I'd
imagine that we'd be making sure that ensure_full_index() to be
called.

I tried coming up with a user-facing name that wasn't too focused on the
internal implementation, but it ends up being misleading. The intention was
to have this be a variation of `git update-index` that uses the default
setting for `command_requires_full_index` but then proceeds to read and
write the index as `update-index` normally would. Something like
`--use-default-index-sparsity` might have been more accurate?

I do not see anything in the code that ensures active_cache_changed
to be flipped on.  So the new test that says

    git -C sparse-index -c core.fsmonitor="" update-index --force-full-index

may not call ensure_full_index(), but because nothing marks
the_index as changed, I think we won't call write_locked_index() at
the end of cmd_update_index().  IOW, what we have in the test patch
may be an expensive noop, no?

In the test's use-case, `active_cache_changed` ends up set to
`CACHE_TREE_CHANGED`, which forces writing the index. It is still
effectively a no-op, but it serves the needs of the test.

In any case, Elijah suggested using a `test-tool` subcommand for this
purpose [1], which I think is more appropriate overall. Something like
`test-tool read-write-cache` can be implemented to make no mention of
`command_requires_full_index` (therefore using its default value) and force
a basic read & write of the index. It also eliminates the issue of having a
user-facing name at all, and can easily be removed once all sparse index
integrations are done.

[1] https://lore.kernel.org/git/CABPp-BF+bEUcyE0N79uRCkpCayJx_NMqOpnMSHHrpJM5a9hAWw@mail.gmail.com/ (local)