On 9/20/2021 11:52 AM, Taylor Blau wrote: > On Mon, Sep 20, 2021 at 09:45:14PM +0930, Calbabreaker wrote: >> What did you do before the bug happened? (Steps to reproduce your issue) >> >> This was ran: >> >> git clone https://github.com/Calbabreaker/piano --sparse >> cd piano >> git sparse-checkout add any_text >> git checkout deploy-frontend >> git sparse-checkout init --cone >> git sparse-checkout add any_text Thank you for pointing this out. I'll point out that this was likely found because "--sparse" does not initialize cone mode patterns, but you might have expected it to. This will increase the priority of adding something like "--sparse=cone" to the 'git clone' options. > Thanks for the reproduction. An even simpler one may be (inside of any > repository): > > git sparse-checkout init > git sparse-checkout add dir > git sparse-checkout init --cone > git sparse-checkout add dir > > The problem occurs because we keep existing entries when adding to the > sparse-checkout list, and cone-mode patterns do not mix with > non cone-mode patterns. > > So after the first init and "add dir", your sparse-checkout file looks > like: > > /* > !/*/ > dir > > but then when we convert to cone-mode and try and add "dir" (which in > cone-mode we'll convert to "/dir/"), we run into trouble when adding the > existing "dir" entry. That's because add_patterns_cone_mode() calls > insert_recursive_pattern() on every entry in the existing list, > including "dir". > > So when we call insert_recursive_pattern() with any pattern list and > path containing "dir", we first insert "dir" into the list, and then: > > char *slash = strrchr(e->pattern, '/'); > char *oldpattern = e->pattern; > > if (slash == e->pattern) > break; > // trim off a slash, repeat > > except slash is NULL because "dir" doesn't contain a slash. And that > explains the problem you're seeing, because (a) we'll stay in that while > loop forever, and (b) because each iteration allocates memory to > accommodate the new pattern, so we'll eventually run out of memory. Yikes! Thanks for digging into the details. > The wrong thing to do would be to handle this case by changing the > conditional to "if (!slash || slash == e->pattern)", because we can't > blindly carry forward some patterns which look like cone-mode patterns, > since together the list of sparse-checkout entries may not represent a > cone. > > (An example here is if we added /foo/bar/baz/* without the corresponding > /foo/, !/foo/*, and so on). > > 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_. Thanks, -Stolee
