Re: Memory leak with sparse-checkout

On 9/20/2021 12:42 PM, Taylor Blau wrote:

On Mon, Sep 20, 2021 at 12:29:36PM -0400, Derrick Stolee wrote:

quoted

quoted

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