On Wed, Oct 6, 2021 at 1:40 PM Victoria Dye <vdye@xxxxxxxxxx> wrote: > > Elijah Newren wrote: > > On Tue, Oct 5, 2021 at 6:20 AM Victoria Dye via GitGitGadget > > <gitgitgadget@xxxxxxxxx> wrote: > >> > >> From: Victoria Dye <vdye@xxxxxxxxxx> > >> > >> 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. If it's a test-only capability you need, I'd say add it under t/helpers/ somewhere, either a new flag for an existing subcommand of test-tool, or a new subcommand for test-tool.
