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

From: Elijah Newren <newren@gmail.com>
Date: Oct 6, 2021, 02:00 UTC
Message-ID: <CABPp-BHD3khX+q7crk4JnNbGd9fN-vJW0L1x=5JdzkFL6X1BBg@mail.gmail.com>
In-Reply-To: <f7cb9013d46731855c3ed42add62b021c0ad0c73.1633440057.git.gitgitgadget@gmail.com>

On Tue, Oct 5, 2021 at 6:20 AM Victoria Dye via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 13 quoted lines

>
> From: Victoria Dye <vdye@github.com>
>
> 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.

Show 81 quoted lines

> Signed-off-by: Victoria Dye <vdye@github.com>
> ---
>  Documentation/git-update-index.txt       |  5 +++++
>  builtin/update-index.c                   | 11 +++++++++++
>  t/t1092-sparse-checkout-compatibility.sh |  2 +-
>  3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
> index 2853f168d97..06255e321a3 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -24,6 +24,7 @@ SYNOPSIS
>              [--[no-]fsmonitor]
>              [--really-refresh] [--unresolve] [--again | -g]
>              [--info-only] [--index-info]
> +            [--force-full-index]
>              [-z] [--stdin] [--index-version <n>]
>              [--verbose]
>              [--] [<file>...]
> @@ -170,6 +171,10 @@ time. Version 4 is relatively young (first released in 1.8.0 in
>  October 2012). Other Git implementations such as JGit and libgit2
>  may not support it yet.
>
> +--force-full-index::
> +       Force the command to operate on a full index, expanding a sparse
> +       index if necessary.
> +
>  -z::
>         Only meaningful with `--stdin` or `--index-info`; paths are
>         separated with NUL character instead of LF.
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 187203e8bb5..32ada3ead77 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -964,6 +964,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>         int split_index = -1;
>         int force_write = 0;
>         int fsmonitor = -1;
> +       int use_default_full_index = 0;
>         struct lock_file lock_file = LOCK_INIT;
>         struct parse_opt_ctx_t ctx;
>         strbuf_getline_fn getline_fn;
> @@ -1069,6 +1070,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>                 {OPTION_SET_INT, 0, "no-fsmonitor-valid", &mark_fsmonitor_only, NULL,
>                         N_("clear fsmonitor valid bit"),
>                         PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, UNMARK_FLAG},
> +               OPT_SET_INT(0, "force-full-index", &use_default_full_index,
> +                       N_("run with full index explicitly required"), 1),
>                 OPT_END()
>         };
>
> @@ -1082,6 +1085,14 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>         if (newfd < 0)
>                 lock_error = errno;
>
> +       /*
> +        * If --force-full-index is set, the command should skip manually
> +        * setting `command_requires_full_index`.
> +        */
> +       prepare_repo_settings(r);
> +       if (!use_default_full_index)
> +               r->settings.command_requires_full_index = 1;
> +
>         entries = read_cache();
>         if (entries < 0)
>                 die("cache corrupted");
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index c5977152661..b3c0d3b98ee 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -642,7 +642,7 @@ test_expect_success 'sparse-index is expanded and converted back' '
>         init_repos &&
>
>         GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
> -               git -C sparse-index -c core.fsmonitor="" reset --hard &&
> +               git -C sparse-index -c core.fsmonitor="" update-index --force-full-index &&
>         test_region index convert_to_sparse trace2.txt &&
>         test_region index ensure_full_index trace2.txt
>  '
> --
> gitgitgadget