Re: [PATCH v1] worktree: integrate with sparse-index
- From
Shuqi Liang <cheskaqiqi@gmail.com>
- Date
- Jun 5, 2023, 20:16 UTC
- Message-ID
- <CAMO4yUEQZz8DqPb7RyN8Owb=23p==6XS6G7Bza77p4-iydo6Qg@mail.gmail.com>
- In-Reply-To
- <773c2f7a-8637-ab0b-e0a8-ab553c90e88b@github.com>
On Mon, Jun 5, 2023 at 3:16 PM Victoria Dye <vdye@github.com> wrote:
Show 12 quoted lines
> > > > 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!
Show 35 quoted lines
> >
> > +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 !
Show 7 quoted lines
> > +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!