fix: .gitignored dirs should be excluded from workspace, pedanticPackages should handle gitignore patterns with leading slash

[cakekindel]

Description of the change

Fixes a bug with the globbing changes introduced in 56a36d5 where a .gitignore pattern of /.spago causes spago to (incorrectly) ignore .spago when building the module dependency graph.

This is felt when building with --pedantic-packages, but I imagine it probably causes some other issues, too.

This is caused by some subtle micromatch behavior differences compared with the previous globbing strategy:

--== 56a36d5 src/Spago/Glob.purs L93 ==--
try (SyncFS.readTextFile UTF8 gitignorePath) >>= case _ of
  Left _ -> pure unit
  Right gitignore -> do
    let { ignore, patterns } = gitignoreToMicromatchPatterns base gitignore
    -- when .gitignore is `/.spago`, patterns is `[".spago"]`.
    
    let matcherForThisGitignore = micromatch { ignore } patterns
    let anyIncludePatternWouldBeIgnored = any matcherForThisGitignore includePatterns
    -- when patterns is `[".spago"]`
    -- and includePatterns is `[".spago/p/aff-1.0.0/**/*.purs"]`
    -- then this returns `false` (because `minimatch('foo/**/*')('foo')` returns `false`)

    when (not anyIncludePatternWouldBeIgnored) do
      -- We add all .gitignore patterns to `fsWalk`
      let addMatcher currentMatcher = or [ currentMatcher, matcherForThisGitignore ]
      void $ Ref.modify addMatcher ignoreMatcherRef

Reproduce with

> npm i -g [email protected]
> mkdir gitignore-repro
> cd gitignore-repro
> spago init
> spago build --pedantic-packages
# succeeds
> cat .gitignore | awk '{print "/" $0}' > .gitignore.new; mv .gitignore.new .gitignore
> spago build --pedantic-packages
Sources for package 'gitignore-repro' declares unused dependencies - please remove them from the project config:
  - console
  - effect
  - prelude

These errors can be fixed by running the below command(s):
spago uninstall -p gitignore-repro console effect prelude

Checklist:

• Added the change to the "Unreleased" section of the changelog
• Added some example of the new feature to the README
• Added a test for the contribution (if applicable)

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]

b0bc0de

If this change is contentious for any reason I can revert it - something that's been bothering me about bleeding spago is that it walks gitignored dirs looking for workspace members, including node_modules and errors when there are .gitignored read-protected dirs (ex. a docker container volume)

[f-f]

The change looks good - could you add the repro as a test?
It should go in the Pedantic suite, but should use the new copyTree structure from the more recent tests (it's shorter and nicer to work with, we haven't managed to refactor the old tests yet)

[f-f]

something that's been bothering me about bleeding spago is that it walks gitignored dirs looking for workspace members, including node_modules and errors when there are .gitignored read-protected dirs (ex. a docker container volume)

But after the latest changes we shouldn't be walking any directories that are being gitignored? Or what am I missing here?

[cakekindel]

Sorry! To clarify, the partial gitignore change both significantly improves performance when large node_modules is present and aligns with the behavior I'd expect of not looking for source packages in gitignored dirs.

If scanning gitignored dirs for packages is desired though I can revert that change :)

[Blugatroff]

But after the latest changes we shouldn't be walking any directories that are being gitignored? Or what am I missing here?

I think my previous fix, for not respecting a gitignore in case we are explicitly looking for something which would be ignored (like .spago), was too coarse grained.
Instead of only ignoring the patterns in the gitignore which caused the glob target to be excluded, my change simply ignored the entire .gitignore file along with every pattern inside.
This means, that if your root .gitignore contains the patterns [".spago", "node_modules"] and you glob for something in .spago, then node_modules would also (along with .spago) not be ignored.
On the other hand, this PR creates a distinct micromatch for every pattern in the .gitignore and checks each one of them for a collision with the glob targets separately.

[f-f]

Thanks for the clarifications! This looks great now 🙂

[cakekindel]

@f-f Gentle bump on this, I'm not sure why actions thinks the macos task has been running for 16 days when it has clearly finished 😝

[cakekindel]

rebased onto master

[f-f]

Thanks for the ping! I was waiting for CI to be done then forgot to check on it, sorry 🥲