Re: 2.34 regression (and workaround): deleting untracked files both outside *and inside* desired sparsity cone

[Derrick Stolee <stolee@gmail.com>]

On 12/1/2021 12:16 PM, Elijah Newren wrote:
> Hi,
> 
> I've got a proposal for changing the sparse-checkout command slightly;
> but it probably doesn't make sense without the context of the bugs
> (old and new) we are facing.  Consider this an RFC, with the final
> bullet point particularly in need of comment and ideas.
> 
> == Background ==
> 
> sparse-checkouts in cone mode are documented as being obtained either
> by using the `--sparse` flag to `git clone`, ...

The `--sparse` flag doesn't initialize cone mode, unfortunately.

>     git sparse-checkout init --cone [--sparse-index]
>     git sparse-checkout set ...
> 
> The first step has traditionally deleted all the tracked files from
> the working tree, except in the toplevel directory, and the second
> restores all the tracked files that are wanted.
> 
> (Usage context:)
> My understanding is Microsoft never uses this sequence, instead using
> the --sparse flag to `git clone`.  In contrast, at Palantir the
> --sparse flag to clone is rarely used.

We use the sparse-checkout builtin. From the Scalar patch series,
you can see that we don't call "git clone" at all, but instead
"scalar clone" does a lot with "git init" and works from there by
setting config and fetching at the correct time.
 
> == The (New) Bug ==
> 
> Starting with Git 2.34, each step will delete all ignored files
> outside the sparsity paths specified to the individual command in
> question.  We are totally onboard with deleting ignored files outside
> the sparsity paths the user wants, but the first command is required
> according to the documentation and does not allow specifying any
> sparsity paths.  Since it does not allow specifying any sparsity
> paths, it treats *everything* as outside and essentially deletes all
> ignored files everywhere.  That's not workable for us.  We want a
> single command for changing to a sparse-checkout.

Ah, since 'git sparse-checkout set' would work differently if not
in cone mode, I see the problem. It's a little too much to manually
set core.sparseCheckoutCone=true before running the 'set' command,
probably.

> == The Current Workaround ==
> 
> Luckily, having these two commands separate isn't enforced, and the
> first command is basically roughly equivalent to setting a few config
> variables and then running `sparse-checkout set` with an empty set of
> paths.  So, currently, we can just do the config setting part of init
> manually, and then skip the rest of init, and then call our desired
> `set` command:
>     git config extensions.worktreeConfig true
>     git config --worktree core.sparseCheckout true
>     git config --worktree core.sparseCheckoutCone true
>     git sparse-checkout set ...

Which you have already worked out.

> Since we're using a wrapper anyway (for computing dependencies and
> determining the list of directories to include), it was relatively
> easy for us to add this workaround.
> 
> However, it is not clear that our current workaround will continue
> functioning with future versions of git, particularly if
> `sparse-checkout init` gains more options.  In fact, it already
> doesn't handle --sparse-index.

Right. It's _yet another_ config option to tweak.

> == Long term proposal ==
> 
> Make `set` do both the work of `init` and `set`.
> 
> This means:
>   * `set` gains the ability to parse both --cone and --sparse-index
> (in addition to --stdin, etc.)
>   * If the sparse-index is not initialized, `set` does the
> initialization work of `init`.
>   * Modify the `init` documentation to mark it as deprecated,
> mentioning the 2-3 bugs above as reasons why.
>   * We could effectively just turn `git sparse-checkout init ...` into
> an alias for `git sparse-checkout set ...`, since init's parameters
> would be a subset of those that `set` accepts.  However, the latter
> might interact badly with allowing a user to toggle sparse-index on
> and off in the middle of a sparse-checkout...so maybe we need
> something more?  Alternatively, we could leave `init` as-is and just
> consider it set in concrete, possibly risking it becoming
> non-functional in a future upgrade.  Hmm...

I think this is a good plan. Making 'init' the same as 'set' with
no paths makes sense to me. We would want to be careful now that
"--option" could be interpreted as a path to recommend using

  git sparse-checkout set <options> -- <path1> ... <pathN>

While you are here, I would be interested in making 'git clone
--sparse' default to cone mode. Or, should it be 'git clone
--sparse=cone' or something? Not making it default to cone mode
is a big regret of mine.

Thanks,
-Stolee