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

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