Re: [PATCH v2 0/5] Sparse checkout: fix bug with worktree of bare repo
- From
Elijah Newren <newren@gmail.com>
- Date
- Dec 30, 2021, 18:38 UTC
- Message-ID
- <CABPp-BH-oOUhTzcXMFWRfG=zjUw6C5h-oHSU=-juPTjpW92_vA@mail.gmail.com>
- In-Reply-To
- <CAPig+cRJj4Aa9Nm_yWD0=WGdn9tRmT2PpTwo5yeGZHg2Qz2rgA@mail.gmail.com>
On Wed, Dec 29, 2021 at 10:41 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Tue, Dec 28, 2021 at 1:16 PM Elijah Newren <newren@gmail.com> wrote: > > On Mon, Dec 27, 2021 at 11:33 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 20 quoted lines
> > > A more general approach might be for the new worktree to copy all the
> > > per-worktree configuration from the worktree in which the command was
> > > invoked, thus sparsity would be inherited "for free" along with other
> > > settings. This has the benefits of not requiring sparse-checkout
> > > special-cases in the code and it's easy to document ("the new worktree
> > > inherits/copies configuration settings from the worktree in which `git
> > > worktree add` was invoked") and easy to understand.
> >
> > Ooh, this is a good point and I *really* like this simple solution.
> > Thanks for pointing it out.
>
> I do wonder, though, if there are traps waiting for us with this
> all-inclusive approach. I don't know what sort of worktree-specific
> configuration people use, so I do worry a bit that this could be
> casting a too-wide net, and that it might in fact be better to only
> copy the sparse-checkout settings (as ugly as it is to special-case
> that -- but we need to special-case `core.bare` and `core.worktree`
> anyhow[1]).
>
> [1]: https://lore.kernel.org/git/CAPig+cSUOknNC9GMyPvAqdBU0r1MVgvSpvgpSpXUmBm67HO7PQ@mail.gmail.com/I could probably be persuaded either way (do users want to copy something and tweak it, or start with a clean slate?), and it might even make sense to have a flag for users to specify.
My hunch, at least with the developers I work with, is that they're more likely to think in terms of "I want another worktree like this one, except that I'm going to change..."
Also, another reason to prefer copying all of core.worktree (minus the always worktree-specific value of core.worktree, and core.bare), is because it's easy to explain in the documentation, and I think we'd be much less likely to obsolete user's knowledge over time. (I think additional sparse-checkout things, or new other features that also want to be copied over, are much more likely than the addition of keys that are always worktree-specific like core.worktree).
...
Show 10 quoted lines
> > An increasingly unworkable alternative is the current behavior of > > defaulting to a full checkout in all cases (and forcing users to > > sparsify afterwards). A full checkout is fine if the user came from > > one (and probably preferable in such a case), but it's increasingly > > problematic for us even with our repo being nowhere near the size of > > the microsoft repos. > > It feels unfortunate and a bit dirty to spread around this > special-case knowledge about sparse-checkout to various parts of the > system,
This might makes things worse, but it's far too late to avoid that:
$ git grep -il -e sparse-checkout -e skip_worktree .gitignore Documentation/RelNotes/2.19.0.txt Documentation/RelNotes/2.25.0.txt Documentation/RelNotes/2.26.0.txt Documentation/RelNotes/2.27.0.txt Documentation/RelNotes/2.28.0.txt Documentation/RelNotes/2.34.0.txt Documentation/RelNotes/2.6.0.txt Documentation/config/checkout.txt Documentation/config/core.txt Documentation/git-add.txt Documentation/git-checkout.txt Documentation/git-clone.txt Documentation/git-read-tree.txt Documentation/git-restore.txt Documentation/git-rm.txt Documentation/git-sparse-checkout.txt Documentation/git-worktree.txt Documentation/gitrepository-layout.txt Documentation/rev-list-options.txt Documentation/technical/index-format.txt Documentation/technical/parallel-checkout.txt Documentation/technical/sparse-index.txt Makefile advice.c apply.c attr.c builtin/add.c builtin/check-ignore.c builtin/checkout.c builtin/clone.c builtin/commit.c builtin/grep.c builtin/ls-files.c builtin/mv.c builtin/read-tree.c builtin/rm.c builtin/sparse-checkout.c builtin/stash.c builtin/update-index.c cache-tree.c cache.h command-list.txt contrib/completion/git-prompt.sh diff-lib.c diff.c dir.c dir.h entry.c git.c list-objects-filter.c list-objects-filter.h merge-ort.c merge-recursive.c path.c pathspec.c pathspec.h po/bg.po po/ca.po po/de.po po/el.po po/es.po po/fr.po po/git.pot po/id.po po/it.po po/ko.po po/pl.po po/pt_PT.po po/ru.po po/sv.po po/tr.po po/vi.po po/zh_CN.po po/zh_TW.po preload-index.c read-cache.c sparse-index.c sparse-index.h t/perf/p0005-status.sh t/perf/p0006-read-tree-checkout.sh t/perf/p0007-write-cache.sh t/perf/p2000-sparse-operations.sh t/perf/repos/inflate-repo.sh t/perf/repos/many-files.sh t/t0060-path-utils.sh t/t1011-read-tree-sparse-checkout.sh t/t1090-sparse-checkout-scope.sh t/t1091-sparse-checkout-builtin.sh t/t1092-sparse-checkout-compatibility.sh t/t2018-checkout-branch.sh t/t3507-cherry-pick-conflict.sh t/t3602-rm-sparse-checkout.sh t/t3705-add-sparse-checkout.sh t/t5317-pack-objects-filter-objects.sh t/t6112-rev-list-filters-objects.sh t/t6428-merge-conflicts-sparse.sh t/t6435-merge-sparse.sh t/t7002-mv-sparse-checkout.sh t/t7012-skip-worktree-writing.sh t/t7063-status-untracked-cache.sh t/t7418-submodule-sparse-gitmodules.sh t/t7519-status-fsmonitor.sh t/t7817-grep-sparse-checkout.sh unpack-trees.c wt-status.c
> but based upon the pain-points you describe, having a new > worktree inherit the sparsity from the originating worktree does sound > (given my limited knowledge of the topic) like it would ease the pain > for users.
:-)