Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid globbing the spago cache folder when building #1293

Merged
merged 3 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Spago/Command/Fetch.purs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ run { packages: packagesRequestedToInstall, ensureRanges, isTest, isRepl } = do
Nothing -> currentWorkspace
Just { newWorkspacePackage } -> currentWorkspace
{ packageSet = currentWorkspace.packageSet
{ lockfile = Left "Lockfile is out of date (installing new packages)"
{ lockfile = Left "Lockfile is out of date (reason: installing new packages)"
-- If we are installing packages, we need to add the new deps to the selected package
, buildType = case currentWorkspace.packageSet.buildType of
RegistrySolverBuild packageMap -> RegistrySolverBuild $ Map.insert newWorkspacePackage.package.name (WorkspacePackage newWorkspacePackage) packageMap
Expand Down
4 changes: 2 additions & 2 deletions src/Spago/Command/Uninstall.purs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ run args = do

newWorkspace = workspace
{ packageSet = workspace.packageSet
{ lockfile = Left "Lockfile is out of date (installing new packages)"
{ lockfile = Left "Lockfile is out of date (reason: installing new packages)"
-- If we are installing packages, we need to add the new deps to the selected package
, buildType = case workspace.packageSet.buildType of
RegistrySolverBuild packageMap -> RegistrySolverBuild $ Map.insert newWorkspacePackage.package.name (WorkspacePackage newWorkspacePackage) packageMap
Expand All @@ -78,7 +78,7 @@ run args = do
}

local (_ { workspace = newWorkspace }) do
void $ writeNewLockfile "Lockfile is out of date (uninstalled packages)"
void $ writeNewLockfile "Lockfile is out of date (reason: uninstalled packages)"

where
writeNewLockfile reason = do
Expand Down
62 changes: 46 additions & 16 deletions src/Spago/Config.purs
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,16 @@ readWorkspace { maybeSelectedPackage, pureBuild, migrateConfig } = do
, "See the relevant documentation here: https://github.com/purescript/spago#the-workspace"
]
Right config@{ yaml: { workspace: Just workspace, package }, doc } -> do
logDebug "Read the root config"
doMigrateConfig "spago.yaml" config
pure { workspace, package, workspaceDoc: doc }

logDebug "Gathering all the spago configs in the tree..."
otherConfigPaths <- liftAff $ Glob.gitignoringGlob Paths.cwd [ "**/spago.yaml" ]
otherConfigPaths <- liftAff $ Glob.gitignoringGlob
{ cwd: Paths.cwd
, includePatterns: [ "**/spago.yaml" ]
, ignorePatterns: [ "**/node_modules/**", "**/.spago/**" ]
}
unless (Array.null otherConfigPaths) do
logDebug $ [ toDoc "Found packages at these paths:", Log.indent $ Log.lines (map toDoc otherConfigPaths) ]

Expand Down Expand Up @@ -300,8 +305,10 @@ readWorkspace { maybeSelectedPackage, pureBuild, migrateConfig } = do
true, _ -> do
logDebug "Using lockfile because of --pure flag"
pure (Right contents)
false, true -> pure (Left "Lockfile is out of date")
false, false -> do
false, lockfileIsOutOfDate@{ result: true } -> do
logDebug $ "Reason for recomputing the lockfile: " <> show lockfileIsOutOfDate
pure $ Left $ "Lockfile is out of date (reason: " <> lockfileIsOutOfDate.reasons <> ")"
false, { result: false } -> do
logDebug "Lockfile is up to date, using it"
pure (Right contents)

Expand Down Expand Up @@ -455,22 +462,45 @@ workspacePackageToLockfilePackage { path, package } = Tuple package.name
, test: { dependencies: foldMap _.dependencies package.test, build_plan: mempty }
}

shouldComputeNewLockfile :: { workspace :: Core.WorkspaceConfig, workspacePackages :: Map PackageName WorkspacePackage } -> Lock.WorkspaceLock -> Boolean
type LockfileRecomputeResult =
{ workspacesDontMatch :: Boolean
, extraPackagesDontMatch :: Boolean
, packageSetAddressIsDifferent :: Boolean
, packageSetIsLocal :: Boolean
, result :: Boolean
, reasons :: String
}

shouldComputeNewLockfile :: { workspace :: Core.WorkspaceConfig, workspacePackages :: Map PackageName WorkspacePackage } -> Lock.WorkspaceLock -> LockfileRecomputeResult
shouldComputeNewLockfile { workspace, workspacePackages } workspaceLock =
-- the workspace packages should exactly match, except for the needed_by field, which is filled in during build plan construction
((workspacePackageToLockfilePackage >>> snd <$> workspacePackages) /= (eraseBuildPlan <$> workspaceLock.packages))
-- and the extra packages should exactly match
|| (fromMaybe Map.empty workspace.extraPackages /= workspaceLock.extra_packages)
-- and the package set address needs to match - we have no way to match the package set contents at this point, so we let it be
|| (workspace.packageSet /= map _.address workspaceLock.package_set)
-- and the package set is not a local file - if it is then we always recompute the lockfile because we have no way to check if it's changed
||
( case workspace.packageSet of
Just (Core.SetFromPath _) -> true
_ -> false
)
{ workspacesDontMatch
, extraPackagesDontMatch
, packageSetAddressIsDifferent
, packageSetIsLocal
, result: workspacesDontMatch || extraPackagesDontMatch || packageSetAddressIsDifferent || packageSetIsLocal
, reasons: String.joinWith ", " $ Array.mapMaybe identity
[ explainReason workspacesDontMatch "workspace packages changed"
, explainReason extraPackagesDontMatch "extraPackages changed"
, explainReason packageSetAddressIsDifferent "package set address changed"
, explainReason packageSetIsLocal "package set is local"
]
}
where
eraseBuildPlan = _ { core { build_plan = mempty }, test { build_plan = mempty } }
-- surely this already exists
explainReason flag reason = if flag then Just reason else Nothing

-- Conditions for recomputing the lockfile:
-- 1. the workspace packages should exactly match, except for the needed_by field, which is filled in during build plan construction
workspacesDontMatch = (workspacePackageToLockfilePackage >>> snd <$> workspacePackages) /= (eraseBuildPlan <$> workspaceLock.packages)
-- 2. the extra packages should exactly match
extraPackagesDontMatch = fromMaybe Map.empty workspace.extraPackages /= workspaceLock.extra_packages
-- 3. the package set address needs to match - we have no way to match the package set contents at this point, so we let it be
packageSetAddressIsDifferent = workspace.packageSet /= map _.address workspaceLock.package_set
-- 4. the package set is not a local file - if it is then we always recompute the lockfile because we have no way to check if it's changed
packageSetIsLocal = case workspace.packageSet of
Just (Core.SetFromPath _) -> true
_ -> false

getPackageLocation :: PackageName -> Package -> FilePath
getPackageLocation name = Paths.mkRelative <<< case _ of
Expand Down
18 changes: 13 additions & 5 deletions src/Spago/Glob.purs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ gitignoreFileToGlob base =
| leadingSlash pattern = dropPrefixSlash pattern <> "/**"
| otherwise = "**/" <> pattern <> "/**"

fsWalk :: String -> Array String -> Array String -> Aff (Array Entry)
fsWalk cwd ignorePatterns includePatterns = Aff.makeAff \cb -> do
fsWalk :: GlobParams -> Aff (Array Entry)
fsWalk { cwd, ignorePatterns, includePatterns } = Aff.makeAff \cb -> do
let includeMatcher = testGlob { ignore: [], include: includePatterns }

-- Pattern for directories which can be outright ignored.
Expand Down Expand Up @@ -214,6 +214,14 @@ fsWalk cwd ignorePatterns includePatterns = Aff.makeAff \cb -> do
pure $ Aff.Canceler \_ ->
void $ liftEffect $ Ref.write true canceled

gitignoringGlob :: String -> Array String -> Aff (Array String)
gitignoringGlob dir patterns = map (withForwardSlashes <<< Path.relative dir <<< _.path)
<$> fsWalk dir [ ".git" ] patterns
type GlobParams = { ignorePatterns :: Array String, includePatterns :: Array String, cwd :: FilePath }

gitignoringGlob :: GlobParams -> Aff (Array String)
gitignoringGlob { cwd, ignorePatterns, includePatterns } = map (withForwardSlashes <<< Path.relative cwd <<< _.path)
<$> fsWalk
{ cwd
, ignorePatterns: ignorePatterns
-- The ones in the base directory are always ignored
<> [ ".git", "spago.yaml" ]
, includePatterns
}
2 changes: 1 addition & 1 deletion src/Spago/Purs/Graph.purs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ getModuleGraphWithPackage (ModuleGraph graph) = do
pure packageGraph

compileGlob :: forall a. FilePath -> Spago a (Array FilePath)
compileGlob sourcePath = liftAff $ Glob.gitignoringGlob Paths.cwd [ withForwardSlashes sourcePath ]
compileGlob sourcePath = liftAff $ Glob.gitignoringGlob { cwd: Paths.cwd, includePatterns: [ withForwardSlashes sourcePath ], ignorePatterns: [] }

--------------------------------------------------------------------------------
-- Package graph
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Reading Spago workspace configuration...

Adding 1 package to the config in spago.yaml
Downloading dependencies...
Lockfile is out of date (installing new packages), generating it...
Lockfile is out of date (reason: installing new packages), generating it...
Lockfile written to spago.lock. Please commit this file.
Building...
Src Lib All
Expand Down
2 changes: 1 addition & 1 deletion test-fixtures/publish-no-config.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Reading Spago workspace configuration...
✓ Selecting package to build: aaaa

Downloading dependencies...
Lockfile is out of date, generating it...
Lockfile is out of date (reason: workspace packages changed), generating it...
Lockfile written to spago.lock. Please commit this file.
Building...
Src Lib All
Expand Down
2 changes: 1 addition & 1 deletion test-fixtures/uninstall-remove-src-deps.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ Reading Spago workspace configuration...
✓ Selecting package to build: uninstall-tests

Removing the following source dependencies: either
Lockfile is out of date (uninstalled packages), generating it...
Lockfile is out of date (reason: uninstalled packages), generating it...
Lockfile written to spago.lock. Please commit this file.
2 changes: 1 addition & 1 deletion test-fixtures/uninstall-remove-test-deps.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ Reading Spago workspace configuration...
✓ Selecting package to build: uninstall-tests

Removing the following test dependencies: either
Lockfile is out of date (uninstalled packages), generating it...
Lockfile is out of date (reason: uninstalled packages), generating it...
Lockfile written to spago.lock. Please commit this file.
23 changes: 12 additions & 11 deletions test/Spago/Glob.purs
Original file line number Diff line number Diff line change
Expand Up @@ -46,48 +46,49 @@ globTmpDir m = Aff.bracket make cleanup m

spec :: Spec Unit
spec = Spec.around globTmpDir do
let glob cwd includePatterns = Glob.gitignoringGlob { cwd, includePatterns, ignorePatterns: [] }
Spec.describe "glob" do
Spec.describe "glob behavior" do
Spec.it "'**/..' matches 0 or more directories" \p -> do
a <- Glob.gitignoringGlob (Path.concat [ p, "fruits/left" ]) [ "**/apple" ]
b <- Glob.gitignoringGlob (Path.concat [ p, "fruits" ]) [ "**/apple" ]
a <- glob (Path.concat [ p, "fruits/left" ]) [ "**/apple" ]
b <- glob (Path.concat [ p, "fruits" ]) [ "**/apple" ]
Array.sort a `Assert.shouldEqual` [ "apple" ]
Array.sort b `Assert.shouldEqual` [ "left/apple", "right/apple" ]

Spec.it "'../**/..' matches 0 or more directories" \p -> do
a <- Glob.gitignoringGlob p [ "fruits/**/apple" ]
a <- glob p [ "fruits/**/apple" ]
Array.sort a `Assert.shouldEqual` [ "fruits/left/apple", "fruits/right/apple" ]

Spec.it "'../**' matches 0 or more directories" \p -> do
a <- Glob.gitignoringGlob p [ "fruits/left/**" ]
a <- glob p [ "fruits/left/**" ]
Array.sort a `Assert.shouldEqual` [ "fruits/left", "fruits/left/apple" ]

Spec.describe "gitignoringGlob" do
Spec.it "when no .gitignore, yields all matches" \p -> do
a <- Glob.gitignoringGlob p [ "**/apple" ]
a <- glob p [ "**/apple" ]
Array.sort a `Assert.shouldEqual` [ "fruits/left/apple", "fruits/right/apple", "src/fruits/apple" ]

Spec.it "respects a .gitignore pattern that doesn't conflict with search" \p -> do
FS.writeTextFile (Path.concat [ p, ".gitignore" ]) "fruits/right"
a <- Glob.gitignoringGlob p [ "fruits/**/apple" ]
a <- glob p [ "fruits/**/apple" ]
Array.sort a `Assert.shouldEqual` [ "fruits/left/apple" ]

Spec.it "respects some .gitignore patterns" \p -> do
FS.writeTextFile (Path.concat [ p, ".gitignore" ]) "fruits\nfruits/right"
a <- Glob.gitignoringGlob p [ "fruits/**/apple" ]
a <- glob p [ "fruits/**/apple" ]
Array.sort a `Assert.shouldEqual` [ "fruits/left/apple" ]

Spec.it "respects a negated .gitignore pattern" \p -> do
FS.writeTextFile (Path.concat [ p, ".gitignore" ]) "!/fruits/left/apple\n/fruits/**/apple"
a <- Glob.gitignoringGlob p [ "**/apple" ]
a <- glob p [ "**/apple" ]
Array.sort a `Assert.shouldEqual` [ "fruits/left/apple", "src/fruits/apple" ]

for_ [ "/fruits", "fruits", "fruits/", "**/fruits", "fruits/**", "**/fruits/**" ] \gitignore -> do
Spec.it
("does not respect a .gitignore pattern that conflicts with search: " <> gitignore)
\p -> do
FS.writeTextFile (Path.concat [ p, ".gitignore" ]) gitignore
a <- Glob.gitignoringGlob p [ "fruits/**/apple" ]
a <- glob p [ "fruits/**/apple" ]
Array.sort a `Assert.shouldEqual` [ "fruits/left/apple", "fruits/right/apple" ]

Spec.it "is stacksafe" \p -> do
Expand All @@ -101,10 +102,10 @@ spec = Spec.around globTmpDir do
FS.writeTextFile (Path.concat [ p, "fruits", ".gitignore" ]) hugeGitignore
FS.writeTextFile (Path.concat [ p, "fruits", "left", ".gitignore" ]) hugeGitignore
FS.writeTextFile (Path.concat [ p, "fruits", "right", ".gitignore" ]) hugeGitignore
a <- Glob.gitignoringGlob p [ "fruits/**/apple" ]
a <- glob p [ "fruits/**/apple" ]
Array.sort a `Assert.shouldEqual` [ "fruits/left/apple", "fruits/right/apple" ]

Spec.it "does respect .gitignore even though it might conflict with a search path without base" $ \p -> do
FS.writeTextFile (Path.concat [ p, ".gitignore" ]) "fruits"
a <- Glob.gitignoringGlob p [ "**/apple" ]
a <- glob p [ "**/apple" ]
Array.sort a `Assert.shouldEqual` []
Loading