Re: [PATCH v2 2/8] sparse-checkout: add basics of 'clean' command
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Sep 11, 2025, 13:37 UTC
- Message-ID
- <4ce92ef9-61ef-491e-80a3-370e92fd10fd@gmail.com>
- In-Reply-To
- <CABPp-BFzMLGJwz4QqYtvw3zRYgmC=Mb8T8GCOsrLZqT2z+8H7A@mail.gmail.com>
On 8/5/25 5:32 PM, Elijah Newren wrote:
Show 34 quoted lines
> 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.Show 50 quoted lines
>> +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).
Show 8 quoted lines
> $ 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.
Show 29 quoted lines
> 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