From ab822a4f2d7dec02e1ff2bf626c1ec885a66ad49 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Sat, 21 Sep 2024 00:03:02 -0400 Subject: [PATCH 1/6] Automatically fill in 'publish.location' in config when absent --- core/src/Config.purs | 1 + src/Spago/Command/Publish.purs | 32 ++++++++++++- src/Spago/Config.js | 8 +++- src/Spago/Config.purs | 36 ++++++++++----- .../1060-autofill-location/project/spago.yaml | 9 ++++ .../project/src/Main.purs | 4 ++ .../project/subdir1/spago.yaml | 6 +++ .../project/subdir1/src/Main.purs | 4 ++ .../project/subdir2/subdir3/spago.yaml | 6 +++ .../project/subdir2/subdir3/src/Main.purs | 4 ++ .../expected-spago.yaml | 10 +++++ .../expected-stderr.txt | 16 +++++++ .../scenario-no-origin/expected-stderr.txt | 16 +++++++ .../scenario-non-github/expected-spago.yaml | 11 +++++ .../scenario-non-github/expected-stderr.txt | 16 +++++++ .../scenario-root/expected-spago.yaml | 12 +++++ .../scenario-root/expected-stderr.txt | 16 +++++++ .../scenario-subdir/expected-spago.yaml | 10 +++++ .../scenario-subdir/expected-stderr.txt | 16 +++++++ test/Spago/Publish.purs | 45 +++++++++++++++++++ 20 files changed, 265 insertions(+), 13 deletions(-) create mode 100644 test-fixtures/publish/1060-autofill-location/project/spago.yaml create mode 100644 test-fixtures/publish/1060-autofill-location/project/src/Main.purs create mode 100644 test-fixtures/publish/1060-autofill-location/project/subdir1/spago.yaml create mode 100644 test-fixtures/publish/1060-autofill-location/project/subdir1/src/Main.purs create mode 100644 test-fixtures/publish/1060-autofill-location/project/subdir2/subdir3/spago.yaml create mode 100644 test-fixtures/publish/1060-autofill-location/project/subdir2/subdir3/src/Main.purs create mode 100644 test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml create mode 100644 test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt create mode 100644 test-fixtures/publish/1060-autofill-location/scenario-no-origin/expected-stderr.txt create mode 100644 test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-spago.yaml create mode 100644 test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-stderr.txt create mode 100644 test-fixtures/publish/1060-autofill-location/scenario-root/expected-spago.yaml create mode 100644 test-fixtures/publish/1060-autofill-location/scenario-root/expected-stderr.txt create mode 100644 test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-spago.yaml create mode 100644 test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-stderr.txt diff --git a/core/src/Config.purs b/core/src/Config.purs index 5dd95970e..47dbdf25b 100644 --- a/core/src/Config.purs +++ b/core/src/Config.purs @@ -31,6 +31,7 @@ module Spago.Core.Config , parseBundleType , parsePlatform , printSpagoRange + , publishLocationCodec , remotePackageCodec , setAddressCodec , widestRange diff --git a/src/Spago/Command/Publish.purs b/src/Spago/Command/Publish.purs index f8b88207b..cb76376fc 100644 --- a/src/Spago/Command/Publish.purs +++ b/src/Spago/Command/Publish.purs @@ -155,7 +155,11 @@ publish _args = do , "See the configuration file's documentation: https://github.com/purescript/spago#the-configuration-file" ] Just publishConfig -> case publishConfig.location of - Nothing -> addError $ toDoc "Need to specify a publish.location field." + Nothing -> do + addedToConfig <- inferLocationAndWriteToConfig selected + if addedToConfig + then addError $ toDoc "The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed." + else addError $ toDoc "The `publish.location` field is empty and Spago is unable to infer it from Git remotes." Just location -> do -- Get the metadata file for this package. -- It will exist if the package has been published at some point, it will not if the package is new. @@ -476,5 +480,31 @@ locationIsInGitRemotes location = do Location.Git { url } -> r.url == url Location.GitHub { owner, repo } -> r.owner == owner && r.repo == repo +inferLocationAndWriteToConfig :: ∀ a. WorkspacePackage -> Spago (PublishEnv a) Boolean +inferLocationAndWriteToConfig selectedPackage = do + Git.getRemotes Nothing >>= case _ of + Left err -> + die $ toDoc err + Right remotes -> + case Array.find (_.name >>> eq "origin") remotes of + Nothing -> + pure false + Just origin -> do + let subdir + | Config.isRootPackage selectedPackage = Nothing + | otherwise = Just selectedPackage.path + configPath + | Config.isRootPackage selectedPackage = "spago.yaml" + | otherwise = Path.concat [ selectedPackage.path, "spago.yaml" ] + location + | String.contains (String.Pattern "github.com") origin.url = + Location.GitHub { owner: origin.owner, repo: origin.repo, subdir } + | otherwise = + Location.Git { url: origin.url, subdir } + + liftEffect $ Config.addPublishLocationToConfig selectedPackage.doc location + liftAff $ FS.writeYamlDocFile configPath selectedPackage.doc + pure true + baseApi :: String baseApi = "https://registry.purescript.org" diff --git a/src/Spago/Config.js b/src/Spago/Config.js index 119c533fb..5bb29bae4 100644 --- a/src/Spago/Config.js +++ b/src/Spago/Config.js @@ -19,8 +19,8 @@ export function addPackagesToConfigImpl(doc, isTest, newPkgs) { } })(); - // Stringify this collection as - // - dep1 + // Stringify this collection as + // - dep1 // - dep2 // rather than // [ dep1, dep2 ] @@ -121,3 +121,7 @@ export function migrateV1ConfigImpl(doc) { return doc; } } + +export function addPublishLocationToConfigImpl(doc, location) { + doc.setIn(["package", "publish", "location"], location); +} diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 0f510f0fa..5759079dc 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -1,27 +1,30 @@ module Spago.Config ( BuildType(..) , Package(..) - , PackageSet(..) , PackageMap + , PackageSet(..) , WithTestGlobs(..) , Workspace , WorkspaceBuildOptions , WorkspacePackage , addPackagesToConfig + , addPublishLocationToConfig , addRangesToConfig - , removePackagesFromConfig - , rootPackageToWorkspacePackage - , getPackageLocation , fileSystemCharEscape - , getWorkspacePackages + , getPackageLocation , getTopologicallySortedWorkspacePackages + , getWorkspacePackages + , isRootPackage , module Core + , readConfig , readWorkspace - , sourceGlob + , removePackagesFromConfig + , rootPackageToWorkspacePackage , setPackageSetVersionInConfig + , sourceGlob , workspacePackageToLockfilePackage - , readConfig - ) where + ) + where import Spago.Prelude @@ -51,12 +54,14 @@ import Dodo as Log import Effect.Aff as Aff import Effect.Uncurried (EffectFn2, EffectFn3, runEffectFn2, runEffectFn3) import Foreign.Object as Foreign +import JSON (JSON) import Node.Path as Path import Registry.Internal.Codec as Internal.Codec import Registry.PackageName as PackageName import Registry.PackageSet as Registry.PackageSet import Registry.Range as Range import Registry.Version as Version +import Spago.Core.Config (publishLocationCodec) import Spago.Core.Config as Core import Spago.FS as FS import Spago.Glob as Glob @@ -164,6 +169,12 @@ type ReadWorkspaceOptions = , migrateConfig :: Boolean } +isRootPackage :: WorkspacePackage -> Boolean +isRootPackage p = p.path == rootPackagePath + +rootPackagePath :: String +rootPackagePath = "./" + -- | 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 a) Workspace @@ -446,7 +457,7 @@ rootPackageToWorkspacePackage -> m WorkspacePackage rootPackageToWorkspacePackage { rootPackage, workspaceDoc } = do hasTests <- liftEffect $ FS.exists "test" - pure { path: "./", doc: workspaceDoc, package: rootPackage, hasTests } + pure { path: rootPackagePath, doc: workspaceDoc, package: rootPackage, hasTests } workspacePackageToLockfilePackage :: WorkspacePackage -> Tuple PackageName Lock.WorkspaceLockPackage workspacePackageToLockfilePackage { path, package } = Tuple package.name @@ -619,8 +630,13 @@ addRangesToConfig doc = runEffectFn2 addRangesToConfigImpl doc <<< map (\(Tuple name range) -> Tuple (PackageName.print name) (Core.printSpagoRange range)) <<< (Map.toUnfoldable :: Map _ _ -> Array _) +addPublishLocationToConfig :: YamlDoc Core.Config -> Location -> Effect Unit +addPublishLocationToConfig doc loc = + runEffectFn2 addPublishLocationToConfigImpl doc (CJ.encode publishLocationCodec loc) + foreign import setPackageSetVersionInConfigImpl :: EffectFn2 (YamlDoc Core.Config) String Unit foreign import addPackagesToConfigImpl :: EffectFn3 (YamlDoc Core.Config) Boolean (Array String) Unit foreign import removePackagesFromConfigImpl :: EffectFn3 (YamlDoc Core.Config) Boolean (PackageName -> Boolean) Unit foreign import addRangesToConfigImpl :: EffectFn2 (YamlDoc Core.Config) (Foreign.Object String) Unit -foreign import migrateV1ConfigImpl :: forall a. YamlDoc a -> Nullable (YamlDoc Core.Config) +foreign import migrateV1ConfigImpl :: ∀ a. YamlDoc a -> Nullable (YamlDoc Core.Config) +foreign import addPublishLocationToConfigImpl :: EffectFn2 (YamlDoc Core.Config) JSON Unit diff --git a/test-fixtures/publish/1060-autofill-location/project/spago.yaml b/test-fixtures/publish/1060-autofill-location/project/spago.yaml new file mode 100644 index 000000000..787e418f6 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/project/spago.yaml @@ -0,0 +1,9 @@ +package: + name: aaa + dependencies: [] + publish: + version: 0.0.1 + license: MIT +workspace: + packageSet: + registry: 58.0.0 diff --git a/test-fixtures/publish/1060-autofill-location/project/src/Main.purs b/test-fixtures/publish/1060-autofill-location/project/src/Main.purs new file mode 100644 index 000000000..0cad20d81 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/project/src/Main.purs @@ -0,0 +1,4 @@ +module Lib where + +anExport :: String +anExport = "Hello, World!" diff --git a/test-fixtures/publish/1060-autofill-location/project/subdir1/spago.yaml b/test-fixtures/publish/1060-autofill-location/project/subdir1/spago.yaml new file mode 100644 index 000000000..2e2680441 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/project/subdir1/spago.yaml @@ -0,0 +1,6 @@ +package: + name: bbb + dependencies: [] + publish: + version: 0.0.1 + license: MIT diff --git a/test-fixtures/publish/1060-autofill-location/project/subdir1/src/Main.purs b/test-fixtures/publish/1060-autofill-location/project/subdir1/src/Main.purs new file mode 100644 index 000000000..583ffd295 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/project/subdir1/src/Main.purs @@ -0,0 +1,4 @@ +module Lib1 where + +anExport :: String +anExport = "Hello, World!" diff --git a/test-fixtures/publish/1060-autofill-location/project/subdir2/subdir3/spago.yaml b/test-fixtures/publish/1060-autofill-location/project/subdir2/subdir3/spago.yaml new file mode 100644 index 000000000..c158134ca --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/project/subdir2/subdir3/spago.yaml @@ -0,0 +1,6 @@ +package: + name: ccc + dependencies: [] + publish: + version: 0.0.1 + license: MIT diff --git a/test-fixtures/publish/1060-autofill-location/project/subdir2/subdir3/src/Main.purs b/test-fixtures/publish/1060-autofill-location/project/subdir2/subdir3/src/Main.purs new file mode 100644 index 000000000..edd726f18 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/project/subdir2/subdir3/src/Main.purs @@ -0,0 +1,4 @@ +module Lib3 where + +anExport :: String +anExport = "Hello, World!" diff --git a/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml b/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml new file mode 100644 index 000000000..5989a7fb7 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml @@ -0,0 +1,10 @@ +package: + name: ccc + dependencies: [] + publish: + version: 0.0.1 + license: MIT + location: + githubOwner: purescript + githubRepo: aaa + subdir: subdir2/subdir3 diff --git a/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt new file mode 100644 index 000000000..5faf42fbc --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt @@ -0,0 +1,16 @@ +Reading Spago workspace configuration... + +✓ Selecting package to build: ccc + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✓ Build succeeded. + +Your package "ccc" is not ready for publishing yet, encountered 1 error: + + +✘ The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed. diff --git a/test-fixtures/publish/1060-autofill-location/scenario-no-origin/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-no-origin/expected-stderr.txt new file mode 100644 index 000000000..73da5cb6c --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/scenario-no-origin/expected-stderr.txt @@ -0,0 +1,16 @@ +Reading Spago workspace configuration... + +✓ Selecting package to build: aaa + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✓ Build succeeded. + +Your package "aaa" is not ready for publishing yet, encountered 1 error: + + +✘ The `publish.location` field is empty and Spago is unable to infer it from Git remotes. diff --git a/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-spago.yaml b/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-spago.yaml new file mode 100644 index 000000000..241b95b83 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-spago.yaml @@ -0,0 +1,11 @@ +package: + name: aaa + dependencies: [] + publish: + version: 0.0.1 + license: MIT + location: + gitUrl: https://not.git-hub.net/foo/bar.git +workspace: + packageSet: + registry: 58.0.0 diff --git a/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-stderr.txt new file mode 100644 index 000000000..fa4356383 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-stderr.txt @@ -0,0 +1,16 @@ +Reading Spago workspace configuration... + +✓ Selecting package to build: aaa + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✓ Build succeeded. + +Your package "aaa" is not ready for publishing yet, encountered 1 error: + + +✘ The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed. diff --git a/test-fixtures/publish/1060-autofill-location/scenario-root/expected-spago.yaml b/test-fixtures/publish/1060-autofill-location/scenario-root/expected-spago.yaml new file mode 100644 index 000000000..dc4748e82 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/scenario-root/expected-spago.yaml @@ -0,0 +1,12 @@ +package: + name: aaa + dependencies: [] + publish: + version: 0.0.1 + license: MIT + location: + githubOwner: purescript + githubRepo: aaa +workspace: + packageSet: + registry: 58.0.0 diff --git a/test-fixtures/publish/1060-autofill-location/scenario-root/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-root/expected-stderr.txt new file mode 100644 index 000000000..fa4356383 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/scenario-root/expected-stderr.txt @@ -0,0 +1,16 @@ +Reading Spago workspace configuration... + +✓ Selecting package to build: aaa + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✓ Build succeeded. + +Your package "aaa" is not ready for publishing yet, encountered 1 error: + + +✘ The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed. diff --git a/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-spago.yaml b/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-spago.yaml new file mode 100644 index 000000000..2d1c600f8 --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-spago.yaml @@ -0,0 +1,10 @@ +package: + name: bbb + dependencies: [] + publish: + version: 0.0.1 + license: MIT + location: + githubOwner: purescript + githubRepo: aaa + subdir: subdir1 diff --git a/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-stderr.txt new file mode 100644 index 000000000..26ae6739d --- /dev/null +++ b/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-stderr.txt @@ -0,0 +1,16 @@ +Reading Spago workspace configuration... + +✓ Selecting package to build: bbb + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✓ Build succeeded. + +Your package "bbb" is not ready for publishing yet, encountered 1 error: + + +✘ The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed. diff --git a/test/Spago/Publish.purs b/test/Spago/Publish.purs index 36bdd247c..3747bf865 100644 --- a/test/Spago/Publish.purs +++ b/test/Spago/Publish.purs @@ -122,6 +122,51 @@ spec = Spec.around withTempDir do rmRf ".spago/p/console-6.1.0/output" spago [ "publish", "--offline" ] >>= shouldBeFailureErr' (fixture "publish/1110-solver-different-version/failure-stderr.txt") + Spec.describe "#1060 auto-filling the `publish.location` field" do + let prepareProject spago fixture = do + FS.copyTree { src: fixture "publish/1060-autofill-location/project", dst: "." } + spago [ "build" ] >>= shouldBeSuccess + doTheGitThing + spago [ "fetch" ] >>= shouldBeSuccess + + Spec.it "happens for root package" \{ fixture, spago } -> do + prepareProject spago fixture + spago [ "publish", "-p", "aaa", "--offline" ] >>= + shouldBeFailureErr (fixture "publish/1060-autofill-location/scenario-root/expected-stderr.txt") + checkFixture "spago.yaml" + (fixture "publish/1060-autofill-location/scenario-root/expected-spago.yaml") + + Spec.it "happens for non-root package" \{ fixture, spago } -> do + prepareProject spago fixture + spago [ "publish", "-p", "bbb", "--offline" ] >>= + shouldBeFailureErr (fixture "publish/1060-autofill-location/scenario-subdir/expected-stderr.txt") + checkFixture "subdir1/spago.yaml" + (fixture "publish/1060-autofill-location/scenario-subdir/expected-spago.yaml") + + Spec.it "happens for nested non-root package" \{ fixture, spago } -> do + prepareProject spago fixture + spago [ "publish", "-p", "ccc", "--offline" ] >>= + shouldBeFailureErr (fixture "publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt") + checkFixture "subdir2/subdir3/spago.yaml" + (fixture "publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml") + + Spec.it "uses URL when not a GitHub remote" \{ fixture, spago } -> do + prepareProject spago fixture + git [ "remote", "set-url", "origin", "https://not.git-hub.net/foo/bar.git" ] + spago [ "publish", "-p", "aaa", "--offline" ] >>= + shouldBeFailureErr (fixture "publish/1060-autofill-location/scenario-non-github/expected-stderr.txt") + checkFixture "spago.yaml" + (fixture "publish/1060-autofill-location/scenario-non-github/expected-spago.yaml") + + Spec.it "prints error when no origin remote" \{ fixture, spago } -> do + prepareProject spago fixture + git [ "remote", "remove", "origin" ] + git [ "remote", "add", "upstream", "git@github.com:foo/bar.git" ] + spago [ "publish", "-p", "aaa", "--offline" ] >>= + shouldBeFailureErr (fixture "publish/1060-autofill-location/scenario-no-origin/expected-stderr.txt") + checkFixture "spago.yaml" + (fixture "publish/1060-autofill-location/project/spago.yaml") + doTheGitThing :: Aff Unit doTheGitThing = do git [ "init" ] From 2bf72d2a879f80800f319bdd3374a316cf06d4b7 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Sat, 21 Sep 2024 00:06:12 -0400 Subject: [PATCH 2/6] Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c0fc05be..38d8dbb3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ Other improvements: before trying to build with them. - Spago no longer ignores config fields that it doesn't recognize. This should help catch typos in field names. +- 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`. ## [0.21.0] - 2023-05-04 From 9cd73b05208ead8ddbfa78c31672e8e8b92273c2 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Sat, 21 Sep 2024 14:38:16 -0400 Subject: [PATCH 3/6] Tidy --- src/Spago/Command/Publish.purs | 30 ++++++++++++++++-------------- src/Spago/Config.purs | 3 +-- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/Spago/Command/Publish.purs b/src/Spago/Command/Publish.purs index cb76376fc..0ed6cc26c 100644 --- a/src/Spago/Command/Publish.purs +++ b/src/Spago/Command/Publish.purs @@ -157,9 +157,10 @@ publish _args = do Just publishConfig -> case publishConfig.location of Nothing -> do addedToConfig <- inferLocationAndWriteToConfig selected - if addedToConfig - then addError $ toDoc "The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed." - else addError $ toDoc "The `publish.location` field is empty and Spago is unable to infer it from Git remotes." + if addedToConfig then + addError $ toDoc "The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed." + else + addError $ toDoc "The `publish.location` field is empty and Spago is unable to infer it from Git remotes." Just location -> do -- Get the metadata file for this package. -- It will exist if the package has been published at some point, it will not if the package is new. @@ -490,17 +491,18 @@ inferLocationAndWriteToConfig selectedPackage = do Nothing -> pure false Just origin -> do - let subdir - | Config.isRootPackage selectedPackage = Nothing - | otherwise = Just selectedPackage.path - configPath - | Config.isRootPackage selectedPackage = "spago.yaml" - | otherwise = Path.concat [ selectedPackage.path, "spago.yaml" ] - location - | String.contains (String.Pattern "github.com") origin.url = - Location.GitHub { owner: origin.owner, repo: origin.repo, subdir } - | otherwise = - Location.Git { url: origin.url, subdir } + let + subdir + | Config.isRootPackage selectedPackage = Nothing + | otherwise = Just selectedPackage.path + configPath + | Config.isRootPackage selectedPackage = "spago.yaml" + | otherwise = Path.concat [ selectedPackage.path, "spago.yaml" ] + location + | String.contains (String.Pattern "github.com") origin.url = + Location.GitHub { owner: origin.owner, repo: origin.repo, subdir } + | otherwise = + Location.Git { url: origin.url, subdir } liftEffect $ Config.addPublishLocationToConfig selectedPackage.doc location liftAff $ FS.writeYamlDocFile configPath selectedPackage.doc diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 5759079dc..5940bb773 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -23,8 +23,7 @@ module Spago.Config , setPackageSetVersionInConfig , sourceGlob , workspacePackageToLockfilePackage - ) - where + ) where import Spago.Prelude From 873fbfeb6de756c4dd9773de14fda88cc0200226 Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Sat, 21 Sep 2024 15:04:23 -0400 Subject: [PATCH 4/6] With forward slashes we celebrate the majesty of our Lord and Savior the mighty Windows --- src/Spago/Command/Publish.purs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Spago/Command/Publish.purs b/src/Spago/Command/Publish.purs index 0ed6cc26c..9d11f78e9 100644 --- a/src/Spago/Command/Publish.purs +++ b/src/Spago/Command/Publish.purs @@ -494,7 +494,7 @@ inferLocationAndWriteToConfig selectedPackage = do let subdir | Config.isRootPackage selectedPackage = Nothing - | otherwise = Just selectedPackage.path + | otherwise = Just $ withForwardSlashes selectedPackage.path configPath | Config.isRootPackage selectedPackage = "spago.yaml" | otherwise = Path.concat [ selectedPackage.path, "spago.yaml" ] From 0ae893e4eec2c1c795cfeff0839c9e7f1aa6ea1d Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Tue, 29 Oct 2024 11:41:11 -0400 Subject: [PATCH 5/6] Review feedback --- src/Spago/Command/Publish.purs | 39 +++++++++++++------ .../expected-spago.yaml | 10 ----- .../expected-stderr.txt | 4 +- .../scenario-no-origin/expected-stderr.txt | 4 +- .../scenario-non-github/expected-spago.yaml | 2 - .../scenario-non-github/expected-stderr.txt | 5 +-- .../scenario-root/expected-stderr.txt | 2 +- .../scenario-subdir/expected-spago.yaml | 10 ----- .../scenario-subdir/expected-stderr.txt | 4 +- test/Spago/Publish.purs | 10 ++--- 10 files changed, 39 insertions(+), 51 deletions(-) delete mode 100644 test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml delete mode 100644 test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-spago.yaml diff --git a/src/Spago/Command/Publish.purs b/src/Spago/Command/Publish.purs index 9d11f78e9..c89d4c1b7 100644 --- a/src/Spago/Command/Publish.purs +++ b/src/Spago/Command/Publish.purs @@ -158,9 +158,14 @@ publish _args = do Nothing -> do addedToConfig <- inferLocationAndWriteToConfig selected if addedToConfig then - addError $ toDoc "The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed." + addError $ toDoc $ + "The `publish.location` field of your `spago.yaml` file was empty. Spago filled it in, please commit it and try again." else - addError $ toDoc "The `publish.location` field is empty and Spago is unable to infer it from Git remotes." + addError $ toDoc + [ "The `publish.location` field is not present in the `spago.yaml` file." + , "Spago is unable to infer it from Git remotes, so please populate it manually." + , "See the docs for more info: https://github.com/purescript/spago#the-configuration-file" + ] Just location -> do -- Get the metadata file for this package. -- It will exist if the package has been published at some point, it will not if the package is new. @@ -475,7 +480,7 @@ locationIsInGitRemotes location = do else Git.getRemotes Nothing >>= case _ of Left err -> - die $ toDoc err + die [ toDoc "Couldn't parse Git remotes: ", err ] Right remotes -> pure $ remotes # Array.any \r -> case location of Location.Git { url } -> r.url == url @@ -491,18 +496,30 @@ inferLocationAndWriteToConfig selectedPackage = do Nothing -> pure false Just origin -> do + -- TODO: at the moment the registry supports only `subdir: + -- Nothing` cases, so we error out when the package is nested. + -- Once the registry supports subdirs, the `else` branch should + -- return `selectedPackage.path` + subdir <- + if Config.isRootPackage selectedPackage + then pure Nothing + else die "The registry does not support nested packages yet. Only the root package can be published." + + -- TODO: similarly, the registry only supports `GitHub` packages, so + -- we error out in other cases. Once the registry supports non-GitHub + -- packages, the `else` branch should return `Git { url: origin.url, subdir }` + location <- + if String.contains (String.Pattern "github.com") origin.url + then pure $ Location.GitHub { owner: origin.owner, repo: origin.repo, subdir } + else die + [ "The registry only supports packages hosted on GitHub at the moment." + , "Cannot publish this package because it is hosted at " <> origin.url + ] + let - subdir - | Config.isRootPackage selectedPackage = Nothing - | otherwise = Just $ withForwardSlashes selectedPackage.path configPath | Config.isRootPackage selectedPackage = "spago.yaml" | otherwise = Path.concat [ selectedPackage.path, "spago.yaml" ] - location - | String.contains (String.Pattern "github.com") origin.url = - Location.GitHub { owner: origin.owner, repo: origin.repo, subdir } - | otherwise = - Location.Git { url: origin.url, subdir } liftEffect $ Config.addPublishLocationToConfig selectedPackage.doc location liftAff $ FS.writeYamlDocFile configPath selectedPackage.doc diff --git a/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml b/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml deleted file mode 100644 index 5989a7fb7..000000000 --- a/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml +++ /dev/null @@ -1,10 +0,0 @@ -package: - name: ccc - dependencies: [] - publish: - version: 0.0.1 - license: MIT - location: - githubOwner: purescript - githubRepo: aaa - subdir: subdir2/subdir3 diff --git a/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt index 5faf42fbc..00c341ac2 100644 --- a/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt +++ b/test-fixtures/publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt @@ -10,7 +10,5 @@ Errors 0 0 0 ✓ Build succeeded. -Your package "ccc" is not ready for publishing yet, encountered 1 error: - -✘ The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed. +✘ The registry does not support nested packages yet. Only the root package can be published. diff --git a/test-fixtures/publish/1060-autofill-location/scenario-no-origin/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-no-origin/expected-stderr.txt index 73da5cb6c..9ea7b6cae 100644 --- a/test-fixtures/publish/1060-autofill-location/scenario-no-origin/expected-stderr.txt +++ b/test-fixtures/publish/1060-autofill-location/scenario-no-origin/expected-stderr.txt @@ -13,4 +13,6 @@ Errors 0 0 0 Your package "aaa" is not ready for publishing yet, encountered 1 error: -✘ The `publish.location` field is empty and Spago is unable to infer it from Git remotes. +✘ The `publish.location` field is not present in the `spago.yaml` file. +Spago is unable to infer it from Git remotes, so please populate it manually. +See the docs for more info: https://github.com/purescript/spago#the-configuration-file diff --git a/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-spago.yaml b/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-spago.yaml index 241b95b83..787e418f6 100644 --- a/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-spago.yaml +++ b/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-spago.yaml @@ -4,8 +4,6 @@ package: publish: version: 0.0.1 license: MIT - location: - gitUrl: https://not.git-hub.net/foo/bar.git workspace: packageSet: registry: 58.0.0 diff --git a/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-stderr.txt index fa4356383..e144c1606 100644 --- a/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-stderr.txt +++ b/test-fixtures/publish/1060-autofill-location/scenario-non-github/expected-stderr.txt @@ -10,7 +10,6 @@ Errors 0 0 0 ✓ Build succeeded. -Your package "aaa" is not ready for publishing yet, encountered 1 error: - -✘ The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed. +✘ The registry only supports packages hosted on GitHub at the moment. +Cannot publish this package because it is hosted at https://not.git-hub.net/foo/bar.git diff --git a/test-fixtures/publish/1060-autofill-location/scenario-root/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-root/expected-stderr.txt index fa4356383..6173342fc 100644 --- a/test-fixtures/publish/1060-autofill-location/scenario-root/expected-stderr.txt +++ b/test-fixtures/publish/1060-autofill-location/scenario-root/expected-stderr.txt @@ -13,4 +13,4 @@ Errors 0 0 0 Your package "aaa" is not ready for publishing yet, encountered 1 error: -✘ The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed. +✘ The `publish.location` field of your `spago.yaml` file was empty. Spago filled it in, please commit it and try again. diff --git a/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-spago.yaml b/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-spago.yaml deleted file mode 100644 index 2d1c600f8..000000000 --- a/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-spago.yaml +++ /dev/null @@ -1,10 +0,0 @@ -package: - name: bbb - dependencies: [] - publish: - version: 0.0.1 - license: MIT - location: - githubOwner: purescript - githubRepo: aaa - subdir: subdir1 diff --git a/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-stderr.txt b/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-stderr.txt index 26ae6739d..f89dda5e3 100644 --- a/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-stderr.txt +++ b/test-fixtures/publish/1060-autofill-location/scenario-subdir/expected-stderr.txt @@ -10,7 +10,5 @@ Errors 0 0 0 ✓ Build succeeded. -Your package "bbb" is not ready for publishing yet, encountered 1 error: - -✘ The `publish.location` field was empty. Spago filled it in, but the config file needs to be committed. +✘ The registry does not support nested packages yet. Only the root package can be published. diff --git a/test/Spago/Publish.purs b/test/Spago/Publish.purs index 3747bf865..188fce48b 100644 --- a/test/Spago/Publish.purs +++ b/test/Spago/Publish.purs @@ -136,21 +136,17 @@ spec = Spec.around withTempDir do checkFixture "spago.yaml" (fixture "publish/1060-autofill-location/scenario-root/expected-spago.yaml") - Spec.it "happens for non-root package" \{ fixture, spago } -> do + Spec.it "errors out for non-root package" \{ fixture, spago } -> do prepareProject spago fixture spago [ "publish", "-p", "bbb", "--offline" ] >>= shouldBeFailureErr (fixture "publish/1060-autofill-location/scenario-subdir/expected-stderr.txt") - checkFixture "subdir1/spago.yaml" - (fixture "publish/1060-autofill-location/scenario-subdir/expected-spago.yaml") - Spec.it "happens for nested non-root package" \{ fixture, spago } -> do + Spec.it "errors out for nested non-root package" \{ fixture, spago } -> do prepareProject spago fixture spago [ "publish", "-p", "ccc", "--offline" ] >>= shouldBeFailureErr (fixture "publish/1060-autofill-location/scenario-nested-subdir/expected-stderr.txt") - checkFixture "subdir2/subdir3/spago.yaml" - (fixture "publish/1060-autofill-location/scenario-nested-subdir/expected-spago.yaml") - Spec.it "uses URL when not a GitHub remote" \{ fixture, spago } -> do + Spec.it "errors out when not a GitHub remote" \{ fixture, spago } -> do prepareProject spago fixture git [ "remote", "set-url", "origin", "https://not.git-hub.net/foo/bar.git" ] spago [ "publish", "-p", "aaa", "--offline" ] >>= From da6d5178f8951154fee0ff0bc0df76b9ed689f7b Mon Sep 17 00:00:00 2001 From: Fyodor Soikin Date: Tue, 29 Oct 2024 11:43:58 -0400 Subject: [PATCH 6/6] Missed a spot + formatting --- src/Spago/Command/Publish.purs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Spago/Command/Publish.purs b/src/Spago/Command/Publish.purs index c89d4c1b7..d7ec35c8d 100644 --- a/src/Spago/Command/Publish.purs +++ b/src/Spago/Command/Publish.purs @@ -490,7 +490,7 @@ inferLocationAndWriteToConfig :: ∀ a. WorkspacePackage -> Spago (PublishEnv a) inferLocationAndWriteToConfig selectedPackage = do Git.getRemotes Nothing >>= case _ of Left err -> - die $ toDoc err + die [ toDoc "Couldn't parse Git remotes: ", err ] Right remotes -> case Array.find (_.name >>> eq "origin") remotes of Nothing -> @@ -501,17 +501,19 @@ inferLocationAndWriteToConfig selectedPackage = do -- Once the registry supports subdirs, the `else` branch should -- return `selectedPackage.path` subdir <- - if Config.isRootPackage selectedPackage - then pure Nothing - else die "The registry does not support nested packages yet. Only the root package can be published." + if Config.isRootPackage selectedPackage then + pure Nothing + else + die "The registry does not support nested packages yet. Only the root package can be published." -- TODO: similarly, the registry only supports `GitHub` packages, so -- we error out in other cases. Once the registry supports non-GitHub -- packages, the `else` branch should return `Git { url: origin.url, subdir }` location <- - if String.contains (String.Pattern "github.com") origin.url - then pure $ Location.GitHub { owner: origin.owner, repo: origin.repo, subdir } - else die + if String.contains (String.Pattern "github.com") origin.url then + pure $ Location.GitHub { owner: origin.owner, repo: origin.repo, subdir } + else + die [ "The registry only supports packages hosted on GitHub at the moment." , "Cannot publish this package because it is hosted at " <> origin.url ]