Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Automatically fill in 'publish.location' in config when absent #1288

Merged
merged 12 commits into from
Oct 31, 2024
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions core/src/Config.purs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ module Spago.Core.Config
, parseBundleType
, parsePlatform
, printSpagoRange
, publishLocationCodec
, remotePackageCodec
, setAddressCodec
, widestRange
Expand Down
55 changes: 53 additions & 2 deletions src/Spago/Command/Publish.purs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,17 @@ 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 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 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.
Expand Down Expand Up @@ -470,11 +480,52 @@ 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
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 "Couldn't parse Git remotes: ", err ]
Right remotes ->
case Array.find (_.name >>> eq "origin") remotes of
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
configPath
| Config.isRootPackage selectedPackage = "spago.yaml"
| otherwise = Path.concat [ selectedPackage.path, "spago.yaml" ]

liftEffect $ Config.addPublishLocationToConfig selectedPackage.doc location
liftAff $ FS.writeYamlDocFile configPath selectedPackage.doc
pure true

baseApi :: String
baseApi = "https://registry.purescript.org"
8 changes: 6 additions & 2 deletions src/Spago/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
Expand Down Expand Up @@ -121,3 +121,7 @@ export function migrateV1ConfigImpl(doc) {
return doc;
}
}

export function addPublishLocationToConfigImpl(doc, location) {
doc.setIn(["package", "publish", "location"], location);
}
33 changes: 24 additions & 9 deletions src/Spago/Config.purs
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
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

import Spago.Prelude
Expand Down Expand Up @@ -51,12 +53,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
Expand Down Expand Up @@ -164,6 +168,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
Expand Down Expand Up @@ -453,7 +463,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
Expand Down Expand Up @@ -649,8 +659,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
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package:
name: aaa
dependencies: []
publish:
version: 0.0.1
license: MIT
workspace:
packageSet:
registry: 58.0.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module Lib where

anExport :: String
anExport = "Hello, World!"
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package:
name: bbb
dependencies: []
publish:
version: 0.0.1
license: MIT
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module Lib1 where

anExport :: String
anExport = "Hello, World!"
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package:
name: ccc
dependencies: []
publish:
version: 0.0.1
license: MIT
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module Lib3 where

anExport :: String
anExport = "Hello, World!"
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
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.


✘ The registry does not support nested packages yet. Only the root package can be published.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
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 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package:
name: aaa
dependencies: []
publish:
version: 0.0.1
license: MIT
workspace:
packageSet:
registry: 58.0.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
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.


✘ 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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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 of your `spago.yaml` file was empty. Spago filled it in, please commit it and try again.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
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.


✘ The registry does not support nested packages yet. Only the root package can be published.
41 changes: 41 additions & 0 deletions test/Spago/Publish.purs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,47 @@ 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 "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")

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")

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" ] >>=
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", "[email protected]: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" ]
Expand Down
Loading