From b0187a791b75dc8678f32ac2c0230ded250096ee Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Wed, 18 Dec 2024 19:58:10 -0500 Subject: [PATCH 01/25] Run Spago from a nested directory other than workspace root --- CHANGELOG.md | 2 + bin/src/Main.purs | 52 ++- core/src/Log.purs | 14 +- core/src/Prelude.purs | 3 +- src/Spago/Command/Ls.purs | 12 +- src/Spago/Config.purs | 405 ++++++++++-------- src/Spago/Prelude.purs | 3 +- test-fixtures/config/discovery/a/spago.yaml | 4 + test-fixtures/config/discovery/b/spago.yaml | 4 + test-fixtures/config/discovery/from-a.txt | 12 + test-fixtures/config/discovery/from-d.txt | 12 + .../config/discovery/from-nested.txt | 17 + test-fixtures/config/discovery/from-root.txt | 17 + .../discovery/nested-workspace/c/spago.yaml | 4 + .../discovery/nested-workspace/d/spago.yaml | 4 + .../discovery/nested-workspace/spago.yaml | 8 + test-fixtures/config/discovery/spago.yaml | 10 + .../config/misnamed-configs/a/b/c/spago.yml | 0 .../config/misnamed-configs/a/b/d/.keep | 0 .../config/misnamed-configs/a/b/spago.yml | 0 .../config/misnamed-configs/from-a.txt | 10 + .../config/misnamed-configs/from-b.txt | 12 + .../config/misnamed-configs/from-c.txt | 13 + .../config/misnamed-configs/from-d.txt | 12 + .../config/misnamed-configs/from-root.txt | 10 + .../config/misnamed-configs/spago.yml | 0 .../config/no-workspace-anywhere.txt | 3 + test-fixtures/spago-yml-check-stderr.txt | 6 +- test/Spago/Config.purs | 63 ++- 29 files changed, 490 insertions(+), 222 deletions(-) create mode 100644 test-fixtures/config/discovery/a/spago.yaml create mode 100644 test-fixtures/config/discovery/b/spago.yaml create mode 100644 test-fixtures/config/discovery/from-a.txt create mode 100644 test-fixtures/config/discovery/from-d.txt create mode 100644 test-fixtures/config/discovery/from-nested.txt create mode 100644 test-fixtures/config/discovery/from-root.txt create mode 100644 test-fixtures/config/discovery/nested-workspace/c/spago.yaml create mode 100644 test-fixtures/config/discovery/nested-workspace/d/spago.yaml create mode 100644 test-fixtures/config/discovery/nested-workspace/spago.yaml create mode 100644 test-fixtures/config/discovery/spago.yaml create mode 100644 test-fixtures/config/misnamed-configs/a/b/c/spago.yml create mode 100644 test-fixtures/config/misnamed-configs/a/b/d/.keep create mode 100644 test-fixtures/config/misnamed-configs/a/b/spago.yml create mode 100644 test-fixtures/config/misnamed-configs/from-a.txt create mode 100644 test-fixtures/config/misnamed-configs/from-b.txt create mode 100644 test-fixtures/config/misnamed-configs/from-c.txt create mode 100644 test-fixtures/config/misnamed-configs/from-d.txt create mode 100644 test-fixtures/config/misnamed-configs/from-root.txt create mode 100644 test-fixtures/config/misnamed-configs/spago.yml create mode 100644 test-fixtures/config/no-workspace-anywhere.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 82e513f55..363aa3eb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ Other improvements: - When the `publish.location` field is missing, `spago publish` will attempt to figure out the location from Git remotes and write it back to `spago.yaml`. - Internally Spago uses stricter-typed file paths. +- Spago can now be launched from a directory nested within the workspace, not + just from workspace root. ## [0.21.0] - 2023-05-04 diff --git a/bin/src/Main.purs b/bin/src/Main.purs index 4056387d3..b2e5312f4 100644 --- a/bin/src/Main.purs +++ b/bin/src/Main.purs @@ -536,8 +536,7 @@ main = do \c -> Aff.launchAff_ case c of Cmd'SpagoCmd (SpagoCmd globalArgs@{ offline, migrateConfig } command) -> do logOptions <- mkLogOptions startingTime globalArgs - rootPath <- Path.mkRoot =<< Paths.cwd - runSpago { logOptions, rootPath } case command of + runSpago { logOptions } case command of Sources args -> do { env } <- mkFetchEnv { packages: mempty @@ -552,6 +551,7 @@ main = do void $ runSpago env (Sources.run { json: args.json }) Init args@{ useSolver } -> do -- Fetch the registry here so we can select the right package set later + rootPath <- Path.mkRoot =<< Paths.cwd env <- mkRegistryEnv offline <#> Record.union { rootPath } setVersion <- parseSetVersion args.setVersion void $ runSpago env $ Init.run { mode: args.mode, setVersion, useSolver } @@ -599,7 +599,8 @@ main = do void $ runSpago publishEnv (Publish.publish {}) Repl args@{ selectedPackage } -> do - packages <- FS.exists (rootPath "spago.yaml") >>= case _ of + cwd <- Paths.cwd + packages <- FS.exists (cwd "spago.yaml") >>= case _ of true -> do -- if we have a config then we assume it's a workspace, and we can run a repl in the project pure mempty -- TODO newPackages @@ -661,13 +662,14 @@ main = do testEnv <- runSpago env (mkTestEnv args buildEnv) runSpago testEnv Test.run LsPaths args -> do - runSpago { logOptions, rootPath } $ Ls.listPaths args + let fetchArgs = { packages: mempty, selectedPackage: Nothing, pure: false, ensureRanges: false, testDeps: false, isRepl: false, migrateConfig, offline } + { env } <- mkFetchEnv fetchArgs + runSpago env $ Ls.listPaths args LsPackages args@{ pure } -> do let fetchArgs = { packages: mempty, selectedPackage: Nothing, pure, ensureRanges: false, testDeps: false, isRepl: false, migrateConfig, offline } - { env: env@{ workspace }, fetchOpts } <- mkFetchEnv fetchArgs + { env, fetchOpts } <- mkFetchEnv fetchArgs dependencies <- runSpago env (Fetch.run fetchOpts) - let lsEnv = { workspace, dependencies, logOptions, rootPath } - runSpago lsEnv (Ls.listPackageSet args) + runSpago (Record.union env { dependencies }) (Ls.listPackageSet args) LsDeps { selectedPackage, json, transitive, pure } -> do let fetchArgs = { packages: mempty, selectedPackage, pure, ensureRanges: false, testDeps: false, isRepl: false, migrateConfig, offline } { env, fetchOpts } <- mkFetchEnv fetchArgs @@ -690,13 +692,11 @@ main = do GraphModules args -> do { env, fetchOpts } <- mkFetchEnv { packages: mempty, selectedPackage: Nothing, pure: false, ensureRanges: false, testDeps: false, isRepl: false, migrateConfig, offline } dependencies <- runSpago env (Fetch.run fetchOpts) - purs <- Purs.getPurs - runSpago { dependencies, logOptions, rootPath, purs, workspace: env.workspace } (Graph.graphModules args) + runSpago (Record.union env { dependencies }) (Graph.graphModules args) GraphPackages args -> do { env, fetchOpts } <- mkFetchEnv { packages: mempty, selectedPackage: Nothing, pure: false, ensureRanges: false, testDeps: false, isRepl: false, migrateConfig, offline } dependencies <- runSpago env (Fetch.run fetchOpts) - purs <- Purs.getPurs - runSpago { dependencies, logOptions, rootPath, purs, workspace: env.workspace } (Graph.graphPackages args) + runSpago (Record.union env { dependencies }) (Graph.graphPackages args) Cmd'VersionCmd v -> when v do output (OutputLines [ BuildInfo.packages."spago-bin" ]) @@ -951,7 +951,13 @@ mkReplEnv replArgs dependencies supportPackage = do , selected } -mkFetchEnv :: forall a b. { offline :: OnlineStatus, migrateConfig :: Boolean, isRepl :: Boolean | FetchArgsRow b } -> Spago (SpagoBaseEnv a) { env :: Fetch.FetchEnv (), fetchOpts :: Fetch.FetchOpts } +mkFetchEnv :: forall a b. + { offline :: OnlineStatus + , migrateConfig :: Boolean + , isRepl :: Boolean + | FetchArgsRow b + } + -> Spago { logOptions :: LogOptions | a } { env :: Fetch.FetchEnv (), fetchOpts :: Fetch.FetchOpts } mkFetchEnv args@{ migrateConfig, offline } = do let parsePackageName p = @@ -966,24 +972,26 @@ mkFetchEnv args@{ migrateConfig, offline } = do Left _err -> die $ "Failed to parse selected package name, was: " <> show args.selectedPackage env <- mkRegistryEnv offline - { rootPath } <- ask - workspace <- - runSpago (Record.union env { rootPath }) - (Config.readWorkspace { maybeSelectedPackage, pureBuild: args.pure, migrateConfig }) + cwd <- Paths.cwd + { workspace, rootPath } <- + runSpago env + (Config.discoverWorkspace { maybeSelectedPackage, pureBuild: args.pure, migrateConfig } cwd) + + FS.mkdirp $ rootPath Paths.localCachePath + FS.mkdirp $ rootPath Paths.localCachePackagesPath + logDebug $ "Workspace root path: " <> Path.quote rootPath + logDebug $ "Local cache: " <> Paths.localCachePath + let fetchOpts = { packages: packageNames, ensureRanges: args.ensureRanges, isTest: args.testDeps, isRepl: args.isRepl } pure { fetchOpts, env: Record.union { workspace, rootPath } env } mkRegistryEnv :: forall a. OnlineStatus -> Spago (SpagoBaseEnv a) (Registry.RegistryEnv ()) mkRegistryEnv offline = do - { logOptions, rootPath } <- ask + { logOptions } <- ask -- Take care of the caches FS.mkdirp Paths.globalCachePath - FS.mkdirp $ rootPath Paths.localCachePath - FS.mkdirp $ rootPath Paths.localCachePackagesPath - logDebug $ "Workspace root path: " <> Path.quote rootPath logDebug $ "Global cache: " <> Path.quote Paths.globalCachePath - logDebug $ "Local cache: " <> Paths.localCachePath -- Make sure we have git and purs git <- Git.getGit @@ -1004,7 +1012,7 @@ mkRegistryEnv offline = do , db } -mkLsEnv :: forall a. Fetch.PackageTransitiveDeps -> Spago (Fetch.FetchEnv a) Ls.LsEnv +mkLsEnv :: ∀ a. Fetch.PackageTransitiveDeps -> Spago (Fetch.FetchEnv a) (Ls.LsEnv ()) mkLsEnv dependencies = do { logOptions, workspace, rootPath } <- ask selected <- case workspace.selected of diff --git a/core/src/Log.purs b/core/src/Log.purs index ea7eb4694..046fa6b26 100644 --- a/core/src/Log.purs +++ b/core/src/Log.purs @@ -22,7 +22,6 @@ module Spago.Log , rightOrDie , rightOrDie_ , rightOrDieWith - , rightOrDieWith' , toDoc ) where @@ -183,25 +182,18 @@ justOrDieWith' value msg = case value of die' msg rightOrDie :: ∀ b m err x. MonadEffect m => MonadAsk (LogEnv b) m => Loggable err => Either err x -> m x -rightOrDie value = rightOrDieWith value identity +rightOrDie = rightOrDieWith identity rightOrDie_ :: ∀ b m err x. MonadEffect m => MonadAsk (LogEnv b) m => Loggable err => Either err x -> m Unit rightOrDie_ = void <<< rightOrDie -rightOrDieWith :: ∀ a b m err x. MonadEffect m => MonadAsk (LogEnv b) m => Loggable a => Either err x -> (err -> a) -> m x -rightOrDieWith value toMsg = case value of +rightOrDieWith :: ∀ a b m err x. MonadEffect m => MonadAsk (LogEnv b) m => Loggable a => (err -> a) -> Either err x -> m x +rightOrDieWith toMsg value = case value of Right a -> pure a Left err -> die $ toMsg err -rightOrDieWith' :: ∀ a b m err x. MonadEffect m => MonadAsk (LogEnv b) m => Loggable a => Either err x -> (err -> Array a) -> m x -rightOrDieWith' value toMsg = case value of - Right a -> - pure a - Left err -> - die' $ toMsg err - data OutputFormat a = OutputJson (CJ.Codec a) a | OutputYaml (CJ.Codec a) a diff --git a/core/src/Prelude.purs b/core/src/Prelude.purs index 585724c71..f6ca91459 100644 --- a/core/src/Prelude.purs +++ b/core/src/Prelude.purs @@ -21,6 +21,7 @@ import Data.DateTime.Instant (Instant) as Extra import Data.Either (Either(..), isLeft, isRight, either, hush) as Extra import Data.Filterable (partition, partitionMap) as Extra import Data.Foldable (foldMap, for_, foldl, and, or) as Extra +import Data.FoldableWithIndex (forWithIndex_) as Extra import Data.Function (on) as Extra import Data.Generic.Rep (class Generic) as Extra import Data.Identity (Identity(..)) as Extra @@ -47,7 +48,7 @@ import Partial.Unsafe (unsafeCrashWith) import Registry.ManifestIndex (ManifestIndex) as Extra import Registry.Types (PackageName, Version, Range, Location, License, Manifest(..), Metadata(..), Sha256) as Extra import Spago.Json (printJson, parseJson) as Extra -import Spago.Log (logDebug, logError, logInfo, Docc, logSuccess, logWarn, die, die', justOrDieWith, justOrDieWith', rightOrDie, rightOrDie_, rightOrDieWith, rightOrDieWith', toDoc, indent, indent2, output, LogEnv, LogOptions, OutputFormat(..)) as Extra +import Spago.Log (logDebug, logError, logInfo, Docc, logSuccess, logWarn, die, die', justOrDieWith, justOrDieWith', rightOrDie, rightOrDie_, rightOrDieWith, toDoc, indent, indent2, output, LogEnv, LogOptions, OutputFormat(..)) as Extra import Spago.Path (RawFilePath, GlobalPath, LocalPath, RootPath, class AppendPath, appendPath, ()) as Extra import Spago.Yaml (YamlDoc, printYaml, parseYaml) as Extra diff --git a/src/Spago/Command/Ls.purs b/src/Spago/Command/Ls.purs index 8f0b15056..3a01144ff 100644 --- a/src/Spago/Command/Ls.purs +++ b/src/Spago/Command/Ls.purs @@ -50,22 +50,24 @@ type LsPathsArgs = { json :: Boolean } -type LsSetEnv = +type LsSetEnv r = { dependencies :: Fetch.PackageTransitiveDeps , logOptions :: LogOptions , workspace :: Workspace , rootPath :: RootPath + | r } -type LsEnv = +type LsEnv r = { dependencies :: Fetch.PackageTransitiveDeps , logOptions :: LogOptions , workspace :: Workspace , selected :: WorkspacePackage , rootPath :: RootPath + | r } -listPaths :: LsPathsArgs -> Spago { logOptions :: LogOptions, rootPath :: RootPath } Unit +listPaths :: ∀ r. LsPathsArgs -> Spago { logOptions :: LogOptions, rootPath :: RootPath | r } Unit listPaths { json } = do logDebug "Running `listPaths`" { rootPath } <- ask @@ -90,7 +92,7 @@ listPaths { json } = do -- TODO: add LICENSE field -listPackageSet :: LsPackagesArgs -> Spago LsSetEnv Unit +listPackageSet :: ∀ r. LsPackagesArgs -> Spago (LsSetEnv r) Unit listPackageSet { json } = do logDebug "Running `listPackageSet`" { workspace, rootPath } <- ask @@ -102,7 +104,7 @@ listPackageSet { json } = do true -> formatPackagesJson rootPath packages false -> formatPackagesTable packages -listPackages :: LsDepsOpts -> Spago LsEnv Unit +listPackages :: ∀ r. LsDepsOpts -> Spago (LsEnv r) Unit listPackages { transitive, json } = do logDebug "Running `listPackages`" { dependencies, selected, rootPath } <- ask diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 5c2885f7b..7bacba45d 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -19,9 +19,8 @@ module Spago.Config , isRootPackage , module Core , readConfig - , readWorkspace + , discoverWorkspace , removePackagesFromConfig - , rootPackageToWorkspacePackage , setPackageSetVersionInConfig , sourceGlob , workspacePackageToLockfilePackage @@ -33,12 +32,14 @@ import Affjax.Node as Http import Affjax.ResponseFormat as Response import Affjax.StatusCode (StatusCode(..)) import Codec.JSON.DecodeError as CJ.DecodeError +import Control.Monad.State as State import Data.Array as Array import Data.Array.NonEmpty as NEA import Data.CodePoint.Unicode as Unicode import Data.Codec.JSON as CJ import Data.Codec.JSON.Record as CJ.Record import Data.Enum as Enum +import Data.Foldable (traverse_) import Data.Graph as Graph import Data.HTTP.Method as Method import Data.Int as Int @@ -174,119 +175,192 @@ type ReadWorkspaceOptions = isRootPackage :: WorkspacePackage -> Boolean isRootPackage p = Path.localPart p.path == "" --- | Reads all the configurations in the tree and builds up the Map of local --- | packages to be integrated in the package set -readWorkspace :: ∀ a. ReadWorkspaceOptions -> Spago (Registry.RegistryEnv (rootPath :: RootPath | a)) Workspace -readWorkspace { maybeSelectedPackage, pureBuild, migrateConfig } = do - { rootPath } <- ask +spagoYaml = "spago.yaml" :: String + +discoverWorkspace :: ∀ a. ReadWorkspaceOptions -> GlobalPath -> Spago (Registry.RegistryEnv a) { workspace :: Workspace, rootPath :: RootPath } +discoverWorkspace options cwd = do logInfo "Reading Spago workspace configuration..." + logDebug $ "Discovering nearest workspace " <> spagoYaml <> " starting at " <> Path.quote cwd + + { workspace, rootPath } /\ { loadedPackages, closestPackage } <- + State.runStateT (walkDirectoriesUpFrom cwd) + { loadedPackages: Map.empty, otherWorkspaceRoots: [], misnamedConfigs: [], closestPackage: Nothing } + + migrateConfigsWhereNeeded rootPath loadedPackages + + packagesByName <- + Map.fromFoldable <$> + for (Map.toUnfoldable loadedPackages :: Array _) \(path /\ { package, config }) -> do + hasTests <- FS.exists (path "test") + let wsp :: WorkspacePackage + wsp = { package, path: path `Path.relativeTo` rootPath, doc: Just config.doc, hasTests } + pure (package.name /\ wsp) + + selected <- + determineSelectedPackage + { explicitlySelected: options.maybeSelectedPackage + , inferredFromCwd: closestPackage + , rootPackage: workspace.rootPackage <#> _.name + , loadedPackages: packagesByName + } - let - doMigrateConfig :: ∀ path. Path.IsPath path => path -> _ -> Spago (Registry.RegistryEnv _) Unit - doMigrateConfig path config = do - case migrateConfig, config.wasMigrated of - true, true -> do - logInfo $ "Migrating your " <> Path.quote path <> " to the latest version..." - liftAff $ FS.writeYamlDocFile path config.doc - false, true -> logWarn $ "Your " <> Path.quote path <> " is using an outdated format. Run Spago with the --migrate flag to update it to the latest version." - _, false -> pure unit - - rootConfigPath = rootPath "spago.yaml" - - -- First try to read the config in the root. It _has_ to contain a workspace - -- configuration, or we fail early. - { workspace, package: maybePackage, workspaceDoc } <- readConfig rootConfigPath >>= case _ of - Left errLines -> - die - [ toDoc "Couldn't parse Spago config, error:" - , Log.break - , indent $ toDoc errLines - , Log.break - , toDoc "The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file" - ] - Right { yaml: { workspace: Nothing } } -> die - [ "Your spago.yaml doesn't contain a workspace section." - , "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 (rootPath "spago.yaml") config - pure { workspace, package, workspaceDoc: doc } - - logDebug "Gathering all the spago configs in the tree..." - otherConfigPaths <- liftAff $ Array.delete rootConfigPath <$> Glob.gitignoringGlob - { root: rootPath - , includePatterns: [ "**/spago.yaml" ] - , ignorePatterns: [ "**/node_modules/**", "**/.spago/**" ] + lockfile <- + loadLockfile { pureBuild: options.pureBuild, workspaceConfig: workspace.config, loadedPackages: packagesByName, rootPath } + + { packageSet, compiler } <- + loadPackageSet { workspaceConfig: workspace.config, loadedPackages: packagesByName, rootPath, lockfile } + + pure + { rootPath + , workspace: + { selected + , packageSet + , compatibleCompiler: compiler + , backend: workspace.config.backend + , buildOptions: + { output: workspace.config.buildOpts >>= _.output <#> \o -> withForwardSlashes $ rootPath o + , censorLibWarnings: _.censorLibraryWarnings =<< workspace.config.buildOpts + , statVerbosity: _.statVerbosity =<< workspace.config.buildOpts + } + , doc: Just workspace.doc + , workspaceConfig: workspace.config + , rootPackage: workspace.rootPackage + } } + where + readConfig' = State.lift <<< readConfig + + walkDirectoriesUpFrom dir = do + maybeConfig <- tryReadConfigAt configFile + + for_ maybeConfig \config -> + for_ config.yaml.package \package -> + -- If there is a package in this directory, remember it + State.modify_ \s -> s + { loadedPackages = Map.insert dir { package, config } s.loadedPackages + , closestPackage = s.closestPackage <|> Just package.name + } + + whenM (FS.exists $ dir "spago.yml") $ + State.modify_ \s -> s { misnamedConfigs = Array.cons dir s.misnamedConfigs } - unless (Array.null otherConfigPaths) do - logDebug $ [ toDoc "Found packages at these paths:", Log.indent $ Log.lines (map (toDoc <<< Path.quote) otherConfigPaths) ] + case maybeConfig of + Just { doc, yaml: { workspace: Just workspace, package } } -> do + -- Finally, found the "workspace" config! + rootPath <- Path.mkRoot dir + loadSubprojectConfigs rootPath + pure { workspace: { config: workspace, doc, rootPackage: package }, rootPath } + _ -> do + -- No workspace in this directory => recur to parent directory (unless it's already root) + when (parentDir == dir) $ + dieForLackOfSpagoYaml + walkDirectoriesUpFrom parentDir + + where + configFile = dir spagoYaml + parentDir = Path.dirname dir + + loadSubprojectConfigs rootPath = do + candidates <- liftAff $ Glob.gitignoringGlob + { root: rootPath + , includePatterns: [ "**/" <> spagoYaml ] + , ignorePatterns: [ "**/node_modules/**", "**/.spago/**" ] + } + + -- Traversing directories (not files) and doing it in sorted order ensures + -- that parent directories come before their subdirectories. That way we + -- can remember workspaces that we find along the way and avoid trying to + -- load their subprojects that come later. + candidates <#> Path.toGlobal <#> Path.dirname # Array.sort # traverse_ \dir -> do + st <- State.get + let configFile = dir spagoYaml + alreadyLoaded = st.loadedPackages # Map.member configFile + anotherParentWorkspace = st.otherWorkspaceRoots # Array.find (_ `Path.isPrefixOf` dir) + case alreadyLoaded, anotherParentWorkspace of + true, _ -> + pure unit + _, Just ws -> do + logDebug $ "Not trying to load " <> Path.quote configFile <> " because it belongs to a different workspace at " <> Path.quote ws + pure unit + false, Nothing -> + readConfig' configFile >>= case _ of + Left _ -> + logWarn $ "Failed to read config at " <> Path.quote configFile + Right { yaml: { workspace: Just _ } } -> + State.modify_ \s -> s { otherWorkspaceRoots = Array.cons dir s.otherWorkspaceRoots } + Right config@{ yaml: { package: Just package } } -> do + logDebug $ "Loaded a subproject config at " <> Path.quote configFile + State.modify_ \s -> s { loadedPackages = Map.insert dir { package, config } s.loadedPackages } + Right _ -> do + logWarn $ "Neither workspace nor package found in " <> Path.quote configFile + + tryReadConfigAt path = do + exists <- FS.exists path + if exists then + Just <$> do + logDebug $ "Loading spago.yaml at " <> Path.quote path + readConfig' path >>= rightOrDieWith \errLines -> + [ toDoc $ "Couldn't parse Spago config file at: " <> Path.quote path + , indent $ toDoc errLines + , Log.break + , toDoc "The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file" + ] + else + pure Nothing + + migrateConfigsWhereNeeded rootPath loadedConfigs = do + forWithIndex_ loadedConfigs \path' { config } -> do + let path = (path' spagoYaml) `Path.relativeTo` rootPath + case options.migrateConfig, config.wasMigrated of + true, true -> do + logInfo $ "Migrating your " <> Path.quote path <> " to the latest version..." + liftAff $ FS.writeYamlDocFile path config.doc + false, true -> + logWarn $ "Your " <> Path.quote path <> " is using an outdated format. Run Spago with the --migrate flag to update it to the latest version." + _, false -> + pure unit + + dieForLackOfSpagoYaml = do + root <- Path.mkRoot cwd + misnamedConfigs <- State.gets _.misnamedConfigs + let misnamedConfigsList = + case misnamedConfigs <#> \c -> Path.quote $ (c "spago.yml") `Path.relativeTo` root of + [] -> [] + [one] -> [ toDoc $ "Instead found " <> one ] + many -> [ toDoc "Instead found these:" , indent $ toDoc many ] + die + [ toDoc $ "No " <> spagoYaml <> " found in the current directory or any of its parents." + , if Array.null misnamedConfigsList then + toDoc "" + else + toDoc + [ Log.break + , indent $ toDoc $ misnamedConfigsList + , indent $ toDoc $ "Note that Spago config files should be named " <> spagoYaml <> ", not spago.yml." + , Log.break + , toDoc "The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file" + ] + ] - -- We read all of them in, and only read the package section, if any. +determineSelectedPackage :: ∀ a. + { explicitlySelected :: Maybe PackageName + , inferredFromCwd :: Maybe PackageName + , rootPackage :: Maybe PackageName + , loadedPackages :: Map PackageName WorkspacePackage + } + -> Spago (Registry.RegistryEnv a) (Maybe WorkspacePackage) +determineSelectedPackage { explicitlySelected, inferredFromCwd, rootPackage, loadedPackages } = do let - readWorkspaceConfig :: LocalPath -> Spago (Registry.RegistryEnv _) (Either Docc ReadWorkspaceConfigResult) - readWorkspaceConfig path = do - maybeConfig <- readConfig path - -- We try to figure out if this package has tests - look for test sources - hasTests <- FS.exists (Path.dirname path "test") - pure $ case maybeConfig of - Left eLines -> Left $ toDoc - [ toDoc $ "Could not read config at path " <> Path.quote path - , toDoc "Error was: " - , indent $ toDoc eLines - ] - Right config -> Right - { config - , hasTests - , configPath: path - , packagePath: Path.dirname path `Path.relativeTo` rootPath - } - - { right: otherPackages, left: failedPackages } <- partitionMap identity <$> traverse readWorkspaceConfig otherConfigPaths - unless (Array.null failedPackages) do - logWarn $ [ toDoc "Failed to read some configs:" ] <> failedPackages - - -- We prune any configs that use a different workspace. - -- For reasoning, see https://github.com/purescript/spago/issues/951 - let configPathsWithWorkspaces = otherPackages # Array.mapMaybe \readResult -> readResult.packagePath <$ readResult.config.yaml.workspace - unless (Array.null configPathsWithWorkspaces) do - logDebug $ "Found these paths with workspaces: " <> String.joinWith ", " (Path.quote <$> configPathsWithWorkspaces) - - { right: configsNoWorkspaces, left: prunedConfigs } <- - let - fn { left, right } readResult@{ configPath, packagePath, hasTests, config } = do - if Array.any (_ `Path.isPrefixOf` packagePath) configPathsWithWorkspaces then - pure { right, left: Array.cons packagePath left } - else - case readResult.config.yaml.package of - Nothing -> - pure { right, left: Array.cons packagePath left } - Just package -> do - -- Note: we migrate configs only at this point - this is because we read a whole lot of them but we are - -- supposed to ignore any subtrees that contain a different workspace, and those we don't want to migrate - doMigrateConfig configPath config - -- We store the path of the package, so we can treat it basically as a LocalPackage - pure { left, right: Array.cons (Tuple package.name { package, hasTests, path: packagePath, doc: Just config.doc }) right } - in - Array.foldM fn { right: [], left: [] } otherPackages - - unless (Array.null prunedConfigs) do - logDebug - $ [ "Excluding configs that use a different workspace (directly or implicitly via parent directory's config):" ] - <> Array.sort (Path.quote <$> prunedConfigs) - - rootPackage <- case maybePackage of - Nothing -> pure [] - Just rootPackage -> do - rootPackage' <- rootPackageToWorkspacePackage rootPath { rootPackage, workspaceDoc } - pure [ Tuple rootPackage.name rootPackage' ] - - let workspacePackages = Map.fromFoldable $ configsNoWorkspaces <> rootPackage + inferredFromCwd' = + -- Do not auto-select the root package if Spago is being run from the root directory. + if inferredFromCwd == rootPackage then Nothing else inferredFromCwd + + selectedName = + explicitlySelected <|> inferredFromCwd' -- Select the package for spago to handle during the rest of the execution - maybeSelected <- case maybeSelectedPackage of - Nothing -> case Array.uncons (Map.toUnfoldable workspacePackages) of + maybeSelected <- case selectedName of + Nothing -> case Array.uncons (Map.toUnfoldable loadedPackages) of Nothing -> die "No valid packages found in the current project, halting." -- If there's only one package and it's not in the root we still select that Just { head: (Tuple packageName package), tail: [] } -> do @@ -294,10 +368,10 @@ readWorkspace { maybeSelectedPackage, pureBuild, migrateConfig } = do pure (Just package) -- If no package has been selected and we have many packages, then we build all of them but select none _ -> pure Nothing - Just name -> case Map.lookup name workspacePackages of + Just name -> case Map.lookup name loadedPackages of Nothing -> die $ [ toDoc $ "Selected package " <> PackageName.print name <> " was not found in the local packages." ] - <> case (Array.fromFoldable $ Map.keys workspacePackages) of + <> case (Array.fromFoldable $ Map.keys loadedPackages) of [] -> [ toDoc "No available packages." ] pkgs -> @@ -307,9 +381,30 @@ readWorkspace { maybeSelectedPackage, pureBuild, migrateConfig } = do Just p -> pure (Just p) + -- Figure out if we are selecting a single package or not + case maybeSelected of + Just selected -> do + logSuccess $ "Selecting package to build: " <> PackageName.print selected.package.name + logDebug $ "Package path: " <> Path.quote selected.path + Nothing -> do + logSuccess + [ toDoc $ "Selecting " <> show (Map.size loadedPackages) <> " packages to build:" + , indent2 (toDoc (Set.toUnfoldable $ Map.keys loadedPackages :: Array PackageName)) + ] + + pure maybeSelected + +loadLockfile :: ∀ a. + { pureBuild :: Boolean + , workspaceConfig :: Core.WorkspaceConfig + , loadedPackages :: Map PackageName WorkspacePackage + , rootPath :: RootPath + } + -> Spago (Registry.RegistryEnv a) (Either String Lockfile) +loadLockfile { pureBuild, workspaceConfig, loadedPackages, rootPath } = do logDebug "Parsing the lockfile..." let lockFilePath = rootPath "spago.lock" - maybeLockfileContents <- FS.exists lockFilePath >>= case _ of + FS.exists lockFilePath >>= case _ of false -> pure (Left "No lockfile found") true -> liftAff (FS.readJsonFile Lock.lockfileCodec lockFilePath) >>= case _ of Left error -> do @@ -323,7 +418,7 @@ readWorkspace { maybeSelectedPackage, pureBuild, migrateConfig } = do -- Unless! the user is passing the --pure flag, in which case we just use the lockfile Right contents -> do logDebug "Parsed the lockfile" - case pureBuild, shouldComputeNewLockfile { workspace, workspacePackages } contents.workspace of + case pureBuild, shouldComputeNewLockfile { workspace: workspaceConfig, workspacePackages: loadedPackages } contents.workspace of true, _ -> do logDebug "Using lockfile because of --pure flag" pure (Right contents) @@ -334,18 +429,25 @@ readWorkspace { maybeSelectedPackage, pureBuild, migrateConfig } = do logDebug "Lockfile is up to date, using it" pure (Right contents) - -- Read in the package database +loadPackageSet :: ∀ a. + { workspaceConfig :: Core.WorkspaceConfig + , loadedPackages :: Map PackageName WorkspacePackage + , rootPath :: RootPath + , lockfile :: Either String Lockfile + } + -> Spago (Registry.RegistryEnv a) { packageSet :: PackageSet, compiler :: Range } +loadPackageSet { lockfile, workspaceConfig, loadedPackages, rootPath } = do { offline } <- ask - packageSetInfo <- case maybeLockfileContents, workspace.packageSet of + packageSetInfo <- case lockfile, workspaceConfig.packageSet of _, Nothing -> do logDebug "Did not find a package set in your config, using Registry solver" pure Nothing -- If there's a lockfile we don't attempt to fetch the package set from the registry -- repo nor from the internet, since we already have the whole set right there - Right lockfile, _ -> do + Right lf, _ -> do logDebug "Found the lockfile, using the package set from there" - pure lockfile.workspace.package_set + pure lf.workspace.package_set Left reason, Just address@(Core.SetFromRegistry { registry: v }) -> do logDebug reason @@ -416,16 +518,12 @@ readWorkspace { maybeSelectedPackage, pureBuild, migrateConfig } = do -- This is to (1) easily allow overriding packages, (2) easily allow "private registries" -- and (3) prevent the security hole where people can register new names and take precedence in your build. let - extraPackages = map fromExtraPackage (fromMaybe Map.empty workspace.extraPackages) - localPackagesOverlap = Set.intersection (Map.keys workspacePackages) (Map.keys extraPackages) - buildType = - let - localPackages = Map.union (map WorkspacePackage workspacePackages) extraPackages - in - case packageSetInfo of - Nothing -> RegistrySolverBuild localPackages - Just info -> PackageSetBuild info $ Map.union localPackages (map fromRemotePackage info.content) - packageSet = { buildType, lockfile: maybeLockfileContents } + extraPackages = map fromExtraPackage (fromMaybe Map.empty workspaceConfig.extraPackages) + localPackagesOverlap = Set.intersection (Map.keys loadedPackages) (Map.keys extraPackages) + localPackages = Map.union (map WorkspacePackage loadedPackages) extraPackages + buildType = case packageSetInfo of + Nothing -> RegistrySolverBuild localPackages + Just info -> PackageSetBuild info $ Map.union localPackages (map fromRemotePackage info.content) -- Note again: we only try to prevent collisions between workspace packages and local overrides. -- We otherwise want local packages to override _remote_ ones, e.g. in the case where you are @@ -438,46 +536,11 @@ readWorkspace { maybeSelectedPackage, pureBuild, migrateConfig } = do ] <> map (\p -> indent $ toDoc $ "- " <> PackageName.print p) (Array.fromFoldable localPackagesOverlap) - -- Figure out if we are selecting a single package or not - case maybeSelected of - Just selected -> do - logSuccess $ "Selecting package to build: " <> PackageName.print selected.package.name - logDebug $ "Package path: " <> Path.quote selected.path - Nothing -> do - logSuccess - [ toDoc $ "Selecting " <> show (Map.size workspacePackages) <> " packages to build:" - , indent2 (toDoc (Set.toUnfoldable $ Map.keys workspacePackages :: Array PackageName)) - ] - - let - buildOptions :: WorkspaceBuildOptions - buildOptions = - { output: workspace.buildOpts >>= _.output <#> \o -> withForwardSlashes $ rootPath o - , censorLibWarnings: _.censorLibraryWarnings =<< workspace.buildOpts - , statVerbosity: _.statVerbosity =<< workspace.buildOpts - } - pure - { selected: maybeSelected - , packageSet - , compatibleCompiler: fromMaybe Core.widestRange $ map _.compiler packageSetInfo - , backend: workspace.backend - , buildOptions - , doc: Just workspaceDoc - , workspaceConfig: workspace - , rootPackage: maybePackage + { packageSet: { buildType, lockfile } + , compiler: packageSetInfo <#> _.compiler # fromMaybe Core.widestRange } -rootPackageToWorkspacePackage - :: forall m - . MonadEffect m - => RootPath - -> { rootPackage :: Core.PackageConfig, workspaceDoc :: YamlDoc Core.Config } - -> m WorkspacePackage -rootPackageToWorkspacePackage rootPath { rootPackage, workspaceDoc } = do - hasTests <- liftEffect $ FS.exists (rootPath "test") - pure { path: rootPath "", doc: Just workspaceDoc, package: rootPackage, hasTests } - workspacePackageToLockfilePackage :: WorkspacePackage -> Tuple PackageName Lock.WorkspaceLockPackage workspacePackageToLockfilePackage { path, package } = Tuple package.name { path: case Path.localPart (withForwardSlashes path) of @@ -622,19 +685,15 @@ readConfig path = do Just yml else Nothing - pure $ Left $ case Path.basename path, yml of - "spago.yaml", Nothing -> - [ "Did not find " <> Path.quote path <> ". Run `spago init` to initialize a new project." ] - "spago.yaml", Just y -> + pure $ Left $ case yml of + Just y -> [ "Did not find " <> Path.quote path <> ". Spago's configuration files must end with `.yaml`, not `.yml`." - , "Try renaming " <> Path.quote y <> " to " <> Path.quote path <> " or run `spago init` to initialize a new project." + , "Try renaming " <> Path.basename y <> " to " <> Path.basename path <> " or run `spago init` to initialize a new project." ] - _, Nothing -> + Nothing | Path.basename path == spagoYaml -> + [ "Did not find " <> Path.quote path <> ". Run `spago init` to initialize a new project." ] + Nothing -> [ "Did not find " <> Path.quote path <> "." ] - _, Just y -> - [ "Did not find " <> Path.quote path <> ". Spago's configuration files must end with `.yaml`, not `.yml`." - , "Try renaming " <> Path.quote y <> " to " <> Path.quote path <> "." - ] Right yamlString -> do case lmap (\err -> CJ.DecodeError.basic ("YAML: " <> err)) (Yaml.parser yamlString) of Left err -> pure $ Left [ CJ.DecodeError.print err ] @@ -658,7 +717,7 @@ readConfig path = do setPackageSetVersionInConfig :: forall m. MonadAff m => MonadEffect m => RootPath -> YamlDoc Core.Config -> Version -> m Unit setPackageSetVersionInConfig root doc version = do liftEffect $ runEffectFn2 setPackageSetVersionInConfigImpl doc (Version.print version) - liftAff $ FS.writeYamlDocFile (root "spago.yaml") doc + liftAff $ FS.writeYamlDocFile (root spagoYaml) doc addPackagesToConfig :: forall m path. Path.IsPath path => MonadAff m => path -> YamlDoc Core.Config -> Boolean -> Array PackageName -> m Unit addPackagesToConfig configPath doc isTest pkgs = do diff --git a/src/Spago/Prelude.purs b/src/Spago/Prelude.purs index 04df4e7ed..9fa336f7a 100644 --- a/src/Spago/Prelude.purs +++ b/src/Spago/Prelude.purs @@ -51,8 +51,7 @@ data OnlineStatus = Offline | Online | OnlineBypassCache derive instance Eq OnlineStatus type SpagoBaseEnv a = - { rootPath :: Path.RootPath - , logOptions :: LogOptions + { logOptions :: LogOptions | a } diff --git a/test-fixtures/config/discovery/a/spago.yaml b/test-fixtures/config/discovery/a/spago.yaml new file mode 100644 index 000000000..069d839a5 --- /dev/null +++ b/test-fixtures/config/discovery/a/spago.yaml @@ -0,0 +1,4 @@ +package: + name: a + dependencies: + - prelude diff --git a/test-fixtures/config/discovery/b/spago.yaml b/test-fixtures/config/discovery/b/spago.yaml new file mode 100644 index 000000000..d33f67709 --- /dev/null +++ b/test-fixtures/config/discovery/b/spago.yaml @@ -0,0 +1,4 @@ +package: + name: b + dependencies: + - prelude diff --git a/test-fixtures/config/discovery/from-a.txt b/test-fixtures/config/discovery/from-a.txt new file mode 100644 index 000000000..24796559e --- /dev/null +++ b/test-fixtures/config/discovery/from-a.txt @@ -0,0 +1,12 @@ +Reading Spago workspace configuration... + +✓ Selecting package to build: a + +Downloading dependencies... +Building... +purs compile: No files found using pattern: a/src/**/*.purs + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✓ Build succeeded. diff --git a/test-fixtures/config/discovery/from-d.txt b/test-fixtures/config/discovery/from-d.txt new file mode 100644 index 000000000..2a1953635 --- /dev/null +++ b/test-fixtures/config/discovery/from-d.txt @@ -0,0 +1,12 @@ +Reading Spago workspace configuration... + +✓ Selecting package to build: d + +Downloading dependencies... +Building... +purs compile: No files found using pattern: d/src/**/*.purs + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✓ Build succeeded. diff --git a/test-fixtures/config/discovery/from-nested.txt b/test-fixtures/config/discovery/from-nested.txt new file mode 100644 index 000000000..d72721f17 --- /dev/null +++ b/test-fixtures/config/discovery/from-nested.txt @@ -0,0 +1,17 @@ +Reading Spago workspace configuration... + +✓ Selecting 3 packages to build: + c + d + nested-workspace + +Downloading dependencies... +Building... +purs compile: No files found using pattern: c/src/**/*.purs +purs compile: No files found using pattern: d/src/**/*.purs +purs compile: No files found using pattern: src/**/*.purs + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✓ Build succeeded. diff --git a/test-fixtures/config/discovery/from-root.txt b/test-fixtures/config/discovery/from-root.txt new file mode 100644 index 000000000..285b01001 --- /dev/null +++ b/test-fixtures/config/discovery/from-root.txt @@ -0,0 +1,17 @@ +Reading Spago workspace configuration... + +✓ Selecting 3 packages to build: + a + b + foo + +Downloading dependencies... +Building... +purs compile: No files found using pattern: a/src/**/*.purs +purs compile: No files found using pattern: b/src/**/*.purs +purs compile: No files found using pattern: src/**/*.purs + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✓ Build succeeded. diff --git a/test-fixtures/config/discovery/nested-workspace/c/spago.yaml b/test-fixtures/config/discovery/nested-workspace/c/spago.yaml new file mode 100644 index 000000000..a9c44e0f0 --- /dev/null +++ b/test-fixtures/config/discovery/nested-workspace/c/spago.yaml @@ -0,0 +1,4 @@ +package: + name: c + dependencies: + - prelude diff --git a/test-fixtures/config/discovery/nested-workspace/d/spago.yaml b/test-fixtures/config/discovery/nested-workspace/d/spago.yaml new file mode 100644 index 000000000..77ee1ae44 --- /dev/null +++ b/test-fixtures/config/discovery/nested-workspace/d/spago.yaml @@ -0,0 +1,4 @@ +package: + name: d + dependencies: + - prelude diff --git a/test-fixtures/config/discovery/nested-workspace/spago.yaml b/test-fixtures/config/discovery/nested-workspace/spago.yaml new file mode 100644 index 000000000..3277c1e9b --- /dev/null +++ b/test-fixtures/config/discovery/nested-workspace/spago.yaml @@ -0,0 +1,8 @@ +package: + name: nested-workspace + dependencies: + - prelude + +workspace: + packageSet: + registry: 41.5.0 diff --git a/test-fixtures/config/discovery/spago.yaml b/test-fixtures/config/discovery/spago.yaml new file mode 100644 index 000000000..164648573 --- /dev/null +++ b/test-fixtures/config/discovery/spago.yaml @@ -0,0 +1,10 @@ +package: + name: foo + dependencies: + - console + - effect + - prelude + +workspace: + packageSet: + registry: 41.5.0 diff --git a/test-fixtures/config/misnamed-configs/a/b/c/spago.yml b/test-fixtures/config/misnamed-configs/a/b/c/spago.yml new file mode 100644 index 000000000..e69de29bb diff --git a/test-fixtures/config/misnamed-configs/a/b/d/.keep b/test-fixtures/config/misnamed-configs/a/b/d/.keep new file mode 100644 index 000000000..e69de29bb diff --git a/test-fixtures/config/misnamed-configs/a/b/spago.yml b/test-fixtures/config/misnamed-configs/a/b/spago.yml new file mode 100644 index 000000000..e69de29bb diff --git a/test-fixtures/config/misnamed-configs/from-a.txt b/test-fixtures/config/misnamed-configs/from-a.txt new file mode 100644 index 000000000..ac2cb95de --- /dev/null +++ b/test-fixtures/config/misnamed-configs/from-a.txt @@ -0,0 +1,10 @@ +Reading Spago workspace configuration... + +✘ No spago.yaml found in the current directory or any of its parents. + + + Instead found "../spago.yml" + Note that Spago config files should be named spago.yaml, not spago.yml. + + +The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file diff --git a/test-fixtures/config/misnamed-configs/from-b.txt b/test-fixtures/config/misnamed-configs/from-b.txt new file mode 100644 index 000000000..136d1f2b2 --- /dev/null +++ b/test-fixtures/config/misnamed-configs/from-b.txt @@ -0,0 +1,12 @@ +Reading Spago workspace configuration... + +✘ No spago.yaml found in the current directory or any of its parents. + + + Instead found these: + "../../spago.yml" + "spago.yml" + Note that Spago config files should be named spago.yaml, not spago.yml. + + +The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file diff --git a/test-fixtures/config/misnamed-configs/from-c.txt b/test-fixtures/config/misnamed-configs/from-c.txt new file mode 100644 index 000000000..eab873467 --- /dev/null +++ b/test-fixtures/config/misnamed-configs/from-c.txt @@ -0,0 +1,13 @@ +Reading Spago workspace configuration... + +✘ No spago.yaml found in the current directory or any of its parents. + + + Instead found these: + "../../../spago.yml" + "../spago.yml" + "spago.yml" + Note that Spago config files should be named spago.yaml, not spago.yml. + + +The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file diff --git a/test-fixtures/config/misnamed-configs/from-d.txt b/test-fixtures/config/misnamed-configs/from-d.txt new file mode 100644 index 000000000..61e6ff5aa --- /dev/null +++ b/test-fixtures/config/misnamed-configs/from-d.txt @@ -0,0 +1,12 @@ +Reading Spago workspace configuration... + +✘ No spago.yaml found in the current directory or any of its parents. + + + Instead found these: + "../../../spago.yml" + "../spago.yml" + Note that Spago config files should be named spago.yaml, not spago.yml. + + +The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file diff --git a/test-fixtures/config/misnamed-configs/from-root.txt b/test-fixtures/config/misnamed-configs/from-root.txt new file mode 100644 index 000000000..349bf5548 --- /dev/null +++ b/test-fixtures/config/misnamed-configs/from-root.txt @@ -0,0 +1,10 @@ +Reading Spago workspace configuration... + +✘ No spago.yaml found in the current directory or any of its parents. + + + Instead found "spago.yml" + Note that Spago config files should be named spago.yaml, not spago.yml. + + +The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file diff --git a/test-fixtures/config/misnamed-configs/spago.yml b/test-fixtures/config/misnamed-configs/spago.yml new file mode 100644 index 000000000..e69de29bb diff --git a/test-fixtures/config/no-workspace-anywhere.txt b/test-fixtures/config/no-workspace-anywhere.txt new file mode 100644 index 000000000..5fd21e84a --- /dev/null +++ b/test-fixtures/config/no-workspace-anywhere.txt @@ -0,0 +1,3 @@ +Reading Spago workspace configuration... + +✘ No spago.yaml found in the current directory or any of its parents. diff --git a/test-fixtures/spago-yml-check-stderr.txt b/test-fixtures/spago-yml-check-stderr.txt index ccd6509e4..349bf5548 100644 --- a/test-fixtures/spago-yml-check-stderr.txt +++ b/test-fixtures/spago-yml-check-stderr.txt @@ -1,10 +1,10 @@ Reading Spago workspace configuration... -✘ Couldn't parse Spago config, error: +✘ No spago.yaml found in the current directory or any of its parents. - Did not find "spago.yaml". Spago's configuration files must end with `.yaml`, not `.yml`. - Try renaming "spago.yml" to "spago.yaml" or run `spago init` to initialize a new project. + Instead found "spago.yml" + Note that Spago config files should be named spago.yaml, not spago.yml. The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 89a0f138f..7e320494b 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -9,6 +9,8 @@ import Registry.PackageName as PackageName import Registry.Version as Version import Spago.Core.Config (SetAddress(..)) import Spago.Core.Config as C +import Spago.FS as FS +import Spago.Paths as Paths import Spago.Yaml as Yaml import Test.Spec (Spec) import Test.Spec as Spec @@ -64,11 +66,62 @@ spec = <> "\n - Could not decode GitHub:" <> "\n Unknown field(s): bogus_field" ) - where - shouldFailWith result expectedError = - case result of - Right _ -> Assert.fail "Expected an error, but parsed successfully" - Left err -> CJ.print err `shouldEqual` expectedError + + Spec.around withTempDir $ + Spec.describe "spago.yaml discovery" do + Spec.it "discovers config up the directory tree" \{ testCwd, fixture, spago } -> do + FS.copyTree { src: fixture "config/discovery", dst: testCwd } + spago [ "build" ] >>= shouldBeSuccess + spago [ "build" ] >>= shouldBeSuccessErr (fixture "config/discovery/from-root.txt") + + -- Running from `./a`, Spago should discover the workspace root at + -- './' and select package 'a' + Paths.chdir $ testCwd "a" + spago [ "build" ] >>= shouldBeSuccessErr (fixture "config/discovery/from-a.txt") + + -- Running from `./nested-workspace`, Spago should use the workspace + -- root at './nested-workspace' and not the one at './' + Paths.chdir $ testCwd "nested-workspace" + spago [ "build" ] >>= shouldBeSuccess + spago [ "build" ] >>= shouldBeSuccessErr (fixture "config/discovery/from-nested.txt") + + -- Running from `./nested-workspace/d`, Spago should use the workspace + -- root at './nested-workspace', because that's the closest one, and + -- select package 'd' + Paths.chdir $ testCwd "nested-workspace" "d" + spago [ "build" ] >>= shouldBeSuccessErr (fixture "config/discovery/from-d.txt") + + -- At workspace roots, a ".spago" directory should be created for + -- local cache, but not in subdirs + FS.exists (testCwd ".spago") `Assert.shouldReturn` true + FS.exists (testCwd "a" ".spago") `Assert.shouldReturn` false + FS.exists (testCwd "nested-workspace" ".spago") `Assert.shouldReturn` true + FS.exists (testCwd "nested-workspace" "d" ".spago") `Assert.shouldReturn` false + + Spec.it "reports no config in any parent directories" \{ spago, fixture } -> + spago [ "build" ] >>= shouldBeFailureErr (fixture "config/no-workspace-anywhere.txt") + + Spec.it "reports possible misnamed configs up the directory tree" \{ testCwd, spago, fixture } -> do + FS.copyTree { src: fixture "config/misnamed-configs", dst: testCwd } + spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-root.txt") + + Paths.chdir $ testCwd "a" + spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-a.txt") + + Paths.chdir $ testCwd "a" "b" + spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-b.txt") + + Paths.chdir $ testCwd "a" "b" "c" + spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-c.txt") + + Paths.chdir $ testCwd "a" "b" "d" + spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-d.txt") + + where + shouldFailWith result expectedError = + case result of + Right _ -> Assert.fail "Expected an error, but parsed successfully" + Left err -> CJ.print err `shouldEqual` expectedError validSpagoYaml :: { serialized :: String, parsed :: C.Config } validSpagoYaml = From fbab1c3bd73d40b908f1c0d62a658cc089859264 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Sun, 19 Jan 2025 18:46:28 -0500 Subject: [PATCH 02/25] The high spirits of questionable formatting are not to be trifled with --- bin/src/Main.purs | 13 +- src/Spago/Config.purs | 284 +++++++++++++++++++++--------------------- 2 files changed, 152 insertions(+), 145 deletions(-) diff --git a/bin/src/Main.purs b/bin/src/Main.purs index b2e5312f4..93fed2a45 100644 --- a/bin/src/Main.purs +++ b/bin/src/Main.purs @@ -951,12 +951,13 @@ mkReplEnv replArgs dependencies supportPackage = do , selected } -mkFetchEnv :: forall a b. - { offline :: OnlineStatus - , migrateConfig :: Boolean - , isRepl :: Boolean - | FetchArgsRow b - } +mkFetchEnv + :: ∀ a b + . { offline :: OnlineStatus + , migrateConfig :: Boolean + , isRepl :: Boolean + | FetchArgsRow b + } -> Spago { logOptions :: LogOptions | a } { env :: Fetch.FetchEnv (), fetchOpts :: Fetch.FetchOpts } mkFetchEnv args@{ migrateConfig, offline } = do let diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 7bacba45d..60b381b2f 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -192,8 +192,9 @@ discoverWorkspace options cwd = do Map.fromFoldable <$> for (Map.toUnfoldable loadedPackages :: Array _) \(path /\ { package, config }) -> do hasTests <- FS.exists (path "test") - let wsp :: WorkspacePackage - wsp = { package, path: path `Path.relativeTo` rootPath, doc: Just config.doc, hasTests } + let + wsp :: WorkspacePackage + wsp = { package, path: path `Path.relativeTo` rootPath, doc: Just config.doc, hasTests } pure (package.name /\ wsp) selected <- @@ -213,141 +214,144 @@ discoverWorkspace options cwd = do pure { rootPath , workspace: - { selected - , packageSet - , compatibleCompiler: compiler - , backend: workspace.config.backend - , buildOptions: - { output: workspace.config.buildOpts >>= _.output <#> \o -> withForwardSlashes $ rootPath o - , censorLibWarnings: _.censorLibraryWarnings =<< workspace.config.buildOpts - , statVerbosity: _.statVerbosity =<< workspace.config.buildOpts + { selected + , packageSet + , compatibleCompiler: compiler + , backend: workspace.config.backend + , buildOptions: + { output: workspace.config.buildOpts >>= _.output <#> \o -> withForwardSlashes $ rootPath o + , censorLibWarnings: _.censorLibraryWarnings =<< workspace.config.buildOpts + , statVerbosity: _.statVerbosity =<< workspace.config.buildOpts + } + , doc: Just workspace.doc + , workspaceConfig: workspace.config + , rootPackage: workspace.rootPackage } - , doc: Just workspace.doc - , workspaceConfig: workspace.config - , rootPackage: workspace.rootPackage - } } where - readConfig' = State.lift <<< readConfig - - walkDirectoriesUpFrom dir = do - maybeConfig <- tryReadConfigAt configFile - - for_ maybeConfig \config -> - for_ config.yaml.package \package -> - -- If there is a package in this directory, remember it - State.modify_ \s -> s - { loadedPackages = Map.insert dir { package, config } s.loadedPackages - , closestPackage = s.closestPackage <|> Just package.name - } - - whenM (FS.exists $ dir "spago.yml") $ - State.modify_ \s -> s { misnamedConfigs = Array.cons dir s.misnamedConfigs } - - case maybeConfig of - Just { doc, yaml: { workspace: Just workspace, package } } -> do - -- Finally, found the "workspace" config! - rootPath <- Path.mkRoot dir - loadSubprojectConfigs rootPath - pure { workspace: { config: workspace, doc, rootPackage: package }, rootPath } - _ -> do - -- No workspace in this directory => recur to parent directory (unless it's already root) - when (parentDir == dir) $ - dieForLackOfSpagoYaml - walkDirectoriesUpFrom parentDir + readConfig' = State.lift <<< readConfig + + walkDirectoriesUpFrom dir = do + maybeConfig <- tryReadConfigAt configFile + + for_ maybeConfig \config -> + for_ config.yaml.package \package -> + -- If there is a package in this directory, remember it + State.modify_ \s -> s + { loadedPackages = Map.insert dir { package, config } s.loadedPackages + , closestPackage = s.closestPackage <|> Just package.name + } + + whenM (FS.exists $ dir "spago.yml") $ + State.modify_ \s -> s { misnamedConfigs = Array.cons dir s.misnamedConfigs } + + case maybeConfig of + Just { doc, yaml: { workspace: Just workspace, package } } -> do + -- Finally, found the "workspace" config! + rootPath <- Path.mkRoot dir + loadSubprojectConfigs rootPath + pure { workspace: { config: workspace, doc, rootPackage: package }, rootPath } + _ -> do + -- No workspace in this directory => recur to parent directory (unless it's already root) + when (parentDir == dir) $ + dieForLackOfSpagoYaml + walkDirectoriesUpFrom parentDir + + where + configFile = dir spagoYaml + parentDir = Path.dirname dir + + loadSubprojectConfigs rootPath = do + candidates <- liftAff $ Glob.gitignoringGlob + { root: rootPath + , includePatterns: [ "**/" <> spagoYaml ] + , ignorePatterns: [ "**/node_modules/**", "**/.spago/**" ] + } - where + -- Traversing directories (not files) and doing it in sorted order ensures + -- that parent directories come before their subdirectories. That way we + -- can remember workspaces that we find along the way and avoid trying to + -- load their subprojects that come later. + candidates <#> Path.toGlobal <#> Path.dirname # Array.sort # traverse_ \dir -> do + st <- State.get + let configFile = dir spagoYaml - parentDir = Path.dirname dir - - loadSubprojectConfigs rootPath = do - candidates <- liftAff $ Glob.gitignoringGlob - { root: rootPath - , includePatterns: [ "**/" <> spagoYaml ] - , ignorePatterns: [ "**/node_modules/**", "**/.spago/**" ] - } + alreadyLoaded = st.loadedPackages # Map.member configFile + anotherParentWorkspace = st.otherWorkspaceRoots # Array.find (_ `Path.isPrefixOf` dir) + case alreadyLoaded, anotherParentWorkspace of + true, _ -> + pure unit + _, Just ws -> do + logDebug $ "Not trying to load " <> Path.quote configFile <> " because it belongs to a different workspace at " <> Path.quote ws + pure unit + false, Nothing -> + readConfig' configFile >>= case _ of + Left _ -> + logWarn $ "Failed to read config at " <> Path.quote configFile + Right { yaml: { workspace: Just _ } } -> + State.modify_ \s -> s { otherWorkspaceRoots = Array.cons dir s.otherWorkspaceRoots } + Right config@{ yaml: { package: Just package } } -> do + logDebug $ "Loaded a subproject config at " <> Path.quote configFile + State.modify_ \s -> s { loadedPackages = Map.insert dir { package, config } s.loadedPackages } + Right _ -> do + logWarn $ "Neither workspace nor package found in " <> Path.quote configFile + + tryReadConfigAt path = do + exists <- FS.exists path + if exists then + Just <$> do + logDebug $ "Loading spago.yaml at " <> Path.quote path + readConfig' path >>= rightOrDieWith \errLines -> + [ toDoc $ "Couldn't parse Spago config file at: " <> Path.quote path + , indent $ toDoc errLines + , Log.break + , toDoc "The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file" + ] + else + pure Nothing - -- Traversing directories (not files) and doing it in sorted order ensures - -- that parent directories come before their subdirectories. That way we - -- can remember workspaces that we find along the way and avoid trying to - -- load their subprojects that come later. - candidates <#> Path.toGlobal <#> Path.dirname # Array.sort # traverse_ \dir -> do - st <- State.get - let configFile = dir spagoYaml - alreadyLoaded = st.loadedPackages # Map.member configFile - anotherParentWorkspace = st.otherWorkspaceRoots # Array.find (_ `Path.isPrefixOf` dir) - case alreadyLoaded, anotherParentWorkspace of - true, _ -> - pure unit - _, Just ws -> do - logDebug $ "Not trying to load " <> Path.quote configFile <> " because it belongs to a different workspace at " <> Path.quote ws - pure unit - false, Nothing -> - readConfig' configFile >>= case _ of - Left _ -> - logWarn $ "Failed to read config at " <> Path.quote configFile - Right { yaml: { workspace: Just _ } } -> - State.modify_ \s -> s { otherWorkspaceRoots = Array.cons dir s.otherWorkspaceRoots } - Right config@{ yaml: { package: Just package } } -> do - logDebug $ "Loaded a subproject config at " <> Path.quote configFile - State.modify_ \s -> s { loadedPackages = Map.insert dir { package, config } s.loadedPackages } - Right _ -> do - logWarn $ "Neither workspace nor package found in " <> Path.quote configFile - - tryReadConfigAt path = do - exists <- FS.exists path - if exists then - Just <$> do - logDebug $ "Loading spago.yaml at " <> Path.quote path - readConfig' path >>= rightOrDieWith \errLines -> - [ toDoc $ "Couldn't parse Spago config file at: " <> Path.quote path - , indent $ toDoc errLines + migrateConfigsWhereNeeded rootPath loadedConfigs = do + forWithIndex_ loadedConfigs \path' { config } -> do + let path = (path' spagoYaml) `Path.relativeTo` rootPath + case options.migrateConfig, config.wasMigrated of + true, true -> do + logInfo $ "Migrating your " <> Path.quote path <> " to the latest version..." + liftAff $ FS.writeYamlDocFile path config.doc + false, true -> + logWarn $ "Your " <> Path.quote path <> " is using an outdated format. Run Spago with the --migrate flag to update it to the latest version." + _, false -> + pure unit + + dieForLackOfSpagoYaml = do + root <- Path.mkRoot cwd + misnamedConfigs <- State.gets _.misnamedConfigs + let + misnamedConfigsList = + case misnamedConfigs <#> \c -> Path.quote $ (c "spago.yml") `Path.relativeTo` root of + [] -> [] + [ one ] -> [ toDoc $ "Instead found " <> one ] + many -> [ toDoc "Instead found these:", indent $ toDoc many ] + die + [ toDoc $ "No " <> spagoYaml <> " found in the current directory or any of its parents." + , if Array.null misnamedConfigsList then + toDoc "" + else + toDoc + [ Log.break + , indent $ toDoc $ misnamedConfigsList + , indent $ toDoc $ "Note that Spago config files should be named " <> spagoYaml <> ", not spago.yml." , Log.break , toDoc "The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file" ] - else - pure Nothing - - migrateConfigsWhereNeeded rootPath loadedConfigs = do - forWithIndex_ loadedConfigs \path' { config } -> do - let path = (path' spagoYaml) `Path.relativeTo` rootPath - case options.migrateConfig, config.wasMigrated of - true, true -> do - logInfo $ "Migrating your " <> Path.quote path <> " to the latest version..." - liftAff $ FS.writeYamlDocFile path config.doc - false, true -> - logWarn $ "Your " <> Path.quote path <> " is using an outdated format. Run Spago with the --migrate flag to update it to the latest version." - _, false -> - pure unit - - dieForLackOfSpagoYaml = do - root <- Path.mkRoot cwd - misnamedConfigs <- State.gets _.misnamedConfigs - let misnamedConfigsList = - case misnamedConfigs <#> \c -> Path.quote $ (c "spago.yml") `Path.relativeTo` root of - [] -> [] - [one] -> [ toDoc $ "Instead found " <> one ] - many -> [ toDoc "Instead found these:" , indent $ toDoc many ] - die - [ toDoc $ "No " <> spagoYaml <> " found in the current directory or any of its parents." - , if Array.null misnamedConfigsList then - toDoc "" - else - toDoc - [ Log.break - , indent $ toDoc $ misnamedConfigsList - , indent $ toDoc $ "Note that Spago config files should be named " <> spagoYaml <> ", not spago.yml." - , Log.break - , toDoc "The configuration file help can be found here https://github.com/purescript/spago#the-configuration-file" - ] - ] + ] -determineSelectedPackage :: ∀ a. - { explicitlySelected :: Maybe PackageName - , inferredFromCwd :: Maybe PackageName - , rootPackage :: Maybe PackageName - , loadedPackages :: Map PackageName WorkspacePackage - } +determineSelectedPackage + :: ∀ a + . { explicitlySelected :: Maybe PackageName + , inferredFromCwd :: Maybe PackageName + , rootPackage :: Maybe PackageName + , loadedPackages :: Map PackageName WorkspacePackage + } -> Spago (Registry.RegistryEnv a) (Maybe WorkspacePackage) determineSelectedPackage { explicitlySelected, inferredFromCwd, rootPackage, loadedPackages } = do let @@ -394,12 +398,13 @@ determineSelectedPackage { explicitlySelected, inferredFromCwd, rootPackage, loa pure maybeSelected -loadLockfile :: ∀ a. - { pureBuild :: Boolean - , workspaceConfig :: Core.WorkspaceConfig - , loadedPackages :: Map PackageName WorkspacePackage - , rootPath :: RootPath - } +loadLockfile + :: ∀ a + . { pureBuild :: Boolean + , workspaceConfig :: Core.WorkspaceConfig + , loadedPackages :: Map PackageName WorkspacePackage + , rootPath :: RootPath + } -> Spago (Registry.RegistryEnv a) (Either String Lockfile) loadLockfile { pureBuild, workspaceConfig, loadedPackages, rootPath } = do logDebug "Parsing the lockfile..." @@ -429,12 +434,13 @@ loadLockfile { pureBuild, workspaceConfig, loadedPackages, rootPath } = do logDebug "Lockfile is up to date, using it" pure (Right contents) -loadPackageSet :: ∀ a. - { workspaceConfig :: Core.WorkspaceConfig - , loadedPackages :: Map PackageName WorkspacePackage - , rootPath :: RootPath - , lockfile :: Either String Lockfile - } +loadPackageSet + :: ∀ a + . { workspaceConfig :: Core.WorkspaceConfig + , loadedPackages :: Map PackageName WorkspacePackage + , rootPath :: RootPath + , lockfile :: Either String Lockfile + } -> Spago (Registry.RegistryEnv a) { packageSet :: PackageSet, compiler :: Range } loadPackageSet { lockfile, workspaceConfig, loadedPackages, rootPath } = do { offline } <- ask From ebba9daccf10921f8db3ec1509e2bb4dbfbaa2a1 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Sun, 19 Jan 2025 20:24:03 -0500 Subject: [PATCH 03/25] When the slash is forward, every forest friend will notice --- test/Spago/Config.purs | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 7e320494b..62cd1240b 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -3,6 +3,7 @@ module Test.Spago.Config where import Test.Prelude import Codec.JSON.DecodeError as CJ +import Data.String as String import Registry.License as License import Registry.Location (Location(..)) import Registry.PackageName as PackageName @@ -72,24 +73,24 @@ spec = Spec.it "discovers config up the directory tree" \{ testCwd, fixture, spago } -> do FS.copyTree { src: fixture "config/discovery", dst: testCwd } spago [ "build" ] >>= shouldBeSuccess - spago [ "build" ] >>= shouldBeSuccessErr (fixture "config/discovery/from-root.txt") + spago [ "build" ] >>= shouldBeSuccessErr' (fixture "config/discovery/from-root.txt") -- Running from `./a`, Spago should discover the workspace root at -- './' and select package 'a' Paths.chdir $ testCwd "a" - spago [ "build" ] >>= shouldBeSuccessErr (fixture "config/discovery/from-a.txt") + spago [ "build" ] >>= shouldBeSuccessErr' (fixture "config/discovery/from-a.txt") -- Running from `./nested-workspace`, Spago should use the workspace -- root at './nested-workspace' and not the one at './' Paths.chdir $ testCwd "nested-workspace" spago [ "build" ] >>= shouldBeSuccess - spago [ "build" ] >>= shouldBeSuccessErr (fixture "config/discovery/from-nested.txt") + spago [ "build" ] >>= shouldBeSuccessErr' (fixture "config/discovery/from-nested.txt") -- Running from `./nested-workspace/d`, Spago should use the workspace -- root at './nested-workspace', because that's the closest one, and -- select package 'd' Paths.chdir $ testCwd "nested-workspace" "d" - spago [ "build" ] >>= shouldBeSuccessErr (fixture "config/discovery/from-d.txt") + spago [ "build" ] >>= shouldBeSuccessErr' (fixture "config/discovery/from-d.txt") -- At workspace roots, a ".spago" directory should be created for -- local cache, but not in subdirs @@ -99,23 +100,23 @@ spec = FS.exists (testCwd "nested-workspace" "d" ".spago") `Assert.shouldReturn` false Spec.it "reports no config in any parent directories" \{ spago, fixture } -> - spago [ "build" ] >>= shouldBeFailureErr (fixture "config/no-workspace-anywhere.txt") + spago [ "build" ] >>= shouldBeFailureErr' (fixture "config/no-workspace-anywhere.txt") Spec.it "reports possible misnamed configs up the directory tree" \{ testCwd, spago, fixture } -> do FS.copyTree { src: fixture "config/misnamed-configs", dst: testCwd } - spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-root.txt") + spago [ "build" ] >>= shouldBeFailureErr' (fixture "config/misnamed-configs/from-root.txt") Paths.chdir $ testCwd "a" - spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-a.txt") + spago [ "build" ] >>= shouldBeFailureErr' (fixture "config/misnamed-configs/from-a.txt") Paths.chdir $ testCwd "a" "b" - spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-b.txt") + spago [ "build" ] >>= shouldBeFailureErr' (fixture "config/misnamed-configs/from-b.txt") Paths.chdir $ testCwd "a" "b" "c" - spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-c.txt") + spago [ "build" ] >>= shouldBeFailureErr' (fixture "config/misnamed-configs/from-c.txt") Paths.chdir $ testCwd "a" "b" "d" - spago [ "build" ] >>= shouldBeFailureErr (fixture "config/misnamed-configs/from-d.txt") + spago [ "build" ] >>= shouldBeFailureErr' (fixture "config/misnamed-configs/from-d.txt") where shouldFailWith result expectedError = @@ -123,6 +124,19 @@ spec = Right _ -> Assert.fail "Expected an error, but parsed successfully" Left err -> CJ.print err `shouldEqual` expectedError + shouldBeSuccessErr' = shouldBeErr isRight + shouldBeFailureErr' = shouldBeErr isLeft + + shouldBeErr result file = checkOutputs' + { stdoutFile: Nothing + , stderrFile: Just file + , result + , sanitize: + String.trim + >>> String.replaceAll (String.Pattern "\\") (String.Replacement "/") + >>> String.replaceAll (String.Pattern "\r\n") (String.Replacement "\n") + } + validSpagoYaml :: { serialized :: String, parsed :: C.Config } validSpagoYaml = { serialized: From 171b93f0824473ac6b4a45eb41e9468d1a5110f0 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 15:33:43 -0500 Subject: [PATCH 04/25] Do not attempt to load subpackages nested under malformed config --- src/Spago/Config.purs | 25 +++++++++++-------- .../config/malformed-configs/a/b/c/spago.yaml | 3 +++ .../config/malformed-configs/a/b/spago.yaml | 3 +++ .../config/malformed-configs/a/d/spago.yaml | 3 +++ .../config/malformed-configs/a/spago.yaml | 3 +++ .../config/malformed-configs/from-root.txt | 9 +++++++ .../config/malformed-configs/spago.yaml | 6 +++++ test/Spago/Config.purs | 15 +++++++++++ 8 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 test-fixtures/config/malformed-configs/a/b/c/spago.yaml create mode 100644 test-fixtures/config/malformed-configs/a/b/spago.yaml create mode 100644 test-fixtures/config/malformed-configs/a/d/spago.yaml create mode 100644 test-fixtures/config/malformed-configs/a/spago.yaml create mode 100644 test-fixtures/config/malformed-configs/from-root.txt create mode 100644 test-fixtures/config/malformed-configs/spago.yaml diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 60b381b2f..6bef2e50d 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -182,9 +182,13 @@ discoverWorkspace options cwd = do logInfo "Reading Spago workspace configuration..." logDebug $ "Discovering nearest workspace " <> spagoYaml <> " starting at " <> Path.quote cwd - { workspace, rootPath } /\ { loadedPackages, closestPackage } <- + { workspace, rootPath } /\ upTree@{ closestPackage } <- State.runStateT (walkDirectoriesUpFrom cwd) - { loadedPackages: Map.empty, otherWorkspaceRoots: [], misnamedConfigs: [], closestPackage: Nothing } + { loadedPackages: Map.empty, misnamedConfigs: [], closestPackage: Nothing } + + { loadedPackages } <- + State.execStateT (loadSubprojectConfigs rootPath) + { loadedPackages: upTree.loadedPackages, blockedSubtrees: [] } migrateConfigsWhereNeeded rootPath loadedPackages @@ -229,6 +233,7 @@ discoverWorkspace options cwd = do } } where + readConfig' :: ∀ s. _ -> State.StateT s _ _ readConfig' = State.lift <<< readConfig walkDirectoriesUpFrom dir = do @@ -249,7 +254,6 @@ discoverWorkspace options cwd = do Just { doc, yaml: { workspace: Just workspace, package } } -> do -- Finally, found the "workspace" config! rootPath <- Path.mkRoot dir - loadSubprojectConfigs rootPath pure { workspace: { config: workspace, doc, rootPackage: package }, rootPath } _ -> do -- No workspace in this directory => recur to parent directory (unless it's already root) @@ -277,19 +281,20 @@ discoverWorkspace options cwd = do let configFile = dir spagoYaml alreadyLoaded = st.loadedPackages # Map.member configFile - anotherParentWorkspace = st.otherWorkspaceRoots # Array.find (_ `Path.isPrefixOf` dir) - case alreadyLoaded, anotherParentWorkspace of + blockedSubtree = st.blockedSubtrees # Array.find (_ `Path.isPrefixOf` dir) + case alreadyLoaded, blockedSubtree of true, _ -> pure unit - _, Just ws -> do - logDebug $ "Not trying to load " <> Path.quote configFile <> " because it belongs to a different workspace at " <> Path.quote ws + _, Just b -> do + logDebug $ "Not trying to load " <> Path.quote configFile <> " because it is nested under " <> Path.quote b pure unit false, Nothing -> readConfig' configFile >>= case _ of - Left _ -> - logWarn $ "Failed to read config at " <> Path.quote configFile + Left _ -> do + logWarn [ "Failed to read config at " <> Path.quote configFile, "Skipping it and all its subprojects" ] + State.modify_ \s -> s { blockedSubtrees = Array.cons dir s.blockedSubtrees } Right { yaml: { workspace: Just _ } } -> - State.modify_ \s -> s { otherWorkspaceRoots = Array.cons dir s.otherWorkspaceRoots } + State.modify_ \s -> s { blockedSubtrees = Array.cons dir s.blockedSubtrees } Right config@{ yaml: { package: Just package } } -> do logDebug $ "Loaded a subproject config at " <> Path.quote configFile State.modify_ \s -> s { loadedPackages = Map.insert dir { package, config } s.loadedPackages } diff --git a/test-fixtures/config/malformed-configs/a/b/c/spago.yaml b/test-fixtures/config/malformed-configs/a/b/c/spago.yaml new file mode 100644 index 000000000..46fb85de8 --- /dev/null +++ b/test-fixtures/config/malformed-configs/a/b/c/spago.yaml @@ -0,0 +1,3 @@ +package: + name: c + dependencies: [] \ No newline at end of file diff --git a/test-fixtures/config/malformed-configs/a/b/spago.yaml b/test-fixtures/config/malformed-configs/a/b/spago.yaml new file mode 100644 index 000000000..665fca193 --- /dev/null +++ b/test-fixtures/config/malformed-configs/a/b/spago.yaml @@ -0,0 +1,3 @@ +package: + title: b # `title` is not a valid field, this config will error out on parsing. + dependencies: [] diff --git a/test-fixtures/config/malformed-configs/a/d/spago.yaml b/test-fixtures/config/malformed-configs/a/d/spago.yaml new file mode 100644 index 000000000..5f009ef1d --- /dev/null +++ b/test-fixtures/config/malformed-configs/a/d/spago.yaml @@ -0,0 +1,3 @@ +package: + name: d + dependencies: [] \ No newline at end of file diff --git a/test-fixtures/config/malformed-configs/a/spago.yaml b/test-fixtures/config/malformed-configs/a/spago.yaml new file mode 100644 index 000000000..4f7c7faa2 --- /dev/null +++ b/test-fixtures/config/malformed-configs/a/spago.yaml @@ -0,0 +1,3 @@ +package: + name: a + dependencies: [] \ No newline at end of file diff --git a/test-fixtures/config/malformed-configs/from-root.txt b/test-fixtures/config/malformed-configs/from-root.txt new file mode 100644 index 000000000..c20ceede2 --- /dev/null +++ b/test-fixtures/config/malformed-configs/from-root.txt @@ -0,0 +1,9 @@ +Reading Spago workspace configuration... +‼ Failed to read config at "/a/b/spago.yaml" +Skipping it and all its subprojects + +✘ Selected package bogus was not found in the local packages. +All available packages: + a + d + root diff --git a/test-fixtures/config/malformed-configs/spago.yaml b/test-fixtures/config/malformed-configs/spago.yaml new file mode 100644 index 000000000..3b928b012 --- /dev/null +++ b/test-fixtures/config/malformed-configs/spago.yaml @@ -0,0 +1,6 @@ +package: + name: root + dependencies: [] +workspace: + packageSet: + registry: 0.0.1 \ No newline at end of file diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 62cd1240b..349f5ed0e 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -11,6 +11,7 @@ import Registry.Version as Version import Spago.Core.Config (SetAddress(..)) import Spago.Core.Config as C import Spago.FS as FS +import Spago.Path as Path import Spago.Paths as Paths import Spago.Yaml as Yaml import Test.Spec (Spec) @@ -118,6 +119,20 @@ spec = Paths.chdir $ testCwd "a" "b" "d" spago [ "build" ] >>= shouldBeFailureErr' (fixture "config/misnamed-configs/from-d.txt") + Spec.it "warns about a malformed config, but stops parsing down the tree" \{ spago, fixture, testCwd } -> do + FS.copyTree { src: fixture "config/malformed-configs", dst: testCwd } + + -- Running with "-p bogus" to get Spago to list all available + -- packages. Packages `b` and `c` shouldn't be in that list because + -- b's config is malformatted, so Spago should warn about it and stop + -- loading configs down the tree from `b`, thus skipping `c`. + spago [ "build", "-p", "bogus" ] >>= checkOutputs' + { result: isLeft + , stdoutFile: Nothing + , stderrFile: Just (fixture "config/malformed-configs/from-root.txt") + , sanitize: String.trim >>> String.replaceAll (String.Pattern $ Path.toRaw testCwd) (String.Replacement "") + } + where shouldFailWith result expectedError = case result of From 2048499c67dceee75060ec1f0ca65630fc6383f0 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 16:44:29 -0500 Subject: [PATCH 05/25] Try to fix CI --- test/Spago/Config.purs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 349f5ed0e..0a727fcef 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -130,7 +130,10 @@ spec = { result: isLeft , stdoutFile: Nothing , stderrFile: Just (fixture "config/malformed-configs/from-root.txt") - , sanitize: String.trim >>> String.replaceAll (String.Pattern $ Path.toRaw testCwd) (String.Replacement "") + , sanitize: + String.trim + >>> String.replaceAll (String.Pattern "\\") (String.Replacement "/") + >>> String.replaceAll (String.Pattern $ Path.toRaw testCwd) (String.Replacement "") } where From b467814d6edf8399bef18068469337ee199f6e0f Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:09:01 -0500 Subject: [PATCH 06/25] Debug MacOS test failure --- .github/workflows/build.yml | 6 +++--- test/Spago/Config.purs | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f4f545a46..d84fc00c9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -78,11 +78,11 @@ jobs: - name: Bootstrap executable run: node ./bin/index.dev.js bundle -p spago-bin - - name: Bundle docs-search client - run: node ./bin/bundle.js bundle -p docs-search-client-halogen + # - name: Bundle docs-search client + # run: node ./bin/bundle.js bundle -p docs-search-client-halogen - name: Run tests - run: node ./bin/bundle.js test + run: node ./bin/bundle.js test -- -e "warns about a malformed config, but stops parsing down the tree" - name: Check formatting (Linux only) if: matrix.os == 'ubuntu-latest' diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 0a727fcef..b7ecb8de4 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -4,6 +4,7 @@ import Test.Prelude import Codec.JSON.DecodeError as CJ import Data.String as String +import Debug (traceM) import Registry.License as License import Registry.Location (Location(..)) import Registry.PackageName as PackageName @@ -122,6 +123,7 @@ spec = Spec.it "warns about a malformed config, but stops parsing down the tree" \{ spago, fixture, testCwd } -> do FS.copyTree { src: fixture "config/malformed-configs", dst: testCwd } + traceM { testCwd } -- Running with "-p bogus" to get Spago to list all available -- packages. Packages `b` and `c` shouldn't be in that list because -- b's config is malformatted, so Spago should warn about it and stop From e15dcb5d09979c8800bc47a4a4f464c76fca8d24 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:10:58 -0500 Subject: [PATCH 07/25] Debug MacOS test failure --- spago.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/spago.yaml b/spago.yaml index a7711a12a..909c98227 100644 --- a/spago.yaml +++ b/spago.yaml @@ -81,6 +81,7 @@ package: - quickcheck - spec - spec-node + - debug workspace: packageSet: registry: 59.0.0 From 6580f30cfd51e6550cfad303e31ac2e108f36b90 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:15:39 -0500 Subject: [PATCH 08/25] Debug MacOS test failure --- test/Spago/Config.purs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index b7ecb8de4..7d9e4a1d0 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -134,8 +134,8 @@ spec = , stderrFile: Just (fixture "config/malformed-configs/from-root.txt") , sanitize: String.trim - >>> String.replaceAll (String.Pattern "\\") (String.Replacement "/") - >>> String.replaceAll (String.Pattern $ Path.toRaw testCwd) (String.Replacement "") + -- >>> String.replaceAll (String.Pattern "\\") (String.Replacement "/") + -- >>> String.replaceAll (String.Pattern $ Path.toRaw testCwd) (String.Replacement "") } where From 41fe0630b6c60535c4833d090a6d963f396bf6ee Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:18:22 -0500 Subject: [PATCH 09/25] Debug MacOS test failure --- .github/workflows/build.yml | 4 ++-- spago.lock | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d84fc00c9..f6d69a5ab 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -13,9 +13,9 @@ jobs: strategy: matrix: include: - - os: ubuntu-latest + # - os: ubuntu-latest - os: macOS-latest - - os: windows-latest + # - os: windows-latest steps: # We set LF endings so that the Windows environment is consistent with the rest # See here for context: https://github.com/actions/checkout/issues/135 diff --git a/spago.lock b/spago.lock index 328e2901d..c8c101494 100644 --- a/spago.lock +++ b/spago.lock @@ -721,6 +721,7 @@ }, "test": { "dependencies": [ + "debug", "exceptions", "quickcheck", "spec", @@ -741,6 +742,7 @@ "contravariant", "control", "datetime", + "debug", "distributive", "effect", "either", @@ -2157,6 +2159,15 @@ "tuples" ] }, + "debug": { + "type": "registry", + "version": "6.0.2", + "integrity": "sha256-vmkYFuXYuELBzeauvgHG6E6Kf/Hp1dAnxwE9ByHfwSg=", + "dependencies": [ + "functions", + "prelude" + ] + }, "distributive": { "type": "registry", "version": "6.0.0", From 124fc0fb9922c0b872bb2a8bc79a51ea254c76bf Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:22:13 -0500 Subject: [PATCH 10/25] Debug MacOS test failure --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f6d69a5ab..93c3db560 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -82,7 +82,7 @@ jobs: # run: node ./bin/bundle.js bundle -p docs-search-client-halogen - name: Run tests - run: node ./bin/bundle.js test -- -e "warns about a malformed config, but stops parsing down the tree" + run: node ./bin/bundle.js test -- -E "(warns about a malformed config, but stops parsing down the tree)|(#1208)" - name: Check formatting (Linux only) if: matrix.os == 'ubuntu-latest' From a5f5bf08db82224101b61bacc6d091f04fbe98e1 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:25:49 -0500 Subject: [PATCH 11/25] Debug MacOS test failure --- test/Spago/Config.purs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 7d9e4a1d0..6ebe0863e 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -123,7 +123,8 @@ spec = Spec.it "warns about a malformed config, but stops parsing down the tree" \{ spago, fixture, testCwd } -> do FS.copyTree { src: fixture "config/malformed-configs", dst: testCwd } - traceM { testCwd } + cwd <- Path.toRaw <$> Paths.cwd + -- Running with "-p bogus" to get Spago to list all available -- packages. Packages `b` and `c` shouldn't be in that list because -- b's config is malformatted, so Spago should warn about it and stop @@ -134,8 +135,8 @@ spec = , stderrFile: Just (fixture "config/malformed-configs/from-root.txt") , sanitize: String.trim - -- >>> String.replaceAll (String.Pattern "\\") (String.Replacement "/") - -- >>> String.replaceAll (String.Pattern $ Path.toRaw testCwd) (String.Replacement "") + >>> String.replaceAll (String.Pattern "\\") (String.Replacement "/") + >>> String.replaceAll (String.Pattern cwd) (String.Replacement "") } where From 443fdc418371ecaa33abff86a8ee6d2bcfca4e87 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:35:52 -0500 Subject: [PATCH 12/25] Debug MacOS test failure --- test/Spago/Config.purs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 6ebe0863e..2499b4390 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -123,8 +123,16 @@ spec = Spec.it "warns about a malformed config, but stops parsing down the tree" \{ spago, fixture, testCwd } -> do FS.copyTree { src: fixture "config/malformed-configs", dst: testCwd } + -- Theoretically `cwd` should equal `testCwd`, but something fishy is + -- happening on MacOS CI, where it turns out that: + -- + -- cwd == "/private/" <> testCwd + -- + -- I don't know why it happens. cwd <- Path.toRaw <$> Paths.cwd + traceM Paths.paths + -- Running with "-p bogus" to get Spago to list all available -- packages. Packages `b` and `c` shouldn't be in that list because -- b's config is malformatted, so Spago should warn about it and stop From 5507c42b7845665fc43771d1c911bd42cd043467 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:40:21 -0500 Subject: [PATCH 13/25] Debug MacOS test failure --- test/Spago/Config.purs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 2499b4390..e820a18b6 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -121,6 +121,12 @@ spec = spago [ "build" ] >>= shouldBeFailureErr' (fixture "config/misnamed-configs/from-d.txt") Spec.it "warns about a malformed config, but stops parsing down the tree" \{ spago, fixture, testCwd } -> do + let bogusPath = Paths.paths.temp "bogus-dir" + Paths.chdir bogusPath + traceM bogusPath + traceM Paths.paths + traceM =<< Paths.cwd + FS.copyTree { src: fixture "config/malformed-configs", dst: testCwd } -- Theoretically `cwd` should equal `testCwd`, but something fishy is @@ -131,8 +137,6 @@ spec = -- I don't know why it happens. cwd <- Path.toRaw <$> Paths.cwd - traceM Paths.paths - -- Running with "-p bogus" to get Spago to list all available -- packages. Packages `b` and `c` shouldn't be in that list because -- b's config is malformatted, so Spago should warn about it and stop From e3987aba60b45205e38aa9fd8d520f4f1f13de70 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:42:36 -0500 Subject: [PATCH 14/25] Debug MacOS test failure --- test/Spago/Config.purs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index e820a18b6..8fef2f458 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -122,6 +122,7 @@ spec = Spec.it "warns about a malformed config, but stops parsing down the tree" \{ spago, fixture, testCwd } -> do let bogusPath = Paths.paths.temp "bogus-dir" + FS.mkdirp bogusPath Paths.chdir bogusPath traceM bogusPath traceM Paths.paths From d7d0ecfa870ab564622784ce48f53cbec628ae2e Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:46:16 -0500 Subject: [PATCH 15/25] Debug MacOS test failure --- test/Prelude.purs | 7 ++++--- test/Spago/Config.purs | 17 +---------------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/test/Prelude.purs b/test/Prelude.purs index 1048b52a4..1b5881da8 100644 --- a/test/Prelude.purs +++ b/test/Prelude.purs @@ -49,9 +49,10 @@ withTempDir = Aff.bracket createTempDir cleanupTempDir where createTempDir = do oldCwd <- Paths.cwd - temp <- Path.mkRoot =<< mkTemp' (Just "spago-test-") - FS.mkdirp temp - Paths.chdir temp + temp' <- mkTemp' (Just "spago-test-") + FS.mkdirp temp' + Paths.chdir temp' + temp <- Path.mkRoot =<< Paths.cwd isDebug <- liftEffect $ map isJust $ Process.lookupEnv "SPAGO_TEST_DEBUG" when isDebug do log $ "Running test in " <> Path.quote temp diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 8fef2f458..6d47d2ffe 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -121,23 +121,8 @@ spec = spago [ "build" ] >>= shouldBeFailureErr' (fixture "config/misnamed-configs/from-d.txt") Spec.it "warns about a malformed config, but stops parsing down the tree" \{ spago, fixture, testCwd } -> do - let bogusPath = Paths.paths.temp "bogus-dir" - FS.mkdirp bogusPath - Paths.chdir bogusPath - traceM bogusPath - traceM Paths.paths - traceM =<< Paths.cwd - FS.copyTree { src: fixture "config/malformed-configs", dst: testCwd } - -- Theoretically `cwd` should equal `testCwd`, but something fishy is - -- happening on MacOS CI, where it turns out that: - -- - -- cwd == "/private/" <> testCwd - -- - -- I don't know why it happens. - cwd <- Path.toRaw <$> Paths.cwd - -- Running with "-p bogus" to get Spago to list all available -- packages. Packages `b` and `c` shouldn't be in that list because -- b's config is malformatted, so Spago should warn about it and stop @@ -149,7 +134,7 @@ spec = , sanitize: String.trim >>> String.replaceAll (String.Pattern "\\") (String.Replacement "/") - >>> String.replaceAll (String.Pattern cwd) (String.Replacement "") + >>> String.replaceAll (String.Pattern $ Path.toRaw testCwd) (String.Replacement "") } where From c0c918bf120f1deef957ea6afb7ceee97f9bd9b0 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:51:02 -0500 Subject: [PATCH 16/25] Debug MacOS test failure --- test-macos.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 test-macos.js diff --git a/test-macos.js b/test-macos.js new file mode 100644 index 000000000..ca0047a98 --- /dev/null +++ b/test-macos.js @@ -0,0 +1,15 @@ +import path from 'path' +import { mkdirSync } from 'node:fs' +import envPaths from 'env-paths' + +const paths = envPaths('test') +console.log(paths) + +const d = path.join(paths.temp, 'dir') +mkdirSync(d, { recursive: true }) +console.log(d) + +process.chdir(d) +const cwd = process.cwd() +console.log(cwd) + From 9f2b7f416299a0408ef3c9324e5e213ca6984cba Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:52:09 -0500 Subject: [PATCH 17/25] Debug MacOS test failure --- .github/workflows/build.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 93c3db560..40436ec59 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -81,6 +81,8 @@ jobs: # - name: Bundle docs-search client # run: node ./bin/bundle.js bundle -p docs-search-client-halogen + - node ./test-macos.js + - name: Run tests run: node ./bin/bundle.js test -- -E "(warns about a malformed config, but stops parsing down the tree)|(#1208)" From be6712e266eb8e691b7887a8c5bb09443ba79e5f Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:52:30 -0500 Subject: [PATCH 18/25] Debug MacOS test failure --- .github/workflows/build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 40436ec59..dea4327a3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -69,6 +69,8 @@ jobs: - name: Install NPM dependencies run: npm ci + - node ./test-macos.js + - name: Install PureScript dependencies run: spago install @@ -81,8 +83,6 @@ jobs: # - name: Bundle docs-search client # run: node ./bin/bundle.js bundle -p docs-search-client-halogen - - node ./test-macos.js - - name: Run tests run: node ./bin/bundle.js test -- -E "(warns about a malformed config, but stops parsing down the tree)|(#1208)" From 38ff466bed3705acfeb585631974678a74b51918 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:53:09 -0500 Subject: [PATCH 19/25] Debug MacOS test failure --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index dea4327a3..3f0520530 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -69,7 +69,7 @@ jobs: - name: Install NPM dependencies run: npm ci - - node ./test-macos.js + - run: node ./test-macos.js - name: Install PureScript dependencies run: spago install From eb0df61db4d33d346ddf3470e6f45c089dad5702 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 17:55:06 -0500 Subject: [PATCH 20/25] Remove debug cruft --- .github/workflows/build.yml | 12 +++++------- spago.yaml | 1 - test-macos.js | 15 --------------- 3 files changed, 5 insertions(+), 23 deletions(-) delete mode 100644 test-macos.js diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3f0520530..f4f545a46 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -13,9 +13,9 @@ jobs: strategy: matrix: include: - # - os: ubuntu-latest + - os: ubuntu-latest - os: macOS-latest - # - os: windows-latest + - os: windows-latest steps: # We set LF endings so that the Windows environment is consistent with the rest # See here for context: https://github.com/actions/checkout/issues/135 @@ -69,8 +69,6 @@ jobs: - name: Install NPM dependencies run: npm ci - - run: node ./test-macos.js - - name: Install PureScript dependencies run: spago install @@ -80,11 +78,11 @@ jobs: - name: Bootstrap executable run: node ./bin/index.dev.js bundle -p spago-bin - # - name: Bundle docs-search client - # run: node ./bin/bundle.js bundle -p docs-search-client-halogen + - name: Bundle docs-search client + run: node ./bin/bundle.js bundle -p docs-search-client-halogen - name: Run tests - run: node ./bin/bundle.js test -- -E "(warns about a malformed config, but stops parsing down the tree)|(#1208)" + run: node ./bin/bundle.js test - name: Check formatting (Linux only) if: matrix.os == 'ubuntu-latest' diff --git a/spago.yaml b/spago.yaml index 909c98227..a7711a12a 100644 --- a/spago.yaml +++ b/spago.yaml @@ -81,7 +81,6 @@ package: - quickcheck - spec - spec-node - - debug workspace: packageSet: registry: 59.0.0 diff --git a/test-macos.js b/test-macos.js deleted file mode 100644 index ca0047a98..000000000 --- a/test-macos.js +++ /dev/null @@ -1,15 +0,0 @@ -import path from 'path' -import { mkdirSync } from 'node:fs' -import envPaths from 'env-paths' - -const paths = envPaths('test') -console.log(paths) - -const d = path.join(paths.temp, 'dir') -mkdirSync(d, { recursive: true }) -console.log(d) - -process.chdir(d) -const cwd = process.cwd() -console.log(cwd) - From 893d7c8fe6aff892e0f260b81ba33a3271141cdc Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 18:01:37 -0500 Subject: [PATCH 21/25] Remove more debug cruft --- test/Spago/Config.purs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 6d47d2ffe..0a727fcef 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -4,7 +4,6 @@ import Test.Prelude import Codec.JSON.DecodeError as CJ import Data.String as String -import Debug (traceM) import Registry.License as License import Registry.Location (Location(..)) import Registry.PackageName as PackageName From 2fb281b685d2c87e9df73d18b38865a9fc6f75d2 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Mon, 3 Feb 2025 18:31:33 -0500 Subject: [PATCH 22/25] Can't forget about Windows --- test/Spago/Config.purs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Spago/Config.purs b/test/Spago/Config.purs index 0a727fcef..35b8b4322 100644 --- a/test/Spago/Config.purs +++ b/test/Spago/Config.purs @@ -132,8 +132,8 @@ spec = , stderrFile: Just (fixture "config/malformed-configs/from-root.txt") , sanitize: String.trim - >>> String.replaceAll (String.Pattern "\\") (String.Replacement "/") >>> String.replaceAll (String.Pattern $ Path.toRaw testCwd) (String.Replacement "") + >>> String.replaceAll (String.Pattern "\\") (String.Replacement "/") } where From f6d9ec905e4138039443f7c171ae4987c182ad07 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Sat, 8 Feb 2025 20:08:08 -0500 Subject: [PATCH 23/25] Review feedback: rename variables, add comment, add type signatures --- src/Spago/Config.purs | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 6bef2e50d..1527c5a88 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -190,6 +190,8 @@ discoverWorkspace options cwd = do State.execStateT (loadSubprojectConfigs rootPath) { loadedPackages: upTree.loadedPackages, blockedSubtrees: [] } + -- Note: we migrate configs only at this point - this is because we read a whole lot of them but we are + -- supposed to ignore any subtrees that contain a different workspace, and those we don't want to migrate migrateConfigsWhereNeeded rootPath loadedPackages packagesByName <- @@ -197,9 +199,9 @@ discoverWorkspace options cwd = do for (Map.toUnfoldable loadedPackages :: Array _) \(path /\ { package, config }) -> do hasTests <- FS.exists (path "test") let - wsp :: WorkspacePackage - wsp = { package, path: path `Path.relativeTo` rootPath, doc: Just config.doc, hasTests } - pure (package.name /\ wsp) + workspacePackage :: WorkspacePackage + workspacePackage = { package, path: path `Path.relativeTo` rootPath, doc: Just config.doc, hasTests } + pure (package.name /\ workspacePackage) selected <- determineSelectedPackage @@ -209,11 +211,11 @@ discoverWorkspace options cwd = do , loadedPackages: packagesByName } - lockfile <- + eitherLockfile <- loadLockfile { pureBuild: options.pureBuild, workspaceConfig: workspace.config, loadedPackages: packagesByName, rootPath } { packageSet, compiler } <- - loadPackageSet { workspaceConfig: workspace.config, loadedPackages: packagesByName, rootPath, lockfile } + loadPackageSet { workspaceConfig: workspace.config, loadedPackages: packagesByName, rootPath, eitherLockfile } pure { rootPath @@ -236,6 +238,12 @@ discoverWorkspace options cwd = do readConfig' :: ∀ s. _ -> State.StateT s _ _ readConfig' = State.lift <<< readConfig + walkDirectoriesUpFrom :: + GlobalPath + -> State.StateT + { loadedPackages :: Map _ _, misnamedConfigs :: Array GlobalPath, closestPackage :: Maybe _ } + _ + { workspace :: _, rootPath :: _ } walkDirectoriesUpFrom dir = do maybeConfig <- tryReadConfigAt configFile @@ -265,6 +273,12 @@ discoverWorkspace options cwd = do configFile = dir spagoYaml parentDir = Path.dirname dir + loadSubprojectConfigs :: + RootPath + -> State.StateT + { loadedPackages :: Map _ _, blockedSubtrees :: Array GlobalPath } + _ + Unit loadSubprojectConfigs rootPath = do candidates <- liftAff $ Glob.gitignoringGlob { root: rootPath @@ -444,21 +458,21 @@ loadPackageSet . { workspaceConfig :: Core.WorkspaceConfig , loadedPackages :: Map PackageName WorkspacePackage , rootPath :: RootPath - , lockfile :: Either String Lockfile + , eitherLockfile :: Either String Lockfile } -> Spago (Registry.RegistryEnv a) { packageSet :: PackageSet, compiler :: Range } -loadPackageSet { lockfile, workspaceConfig, loadedPackages, rootPath } = do +loadPackageSet { eitherLockfile, workspaceConfig, loadedPackages, rootPath } = do { offline } <- ask - packageSetInfo <- case lockfile, workspaceConfig.packageSet of + packageSetInfo <- case eitherLockfile, workspaceConfig.packageSet of _, Nothing -> do logDebug "Did not find a package set in your config, using Registry solver" pure Nothing -- If there's a lockfile we don't attempt to fetch the package set from the registry -- repo nor from the internet, since we already have the whole set right there - Right lf, _ -> do + Right lockfile, _ -> do logDebug "Found the lockfile, using the package set from there" - pure lf.workspace.package_set + pure lockfile.workspace.package_set Left reason, Just address@(Core.SetFromRegistry { registry: v }) -> do logDebug reason @@ -548,7 +562,7 @@ loadPackageSet { lockfile, workspaceConfig, loadedPackages, rootPath } = do <> map (\p -> indent $ toDoc $ "- " <> PackageName.print p) (Array.fromFoldable localPackagesOverlap) pure - { packageSet: { buildType, lockfile } + { packageSet: { buildType, lockfile: eitherLockfile } , compiler: packageSetInfo <#> _.compiler # fromMaybe Core.widestRange } From d1a15aff0e3b47267fb060c9816c34904b94c21c Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Sat, 8 Feb 2025 20:57:36 -0500 Subject: [PATCH 24/25] Autoformatter --- src/Spago/Config.purs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 1527c5a88..a133e295f 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -238,12 +238,12 @@ discoverWorkspace options cwd = do readConfig' :: ∀ s. _ -> State.StateT s _ _ readConfig' = State.lift <<< readConfig - walkDirectoriesUpFrom :: - GlobalPath + walkDirectoriesUpFrom + :: GlobalPath -> State.StateT - { loadedPackages :: Map _ _, misnamedConfigs :: Array GlobalPath, closestPackage :: Maybe _ } - _ - { workspace :: _, rootPath :: _ } + { loadedPackages :: Map _ _, misnamedConfigs :: Array GlobalPath, closestPackage :: Maybe _ } + _ + { workspace :: _, rootPath :: _ } walkDirectoriesUpFrom dir = do maybeConfig <- tryReadConfigAt configFile @@ -273,12 +273,12 @@ discoverWorkspace options cwd = do configFile = dir spagoYaml parentDir = Path.dirname dir - loadSubprojectConfigs :: - RootPath + loadSubprojectConfigs + :: RootPath -> State.StateT - { loadedPackages :: Map _ _, blockedSubtrees :: Array GlobalPath } - _ - Unit + { loadedPackages :: Map _ _, blockedSubtrees :: Array GlobalPath } + _ + Unit loadSubprojectConfigs rootPath = do candidates <- liftAff $ Glob.gitignoringGlob { root: rootPath From 57a476863b67f2e080727aeab3b9cd04531b4fbe Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Sun, 9 Feb 2025 11:10:44 -0500 Subject: [PATCH 25/25] Mention pruning reasoning --- src/Spago/Config.purs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index a133e295f..958cf7694 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -300,6 +300,10 @@ discoverWorkspace options cwd = do true, _ -> pure unit _, Just b -> do + -- We prune any configs that use a different workspace, + -- see https://github.com/purescript/spago/issues/951 + -- We also prune configs that are nested under a config that we couldn't read, + -- see discussion in https://github.com/purescript/spago/pull/1316#issuecomment-2629011927 logDebug $ "Not trying to load " <> Path.quote configFile <> " because it is nested under " <> Path.quote b pure unit false, Nothing ->