Re: [PATCH v2 2/8] sparse-checkout: add basics of 'clean' command

[Derrick Stolee <stolee@gmail.com>]

On 8/5/25 5:32 PM, Elijah Newren wrote:
> On Wed, Jul 16, 2025 at 6:34 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> 
> Sorry for the long delay in responding...
> 
> [...]
>> Add a new subcommand to 'git sparse-checkout' that removes these
>> tracked-but-sparse directories. This necessarily removes all files
>> contained within, including tracked and untracked files. Of particular
> 
> Nice to see tracked files also being addressed in v2.
> 
>> importance are ignored and excluded files which would normally be
>> ignored even by 'git clean -f' unless the '-x' or '-X' option is
>> provided. This is the most extreme method for doing this, but it works
>> when the sparse-checkout is in cone mode and is expected to rescope
>> based on directories, not files.
>>
>> The current implementation always deletes these sparse directories
>> without warning. This is unacceptable for a released version, but those
>> features will be added in changes coming immediately after this one.
>>
>> Note that untracked directories within the sparse-checkout remain.
> 
> You've changed the wording here relative to v1, but you haven't
> addressed the part that was ambiguous/misleading in v1.  In fact, you
> may have made a different part ambiguous as well, and made readers
> think that this sentence contradicts your above claims that this
> command is meant to clean out untracked directories underneath sparse
> directories.  Perhaps something like:
> 
> "Note that untracked directories in the sparse-checkout that are not
> within sparse directories will not be removed by this command; it only
> cleans up paths under directories that are supposed to be sparse."

Both here and in the documentation, things can get a bit confusing.
In the v3 I'm preparing, I'm taking the following approach:

  * In the commit message, focus on the implementation details and how
    that impacts the behavior of the tool.

  * In the documentation, focus on the list of files that will be
    "considered for removal". Use the most broad definition there:
    in a tracked directory that is outside of the sparse-checkout.
    Add pointers that could explain exceptions and how to remove
    these exceptions, but don't attempt to explain all special
    cases.

>> +test_expect_success 'clean with staged sparse change' '
>> +       git -C repo sparse-checkout set --cone deep/deeper1 &&
>> +       mkdir repo/deep/deeper2 repo/folder1 repo/folder2 &&
>> +       touch repo/deep/deeper2/file &&
>> +       touch repo/folder1/file &&
>> +       echo dirty >repo/folder2/a &&
>> +
>> +       git -C repo add --sparse folder1/file &&
>> +
>> +       # deletes deep/deeper2/ but leaves folder1/ and folder2/
>> +       cat >expect <<-\EOF &&
>> +       Removing deep/deeper2/
>> +       EOF
>> +
>> +       git -C repo sparse-checkout clean >out &&
>> +       test_cmp expect out &&
>> +
>> +       test_path_is_missing repo/deep/deeper2 &&
>> +       test_path_exists repo/folder1
> 
> What about repo/folder2/ ?
> 
> Anyway, this test shows that neither staged nor unstaged changes are
> cleaned up (which at least resolves the conflicting documentation you
> provided on the matter) -- or would if you also checked repo/folder2.
> 
> What it doesn't show is that tracked files with neither staged nor
> unstaged changes are not cleaned up either:
> 
> $ mkdir repo/folder2
> $ echo dirty >repo/folder2/a
> $ touch repo/folder2/untracked
> $ cd repo
> $ git status --porcelain
>   M folder2/a
> ?? folder2/untracked
> 
> # So, we have both a unstaged change and an untracked file; let's undo
> the unstaged change
> 
> $ git checkout HEAD folder2/a
> Updated 1 path from 8cc814f
> $ git status --porcelain
> ?? folder2/untracked
> $ ls folder2/
> a  untracked
> 
> # Both files are still present -- the untracked file, and the
> untracked file with no changes either staged or unstaged -- what does
> `git sparse-checkout clean` do?

It seems that the unstaged modification to a tracked, sparse file
is enough to prevent the sparse directory collapse. This is
similar to how 'git sparse-checkout reapply' will refuse to remove
those modified changes. I'll be sure to update my advice around
special cases to include this (and lock it in with a test case).

> $ git sparse-checkout clean
> $ ls folder2/
> a  untracked
> $ git status --porcelain
> ?? folder2/untracked
> 
> # Absolutely nothing.  Not only does it not clean anything up, it
> gives no warnings about not cleaning up what should be cleaned up.

At this point, the SKIP_WORKTREE bit is still removed because
we've staged the change.

> Let's try sparse-checkout reapply:
> 
> $ git sparse-checkout reapply
> warning: directory 'folder2/' contains untracked files, but is not in
> the sparse-checkout cone
> $ git status --porcelain
> ?? folder2/untracked
> $ ls folder2/
> untracked
>
> # So `sparse-checkout reapply` does correctly remove folder2/a for us,
> while warning about the untracked file.  (If folder2/a would have
> still had changes, it would have warned about it instead of
> removing.).  Let's try `sparse-checkout clean` now...
> 
> $ git sparse-checkout clean
> Removing folder2/
> $ git status --porcelain
> $ ls folder2/
> ls: cannot access 'folder2/': No such file or directory
> $
> 
> I think these cases either need to be a new testcase or part of this
> last testcase, and the commit message and documentation should be
> clearer about tracked-and-staged, tracked-with-unstaged-changes, and
> tracked-with-no-changes files...or at least comment that they'll be
> discussed later in the patch series.  (I have a feeling I just did a
> lot of work to discover as I read your next patches that you cover
> these later...)

No! you found interesting ways to test special cases. Thanks!

Describing the lifecycle of a sparse file (with sibling untracked
change) going from modified to staged to sparse to unlock the
cleaning would be helpful documentation.

I do think there is an interesting extra functionality that we
should consider for the future: "What files are in my worktree
that _should_ be sparse? Why is 'clean' not removing them?"

Thanks,
-Stolee