Re: [PATCH v1] worktree: integrate with sparse-index

[Shuqi Liang <cheskaqiqi@gmail.com>]

On Mon, Jun 5, 2023 at 3:16 PM Victoria Dye <vdye@github.com> wrote:

> >
> > The `p2000` tests demonstrate a ~20% execution time reduction for
> > 'git worktree' using a sparse index:
> >
> > (Note:the p2000 test results did't reflect the huge speedup because of
>
> s/did't/didn't
>
> (not worth fixing if you don't end up re-rolling, though!)
>
> > the index reading time is minuscule comparing to the filesystem
> > operations.)

Will fix it!

> >
> > +test_expect_success 'worktree' '
> > +     init_repos &&
> > +
> > +     write_script edit-contents <<-\EOF &&
> > +     echo text >>"$1"
> > +     EOF
> > +
> > +     test_all_match git worktree add .worktrees/hotfix &&
> > +     test_sparse_match ls .worktrees/hotfix &&
>
> I see why you're comparing 'sparse-checkout' to 'sparse-index' here (their
> worktrees should both contain only the files matched by the sparse-checkout
> patterns, unlike 'full-checkout' which will contain all files), but this
> won't catch bugs that apply to both sparse-checkout and sparse-index (e.g.,
> if the sparse checkout patterns weren't applied and the full worktrees were
> checked out).
>
> To make sure that doesn't happen, you could add a section that compares each
> test repo's default worktree to a new worktree, e.g.:
>
>         for repo in full-checkout sparse-checkout sparse-index
>         do
>                 worktree=${repo}-wt &&
>                 git -C $repo worktree add $worktree &&
>
>                 # Compare worktree content with 'ls'
>
>                 # Compare index content with 'ls-files --sparse'
>
>                 # Any other comparisons that are useful
>
>                 git worktree remove $worktree || return 1
>         done
>

Will do !

> > +test_expect_success 'worktree is not expanded' '
> > +     init_repos &&
> > +
> > +     test_all_match git worktree add .worktrees/hotfix &&
>
> Shouldn't 'git worktree add' not expand the index? Why use 'test_all_match'
> instead of 'ensure_not_expanded'?

Here's my perspective on why my use of "test_all_match" instead of
"ensure_not_expanded" in "git worktree add":

The functions "validate_no_submodules" and "check_clean_worktree" are
specifically related to the "git worktree remove" command, and "git
worktree add" doesn't require index reading, so with or without the
"ensure_full_index" wouldn't affect the "git worktree add" command.
I look forward to hearing your thoughts regarding whether my
understanding is correct or not.

Thanks for your valuable feedback！