purescript / spago

🍝 PureScript package manager and build tool
BSD 3-Clause "New" or "Revised" License
792 stars 132 forks source link

fix: gitignoringGlob stack safety + perf #1234

Closed cakekindel closed 5 months ago

cakekindel commented 5 months ago

Description of the change

fixes #1231

Before, we were eagerly turning string glob patterns to matching functions String -> Boolean, and adding new patterns with ||. The instance (HeytingAlgebra b) => HeytingAlgebra (a -> b) isn't stack safe, causing sufficiently large gitignore files to overflow the call stack.

Now, we accumulate the patterns themselves and build them into micromatch patterns when we evaluate them. The actual call to micromatch costs practically nothing, so this flat-out wins time & space cost (as well as fixing the stack-safety issue)

-- Instead of composing the matcher functions, we could also keep a growing array of
-- patterns and regenerate the matcher on every append. I don't know which option is
-- more performant, but composing functions is more convenient.

Turns out we've learned which option is more performant :sweat_smile:

Checklist:

P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊

cakekindel commented 5 months ago

is micromatch not cross-platform? :anger:

f-f commented 5 months ago

I am very confused as well - I went through the code and all the paths seem to be handled properly, so I am not sure why the failures

cakekindel commented 5 months ago

I'd imagine it's because there's an assumption that foo/** will match both foo and foo/a, which i'm guessing doesn't hold on windows :confused: