Re: [PATCH 0/7] RFC: sparse checkout: make --cone mode the default, and check add/set argument validity

[Derrick Stolee <derrickstolee@github.com>]

On 2/15/2022 12:12 AM, Elijah Newren wrote:
> On Mon, Feb 14, 2022 at 8:19 AM Derrick Stolee <derrickstolee@github.com> wrote:
>>
>> On 2/12/2022 7:39 PM, Elijah Newren via GitGitGadget wrote:
>>> 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.

>> 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.

>> 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