From: Derrick Stolee <stolee@gmail.com>
Date: Fri, 16 May 2025 12:49:43 GMT
Subject: Re: [PATCH 1/3] apply: integrate with the sparse index
Message-ID: <3c4ef12a-3ad4-4078-a513-473f13a670fb@gmail.com>
In-Reply-To: <CABPp-BHkgvc0UQbhXfP4POtY8GPVpz9J8ZbX3_jyzL_V7GyBbA@mail.gmail.com>

On 5/9/25 11:18 PM, Elijah Newren wrote:
> On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget


>> +       # Expands when using --index.
>> +       ensure_expanded apply --index ../patch-outside &&
>> +       git -C sparse-index reset --hard &&
> 
> All makes sense up to here.
> 
>> +
>> +       # Does not expand when using --cached.
>> +       ensure_not_expanded apply --cached ../patch-outside
> 
> Wait, what?  That makes no sense.
> 
> After some digging, I see why the test passed, but it's very
> misleading.  Just before this command, if you ran the following
> commands from the sparse-index directory, you'd see the following:
> 
> $ rm testme
> $ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside
> $ grep ensure_full_index testme
> $
> 
> Which matches what you were testing and shows why it passed for you.
> But I'd argue the test is not correct and confusing for anyone that
> reads it, because:
> 
> $ git ls-files -s --sparse | grep folder2
> 040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0    folder2/0/
> 100644 78981922613b2afb6025042ff6bd878ac1994e85 0    folder2/a
> 
> In other words, the index was *already* (partially) expanded by the
> `git apply --index`, and the `git reset --hard` did not fix that
> contrary to expectations.  Continuing from here we see:
> 
> $ git reset --hard
> HEAD is now at 703fd3e initial commit
> $ git ls-files -s --sparse | grep folder2
> 040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0    folder2/0/
> 100644 78981922613b2afb6025042ff6bd878ac1994e85 0    folder2/a
> $ git sparse-checkout reapply
> $ git ls-files -s --sparse | grep folder2
> 040000 123706f6fc38949628eaf0483edbf97ba21123ae 0    folder2/
> 
> So, we need to do a `git sparse-checkout reapply` to make sure we were
> actually in the expected fully sparse state.  From here...
> 
> $ rm testme
> $ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside
> $ grep ensure_full_index testme
> {"event":"region_enter","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.856008Z","file":"sparse-index.c","line":372,"repo":1,"nesting":1,"category":"index","label":"ensure_full_index"}
> {"event":"region_leave","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.856454Z","file":"sparse-index.c","line":455,"repo":1,"t_rel":0.000446,"nesting":1,"category":"index","label":"ensure_full_index"}
> {"event":"region_enter","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.857016Z","file":"sparse-index.c","line":372,"repo":1,"nesting":1,"category":"index","label":"ensure_full_index"}
> {"event":"region_leave","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.857135Z","file":"sparse-index.c","line":455,"repo":1,"t_rel":0.000119,"nesting":1,"category":"index","label":"ensure_full_index"}
> 
> So, indeed, `git apply --cached ../patch-outside` DOES expand the
> index, as I expected.  It has to when folder2/ is a directory in the
> index, so that we can get a folder2/a entry that we can modify.  And
> that's just what we see:
> 
> $ git ls-files -s --sparse | grep folder2
> 040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0    folder2/0/
> 100644 50f4ca4e6265a0497ec2ee6782648138914ad398 0    folder2/a
> 
> 
> Can you add a `git sparse-checkout reapply` right after your `git
> reset --hard`, and then switch the ensure_not_expanded to
> ensure_expanded for the apply --cached call?

Thanks for digging in here. I'm going to augment the test to
include both the ensure_expanded and ensure_not_expanded so we
get the extra coverage for these two scenarios.

Thanks for the careful eye!
-Stolee