Re: [PATCH v2 0/5] Sparse checkout: fix bug with worktree of bare repo

From
Elijah Newren <hidden>
Date
2021-12-30 18:38:20
Reply to
Eric Sunshine, 2021-12-30
On Wed, Dec 29, 2021 at 10:41 PM Eric Sunshine [off-list ref] wrote:
On Tue, Dec 28, 2021 at 1:16 PM Elijah Newren [off-list ref] wrote:
quoted
On Mon, Dec 27, 2021 at 11:33 PM Eric Sunshine [off-list ref] wrote:
quoted
quoted
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/ (local)
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).

...
quoted
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.
:-)
Thread (121 messages) 121 messages, 8 authors, last reply 2022-02-16
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help