On 2/15/2022 12:12 AM, Elijah Newren wrote:

On Mon, Feb 14, 2022 at 8:19 AM Derrick Stolee [off-list ref] wrote:

quoted

On 2/12/2022 7:39 PM, Elijah Newren via GitGitGadget wrote:

quoted

Note (reason for RFC): this is RFC primarily because of dependencies (you
may not want to pick this up yet, Junio), though there is also a question of
whether to split patch 7 into two steps -- one for now and one we take in
some future release. In particular, the first step could be to have
sparse-checkout error out if neither --no-cone nor --cone are specified and
then change the default to be --cone in some future release. I don't think
splitting it into two steps is needed given (a) users who are unaware of the
change will still get useful error messages telling them that directories
are expected due to patches 4-6 of this series, and (b) the huge
"EXPERIMENTAL" warning and explicit note about likely behavioral changes in
git-sparse-checkout.txt serves as warning about the changes. However, the
two step approach is an alternative.

I support this change. This will also require an update to the 'git clone'
documentation around the '--sparse' option, as I imagine we are going to
be changing behavior there. (If not, then we should do that as part of the
deprecation.)

Why would that be needed?  The documentation does not specify anything
about cone vs. non-cone mode, only that the initial working tree will
only have files from the toplevel directory present.  So, the
documentation is correct without any needed changes.

OK. That makes sense.

quoted

I took a close look at these patches and mostly have minor typo fixes. There
was one behavior issue: I don't think you should warn for file paths in non-
cone-mode. Being able to select a single file in a directory full of large
files is one of the main reasons to use non-cone-mode, in my experience.

In which case shouldn't we still show a warning when users specify a
path rather than a pattern, since the former risks selecting more than
one file?  (Adding a leading slash should be recommended for such a
case, right?)

Yes, that is enough of a tweak for me. Thanks.

quoted

We can also consider if we need a release or two where this behavior
change is announced, but not actually done. I'm not sure if that is
necessary. Making '--no-cone' required might stir up some noise that
indicate how much of an impact the change would make.

We can discuss this more later, but I think it's worthwhile to
consider what happens even if folks didn't read the BIG warning about
behavioral changes in the git-sparse-checkout.txt manual, and didn't
know about the default change, and tried to use it anyway.  If they
specify something other than a directory, then they'd get an error
message due to the first six patches of this series -- at which point
they can look to the manual and decide to add --no-cone to their
command.  In other words, it basically does the same thing that we'd
do if we decided to have an interim period with an error when neither
--cone or --no-cone were specified, other than the error message
perhaps being slightly different.  What if the user does specify
directories and doesn't know about the default mode change?  Well,
that's where the two modes overlap and things work fine (with only
minor differences in behavior, such as better performance, and files
from leading directories being included), so the user would be able to
continue with their work.  So, I'm not sure that an interim period
where we error out when neither --cone or --no-cone are specified is
going to buy us much of anything.  And besides, we do have that super
big scary warning in the manual.  Anyway, I'll bring this all up again
when I resubmit the final patch broken up into a separate series.

I suppose the warning/error messages are a way to help users
self-correct to this behavior change. I'm willing to see how that
plays out.

Thanks,
-Stolee