Skip to content

Commit

Permalink
Fix missing git dependencies when building monorepos (#1161)
Browse files Browse the repository at this point in the history
  • Loading branch information
@f-f
f-f authored Jan 15, 2024
1 parent 68b6446 commit 6f9163f
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 97 deletions.
4 changes: 3 additions & 1 deletion src/Spago/Command/Build.purs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ run opts = do
Just p -> NEA.singleton p
Nothing -> Config.getWorkspacePackages workspace.packageSet
globs = getBuildGlobs
{ dependencies: allDependencies
{ dependencies: case workspace.selected of
Just p -> unsafeFromJust $ Map.lookup p.package.name dependencies
Nothing -> allDependencies
, depsOnly: opts.depsOnly
, withTests: true
, selected: selectedPackages
Expand Down
2 changes: 1 addition & 1 deletion src/Spago/Config.purs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ readWorkspace { maybeSelectedPackage, pureBuild } = do

-- TODO: this should be a logwarning, and there should be a test
unless (Array.null prunedConfigs) do
logDebug $ [ "Excluding configs that use a different workspace (directly or implicitly via parent directory's config):" ] <> Array.sort failedPackages
logDebug $ [ "Excluding configs that use a different workspace (directly or implicitly via parent directory's config):" ] <> Array.sort prunedConfigs

rootPackage <- case maybePackage of
Nothing -> pure []
Expand Down
7 changes: 0 additions & 7 deletions test/Spago/Build.purs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,6 @@ spec = Spec.around withTempDir do
FS.exists "myOutput" `Assert.shouldReturn` true
FS.exists "output" `Assert.shouldReturn` false

-- TODO: no-install flag
-- Spec.it "does not install packages when passed the --no-install flag" \{ spago } -> do
-- spago [ "init" ] >>= shouldBeSuccess
-- spago [ "build", "--no-install" ] >>= shouldBeFailure
-- spago [ "install" ] >>= shouldBeSuccess
-- spago [ "build", "--no-install" ] >>= shouldBeSuccess

Spec.it "can build with a local custom package set" \{ spago, fixture } -> do
spago [ "init" ] >>= shouldBeSuccess
FSA.unlink "spago.yaml"
Expand Down
224 changes: 136 additions & 88 deletions test/Spago/Build/Polyrepo.purs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
-- | issue/PR and preview it to visualize it.
-- | For a guide on how to write such a diagram,
-- | see https://mermaid.js.org/syntax/flowchart.html
module Test.Spago.Build.Polyrepo where
module Test.Spago.Build.Polyrepo (spec) where

import Test.Prelude

import Data.Array as Array
import Data.Map as Map
import Data.String (Pattern(..))
import Data.String as String
import Node.Path as Path
Expand Down Expand Up @@ -40,88 +41,7 @@ spec = Spec.describe "polyrepo" do
$ Init.defaultConfig'
$ WorkspaceOnly { setVersion: Just $ unsafeFromRight $ Version.parse "0.0.1" }

setupDir { packageName, spagoYaml, srcMain, testMain } = do
let
src = Path.concat [ packageName, "src" ]
test = Path.concat [ packageName, "test" ]
copyTemplate templateStr path = do
for_ templateStr \fileContent ->
FS.writeTextFile (Path.concat $ Array.cons packageName path) fileContent

FS.mkdirp src
FS.mkdirp test
FS.writeYamlFile Config.configCodec (Path.concat [ packageName, "spago.yaml" ]) spagoYaml
copyTemplate srcMain [ "src", "Main.purs" ]
copyTemplate testMain [ "test", "Main.purs" ]

mkModuleContent { modName, imports, body } = Just $ Array.intercalate "\n"
$
[ "module " <> modName <> " where"
, ""
, "import Prelude"
]
<> imports
<> body

Spec.describe "inter-workspace package dependencies" do

let
setupPackageWithDeps
:: { packageName :: String
, hasTest :: Boolean
, deps :: Array { dep :: String, import :: String, alias :: String }
}
-> Aff { dep :: String, import :: String, alias :: String }
setupPackageWithDeps { packageName, hasTest, deps } = do
let
packageAlias = packageToModuleName packageName
packageNameValues
| Array.null deps = "\"no deps\""
| otherwise = Array.intercalate " <> " $ map (\r -> r.alias <> ".packageNameValue") deps
setupDir
{ packageName: packageName
, spagoYaml: mkPackageOnlyConfig
{ packageName, srcDependencies: [ "prelude" ] <> map _.dep deps }
[ configAddTestMain
, configAddTestDependencies [ "prelude", "console", "effect" ]
]
, srcMain: mkModuleContent
{ modName: mkSrcModuleName packageName
, imports: map _.import deps
, body:
[ ""
, "libraryUsage :: String"
, "libraryUsage = packageNameValue <> " <> packageNameValues
, ""
, "packageNameValue :: String"
, "packageNameValue = \"package name \" <> \"" <> packageName <> "\""
]
}
, testMain:
if not hasTest then Nothing
else mkModuleContent
{ modName: mkTestModuleName packageName
, imports:
[ ""
, "import Effect (Effect)"
, "import Effect.Console (log)"
, "import " <> mkSrcModuleName packageName <> " as " <> packageAlias
]
<> (map _.import deps)
, body:
[ ""
, "main :: Effect Unit"
, "main = do"
, " log $ \"Test for \" <> " <> packageAlias <> ".packageNameValue <> " <> packageNameValues
]
}
}
pure
{ dep: packageName
, import: "import " <> mkSrcModuleName packageName <> " as " <> packageAlias
, alias: packageAlias
}

{-
```mermaid
flowchart TD
Expand All @@ -131,7 +51,7 @@ spec = Spec.describe "polyrepo" do
end
```
-}
Spec.it "Case 1 (independent packages) builds" \{ spago } -> do
Spec.it "Case 1: 'independent packages' builds" \{ spago } -> do
writeWorkspaceOnlySpagoYamlFile
void $ setupPackageWithDeps { packageName: "package-a", hasTest: true, deps: [] }
void $ setupPackageWithDeps { packageName: "package-b", hasTest: true, deps: [] }
Expand All @@ -150,7 +70,7 @@ spec = Spec.describe "polyrepo" do
end
```
-}
Spec.it "Case 2 (shared dependencies packages) builds" \{ spago } -> do
Spec.it "Case 2: 'shared dependencies packages' builds" \{ spago } -> do
writeWorkspaceOnlySpagoYamlFile
shared <- setupPackageWithDeps { packageName: "package-shared", hasTest: false, deps: [] }
void $ setupPackageWithDeps { packageName: "package-a", hasTest: true, deps: [ shared ] }
Expand All @@ -170,7 +90,7 @@ spec = Spec.describe "polyrepo" do
end
```
-}
Spec.it "Case 3 (dependencies: A&B -> C; A -> B) builds" \{ spago, fixture } -> do
Spec.it "Case 3: 'dependencies: A&B -> C; A -> B' builds" \{ spago, fixture } -> do
writeWorkspaceOnlySpagoYamlFile
pkgC <- setupPackageWithDeps { packageName: "package-c", hasTest: false, deps: [] }
pkgB <- setupPackageWithDeps { packageName: "package-b", hasTest: true, deps: [ pkgC ] }
Expand All @@ -188,7 +108,7 @@ spec = Spec.describe "polyrepo" do
end
```
-}
Spec.it "Declaring 2+ modules with the same name across 2+ packages fails to build" \{ spago } -> do
Spec.it "declaring 2+ modules with the same name across 2+ packages fails to build" \{ spago } -> do
writeWorkspaceOnlySpagoYamlFile
let
sameModuleName = "SameModuleName"
Expand Down Expand Up @@ -218,6 +138,52 @@ spec = Spec.describe "polyrepo" do
Assert.fail $ "STDERR did not contain text:\n" <> exp <> "\n\nStderr was:\n" <> stdErr
spago [ "build" ] >>= check { stdout: mempty, stderr: hasExpectedModules, result: isLeft }

{-
```mermaid
flowchart TD
subgraph "Case 1.1"
A ---> Dep0["prelude"]
B ---> Dep1["either-from-git"]
end
```
-}
Spec.it "#1161 regression: 'subpackage with disjoint git dependency' builds" \{ spago } -> do
-- Condition 1: there must be a root package
let
conf = Init.defaultConfig
{ name: mkPackageName "root"
, withWorkspace: Just
{ setVersion: Just $ unsafeFromRight $ Version.parse "20.0.1"
}
, testModuleName: "Test.Main"
}
-- Condition 2: one of the dependencies of the subpackage must be fetched from git.
-- This is a problem only when the git dependency is not a dependency of the root package.
FS.writeYamlFile Config.configCodec "spago.yaml"
( conf
{ workspace = conf.workspace # map
( _
{ extra_packages = Just $ Map.fromFoldable
-- This commit is for the `v6.1.0` release
[ Tuple (mkPackageName "either") $ Config.ExtraRemotePackage $ Config.RemoteGitPackage
{ git: "https://github.com/purescript/purescript-either.git"
, ref: "af655a04ed2fd694b6688af39ee20d7907ad0763"
, subdir: Nothing
, dependencies: Just $ mkDependencies [ "control", "invariant", "maybe", "prelude" ]
}
]
}
)
}
)
-- Condition 3: the workspace needs to contain a subpackage that is using the git dependency
void $ setupPackageWithDeps { packageName: "package-b", hasTest: true, deps: [ { dep: "either", alias: "EITHER", import: "import Data.Either" } ] }
-- Lastly, this broke only when building the root package
spago [ "build", "-p", "root" ] >>= shouldBeSuccess
-- Or getting its graph
spago [ "uninstall", "-p", "root", "console", "effect", "prelude" ] >>= shouldBeSuccess
spago [ "build", "-p", "root", "--pedantic-packages" ] >>= shouldBeSuccess

Spec.describe "warning censoring and error-promotion" do
let
setupPackageWithUnusedNameWarning packageName deps strict censorShadowedName includeTestCode = do
Expand Down Expand Up @@ -332,8 +298,8 @@ spec = Spec.describe "polyrepo" do
Spec.describe "pedantic packages" do
let
{-
/-- tuples (unused dep by `src`)
newtype (transitive dep) <-- control (direct dep) <--+
/-- tuples (unused dep by `src`)
newtype (transitive dep) <-- control (direct dep) <--+
\-- either (unused dep by `test`)
- src and test both import `Data.Newtype` (from `newtype` package) unnecessarily,
Expand Down Expand Up @@ -440,3 +406,85 @@ spec = Spec.describe "polyrepo" do
unless (Array.null unfoundTexts) do
Assert.fail $ "STDERR did not contain expected texts:\n" <> (Array.intercalate "\n\n" unfoundTexts) <> "\n\nStderr was:\n" <> stdErr
spago [ "build", "--pedantic-packages" ] >>= check { stdout: mempty, stderr: hasExpectedErrors, result: isLeft }

type SetupPackage = { packageName :: String, hasTest :: Boolean, deps :: Array { dep :: String, import :: String, alias :: String } }

setupPackageWithDeps :: SetupPackage -> Aff { dep :: String, import :: String, alias :: String }
setupPackageWithDeps { packageName, hasTest, deps } = do
let
packageAlias = packageToModuleName packageName
packageNameValues
| Array.null deps = "\"no deps\""
| otherwise = Array.intercalate " <> " $ map (\r -> r.alias <> ".packageNameValue") deps
setupDir
{ packageName: packageName
, spagoYaml: mkPackageOnlyConfig
{ packageName, srcDependencies: [ "prelude" ] <> map _.dep deps }
[ configAddTestMain
, configAddTestDependencies [ "prelude", "console", "effect" ]
]
, srcMain: mkModuleContent
{ modName: mkSrcModuleName packageName
, imports: map _.import deps
, body:
[ ""
, "libraryUsage :: String"
, "libraryUsage = packageNameValue <> " <> packageNameValues
, ""
, "packageNameValue :: String"
, "packageNameValue = \"package name \" <> \"" <> packageName <> "\""
]
}
, testMain:
if not hasTest then Nothing
else mkModuleContent
{ modName: mkTestModuleName packageName
, imports:
[ ""
, "import Effect (Effect)"
, "import Effect.Console (log)"
, "import " <> mkSrcModuleName packageName <> " as " <> packageAlias
]
<> (map _.import deps)
, body:
[ ""
, "main :: Effect Unit"
, "main = do"
, " log $ \"Test for \" <> " <> packageAlias <> ".packageNameValue <> " <> packageNameValues
]
}
}
pure
{ dep: packageName
, import: "import " <> mkSrcModuleName packageName <> " as " <> packageAlias
, alias: packageAlias
}

type SetupDir = { packageName :: String, spagoYaml :: Config.Config, srcMain :: Maybe String, testMain :: Maybe String }

setupDir :: SetupDir -> Aff Unit
setupDir { packageName, spagoYaml, srcMain, testMain } = do
let
src = Path.concat [ packageName, "src" ]
test = Path.concat [ packageName, "test" ]
copyTemplate templateStr path = do
for_ templateStr \fileContent ->
FS.writeTextFile (Path.concat $ Array.cons packageName path) fileContent

FS.mkdirp src
FS.mkdirp test
FS.writeYamlFile Config.configCodec (Path.concat [ packageName, "spago.yaml" ]) spagoYaml
copyTemplate srcMain [ "src", "Main.purs" ]
copyTemplate testMain [ "test", "Main.purs" ]

type ModuleContent = { body :: Array String, imports :: Array String, modName :: String }

mkModuleContent :: ModuleContent -> Maybe String
mkModuleContent { modName, imports, body } = Just $ Array.intercalate "\n"
$
[ "module " <> modName <> " where"
, ""
, "import Prelude"
]
<> imports
<> body

0 comments on commit 6f9163f

Please sign in to comment.