From: Derrick Stolee <stolee@gmail.com>
Date: Thu, 11 Sep 2025 15:26:56 GMT
Subject: Re: [PATCH v2 8/8] sparse-checkout: make 'clean' clear more files
Message-ID: <9b8d6c20-e91d-4ef6-9260-5016870f27c7@gmail.com>
In-Reply-To: <CABPp-BHLcy-A4yLR8gP1Sjt_EKQ4K08kPyb7G6yifdZj+0MJNg@mail.gmail.com>

On 8/5/25 8:21 PM, Elijah Newren wrote:
> On Wed, Jul 16, 2025 at 6:34 PM Derrick Stolee via GitGitGadget

>>          test_path_is_missing repo/deep/deeper2 &&
>> -       test_path_exists repo/folder1 &&
>> +       test_path_is_missing repo/folder1 &&
>>          test_path_exists repo/folder2
> 
> What this doesn't show is that afterwards:
> 
> $ git -C repo status
> 
> On branch main
> You are in a sparse checkout with 78% of tracked files present.
> 
> Changes to be committed:
>    (use "git restore --staged <file>..." to unstage)
>      new file:   folder1/file
> 
> Changes not staged for commit:
>    (use "git add/rm <file>..." to update what will be committed)
>    (use "git restore <file>..." to discard changes in working directory)
>      deleted:    folder1/file
>      modified:   folder2/a

You make an excellent point about SKIP_WORKTREE bit states across
changes like this. I'll expand on this flow in the test script.

>> +               test_path_exists deep/deeper2/extra &&
>> +
>> +               cat >expect <<-\EOF &&
>> +               Removing deep/deeper2/
>> +               Removing folder1/
>> +               EOF
>>
>>                  # clean does not remove the file, because the
>>                  # SKIP_WORKTREE bit was not cleared by the merge command.
> 
> Shouldn't the comment be updated, given the testcase updates?

Yes. Good catch.

>>                  git sparse-checkout clean -f >out &&
>> -               test_line_count = 0 out &&
>> -               test_path_exists folder1/even/more/dirs/file &&
>> -
>> -               git sparse-checkout reapply &&
>> -               test_path_is_missing folder1
>> +               test_cmp expect out &&
>> +               test_path_is_missing folder1 &&
>> +               test_path_is_missing deep/deeper2
> 
> Yes, but why does `git status` show folder1/even/more/dirs/file as
> being locally deleted?  Does the code forget to update the
> SKIP_WORKTREE status after clearing out the files?

This is an interesting quirk about how "git add --sparse <path>"
works, which is to remove the SKIP_WORKTREE bit. It's further
interesting that the file is still being "tracked as a deletion"
while also being "collapsed to a sparse directory".

By expanding the earlier test cases to include these post-clean
statuses, we can see that this change is causing this new view
of a deleted file. I will see what I can do to determine exactly
what's different this time and how we can minimize that
strangeness (or: consider dropping this patch).

Thanks,
-Stolee