Re: Memory leak with sparse-checkout

[Derrick Stolee <stolee@gmail.com>]

On 9/20/2021 12:42 PM, Taylor Blau wrote:
> On Mon, Sep 20, 2021 at 12:29:36PM -0400, Derrick Stolee wrote:
>>> So I think the problem really is that we need to drop existing patterns
>>> when re-initializing the sparse-checkout in cone mode. We could try to
>>> recognize that existing patterns may already constitute a cone (and/or
>>> create a cone that covers the existing patterns).
>>>
>>> But I think the easiest thing (if a little unfriendly) would be to just
>>> drop them and start afresh when re-initializing the sparse-checkout in
>>> cone mode.
>>
>> This isn't sufficient, as a user can modify their .git/info/sparse-checkout
>> file whenever they want, so we should fix this bug regardless. We could add
>> a "Your existing patterns are not in cone mode" error.
>>
>> It might still be a good idea to let "git sparse-checkout init --cone"
>> overwrite the sparse-checkout file _if the file is not already in cone
>> mode_.
> 
> I'm not sure how helpful such an error message might be to a user in
> this scenario without extra information. After seeing just "this isn't a
> cone", it's not clear what they should do other than drop their
> sparse-checkout configuration and start over.
> 
> It would be nice to have an intermediate step between seeing realizing
> that the existing patterns don't form a cone and dropping them.
> 
> Perhaps we could include an error message and say something like:
> 
>     warning: your sparse-checkout patterns do not from a cone
>        hint: to reinitialize your sparse-checkout configuration
>        hint: try running:
>        hint:
>        hint:   git sparse-checkout init --cone --reinitialize
> 
> Where `--reinitialize` means to drop existing patterns. I suppose it
> could be the default when transitioning from non-cone to cone mode, but
> that would defeat the purpose of the warning.

I've got an initial set of commits in a GGG pull request [1], and I'm
waiting build validation as a double-check before sending them to the
list.

[1] https://github.com/gitgitgadget/git/pull/1043

There, I think the best thing to do is have 'init --cone' overwrite the
old, non-cone-mode patterns. This essentially makes your --reinitialize
suggestion on permanently in that case. Note that it doesn't reinitialize
when the patterns are already in cone mode.

Note that 'git sparse-checkout set' (with no other arguments) initializes
the sparse-checkout file with the default set of cone mode patterns, so
we have a way to do that now.
 
> We would probably want to perform this check both during initialization,
> and when adding patterns in cone mode. It may also be worthwhile to
> check the validity of the cone before running 'list' or 'reapply', too.

'list' definitely seems like a good idea, since it is expecting different
output than the literal patterns when cone mode is enabled.

'reapply' should work in both cases, it will just use the old pattern
matching algorithm when in cone mode. It will present a warning when the
patterns don't match as expected.

One thing that is very interesting about the example here is that a
pattern "dir" is not recognized as a non-cone-mode pattern, but it
should be. The fix is to look for a '/' at the start of the pattern and
reject it otherwise. This is fixed in my series.

Thanks,
-Stolee