Re: [PATCH 2/3] git add: make -p/-i aware of sparse index
- From
Derrick Stolee <stolee@gmail.com>
- Date
- May 16, 2025, 12:54 UTC
- Message-ID
- <bee620e4-1f45-4fff-a7d4-ecb5f99f4e0a@gmail.com>
- In-Reply-To
- <CABPp-BEmMaFQxE9NQgM8M=cgfBHY1p56vnBt7R4CfuiXnq++4Q@mail.gmail.com>
On 5/10/25 12:38 AM, Elijah Newren wrote:
Show 30 quoted lines
> On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> >> From: Derrick Stolee <stolee@gmail.com> >> >> It is slow to expand a sparse index in-memory due to parsing of trees. >> We aim to minimize that performance cost when possible. 'git add -p' >> uses 'git apply' child processes to modify the index, but still there >> are some expansions that occur. > > still there are some expansions that occur...outside of those child > processes? Is that what you're trying to say, or was it something > else? > >> It turns out that control flows out of cmd_add() in the interactive >> cases before the lines that confirm that the builtin is integrated with >> the sparse index. We need to move that earlier to ensure it prevents a >> full index expansion on read. >> >> Add more test cases that confirm that these interactive add options work >> with the sparse index. One interesting aspect here is that the '-i' >> option avoids expanding the sparse index when a sparse directory exists >> on disk while the '-p' option does hit the ensure_full_index() method. >> This leaves some room for improvement, but this case should be atypical >> as users should remain within their sparse-checkout. > > It's not clear whether this paragraph is talking about existing state > (before the patch) or desired state (after the patch). I think based > on the context it's the former, but the last sentence sounds more like > a future work direction that makes it very unclear, to me at least.
I'll try to rewrite to make this clearer.
Show 17 quoted lines
>> + # -p does expand when edits are outside sparse checkout. >> + test_write_lines y n y >in && >> + ensure_expanded add -p <in && >> + >> + # but -i does not expand. >> + git -C sparse-index reset && >> + test_write_lines u 2 3 "" q >in && >> + ensure_not_expanded add -i <in > > This has the same error as patch 1, in that you assume your reset > above (which wasn't even a reset --hard) will re-sparsify the index. > Since it doesn't, your test is misleading and only shows that when > already expanded to include the files of interest it doesn't expand > any further. To re-sparsify your index before the `add -i` call, > you'll need to do a `git reset --hard && git sparse-checkout reapply` > and then recreate folder1/a with "new content" again...and then run > your 'add -i' command.
Thanks. I didn't like that this was different. I appreciate your expertise helping to clarify this issue.
Thanks, -Stolee