Re: [PATCH v3 8/8] sparse-checkout: clear tracked sparse dirs

On Thu, Aug 19, 2021 at 1:48 AM Johannes Schindelin
[off-list ref] wrote:

Hi Stolee,

On Tue, 17 Aug 2021, Derrick Stolee via GitGitGadget wrote:

quoted

From: Derrick Stolee <redacted>

When changing the scope of a sparse-checkout using cone mode, we might
have some tracked directories go out of scope. The current logic removes
the tracked files from within those directories, but leaves the ignored
files within those directories. This is a bit unexpected to users who
have given input to Git saying they don't need those directories
anymore.

This is something that is new to the cone mode pattern type: the user
has explicitly said "I want these directories and _not_ those
directories." The typical sparse-checkout patterns more generally apply
to "I want files with with these patterns" so it is natural to leave
ignored files as they are. This focus on directories in cone mode
provides us an opportunity to change the behavior.

Leaving these ignored files in the sparse directories makes it
impossible to gain performance benefits in the sparse index. When we
track into these directories, we need to know if the files are ignored
or not, which might depend on the _tracked_ .gitignore file(s) within
the sparse directory. This depends on the indexed version of the file,
so the sparse directory must be expanded.

By deleting the sparse directories when changing scope (or running 'git
sparse-checkout reapply') we regain these performance benefits as if the
repository was in a clean state.

Since these ignored files are frequently build output or helper files
from IDEs, the users should not need the files now that the tracked
files are removed. If the tracked files reappear, then they will have
newer timestamps than the build artifacts, so the artifacts will need to
be regenerated anyway.

Use the sparse-index as a data structure in order to find the sparse
directories that can be safely deleted. Re-expand the index to a full
one if it was full before.

This description makes sense, and is easy to explain.

It does not cover the case where untracked files are found and the
directory is _not_ removed as a consequence, though. I would like to ask
to add this to the commit message, because it is kind of important.

The implementation of this behavior looks fine to me.

About this behavior itself: in my experience, the more tricky a feature is
to explain, the more likely the design should have been adjusted in the
first place. And I find myself struggling a little bit to explain what
files `git switch` touches in cone mode in addition to tracked files.

I share this concern.

So I wonder whether an easier-to-explain behavior would be the following:
ignored files in directories that fell out of the sparse-checkout cone are
deleted. (Even if there are untracked, unignored files in the same
directory tree.)

This is different than what this patch implements: we would now have to
delete the ignored and out-of-cone files _also_ when there are untracked
files in the same directory, i.e. we could no longer use the sweet
`remove_dir_recursively()` call. Therefore, the implementation of what I
suggested would be much more complicated: you would have to enumerate the
ignored files and remove them individually.

The ignored files are already enumerated by the fill_directory call;
you just need to iterate over the dir->ignored_nr entries in
dir->ignored.

Having said that, even after mulling over this behavior and sleeping over
it, I am unsure what the best way forward would be. Just because it is
easy to explain does not make it right.

It is tricky to decide mostly because "ignored" files are definitely not
always build output. Apart from VIM's temporary files, users like me
frequently write other files and/or directories that we simply do not want
to see tracked in Git. For example, I often test things in an `a1.c` file
that -- for convenience -- lives in the current worktree. Obviously I
don't want Git to track it, but I also don't want it to be deleted, so I
often add corresponding lines to `.git/info/exclude`. Likewise, I
sometimes download additional information related to what I am
implementing, and that also lives in the current worktree (but then, I
usually am too lazy to add an entry to `.git/info/exclude` in those
cases).

I do the same thing, and I know other users that do as well...but I
don't put such files in directories that are irrelevant to me.  I
create cruft files near other files that I'm working on, or in a
special directory of its own, but not under some directory that is
irrelevant to the areas I'm working on.

For reference, we implemented something like this in our `sparsify`
wrapper we have internally, where 'git clean -fdX <all sparse
directories>` is executed whenever folks sparsify.  (We have our own
tool and don't have users use sparse-checkout directly, because our
tool computes dependencies to determine which directories are needed.)
 I was really hesitant to add that cleaning behavior by default, and
just made it an option.  My colleagues tired of all the bug reports
about left-around directories and made it the default, waiting to hear
complaints.  We never got one.  It's been over a year.

Now, I don't want to over-index on my own habits. There are so many users
out there, and most of them have different preferences from mine.

Which leaves me to wonder whether we need at least a flag to turn this
behavior on and off? Something like
`core.ignoredFilesInSparseConesArePrecious = true` (obviously with a
better, shorter name).

That seems fine, but I suspect you're projecting the same concerns I
had, and it turns out much less useful than you'd expect (perhaps even
totally useless).