diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fee72990..3910b7538 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Other improvements: - builds with Cabal successfully - update to latest `versions` dependency: https://hackage.haskell.org/package/versions-6.0.1/changelog - Fix output truncation with `--json-errors`, many warnings and build failure (#1199) +- Fixed globbing issue where `/.spago` behaves differently than `.spago` in `.gitignore` ## [0.21.0] - 2023-05-04 diff --git a/spago.lock b/spago.lock index 5eb74ab5a..744d94cbb 100644 --- a/spago.lock +++ b/spago.lock @@ -356,148 +356,6 @@ workspace: - unsafe-coerce - variant - versions - perftest: - path: perftest - dependencies: - - aff - - arrays - - codec-argonaut - - console - - js-date - - lists - - node-fs - - node-path - - node-process - - optparse - - ordered-collections - - record - - refs - - registry-lib - - spago - - spago-core - - unsafe-coerce - test_dependencies: [] - build_plan: - - aff - - aff-promise - - affjax - - affjax-node - - ansi - - argonaut-core - - arraybuffer-types - - arrays - - assert - - avar - - bifunctors - - catenable-lists - - codec - - codec-argonaut - - console - - const - - contravariant - - control - - datetime - - distributive - - docs-search-common - - docs-search-index - - dodo-printer - - effect - - either - - enums - - exceptions - - exists - - exitcodes - - filterable - - fixed-points - - foldable-traversable - - foreign - - foreign-object - - fork - - form-urlencoded - - formatters - - free - - functions - - functors - - gen - - graphs - - http-methods - - identity - - integers - - invariant - - js-date - - js-timers - - js-uri - - json-codecs - - language-cst-parser - - language-purescript - - lazy - - lcg - - lists - - maybe - - media-types - - mmorph - - newtype - - node-buffer - - node-child-process - - node-event-emitter - - node-execa - - node-fs - - node-human-signals - - node-os - - node-path - - node-process - - node-streams - - nonempty - - now - - nullable - - numbers - - open-memoize - - optparse - - ordered-collections - - orders - - parallel - - parsing - - partial - - pipes - - posix-types - - prelude - - profunctor - - profunctor-lenses - - psci-support - - quickcheck - - quickcheck-laws - - random - - record - - refs - - registry-lib - - routing-duplex - - safe-coerce - - search-trie - - spago - - spago-core - - spec - - st - - string-parsers - - strings - - stringutils - - tailrec - - these - - transformers - - tuples - - type-equality - - typelevel-prelude - - unfoldable - - unicode - - unsafe-coerce - - unsafe-reference - - variant - - versions - - web-dom - - web-events - - web-file - - web-html - - web-storage - - web-xhr spago: path: ./ dependencies: diff --git a/src/Spago/Glob.purs b/src/Spago/Glob.purs index d99e7b451..1165c2ed4 100644 --- a/src/Spago/Glob.purs +++ b/src/Spago/Glob.purs @@ -3,8 +3,10 @@ module Spago.Glob (gitignoringGlob) where import Spago.Prelude import Data.Array as Array -import Data.String as String +import Data.Filterable (filter) import Data.Foldable (any) +import Data.String as String +import Data.String.CodePoints as String.CodePoint import Effect.Aff as Aff import Effect.Ref as Ref import Node.FS.Sync as SyncFS @@ -48,15 +50,14 @@ gitignoreToMicromatchPatterns base = dropSuffixSlash str = fromMaybe str $ String.stripSuffix (String.Pattern "/") str dropPrefixSlash str = fromMaybe str $ String.stripPrefix (String.Pattern "/") str + leadingSlash str = String.codePointAt 0 str == Just (String.CodePoint.codePointFromChar '/') + trailingSlash str = String.codePointAt (String.length str - 1) str == Just (String.CodePoint.codePointFromChar '/') + gitignorePatternToMicromatch :: String -> String gitignorePatternToMicromatch pattern - -- Git matches every pattern that does not include a `/` by basename. - | not $ String.contains (String.Pattern "/") pattern = "**/" <> pattern <> "/**" - | otherwise = - -- Micromatch treats every pattern like git treats those starting with '/'. - dropPrefixSlash pattern - -- ".spago/" in a .gitignore is the same as ".spago". Micromatch does interpret them differently. - # dropSuffixSlash + | trailingSlash pattern = gitignorePatternToMicromatch $ dropSuffixSlash pattern + | leadingSlash pattern = dropPrefixSlash pattern <> "/**" + | otherwise = "**/" <> pattern <> "/**" fsWalk :: String -> Array String -> Array String -> Aff (Array Entry) fsWalk cwd ignorePatterns includePatterns = Aff.makeAff \cb -> do @@ -92,20 +93,20 @@ fsWalk cwd ignorePatterns includePatterns = Aff.makeAff \cb -> do Left _ -> pure unit Right gitignore -> do let { ignore, patterns } = gitignoreToMicromatchPatterns base gitignore - let matcherForThisGitignore = micromatch { ignore } patterns - - -- Does the .gitignore contain a pattern which would ignore the very - -- thing we are asked to look for? - -- For example, the .gitignore might have the pattern ".spago" but - -- `includePatterns` contains a glob like .spago/p/foo-3.1.4/**/*.purs - let anyIncludePatternWouldBeIgnored = any matcherForThisGitignore includePatterns - - -- In such a case, do not use this .gitignore for pruning the search. - when (not anyIncludePatternWouldBeIgnored) do + let gitignored = (micromatch { ignore } <<< pure) <$> patterns + let wouldConflictWithSearch m = any m includePatterns + + -- Do not add `.gitignore` patterns that explicitly ignore the files + -- we're searching for; + -- + -- ex. if `includePatterns` is [".spago/p/aff-1.0.0/**/*.purs"], + -- and `gitignored` is ["node_modules", ".spago"], + -- then add "node_modules" to `ignoreMatcher` but not ".spago" + for_ (filter (not <<< wouldConflictWithSearch) gitignored) \pat -> do -- 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. - let addMatcher currentMatcher = or [ currentMatcher, matcherForThisGitignore ] + let addMatcher currentMatcher = or [ currentMatcher, pat ] void $ Ref.modify addMatcher ignoreMatcherRef ignoreMatcher <- Ref.read ignoreMatcherRef diff --git a/test-fixtures/pedantic/follow-instructions/.gitignore b/test-fixtures/pedantic/follow-instructions/.gitignore new file mode 100644 index 000000000..c4705363b --- /dev/null +++ b/test-fixtures/pedantic/follow-instructions/.gitignore @@ -0,0 +1,12 @@ + +bower_components/ +node_modules/ +.pulp-cache/ +output/ +output-es/ +generated-docs/ +.psc-package/ +.psc* +.purs* +.psa* +.spago diff --git a/test-fixtures/pedantic/follow-instructions/spago.yaml b/test-fixtures/pedantic/follow-instructions/spago.yaml new file mode 100644 index 000000000..92423b925 --- /dev/null +++ b/test-fixtures/pedantic/follow-instructions/spago.yaml @@ -0,0 +1,12 @@ +package: + name: follow-instructions + run: + main: Test.Pedantic.FollowInstructions.Main + dependencies: + - console + - effect + - prelude +workspace: + package_set: + registry: 0.0.1 + extra_packages: {} diff --git a/test-fixtures/pedantic/follow-instructions/src/Main.purs b/test-fixtures/pedantic/follow-instructions/src/Main.purs new file mode 100644 index 000000000..151aa6c66 --- /dev/null +++ b/test-fixtures/pedantic/follow-instructions/src/Main.purs @@ -0,0 +1,8 @@ +module Test.Pedantic.FollowInstructions.Main where +import Prelude + +import Effect +import Effect.Console as Console + +main :: Effect Unit +main = Console.log "wat" diff --git a/test-fixtures/pedantic/pedantic-instructions-initial-failure.txt b/test-fixtures/pedantic/pedantic-instructions-initial-failure.txt index b68c36af6..eb275ee07 100644 --- a/test-fixtures/pedantic/pedantic-instructions-initial-failure.txt +++ b/test-fixtures/pedantic/pedantic-instructions-initial-failure.txt @@ -1,6 +1,6 @@ Reading Spago workspace configuration... -✅ Selecting package to build: pedantic +✅ Selecting package to build: follow-instructions Downloading dependencies... Building... @@ -14,10 +14,10 @@ Looking for unused and undeclared transitive dependencies... ❌ Found unused and/or undeclared transitive dependencies: -Sources for package 'pedantic' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: +Sources for package 'follow-instructions' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: effect - from `Main`, which imports: + from `Test.Pedantic.FollowInstructions.Main`, which imports: Effect These errors can be fixed by running the below command(s): -spago install -p pedantic effect +spago install -p follow-instructions effect diff --git a/test-fixtures/pedantic/pedantic-instructions-installation-result.txt b/test-fixtures/pedantic/pedantic-instructions-installation-result.txt index 4ceb738b2..53af072cb 100644 --- a/test-fixtures/pedantic/pedantic-instructions-installation-result.txt +++ b/test-fixtures/pedantic/pedantic-instructions-installation-result.txt @@ -1,6 +1,6 @@ Reading Spago workspace configuration... -✅ Selecting package to build: pedantic +✅ Selecting package to build: follow-instructions Adding 1 package to the config in spago.yaml Downloading dependencies... diff --git a/test/Spago.purs b/test/Spago.purs index 262b43b4c..6d0c2053e 100644 --- a/test/Spago.purs +++ b/test/Spago.purs @@ -8,6 +8,7 @@ import Data.Newtype (un) import Effect (Effect) import Effect.Aff (Milliseconds(..)) import Effect.Aff as Aff +import Test.Spago.Glob as Glob import Test.Spago.Build as Build import Test.Spago.Bundle as Bundle import Test.Spago.Docs as Docs @@ -56,4 +57,4 @@ main = Aff.launchAff_ $ void $ un Identity $ Spec.Runner.runSpecT testConfig [ S Spec.describe "miscellaneous" do Lock.spec Unit.spec - + Glob.spec diff --git a/test/Spago/Build/Pedantic.purs b/test/Spago/Build/Pedantic.purs index 8afe8b19f..154a5586c 100644 --- a/test/Spago/Build/Pedantic.purs +++ b/test/Spago/Build/Pedantic.purs @@ -192,28 +192,28 @@ spec = -- where the fix is to install that missing package. -- Following those instructions shouldn't cause an error. Spec.it "following installation instructions does not fail with an unrelated pedantic error" \{ spago, fixture } -> do - setup spago - ( defaultSetupConfig - { installSourcePackages = [ "prelude", "console" ] - , installTestPackages = [] - , main = Just - [ "import Prelude" - , "" - , "import Effect" - , "import Effect.Console as Console" - , "" - , "main :: Effect Unit" - , "main = Console.log \"wat\"" - ] - , testMain = Nothing - } - ) + FS.copyTree { src: fixture "pedantic/follow-instructions", dst: "." } spago [ "uninstall", "effect" ] >>= shouldBeSuccess -- Get rid of "Compiling..." messages spago [ "build" ] >>= shouldBeSuccess editSpagoYaml (addPedanticFlagToSrc) spago [ "build" ] >>= shouldBeFailureErr (fixture "pedantic/pedantic-instructions-initial-failure.txt") - spago [ "install", "-p", "pedantic", "effect" ] >>= shouldBeSuccessErr (fixture "pedantic/pedantic-instructions-installation-result.txt") + spago [ "install", "-p", "follow-instructions", "effect" ] >>= shouldBeSuccessErr (fixture "pedantic/pedantic-instructions-installation-result.txt") + + -- Regression test for https://github.com/purescript/spago/pull/1222 + let gitignores = [".spago", "/.spago", ".spago/**"] + for_ gitignores \gitignore -> + Spec.it + (".gitignore does not affect discovery of transitive deps (" <> gitignore <> ")") + \{ spago, fixture } -> do + FS.copyTree { src: fixture "pedantic/follow-instructions", dst: "." } + FS.writeTextFile ".gitignore" gitignore + spago [ "uninstall", "effect" ] >>= shouldBeSuccess + -- Get rid of "Compiling..." messages + spago [ "build" ] >>= shouldBeSuccess + editSpagoYaml (addPedanticFlagToSrc) + spago [ "build" ] >>= shouldBeFailureErr (fixture "pedantic/pedantic-instructions-initial-failure.txt") + spago [ "install", "-p", "follow-instructions", "effect" ] >>= shouldBeSuccessErr (fixture "pedantic/pedantic-instructions-installation-result.txt") addPedanticFlagToSrc :: Config -> Config addPedanticFlagToSrc config = config diff --git a/test/Spago/Glob.purs b/test/Spago/Glob.purs new file mode 100644 index 000000000..a7a8bc47d --- /dev/null +++ b/test/Spago/Glob.purs @@ -0,0 +1,68 @@ +module Test.Spago.Glob where + +import Test.Prelude + +import Effect.Aff as Aff +import Node.Path as Path +import Spago.FS as FS +import Spago.Glob as Glob +import Test.Spec (Spec) +import Test.Spec as Spec +import Test.Spec.Assertions as Assert + +globTmpDir :: (String -> Aff Unit) -> Aff Unit +globTmpDir m = Aff.bracket make cleanup m + where + touch name base = FS.writeTextFile (Path.concat [base, name]) "" + dir name contents base = do + FS.mkdirp $ Path.concat [base, name] + for_ contents \f -> f $ Path.concat [base, name] + cleanup _ = pure unit + make = do + base <- mkTemp' $ Just "spago-test-" + dir + ".git" + [ dir "fruits" [touch "apple"] ] + base + dir + "fruits" + [touch "apple", touch "orange", touch "banana"] + base + dir + "sports" + [touch "baseball", touch "scrabble"] + base + dir + "src" + [ dir "fruits" [touch "apple"] + , dir "sports" [touch "baseball"] + ] + base + pure base + +spec :: Spec Unit +spec = Spec.around globTmpDir do + Spec.describe "glob" do + Spec.describe "gitignoringGlob" do + Spec.it "when no .gitignore, yields all matches" \p -> do + a <- Glob.gitignoringGlob p ["**/fruits/apple/**"] + a `Assert.shouldEqual` ["fruits/apple", "src/fruits/apple"] + + b <- Glob.gitignoringGlob p ["sports/**"] + b `Assert.shouldEqual` ["sports", "sports/baseball", "sports/scrabble"] + + Spec.it "respects a .gitignore pattern that doesn't conflict with search" \p -> do + FS.writeTextFile (Path.concat [p, ".gitignore"]) "/fruits" + a <- Glob.gitignoringGlob p ["src/fruits/apple/**"] + a `Assert.shouldEqual` ["src/fruits/apple"] + + Spec.it "does not respect a .gitignore pattern that conflicts with search" \p -> do + for_ ["/fruits", "fruits", "fruits/", "**/fruits", "fruits/**", "**/fruits/**"] \gitignore -> do + FS.writeTextFile (Path.concat [p, ".gitignore"]) gitignore + a <- Glob.gitignoringGlob p ["fruits/apple/**"] + a `Assert.shouldEqual` ["fruits/apple"] + + Spec.it "respects some .gitignore patterns" \p -> do + FS.writeTextFile (Path.concat [p, ".gitignore"]) """/fruits\n/src""" + a <- Glob.gitignoringGlob p ["fruits/apple/**"] + a `Assert.shouldEqual` ["fruits/apple"]