On 11/6/22 1:04 AM, Elijah Newren via GitGitGadget wrote:
> The --sparse-index work has been mostly complete (or at least released
> into production even if some small edges remain) for quite some time
> now. We have also had several discussions on flag and config names,
> though we never came to solid conclusions. Stolee once upon a time
> suggested putting all these into some document in
> Documentation/technical[3], which Victoria recently also requested[4].
> I'm behind the times, but here's a patch attempting to finally do that.
This is a correct summary of where the sparse index feature is right now.
It also is a highly-requested document. Thank you for working so hard on
it and sorry for being slow to sign off on your edits since v1.
Today, I'm rereading the whole document anew, but I'll avoid any nits
since I think you are converging on a solid foundation for us to build on.
Mostly, if you asked a question in the doc, I'll reply. Nothing is binding
since the point is to ask the question in the context of the problem
statement and examples. We should remember to update this document when we
actually implement the options, so the decisions are documented here
instead of leaving answered questions lingering.
> + * Do the options --scope={sparse,all} sound good to others? Are there better
> + options?
> + * Names in use, or appearing in patches, or previously suggested:
> + * --sparse/--dense
> + * --ignore-skip-worktree-bits
> + * --ignore-skip-worktree-entries
> + * --ignore-sparsity
> + * --[no-]restrict-to-sparse-paths
> + * --full-tree/--sparse-tree
> + * --[no-]restrict
> + * --scope={sparse,all}
> + * --focus/--unfocus
> + * --limit/--unlimited
I'm partial to --scope={sparse|all} (with the option to add another
value if we see the need).
> + * If a config option is added (sparse.scope?) what should the values and
> + description be? "sparse" (behavior A), "worktree-sparse-history-dense"
> + (behavior B), "dense" (behavior C)? There's a risk of confusion,
> + because even for Behaviors A and B we want some commands to be
> + full-tree and others to operate sparsely, so the wording may need to be
> + more tied to the usecases and somehow explain that. Also, right now,
> + the primary difference we are focusing is just the history-querying
> + commands (log/diff/grep). Previous config suggestion here: [13]
Personally, I think we should have the same values for 'sparse.scope' and
'--scope=<X>'. For now, let's pick one behavior for the 'sparse' value and
we can add a new value to differentiate between A and B when necessary in
the future.
> + * Is `--no-expand` a good alias for ls-files's `--sparse` option?
> + (`--sparse` does not map to either `--scope=sparse` or `--scope=all`,
> + because in non-cone mode it does nothing and in cone-mode it shows the
> + sparse directory entries which are technically outside the sparse
> + specification)
> +
> + * Under Behavior A:
> + * Does ls-files' `--no-expand` override the default `--scope=all`, or
> + does it need an extra flag?
> + * Does ls-files' `-t` option imply `--scope=all`?
> + * Does update-index's `--[no-]skip-worktree` option imply `--scope=all`?
Since the --no-expand option is rather new, and we have a big experimental
banner on the sparse-checkout documentation, it might be good to plan for
a deprecation of these non-standard options. We could start by making them
aliases for the --scope=sparse option, but with a warning that the option
is deprecated and we will _remove_ the option in a future version. We can
document here which versions we expect those removals to happen.
> + * sparse-checkout: once behavior A is fully implemented, should we take
> + an interim measure to ease people into switching the default? Namely,
> + if folks are not already in a sparse checkout, then require
> + `sparse-checkout init/set` to take a
> + `--set-scope=(sparse|worktree-sparse-history-dense|dense)` flag (which
> + would set sparse.scope according to the setting given), and throw an
> + error if the flag is not provided? That error would be a great place
> + to warn folks that the default may change in the future, and get them
> + used to specifying what they want so that the eventual default switch
> + is seamless for them.
I'm not sure that we need a warning here. I think picking an initial default
is good enough. Let's reconsider this warning after we have more implementation
changes that provide a choice between behaviors A and B.
> +=== Implementation Goals/Plans ===
> +
> + * Get buy-in on this document in general.
Consider me bought-in.
> + * Figure out answers to the 'Implementation Questions' sections (above)
> +
> + * Fix bugs in the 'Known bugs' section (below)
> +
> + * Provide some kind of method for backfilling the blobs within the sparse
> + specification in a partial clone
> +
> + [Below here is kind of spitballing since the first two haven't been resolved]
We can update this document as we gain clarity after the first few updates.
> + * update-index: flip the default to --no-ignore-skip-worktree-entries,
> + nuke this stupid "Oh, there's a bug? Let me add a flag to let users
> + request that they not trigger this bug." flag
> +
> + * Flags & Config
> + * Make `--sparse` in add/rm/mv a deprecated alias for `--scope=all`
This '--sparse' deprecation can eventually be a removal, I think.
> + * Make `--ignore-skip-worktree-bits` in checkout-index/checkout/restore
> + a deprecated aliases for `--scope=all`
This one might be harder to remove since it's much older. We can consider
it, though.
> + * Create config option (sparse.scope?), tie it to the "Cliff notes"
> + overview
Implementation detail: it might be nice to create a parse-opt macro that
will read the '--scope={sparse|all}' command-line option but _also_
create a method to initialize the value to the 'sparse.scope' config
option. These can both happen with the very first implementation of the
command-line option and all future integrations can follow that pattern to
get both options.
Thanks for working so hard on this doc. I think this version is ready to
merge down. Let's get started on this work. I'm excited!
Thanks,
-Stolee
