diff --git a/src/Spago/Command/Fetch.purs b/src/Spago/Command/Fetch.purs index d6ae9e732..b4c5429db 100644 --- a/src/Spago/Command/Fetch.purs +++ b/src/Spago/Command/Fetch.purs @@ -7,6 +7,7 @@ module Spago.Command.Fetch , getTransitiveDeps , getTransitiveDepsFromRegistry , getWorkspacePackageDeps + , getWorkspaceTransitiveDeps , fetchPackagesToLocalCache , run , toAllDependencies @@ -25,6 +26,7 @@ import Data.Codec.JSON as CJ import Data.Codec.JSON.Common as CJ.Common import Data.Either as Either import Data.FunctorWithIndex (mapWithIndex) +import Data.Profunctor.Strong ((&&&)) import Data.HTTP.Method as Method import Data.Int as Int import Data.List as List @@ -160,12 +162,9 @@ run { packages: packagesRequestedToInstall, ensureRanges, isTest, isRepl } = do local (_ { workspace = workspace }) do -- We compute the transitive deps for all the packages in the workspace, but keep them - -- split by package - we need all of them so we can stash them in the lockfile, but we + -- split by package. We need all of them for the lockfile's workspace entries, but we -- are going to only download the ones that we need to, if e.g. there's a package selected - dependencies <- traverse getTransitiveDeps - $ Map.fromFoldable - $ map (\p -> Tuple p.package.name p) - $ Config.getWorkspacePackages workspace.packageSet + dependencies <- getWorkspaceTransitiveDeps for_ installingPackagesData \{ configPath, yamlDoc, actualPackagesToInstall } -> do let @@ -427,17 +426,8 @@ writeNewLockfile reason allTransitiveDeps = do dependencies <- corePackageDepsOrEmpty packageName package pure $ FromPath { path, dependencies } - -- For every package that is a `WorkspacePackage`, we just pick up all - -- transitive core and test dependencies of that package from - -- `allTransitiveDeps` and stick them into `core { build_plan }` and - -- `test { build_plan }` respectively. workspacePackageLockEntries = Map.fromFoldable do - name /\ package <- Config.workspacePackageToLockfilePackage <$> Config.getWorkspacePackages workspace.packageSet - let deps = allTransitiveDeps # Map.lookup name # fromMaybe { core: Map.empty, test: Map.empty } - pure $ name /\ package - { core { build_plan = Map.keys deps.core } - , test { build_plan = Map.keys deps.test } - } + Config.workspacePackageToLockfilePackage <$> Config.getWorkspacePackages workspace.packageSet -- For every non-workspace package, we convert it to its respective type of -- lockfile entry via `packageToLockEntry`. @@ -601,6 +591,17 @@ type TransitiveDepsResult = } } +-- | Compute transitive dependencies for all workspace packages. +-- | This is used by multiple commands (fetch, uninstall) that need +-- | to gather transitive deps for the entire workspace. +getWorkspaceTransitiveDeps :: forall a. Spago (FetchEnv a) PackageTransitiveDeps +getWorkspaceTransitiveDeps = do + { workspace } <- ask + traverse getTransitiveDeps + $ Map.fromFoldable + $ map (\p -> Tuple p.package.name p) + $ Config.getWorkspacePackages workspace.packageSet + -- | For a given workspace package, returns a list of all its transitive -- | dependencies, but seperately for core and test. -- | @@ -622,27 +623,59 @@ getTransitiveDeps workspacePackage = do let depsRanges = (map (fromMaybe Config.widestRange) <<< unwrap) `onEachEnv` getWorkspacePackageDeps workspacePackage { workspace } <- ask case workspace.packageSet.lockfile of - -- If we have a lockfile we can just use that - we don't need build a plan, since we store it for every workspace - -- package, so we can just filter out the packages we need. + -- If we have a lockfile we can compute transitive deps from the lockfile data Right lockfile -> do case Map.lookup workspacePackage.package.name lockfile.workspace.packages of Nothing -> die $ "Package " <> PackageName.print workspacePackage.package.name <> " not found in lockfile" Just envs -> pure - { core: fromBuildPlan envs.core.build_plan - , test: fromBuildPlan envs.test.build_plan + { core: computeTransitiveDeps envs.core.dependencies + , test: computeTransitiveDeps envs.test.dependencies } where - fromBuildPlan bp = Map.union otherPackages workspacePackagesWeNeed + -- Note: this transitive closure logic is the same as the one that happens + -- in the Left branch, where we compute the package-set-based plan with + -- getTransitiveDepsFromPackageSet. + -- We could try to extract the overall logic from there, but we have instead + -- reimplemented it here since it's a pure traversal with no IO (since we + -- fetch dependencies as we compute the plan there), nor error handling + -- (cycles, missing packages), since all dependencies data in the lockfile + -- is pre-computed and known to be correct. + allWorkspacePackages = Map.fromFoldable $ (_.package.name &&& WorkspacePackage) <$> Config.getWorkspacePackages workspace.packageSet + + -- Get direct dependencies of a package from the lockfile + getDeps :: PackageName -> Set PackageName + getDeps name = case Map.lookup name lockfile.workspace.packages of + Just pkg -> Set.fromFoldable $ Map.keys $ unwrap pkg.core.dependencies + Nothing -> Set.fromFoldable $ case Map.lookup name lockfile.packages of + Just (FromPath { dependencies }) -> dependencies + Just (FromGit { dependencies }) -> dependencies + Just (FromRegistry { dependencies }) -> dependencies + Nothing -> [] + + transitiveClosure :: Set PackageName -> Set PackageName + transitiveClosure initial = go initial initial where - allWorkspacePackages = Map.fromFoldable $ map (\p -> Tuple p.package.name (WorkspacePackage p)) (Config.getWorkspacePackages workspace.packageSet) - - isInBuildPlan :: forall v. PackageName -> v -> Boolean - isInBuildPlan name _package = Set.member name bp - - workspacePackagesWeNeed = Map.filterWithKey isInBuildPlan allWorkspacePackages - otherPackages = map fromLockEntry $ Map.filterWithKey isInBuildPlan lockfile.packages + go seen new + | Set.isEmpty new = seen + | otherwise = + let + discovered = foldMap getDeps new `Set.difference` seen + in + go (seen <> discovered) discovered + + computeTransitiveDeps :: Dependencies -> PackageMap + computeTransitiveDeps deps = + let + transitiveDeps = transitiveClosure $ Set.fromFoldable $ Map.keys $ unwrap deps + + isInTransitiveDeps :: forall v. PackageName -> v -> Boolean + isInTransitiveDeps name _ = Set.member name transitiveDeps + in + Map.union + (Map.filterWithKey isInTransitiveDeps allWorkspacePackages) + (map fromLockEntry $ Map.filterWithKey isInTransitiveDeps lockfile.packages) -- No lockfile, we need to build a plan from scratch, and hit the Registry and so on Left _ -> case workspace.packageSet.buildType of diff --git a/src/Spago/Command/Uninstall.purs b/src/Spago/Command/Uninstall.purs index 44dc2cb42..17d4bb02f 100644 --- a/src/Spago/Command/Uninstall.purs +++ b/src/Spago/Command/Uninstall.purs @@ -83,11 +83,7 @@ run args = do where writeNewLockfile reason = do - { workspace } <- ask - dependencies <- traverse Fetch.getTransitiveDeps - $ Map.fromFoldable - $ map (\p -> Tuple p.package.name p) - $ Config.getWorkspacePackages workspace.packageSet + dependencies <- Fetch.getWorkspaceTransitiveDeps Fetch.writeNewLockfile reason dependencies toContext :: LocalPath -> Maybe (YamlDoc Core.Config) -> PackageConfig -> Spago _ (_ _) diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 52d8bcb67..96e3640bb 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -579,8 +579,8 @@ workspacePackageToLockfilePackage { path, package } = Tuple package.name { path: case Path.localPart (withForwardSlashes path) of "" -> "./" p -> p - , core: { dependencies: package.dependencies, build_plan: mempty } - , test: { dependencies: foldMap _.dependencies package.test, build_plan: mempty } + , core: { dependencies: package.dependencies } + , test: { dependencies: foldMap _.dependencies package.test } } type LockfileRecomputeResult = @@ -607,13 +607,12 @@ shouldComputeNewLockfile { workspace, workspacePackages } workspaceLock = ] } 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) + -- 1. the workspace packages should exactly match + workspacesDontMatch = (workspacePackageToLockfilePackage >>> snd <$> workspacePackages) /= 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 diff --git a/src/Spago/Lock.purs b/src/Spago/Lock.purs index 6d61ddee7..b8b55a7ba 100644 --- a/src/Spago/Lock.purs +++ b/src/Spago/Lock.purs @@ -17,7 +17,6 @@ import Spago.Prelude import Codec.JSON.DecodeError as CJ.DecodeError import Data.Codec as Codec import Data.Codec.JSON as CJ -import Data.Codec.JSON.Common as CJ.Common import Data.Codec.JSON.Strict as CJS import Data.Profunctor as Profunctor import Record as Record @@ -62,7 +61,6 @@ type WorkspaceLockPackage = type WorkspaceLockPackageEnv = { dependencies :: Core.Dependencies - , build_plan :: Set PackageName } lockfileCodec :: CJ.Codec Lockfile @@ -86,7 +84,6 @@ workspaceLockCodec = CJ.named "WorkspaceLock" $ CJS.objectStrict envCodec = CJ.named "Environment" $ CJS.objectStrict $ CJS.recordProp @"dependencies" Config.dependenciesCodec - $ CJS.recordProp @"build_plan" (CJ.Common.set PackageName.codec) $ CJS.record packageSetCodec :: CJ.Codec PackageSetInfo diff --git a/test-fixtures/build/local-package-set-lockfile/spago.lock.new b/test-fixtures/build/local-package-set-lockfile/spago.lock.new index d423bff0a..1f57ee5fb 100644 --- a/test-fixtures/build/local-package-set-lockfile/spago.lock.new +++ b/test-fixtures/build/local-package-set-lockfile/spago.lock.new @@ -8,16 +8,10 @@ "console", "effect", "prelude" - ], - "build_plan": [ - "console", - "effect", - "prelude" ] }, "test": { - "dependencies": [], - "build_plan": [] + "dependencies": [] } } }, diff --git a/test-fixtures/build/local-package-set-lockfile/spago.lock.old b/test-fixtures/build/local-package-set-lockfile/spago.lock.old index 43320b168..30b827d9e 100644 --- a/test-fixtures/build/local-package-set-lockfile/spago.lock.old +++ b/test-fixtures/build/local-package-set-lockfile/spago.lock.old @@ -8,16 +8,10 @@ "console", "effect", "prelude" - ], - "build_plan": [ - "console", - "effect", - "prelude" ] }, "test": { - "dependencies": [], - "build_plan": [] + "dependencies": [] } } }, diff --git a/test-fixtures/git-declared-deps/spago.lock b/test-fixtures/git-declared-deps/spago.lock index 0f808face..ce1a72141 100644 --- a/test-fixtures/git-declared-deps/spago.lock +++ b/test-fixtures/git-declared-deps/spago.lock @@ -6,21 +6,10 @@ "core": { "dependencies": [ "either" - ], - "build_plan": [ - "control", - "either", - "invariant", - "maybe", - "newtype", - "prelude", - "safe-coerce", - "unsafe-coerce" ] }, "test": { - "dependencies": [], - "build_plan": [] + "dependencies": [] } } }, diff --git a/test-fixtures/lock/1158-doubly-nested-projects/expected-lockfile.txt b/test-fixtures/lock/1158-doubly-nested-projects/expected-lockfile.txt index ee8e7d87e..c6e7c0f65 100644 --- a/test-fixtures/lock/1158-doubly-nested-projects/expected-lockfile.txt +++ b/test-fixtures/lock/1158-doubly-nested-projects/expected-lockfile.txt @@ -6,25 +6,19 @@ "core": { "dependencies": [ "prelude" - ], - "build_plan": [ - "prelude" ] }, "test": { - "dependencies": [], - "build_plan": [] + "dependencies": [] } }, "root": { "path": "./", "core": { - "dependencies": [], - "build_plan": [] + "dependencies": [] }, "test": { - "dependencies": [], - "build_plan": [] + "dependencies": [] } } }, diff --git a/test-fixtures/polyrepo.lock b/test-fixtures/polyrepo.lock index d0cac2d01..5c252d195 100644 --- a/test-fixtures/polyrepo.lock +++ b/test-fixtures/polyrepo.lock @@ -8,11 +8,6 @@ "package-b", "package-c", "prelude" - ], - "build_plan": [ - "package-b", - "package-c", - "prelude" ] }, "test": { @@ -20,11 +15,6 @@ "console", "effect", "prelude" - ], - "build_plan": [ - "console", - "effect", - "prelude" ] } }, @@ -34,20 +24,11 @@ "dependencies": [ "package-c", "prelude" - ], - "build_plan": [ - "package-c", - "prelude" ] }, "test": { "dependencies": [ "console" - ], - "build_plan": [ - "console", - "effect", - "prelude" ] } }, @@ -56,20 +37,12 @@ "core": { "dependencies": [ "prelude" - ], - "build_plan": [ - "prelude" ] }, "test": { "dependencies": [ "console", "effect" - ], - "build_plan": [ - "console", - "effect", - "prelude" ] } } diff --git a/test-fixtures/spago-with-maybe.lock b/test-fixtures/spago-with-maybe.lock index b95b7ea74..6f794ea91 100644 --- a/test-fixtures/spago-with-maybe.lock +++ b/test-fixtures/spago-with-maybe.lock @@ -9,22 +9,10 @@ "effect", "maybe", "prelude" - ], - "build_plan": [ - "console", - "control", - "effect", - "invariant", - "maybe", - "newtype", - "prelude", - "safe-coerce", - "unsafe-coerce" ] }, "test": { - "dependencies": [], - "build_plan": [] + "dependencies": [] } } }, diff --git a/test-fixtures/spago.lock b/test-fixtures/spago.lock index 6948b28a8..2947de95f 100644 --- a/test-fixtures/spago.lock +++ b/test-fixtures/spago.lock @@ -8,16 +8,10 @@ "console", "effect", "prelude" - ], - "build_plan": [ - "console", - "effect", - "prelude" ] }, "test": { - "dependencies": [], - "build_plan": [] + "dependencies": [] } } }, diff --git a/test/Spago/Lock.purs b/test/Spago/Lock.purs index 98a594ca6..f8e8e0c31 100644 --- a/test/Spago/Lock.purs +++ b/test/Spago/Lock.purs @@ -4,7 +4,6 @@ import Test.Prelude import Codec.JSON.DecodeError as CJ.DecodeError import Data.Map as Map -import Data.Set as Set import Registry.Range as Range import Registry.Sha256 as Sha256 import Registry.Version as Version @@ -44,26 +43,18 @@ validLockfile = [ packageTuple "effect" (Just (unsafeFromRight (Range.parse ">=1.0.0 <5.0.0"))) , packageTuple "my-library" Nothing ] - , build_plan: Set.fromFoldable - [ mkPackageName "my-library" - , mkPackageName "effect" - , mkPackageName "prelude" - ] } , test: { dependencies: Dependencies Map.empty - , build_plan: Set.empty } , path: "my-app" } , packageTuple "my-library" { core: { dependencies: Dependencies $ Map.fromFoldable [ packageTuple "prelude" Nothing ] - , build_plan: Set.fromFoldable [ mkPackageName "prelude" ] } , test: { dependencies: Dependencies $ Map.fromFoldable [ packageTuple "console" (Just Config.widestRange) ] - , build_plan: Set.fromFoldable [ mkPackageName "console" ] } , path: "my-library" } @@ -133,11 +124,6 @@ validLockfileString = "my-app": { "path": "my-app", "core": { - "build_plan": [ - "effect", - "my-library", - "prelude" - ], "dependencies": [ { "effect": ">=1.0.0 <5.0.0" @@ -146,16 +132,12 @@ validLockfileString = ] }, "test": { - "dependencies": [], - "build_plan": [] + "dependencies": [] } }, "my-library": { "path": "my-library", "core": { - "build_plan": [ - "prelude" - ], "dependencies": [ "prelude" ] @@ -165,9 +147,6 @@ validLockfileString = { "console": "*" } - ], - "build_plan": [ - "console" ] } }