On 2/15/2022 12:12 AM, Elijah Newren wrote: > On Mon, Feb 14, 2022 at 8:19 AM Derrick Stolee <derrickstolee@xxxxxxxxxx> 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
