Re: [PATCH 2/3] sparse-checkout: add 'clean' command

> On Tue, Jul 8, 2025 at 4:20 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> When users change their sparse-checkout definitions to add new
>> directories and remove old ones, there may be a few reasons why
>> directories no longer in scope remain (ignored or excluded files still
>> exist, Windows handles are still open, etc.).
> 
> Good background; I am still particularly interested in the "etc." part...

>> When these files still
>> exist, the sparse index feature notices that a tracked, but sparse,
>> directory still exists on disk and thus the index expands. This causes a
>> performance hit _and_ the advice printed isn't very helpful. Using 'git
>> clean' isn't enough (generally '-dfx' may be needed) but also this may
>> not be sufficient.
> 
> Very well motivated.
> 
>> Add a new subcommand to 'git sparse-checkout' that removes these
>> tracked-but-sparse directories, including any excluded or ignored files
>> underneath.
> 
> "including"?

>> 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.
> 
> So is this also meant for cone mode without sparse index turned on?
> What about non-cone mode?

>> Be sure to add a --dry-run option so users can predict what will be
>> deleted. In general, output the directories that are being removed so
>> users can know what was removed.
> 
> Is greater fidelity of interest when there are multiple different
> types of files contained?  For example, "git status" lists individual
> files within a directory, unless it find an ignored directory and then
> it simply lists the directory.  That means we get more fidelity when
> it's warranted, and less when it's not.  I'm not sure if that's a
> perfect analogy, though; it may well be that we don't need the same
> kind of fidelity that `git status` provides.  (And I'm kind of
> guessing it isn't needed, except in error cases, but I'm just asking.)
> 
>> Note that untracked directories remain.
> 
> What does this mean?  If the sparse directory had an untracked
> directory within it then it'll be left on disk, you will only clean up
> untracked files at a depth of 1 within the sparse directory?
> 
> Or that untracked directories not contained within a sparse directory
> will be left alone?

>> Further, directories that
>> contain staged changes are not deleted.
> 
> Shouldn't those be safe to delete?  When a sparse directory has files
> underneath it with staged changes, those roll-up into a staged
> sparse-directory tree value, and so we should be able to delete the
> file.

>> This is a detail that is partly
>> hidden by the implementation which relies on collapsing the index to a
>> sparse index in-memory and only deleting directories that are listed as
>> sparse in the index. If a staged change exists, then that entry is not
>> stored as a sparse tree entry and thus remains on-disk until committed
>> or reset. 
> In contrast, the files under the sparse directory with unstaged
> changes would be problematic to simply remove.

>> +'clean'::
>> +       Remove all files in tracked directories that are outside of the
>> +       sparse-checkout definition.
> 
> If literal, this sounds unsafe, particularly if run while resolving
> merge or rebase conflicts (since those conflicts may occur in paths
> outside the sparse checkout definition).

>> +                                                    This subcommand requires cone-mode
>> +       sparse-checkout to be sure that we know which directories are
>> +       both tracked and all contained paths are not in the sparse-checkout.
>> +       This command can be used to be sure the sparse index works
>> +       efficiently.
> 
> So...what does it do when in cone mode and the sparse index is not enabled?

>> +       if (convert_to_sparse(repo->index, SPARSE_INDEX_MEMORY_ONLY))
>> +               die(_("failed to convert index to a sparse index"));
> 
> So, you make the in-memory index sparse; I don't remember the details
> on this function so it might invalidate some things I say below...but
> after this point you then...
> 
>> +
>> +       strbuf_addstr(&full_path, repo->worktree);
>> +       strbuf_addch(&full_path, '/');
>> +       worktree_len = full_path.len;
>> +
>> +       for (size_t i = 0; i < repo->index->cache_nr; i++) {
>> +               DIR* dir;
>> +               struct cache_entry *ce = repo->index->cache[i];
>> +               if (!S_ISSPARSEDIR(ce->ce_mode))
>> +                       continue;
> 
> ...skip the entries that aren't sparse directories.
> 
>> +               strbuf_setlen(&full_path, worktree_len);
>> +               strbuf_add(&full_path, ce->name, ce->ce_namelen);
>> +
>> +               dir = opendir(full_path.buf);
>> +               if (!dir)
>> +                       continue;
> 
> ...skip the sparse directories that, as expected, don't exist on disk.
> 
>> +               else if (ENOENT != errno) {
>> +                       warning_errno(_("failed to check for existence of '%s'"), ce->name);
>> +                       continue;
>> +               }
>> +
>> +               closedir(dir);
>> +
>> +               printf("%s\n", ce->name);
>> +               if (!clean_opts.dry_run) {
>> +                       if (remove_dir_recursively(&full_path, 0))
>> +                               warning_errno(_("failed to remove '%s'"), ce->name);
>> +               }
> 
> ...and then unconditionally remove the directory, as you stated in the
> documentation for this clean option.
> 
> I'm worried whether this is safe; if someone does a merge or rebase,
> there could be tracked-and-modified/conflicted files outside the
> sparse specification in the working tree.

> Even after resolving such a merge and committing, the paths may remain
> around until the user does a 'git sparse-checkout reapply' (I don't
> remember details here, but our documentation for reapply certainly
> says so), and since the file might stick around, the user may make
> further modifications to such a file.
> 
> ...or will the convert_to_sparse() call above fail in all these cases?
>  If it does, should it give a better and more useful error message
> than "failed to convert index to a sparse index" and rather e.g. "path
> %s has modifications; please stage or revert first"?