Thread (92 messages) 92 messages, 9 authors, 2021-12-01

Re: [PATCH v2 2/7] update-index: add --force-full-index option for expand/collapse test

From: Victoria Dye <hidden>
Date: 2021-10-06 20:40:45

Elijah Newren wrote:
On Tue, Oct 5, 2021 at 6:20 AM Victoria Dye via GitGitGadget
[off-list ref] wrote:
quoted
From: Victoria Dye <redacted>

Add a new `--force-full-index` option to `git update-index`, which skips
explicitly setting `command_requires_full_index`. This lets `git
update-index --force-full-index` run as a command without sparse index
compatibility implemented, even after it receives sparse index compatibility
updates.

By using `git update-index --force-full-index` in the `t1092` test
`sparse-index is expanded and converted back`, commands can continue to
integrate with the sparse index without the need to keep modifying the
command used in the test.
So...we're adding a permanent user-facing command line flag, whose
purpose is just to help us with the transition work of implementing
sparse indexes everywhere?  Am I reading that right, or is that just
the reason for t1092 and there are more reasons for it elsewhere?

Also, I'm curious if update-index is the right place to add this.  If
you don't want a sparse index anymore, wouldn't a user want to run
   git sparse-checkout disable
?  Or is the point that you do want to keep the sparse checkout, but
you just don't want the index to also be sparse?  Still, even in that
case, it seems like adding a subcommand or flag to an existing
sparse-checkout subcommand would feel more natural, since
sparse-checkout is the command the user uses to request to get into a
sparse-checkout and sparse index.
This came out of a conversation [1] on an earlier version of this patch.
Because the `t1092 - sparse-index is expanded and converted back` test
verifies sparse index compatibility (i.e., expand the index when reading,
collapse back to sparse when writing) on commands that don't have any sparse
index integration, it needed to be changed from `git reset` to something
else. However, as we keep integrating commands with sparse index we'd need
to keep changing the command in the test, creating a bunch of patches doing
effectively the same thing for no long-term benefit. 

The `--force-full-index` flag isn't meant to be used externally or modify
the index in any "new" way - it's really just a "test" version of `git
update-index` that we guarantee will accurately represent a command using
the default settings. Right now, it does exactly what `git update-index`
(without the flag) does, and will only behave differently once `git
update-index` is integrated with sparse index. Using `--force-full-index`,
the test won't need to be regularly updated and will continue to catch
errors like:

1. Changing the default value of `command_requires_full_index` to 0
2. Not expanding a sparse index to full when `command_requires_full_index`
   is 1
3. Not collapsing the index back to sparse if sparse index is enabled

I see the issue of introducing a test-only option (when sparse index is
integrated everywhere, shouldn't it be deprecated?). If there's a way to
make this more obviously internal/temporary, I'm happy to modify it. Or, if
semi-frequent updates of the command in the test aren't a huge issue, I can
revert to V1.

[1] https://lore.kernel.org/git/xmqqr1d58v9x.fsf@gitster.g/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help