Skip to content

Commit 4381814

Browse files
fsoikinfsoikin
authored
When resolving modules in graph, glob their paths relative to their own roots (#1334)
1 parent de2e98c commit 4381814

23 files changed

+192
-20
lines changed

‎CHANGELOG.md

+3-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ Other improvements:
4949
their specified dependency ranges.
5050
- `spago publish` no longer tries to validate all workspace dependencies, but
5151
only the (transitive) dependencies of the project being published.
52-
- Restored broken search-directed search in generated docs.
52+
- Restored broken type-directed search in generated docs.
53+
- `spago graph modules` now correctly reports extra-packages located outside the
54+
workspace root.
5355

5456
## [0.21.0] - 2023-05-04
5557

‎src/Spago/Purs/Graph.purs

+25-9
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import Registry.PackageName as PackageName
2929
import Spago.Command.Fetch as Fetch
3030
import Spago.Config (Package(..), WithTestGlobs(..), WorkspacePackage)
3131
import Spago.Config as Config
32+
import Spago.FS as FS
3233
import Spago.Glob as Glob
3334
import Spago.Log as Log
3435
import Spago.Path as Path
@@ -96,10 +97,26 @@ getModuleGraphWithPackage (ModuleGraph graph) = do
9697
$ for (Map.toUnfoldable allPackages)
9798
\(Tuple name package) -> do
9899
-- Basically partition the modules of the current package by in src and test packages
99-
let withTestGlobs = if (Set.member name (Map.keys testPackages)) then OnlyTestGlobs else NoTestGlobs
100+
let withTestGlobs = if (Map.member name testPackages) then OnlyTestGlobs else NoTestGlobs
100101
logDebug $ "Getting globs for package " <> PackageName.print name
101-
globMatches :: Array LocalPath <- map Array.fold $ traverse compileGlob (Config.sourceGlob rootPath withTestGlobs name package)
102-
pure $ map (\p -> Tuple p name) globMatches
102+
103+
-- We have to glob this package's sources relative to its own root,
104+
-- not to our workspace root, because the package may be located
105+
-- outside of the workspace root - e.g. if it's a local-file-system
106+
-- package, - in which case the `gitignoringGlob` function won't match
107+
-- any files there. That's just how it works: it walks down the tree
108+
-- from the given root.
109+
packageRoot <- Path.mkRoot $ Config.getLocalPackageLocation rootPath name package
110+
packageExists <- FS.exists packageRoot
111+
112+
if packageExists then
113+
Config.sourceGlob rootPath withTestGlobs name package
114+
<#> (_ `Path.relativeTo` packageRoot)
115+
# traverse (compileGlob packageRoot)
116+
<#> Array.fold
117+
<#> map \p -> (p `Path.relativeTo` rootPath) /\ name
118+
else
119+
pure []
103120

104121
logDebug "Got the pathToPackage map, calling packageGraph"
105122
let
@@ -118,10 +135,9 @@ getModuleGraphWithPackage (ModuleGraph graph) = do
118135

119136
pure packageGraph
120137

121-
compileGlob :: ∀ a. LocalPath -> Spago { rootPath :: RootPath | a } (Array LocalPath)
122-
compileGlob sourcePath = do
123-
{ rootPath } <- ask
124-
liftAff $ Glob.gitignoringGlob
138+
compileGlob :: ∀ a. RootPath -> LocalPath -> Spago { rootPath :: RootPath | a } (Array LocalPath)
139+
compileGlob rootPath sourcePath = liftAff $
140+
Glob.gitignoringGlob
125141
{ root: rootPath
126142
, includePatterns: [ Path.localPart $ withForwardSlashes sourcePath ]
127143
, ignorePatterns: []
@@ -233,7 +249,7 @@ checkImports graph = do
233249
{ rootPath } <- ask
234250
projectFiles :: Set String <-
235251
Config.sourceGlob rootPath testGlobOption packageName (WorkspacePackage selected)
236-
# traverse compileGlob
252+
# traverse (compileGlob rootPath)
237253
<#> Array.fold
238254
<#> map (Path.localPart <<< withForwardSlashes)
239255
<#> Set.fromFoldable
@@ -324,7 +340,7 @@ unusedError isTest selected unused = do
324340
{ errorMessage: toDoc
325341
[ toDoc $ (if isTest then "Tests for package '" else "Sources for package '")
326342
<> PackageName.print selected.package.name
327-
<> "' declares unused dependencies - please remove them from the project config:"
343+
<> "' declare unused dependencies - please remove them from the project config:"
328344
, indent $ toDoc $ map (append "- ") unusedPkgs
329345
]
330346
, correction: toDoc

‎test-fixtures/1281-local-fs-extra-packages/consumer/spago.yaml

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package:
2+
name: consumer
3+
dependencies:
4+
- extra-package
5+
6+
workspace:
7+
packageSet:
8+
registry: 41.5.0
9+
extraPackages:
10+
extra-package:
11+
path: ../extra-package

‎test-fixtures/1281-local-fs-extra-packages/consumer/src/Main.purs

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
module Consumer where
2+
3+
import ExtraPackage (x)
4+
5+
y :: Int
6+
y = x

‎test-fixtures/1281-local-fs-extra-packages/expected-stdout.txt

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Consumer:
2+
depends:
3+
- ExtraPackage
4+
package: consumer
5+
path: src/Main.purs
6+
ExtraPackage:
7+
depends: []
8+
package: extra-package
9+
path: ../extra-package/src/Main.purs

‎test-fixtures/1281-local-fs-extra-packages/extra-package/spago.yaml

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package:
2+
name: extra-package
3+
dependencies: []
4+
5+
workspace:
6+
packageSet:
7+
registry: 41.5.0

‎test-fixtures/1281-local-fs-extra-packages/extra-package/src/Main.purs

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
module ExtraPackage where
2+
3+
x :: Int
4+
x = 42

‎test-fixtures/pedantic/1281-local-fs-extra-packages/expected-stderr-unused.txt

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
Reading Spago workspace configuration...
2+
3+
✓ Selecting package to build: packagec
4+
5+
Downloading dependencies...
6+
Building...
7+
Src Lib All
8+
Warnings 0 0 0
9+
Errors 0 0 0
10+
11+
✓ Build succeeded.
12+
13+
Looking for unused and undeclared transitive dependencies...
14+
15+
✘ Found unused and/or undeclared transitive dependencies:
16+
17+
Sources for package 'packagec' declare unused dependencies - please remove them from the project config:
18+
- packageb
19+
20+
These errors can be fixed by running the below command(s):
21+
spago uninstall -p packagec packageb

‎test-fixtures/pedantic/1281-local-fs-extra-packages/expected-stderr-used.txt

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
Reading Spago workspace configuration...
2+
3+
✓ Selecting package to build: packagea
4+
5+
Downloading dependencies...
6+
Building...
7+
Src Lib All
8+
Warnings 0 0 0
9+
Errors 0 0 0
10+
11+
✓ Build succeeded.
12+
13+
Looking for unused and undeclared transitive dependencies...

‎test-fixtures/pedantic/1281-local-fs-extra-packages/packagea/spago.yaml

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package:
2+
name: packagea
3+
dependencies:
4+
- maybe
5+
- packageb
6+
workspace:
7+
packageSet:
8+
registry: 58.0.1
9+
extraPackages:
10+
packageb:
11+
path: ../packageb

‎test-fixtures/pedantic/1281-local-fs-extra-packages/packagea/src/PackageA.purs

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
module PackageA
2+
( sample
3+
)
4+
where
5+
6+
import Data.Maybe (Maybe(..))
7+
import PackageB (exampleFunc)
8+
9+
sample :: forall a. Maybe a
10+
sample =
11+
case exampleFunc of
12+
Just val -> Just val
13+
Nothing -> Nothing
14+

‎test-fixtures/pedantic/1281-local-fs-extra-packages/packageb/spago.yaml

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package:
2+
name: packageb
3+
dependencies:
4+
- maybe
5+
build:
6+
strict: true
7+
workspace:
8+
packageSet:
9+
registry: 58.0.1

‎test-fixtures/pedantic/1281-local-fs-extra-packages/packageb/src/PackageB.purs

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
module PackageB
2+
( exampleFunc
3+
)
4+
where
5+
6+
import Data.Maybe (Maybe(..))
7+
8+
exampleFunc :: forall a. Maybe a
9+
exampleFunc = Nothing

‎test-fixtures/pedantic/1281-local-fs-extra-packages/packagec/spago.yaml

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package:
2+
name: packagec
3+
dependencies:
4+
- maybe
5+
- packageb
6+
workspace:
7+
packageSet:
8+
registry: 58.0.1
9+
extraPackages:
10+
packageb:
11+
path: ../packageb

‎test-fixtures/pedantic/1281-local-fs-extra-packages/packagec/src/PackageC.purs

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
module PackageC
2+
( sample
3+
)
4+
where
5+
6+
import Data.Maybe (Maybe(..))
7+
8+
sample :: forall a. Maybe a
9+
sample = Nothing

‎test-fixtures/pedantic/check-pedantic-packages.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Looking for unused and undeclared transitive dependencies...
1414

1515
✘ Found unused and/or undeclared transitive dependencies:
1616

17-
Sources for package 'pedantic' declares unused dependencies - please remove them from the project config:
17+
Sources for package 'pedantic' declare unused dependencies - please remove them from the project config:
1818
- console
1919
- effect
2020
- either
@@ -24,7 +24,7 @@ Sources for package 'pedantic' import the following transitive dependencies - pl
2424
from `Main`, which imports:
2525
Data.Newtype
2626

27-
Tests for package 'pedantic' declares unused dependencies - please remove them from the project config:
27+
Tests for package 'pedantic' declare unused dependencies - please remove them from the project config:
2828
- tuples
2929

3030
Tests for package 'pedantic' import the following transitive dependencies - please add them to the project dependencies, or remove the imports:

‎test-fixtures/pedantic/check-unused-dependency-in-source.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Looking for unused and undeclared transitive dependencies...
1414

1515
✘ Found unused and/or undeclared transitive dependencies:
1616

17-
Sources for package 'pedantic' declares unused dependencies - please remove them from the project config:
17+
Sources for package 'pedantic' declare unused dependencies - please remove them from the project config:
1818
- console
1919
- effect
2020

‎test-fixtures/pedantic/check-unused-dependency.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Looking for unused and undeclared transitive dependencies...
1414

1515
✘ Found unused and/or undeclared transitive dependencies:
1616

17-
Sources for package 'pedantic' declares unused dependencies - please remove them from the project config:
17+
Sources for package 'pedantic' declare unused dependencies - please remove them from the project config:
1818
- console
1919
- effect
2020

‎test-fixtures/pedantic/check-unused-source-and-test-dependency.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ Looking for unused and undeclared transitive dependencies...
1414

1515
✘ Found unused and/or undeclared transitive dependencies:
1616

17-
Sources for package 'pedantic' declares unused dependencies - please remove them from the project config:
17+
Sources for package 'pedantic' declare unused dependencies - please remove them from the project config:
1818
- console
1919
- effect
2020

21-
Tests for package 'pedantic' declares unused dependencies - please remove them from the project config:
21+
Tests for package 'pedantic' declare unused dependencies - please remove them from the project config:
2222
- console
2323
- effect
2424

‎test-fixtures/pedantic/check-unused-test-dependency.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Looking for unused and undeclared transitive dependencies...
1414

1515
✘ Found unused and/or undeclared transitive dependencies:
1616

17-
Tests for package 'pedantic' declares unused dependencies - please remove them from the project config:
17+
Tests for package 'pedantic' declare unused dependencies - please remove them from the project config:
1818
- newtype
1919

2020
These errors can be fixed by running the below command(s):

‎test/Spago/Build/Monorepo.purs

+1-1
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ spec = Spec.describe "monorepo" do
192192

193193
mkUnusedDepErr isSrc package =
194194
Array.intercalate "\n"
195-
[ toMsgPrefix isSrc <> " for package '" <> package <> "' declares unused dependencies - please remove them from the project config:"
195+
[ toMsgPrefix isSrc <> " for package '" <> package <> "' declare unused dependencies - please remove them from the project config:"
196196
, " - " <> (if isSrc then "tuples" else "either")
197197
]
198198
mkTransitiveDepErr isSrc package = do

‎test/Spago/Build/Pedantic.purs

+15-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import Data.Array as Array
66
import Data.Map as Map
77
import Spago.Core.Config (Dependencies(..), Config)
88
import Spago.FS as FS
9-
import Spago.Path as Path
9+
import Spago.Paths as Paths
1010
import Test.Spec (SpecT)
1111
import Test.Spec as Spec
1212

@@ -206,14 +206,27 @@ spec =
206206
Spec.it
207207
(".gitignore does not affect discovery of transitive deps (" <> gitignore <> ")") \{ spago, fixture, testCwd } -> do
208208
FS.copyTree { src: fixture "pedantic/follow-instructions", dst: testCwd }
209-
FS.writeTextFile (Path.global ".gitignore") gitignore
209+
FS.writeTextFile (testCwd </> ".gitignore") gitignore
210210
spago [ "uninstall", "effect" ] >>= shouldBeSuccess
211211
-- Get rid of "Compiling..." messages
212212
spago [ "build" ] >>= shouldBeSuccess
213213
editSpagoYaml (addPedanticFlagToSrc)
214214
spago [ "build" ] >>= shouldBeFailureErr (fixture "pedantic/pedantic-instructions-initial-failure.txt")
215215
spago [ "install", "-p", "follow-instructions", "effect" ] >>= shouldBeSuccessErr (fixture "pedantic/pedantic-instructions-installation-result.txt")
216216

217+
Spec.it "#1281 treats extra-packages on the local file system as used" \{ spago, fixture, testCwd } -> do
218+
FS.copyTree { src: fixture "pedantic/1281-local-fs-extra-packages", dst: testCwd }
219+
220+
Paths.chdir $ testCwd </> "packagea"
221+
spago [ "build" ] >>= shouldBeSuccess
222+
spago [ "build", "--pedantic-packages" ]
223+
>>= shouldBeSuccessErr (fixture "pedantic/1281-local-fs-extra-packages/expected-stderr-used.txt")
224+
225+
Paths.chdir $ testCwd </> "packagec"
226+
spago [ "build" ] >>= shouldBeSuccess
227+
spago [ "build", "--pedantic-packages" ]
228+
>>= shouldBeFailureErr (fixture "pedantic/1281-local-fs-extra-packages/expected-stderr-unused.txt")
229+
217230
addPedanticFlagToSrc :: Config -> Config
218231
addPedanticFlagToSrc config = config
219232
{ package = config.package <#> \r -> r

‎test/Spago/Graph.purs

+7
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ module Test.Spago.Graph where
22

33
import Test.Prelude
44

5+
import Spago.FS as FS
6+
import Spago.Paths as Paths
57
import Test.Spec (Spec)
68
import Test.Spec as Spec
79

@@ -32,3 +34,8 @@ spec = Spec.around withTempDir do
3234
Spec.it "can topologically sort packages" \{ spago, fixture } -> do
3335
spago [ "init", "--name", "my-project", "--package-set", "41.2.0" ] >>= shouldBeSuccess
3436
spago [ "graph", "packages", "--topo" ] >>= shouldBeSuccessOutput (fixture "graph-packages-topo.txt")
37+
38+
Spec.it "#1281 correctly reports modules from extra-packages on the local file system" \{ spago, fixture, testCwd } -> do
39+
FS.copyTree { src: fixture "1281-local-fs-extra-packages", dst: testCwd }
40+
Paths.chdir $ testCwd </> "consumer"
41+
spago [ "graph", "modules" ] >>= shouldBeSuccessOutput (fixture "1281-local-fs-extra-packages/expected-stdout.txt")

0 commit comments

Comments
 (0)