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

[Victoria Dye <vdye@github.com>]

Junio C Hamano wrote:
> "Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> +	/*
>> +	 * 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/