On Wed, Jan 5, 2022 at 1:04 PM Elijah Newren [off-list ref] wrote:

On Tue, Jan 4, 2022 at 9:37 AM Victoria Dye via GitGitGadget
[off-list ref] wrote:

quoted

From: Victoria Dye <redacted>

...

quoted

+'
+
+# NEEDSWORK: even in sparse checkouts, checkout-index --all will create all
+# files (even those outside the sparse definition) on disk. However, these files
+# don't appear in the percentage of tracked files in git status.

Ah, so you _are_ noticing the present-despite-SKIP_WORKTREE files.  I
think it might be nice to test for these a bit earlier, but it's nice
to see some test here for them.

I think that present-despite-SKIP_WORKTREE files is an erroneous
condition, one we should avoid triggering, and one we should help
users clean up from.  For checkout-index, the fact that it currently
makes more of these files is buggy.  It should either (1) clear the
SKIP_WORKTREE bit when it writes the files to the working tree, or (2)
avoid writing the files to the working tree.

And if we choose (1), there's already a --no-create option we could
piggy-back on to allow folks to not write the SKIP_WORKTREE files.

quoted

+test_expect_failure 'checkout-index --all' '
+       init_repos &&
+
+       test_all_match git checkout-index --all &&
+       test_sparse_match test_path_is_missing folder1

Ah, it looks like you're choosing (2).  That may be fine, but an
interesting anecdote:

While attempting to adopt sparse checkouts at $DAYJOB (and
particularly using cone mode), we found the code structure just didn't
quite work.  We needed some directories to be ignored for sparse
checkouts to be meaningful at all, but we had some files that were
siblings to those directories that were needed for builds to function.
We came up with a hack to "add a few files back", using "git
checkout-index -- $FILENAME".  We expected that hack to write the
listed file(s) to the working tree -- though I think we also had logic
to then run the stuff we needed and then delete these temporary files.
We used this hack starting in Feb 2020, and eventually restructured
the code to not need this hack in Feb 2021.  I'll have to mull over
whether your choice of option (2) might cause us some problems if
someone (a) uses a new git version to (b) access an old version of our
code and (c) really wants to work with a sparse checkout (since the
"checkout-index" stuff was part of the build logic checked into the
code).  I think your change here is fine (because not using sparse
checkouts is an option, we told folks it was experimental, and those
are old versions that are only getting security fixes in special
circumstances), but let me think about it for a bit...

I brought this up a day or two ago with my co-workers and discussed
the combination of conditions needed to trigger any problems.  Their
response was "meh, it's pretty unlikely that anyone ever hits it, the
feature was labeled as experimental anyway, and there are multiple
easy workarounds for the user to choose from".  So, no need to worry
about this angle.