From fe1ef5c5c7a5ca21664c1c2cb51bb46d08fb9cb6 Mon Sep 17 00:00:00 2001 From: Matthew Pickering Date: Wed, 31 Jan 2024 16:53:43 +0000 Subject: [PATCH] cabal-install: Clarify the semantics of package-db flag In this PR we make the `--package-db` flag apply only to the default immutable initial package stack rather than also applying to the store package database. There are two special package databases which cabal install knows about and treats specially. * The store directory, a global cache of installed packages where cabal builds and installs packages into. * The inplace directory, a local build folder for packages, where cabal builds local packages in. It is very important that cabal registers packages it builds into one of these two locations as there are many assumptions that packages will end up here. With the current design of the `--package-db` flag, you are apparently allowed to override the store location which should have the effect of making the last package database in the package stack the "store" directory. Perhaps this works out in theory but practically this behaviour was broken and things were always registered into the store directory that cabal knew about. (The assertion in `ProjectBuilding.UnpackedPackage` was failing (see added test)). With `--package-db` not being able to modify the location of the store directory then the interaction of `--package-db`, `--store-dir` and `--dist-dir` flags become easy to understand. * `--package-db` modify the initial package database stack, these package database will not be mutated by cabal and provide the initial package environment which is used by cabal. * `--store-dir` modify the location of the store directory * `--dist-dir` modify the location of the dist directory (and hence inplace package database) Treating the flags in this way also fix an assertion failure when building packages. --- .../src/Distribution/Client/CmdInstall.hs | 11 +++--- .../src/Distribution/Client/DistDirLayout.hs | 23 +++++++++---- .../Client/ProjectOrchestration.hs | 7 ++-- .../Distribution/Client/ProjectPlanning.hs | 5 +-- .../UnitTests/Distribution/Client/Store.hs | 4 +-- .../PackageDB/t9678/cabal.test.hs | 11 ++++++ .../PackageDB/t9678/p1/CHANGELOG.md | 5 +++ .../PackageTests/PackageDB/t9678/p1/p1.cabal | 18 ++++++++++ .../PackageDB/t9678/p1/src/MyLib.hs | 4 +++ .../PackageDB/t9678/p2/CHANGELOG.md | 5 +++ .../PackageDB/t9678/p2/cabal.project | 1 + .../PackageTests/PackageDB/t9678/p2/p2.cabal | 18 ++++++++++ .../PackageDB/t9678/p2/src/MyLib.hs | 4 +++ .../t9678/repo/p3-0.1.0.0/CHANGELOG.md | 5 +++ .../PackageDB/t9678/repo/p3-0.1.0.0/p3.cabal | 18 ++++++++++ .../t9678/repo/p3-0.1.0.0/src/MyLib.hs | 4 +++ changelog.d/issue-9678 | 16 +++++++++ doc/cabal-project-description-file.rst | 34 ++++++++++++------- 18 files changed, 163 insertions(+), 30 deletions(-) create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/cabal.test.hs create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/p1/CHANGELOG.md create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/p1/p1.cabal create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/p1/src/MyLib.hs create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/p2/CHANGELOG.md create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/p2/cabal.project create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/p2/p2.cabal create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/p2/src/MyLib.hs create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/CHANGELOG.md create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/p3.cabal create mode 100644 cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/src/MyLib.hs create mode 100644 changelog.d/issue-9678 diff --git a/cabal-install/src/Distribution/Client/CmdInstall.hs b/cabal-install/src/Distribution/Client/CmdInstall.hs index 0adeca99446..6f1aa29c861 100644 --- a/cabal-install/src/Distribution/Client/CmdInstall.hs +++ b/cabal-install/src/Distribution/Client/CmdInstall.hs @@ -440,11 +440,12 @@ installAction flags@NixStyleFlags{extraFlags = clientInstallFlags', ..} targetSt ProjectConfigShared { projectConfigStoreDir + , projectConfigPackageDBs } = projectConfigShared projectConfig mlogsDir = flagToMaybe projectConfigLogsDir mstoreDir = flagToMaybe projectConfigStoreDir - cabalDirLayout <- mkCabalDirLayout mstoreDir mlogsDir + cabalDirLayout <- mkCabalDirLayout mstoreDir mlogsDir projectConfigPackageDBs let buildSettings = @@ -494,6 +495,7 @@ installAction flags@NixStyleFlags{extraFlags = clientInstallFlags', ..} targetSt , projectConfigHcPkg , projectConfigStoreDir , projectConfigProgPathExtra + , projectConfigPackageDBs } , projectConfigLocalPackages = PackageConfig @@ -530,7 +532,7 @@ installAction flags@NixStyleFlags{extraFlags = clientInstallFlags', ..} targetSt envFile <- getEnvFile clientInstallFlags platform compilerVersion existingEnvEntries <- getExistingEnvEntries verbosity compilerFlavor supportsPkgEnvFiles envFile - packageDbs <- getPackageDbStack compiler projectConfigStoreDir projectConfigLogsDir + packageDbs <- getPackageDbStack compiler projectConfigStoreDir projectConfigLogsDir projectConfigPackageDBs installedIndex <- getInstalledPackages verbosity compiler packageDbs progDb let @@ -1214,12 +1216,13 @@ getPackageDbStack :: Compiler -> Flag FilePath -> Flag FilePath + -> [Maybe PackageDB] -> IO PackageDBStack -getPackageDbStack compiler storeDirFlag logsDirFlag = do +getPackageDbStack compiler storeDirFlag logsDirFlag packageDbs = do mstoreDir <- traverse makeAbsolute $ flagToMaybe storeDirFlag let mlogsDir = flagToMaybe logsDirFlag - cabalLayout <- mkCabalDirLayout mstoreDir mlogsDir + cabalLayout <- mkCabalDirLayout mstoreDir mlogsDir packageDbs pure $ storePackageDBStack (cabalStoreDirLayout cabalLayout) compiler -- | This defines what a 'TargetSelector' means for the @bench@ command. diff --git a/cabal-install/src/Distribution/Client/DistDirLayout.hs b/cabal-install/src/Distribution/Client/DistDirLayout.hs index 834bda34705..b20f52f1c3a 100644 --- a/cabal-install/src/Distribution/Client/DistDirLayout.hs +++ b/cabal-install/src/Distribution/Client/DistDirLayout.hs @@ -264,8 +264,8 @@ defaultDistDirLayout projectRoot mdistDirectory haddockOutputDir = distHaddockOutputDir :: Maybe FilePath distHaddockOutputDir = haddockOutputDir -defaultStoreDirLayout :: FilePath -> StoreDirLayout -defaultStoreDirLayout storeRoot = +defaultStoreDirLayout :: FilePath -> [Maybe PackageDB] -> StoreDirLayout +defaultStoreDirLayout storeRoot extraPackageDB = StoreDirLayout{..} where storeDirectory :: Compiler -> FilePath @@ -288,7 +288,10 @@ defaultStoreDirLayout storeRoot = storePackageDBStack :: Compiler -> PackageDBStack storePackageDBStack compiler = - [GlobalPackageDB, storePackageDB compiler] + ( applyPackageDbFlags + [GlobalPackageDB] + extraPackageDB ) + ++ [storePackageDB compiler] storeIncomingDirectory :: Compiler -> FilePath storeIncomingDirectory compiler = @@ -298,19 +301,27 @@ defaultStoreDirLayout storeRoot = storeIncomingLock compiler unitid = storeIncomingDirectory compiler prettyShow unitid <.> "lock" +-- | Append the given package databases to an existing PackageDBStack. +-- A @Nothing@ entry will clear everything before it. +applyPackageDbFlags :: PackageDBStack -> [Maybe PackageDB] -> PackageDBStack +applyPackageDbFlags dbs' [] = dbs' +applyPackageDbFlags _ (Nothing : dbs) = applyPackageDbFlags [] dbs +applyPackageDbFlags dbs' (Just db : dbs) = applyPackageDbFlags (dbs' ++ [db]) dbs + defaultCabalDirLayout :: IO CabalDirLayout defaultCabalDirLayout = - mkCabalDirLayout Nothing Nothing + mkCabalDirLayout Nothing Nothing [] mkCabalDirLayout :: Maybe FilePath -- ^ Store directory. Must be absolute -> Maybe FilePath -- ^ Log directory + -> [Maybe PackageDB] -> IO CabalDirLayout -mkCabalDirLayout mstoreDir mlogDir = do +mkCabalDirLayout mstoreDir mlogDir mExtraPackageDB = do cabalStoreDirLayout <- - defaultStoreDirLayout <$> maybe defaultStoreDir pure mstoreDir + defaultStoreDirLayout <$> maybe defaultStoreDir pure mstoreDir <*> pure mExtraPackageDB cabalLogsDirectory <- maybe defaultLogsDir pure mlogDir pure $ CabalDirLayout{..} diff --git a/cabal-install/src/Distribution/Client/ProjectOrchestration.hs b/cabal-install/src/Distribution/Client/ProjectOrchestration.hs index 4f5c9faeed8..7a08a552ccd 100644 --- a/cabal-install/src/Distribution/Client/ProjectOrchestration.hs +++ b/cabal-install/src/Distribution/Client/ProjectOrchestration.hs @@ -284,6 +284,7 @@ establishProjectBaseContextWithRoot verbosity cliConfig projectRoot currentComma ProjectConfigShared { projectConfigStoreDir + , projectConfigPackageDBs } = projectConfigShared projectConfig mlogsDir = Setup.flagToMaybe projectConfigLogsDir @@ -291,7 +292,8 @@ establishProjectBaseContextWithRoot verbosity cliConfig projectRoot currentComma sequenceA $ makeAbsolute <$> Setup.flagToMaybe projectConfigStoreDir - cabalDirLayout <- mkCabalDirLayout mstoreDir mlogsDir + + cabalDirLayout <- mkCabalDirLayout mstoreDir mlogsDir projectConfigPackageDBs let buildSettings = resolveBuildTimeSettings @@ -1445,12 +1447,13 @@ establishDummyProjectBaseContext verbosity projectConfig distDirLayout localPack ProjectConfigShared { projectConfigStoreDir + , projectConfigPackageDBs } = projectConfigShared projectConfig mlogsDir = flagToMaybe projectConfigLogsDir mstoreDir = flagToMaybe projectConfigStoreDir - cabalDirLayout <- mkCabalDirLayout mstoreDir mlogsDir + cabalDirLayout <- mkCabalDirLayout mstoreDir mlogsDir projectConfigPackageDBs let buildSettings :: BuildTimeSettings buildSettings = diff --git a/cabal-install/src/Distribution/Client/ProjectPlanning.hs b/cabal-install/src/Distribution/Client/ProjectPlanning.hs index deff1f38bf0..fd453fcf9cf 100644 --- a/cabal-install/src/Distribution/Client/ProjectPlanning.hs +++ b/cabal-install/src/Distribution/Client/ProjectPlanning.hs @@ -2315,10 +2315,7 @@ elaborateInstallPlan corePackageDbs ++ [distPackageDB (compilerId compiler)] - corePackageDbs = - applyPackageDbFlags - (storePackageDBStack compiler) - (projectConfigPackageDBs sharedPackageConfig) + corePackageDbs = storePackageDBStack compiler -- For this local build policy, every package that lives in a local source -- dir (as opposed to a tarball), or depends on such a package, will be diff --git a/cabal-install/tests/UnitTests/Distribution/Client/Store.hs b/cabal-install/tests/UnitTests/Distribution/Client/Store.hs index 976bd97a4cb..bcfe496e3e1 100644 --- a/cabal-install/tests/UnitTests/Distribution/Client/Store.hs +++ b/cabal-install/tests/UnitTests/Distribution/Client/Store.hs @@ -32,7 +32,7 @@ tests = testListEmpty :: Assertion testListEmpty = withTempDirectory verbosity "." "store-" $ \tmp -> do - let storeDirLayout = defaultStoreDirLayout (tmp "store") + let storeDirLayout = defaultStoreDirLayout (tmp "store") [] assertStoreEntryExists storeDirLayout compiler unitid False assertStoreContent tmp storeDirLayout compiler Set.empty @@ -53,7 +53,7 @@ testListEmpty = testInstallSerial :: Assertion testInstallSerial = withTempDirectory verbosity "." "store-" $ \tmp -> do - let storeDirLayout = defaultStoreDirLayout (tmp "store") + let storeDirLayout = defaultStoreDirLayout (tmp "store") [] copyFiles file content dir = do -- we copy into a prefix inside the tmp dir and return the prefix let destprefix = dir "prefix" diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/cabal.test.hs b/cabal-testsuite/PackageTests/PackageDB/t9678/cabal.test.hs new file mode 100644 index 00000000000..5cb73595452 --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/cabal.test.hs @@ -0,0 +1,11 @@ +import Test.Cabal.Prelude +main = cabalTest $ do + recordMode DoNotRecord $ withRepo "repo" $ withPackageDb $ do + withDirectory "p1" $ + setup_install [] + + env <- getTestEnv + let pkgDbPath = testPackageDbDir env + + withDirectory "p2" $ do + void $ cabal' "v2-build" ["--package-db=" ++ pkgDbPath] diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/p1/CHANGELOG.md b/cabal-testsuite/PackageTests/PackageDB/t9678/p1/CHANGELOG.md new file mode 100644 index 00000000000..877394c57bd --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/p1/CHANGELOG.md @@ -0,0 +1,5 @@ +# Revision history for p1 + +## 0.1.0.0 -- YYYY-mm-dd + +* First version. Released on an unsuspecting world. diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/p1/p1.cabal b/cabal-testsuite/PackageTests/PackageDB/t9678/p1/p1.cabal new file mode 100644 index 00000000000..78f0f1372e2 --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/p1/p1.cabal @@ -0,0 +1,18 @@ +cabal-version: 3.0 +name: p1 +version: 0.1.0.0 +license: NONE +author: matthewtpickering@gmail.com +maintainer: Matthew Pickering +build-type: Simple +extra-doc-files: CHANGELOG.md + +common warnings + ghc-options: -Wall + +library + import: warnings + exposed-modules: MyLib + build-depends: base ^>=4.18.0.0 + hs-source-dirs: src + default-language: Haskell2010 diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/p1/src/MyLib.hs b/cabal-testsuite/PackageTests/PackageDB/t9678/p1/src/MyLib.hs new file mode 100644 index 00000000000..e657c4403f6 --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/p1/src/MyLib.hs @@ -0,0 +1,4 @@ +module MyLib (someFunc) where + +someFunc :: IO () +someFunc = putStrLn "someFunc" diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/p2/CHANGELOG.md b/cabal-testsuite/PackageTests/PackageDB/t9678/p2/CHANGELOG.md new file mode 100644 index 00000000000..e603f1bd4b2 --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/p2/CHANGELOG.md @@ -0,0 +1,5 @@ +# Revision history for p2 + +## 0.1.0.0 -- YYYY-mm-dd + +* First version. Released on an unsuspecting world. diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/p2/cabal.project b/cabal-testsuite/PackageTests/PackageDB/t9678/p2/cabal.project new file mode 100644 index 00000000000..e6fdbadb439 --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/p2/cabal.project @@ -0,0 +1 @@ +packages: . diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/p2/p2.cabal b/cabal-testsuite/PackageTests/PackageDB/t9678/p2/p2.cabal new file mode 100644 index 00000000000..242d165f652 --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/p2/p2.cabal @@ -0,0 +1,18 @@ +cabal-version: 3.0 +name: p2 +version: 0.1.0.0 +license: NONE +author: matthewtpickering@gmail.com +maintainer: Matthew Pickering +build-type: Simple +extra-doc-files: CHANGELOG.md + +common warnings + ghc-options: -Wall + +library + import: warnings + exposed-modules: MyLib + build-depends: base ^>=4.18.0.0, p1, p3 + hs-source-dirs: src + default-language: Haskell2010 diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/p2/src/MyLib.hs b/cabal-testsuite/PackageTests/PackageDB/t9678/p2/src/MyLib.hs new file mode 100644 index 00000000000..e657c4403f6 --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/p2/src/MyLib.hs @@ -0,0 +1,4 @@ +module MyLib (someFunc) where + +someFunc :: IO () +someFunc = putStrLn "someFunc" diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/CHANGELOG.md b/cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/CHANGELOG.md new file mode 100644 index 00000000000..877394c57bd --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/CHANGELOG.md @@ -0,0 +1,5 @@ +# Revision history for p1 + +## 0.1.0.0 -- YYYY-mm-dd + +* First version. Released on an unsuspecting world. diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/p3.cabal b/cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/p3.cabal new file mode 100644 index 00000000000..bb0d55591bb --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/p3.cabal @@ -0,0 +1,18 @@ +cabal-version: 3.0 +name: p3 +version: 0.1.0.0 +license: NONE +author: matthewtpickering@gmail.com +maintainer: Matthew Pickering +build-type: Simple +extra-doc-files: CHANGELOG.md + +common warnings + ghc-options: -Wall + +library + import: warnings + exposed-modules: MyLib + build-depends: base ^>=4.18.0.0 + hs-source-dirs: src + default-language: Haskell2010 diff --git a/cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/src/MyLib.hs b/cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/src/MyLib.hs new file mode 100644 index 00000000000..e657c4403f6 --- /dev/null +++ b/cabal-testsuite/PackageTests/PackageDB/t9678/repo/p3-0.1.0.0/src/MyLib.hs @@ -0,0 +1,4 @@ +module MyLib (someFunc) where + +someFunc :: IO () +someFunc = putStrLn "someFunc" diff --git a/changelog.d/issue-9678 b/changelog.d/issue-9678 new file mode 100644 index 00000000000..e1eda2b46e9 --- /dev/null +++ b/changelog.d/issue-9678 @@ -0,0 +1,16 @@ +synopsis: Clarify the semantics of the -package-db flag +packages: cabal-install +prs: +issues: #9678 + +description: { + +The `--package-db` flag now only applies to the default +immutable initial package stack rather than also applying to the store +package database. + +This fixes an assertion failure which was triggered when using -package-db and also +clarifies how it should interact with `--store-dir` and `--dist-dir` flags. + +} + diff --git a/doc/cabal-project-description-file.rst b/doc/cabal-project-description-file.rst index a787a221f58..1c99d238c0c 100644 --- a/doc/cabal-project-description-file.rst +++ b/doc/cabal-project-description-file.rst @@ -372,6 +372,11 @@ package, and thus apply globally: :synopsis: PackageDB stack manipulation :since: 3.7 + By modifying ``package-dbs`` you can modify the default package environment + which ``cabal`` will see. The package databases you add using ``package-dbs`` + will not be written into and only used as immutable package stores to initialise + the environment with additional packages that ``cabal`` can choose to use. + There are three package databases involved with most builds: global @@ -381,37 +386,42 @@ package, and thus apply globally: in-place Project-specific build directory - By default, the package stack you will have with v2 commands is: + By default, the initial package stack prefix you will have with v2 commands is: :: - -- [global, store] + -- prefix = [global] - So all remote packages required by your project will be - registered in the store package db (because it is last). + So the initial set of packages which is used by cabal is just the packages + installed in the global package database which comes with ``ghc``. - When cabal starts building your local projects, it appends the in-place db - to the end: + When cabal builds a package it will start populating the ``store`` package database, + whose packages will then be subsequently be available to be used in future runs. :: - -- [global, store, in-place] + -- prefix ++ [store] + + When cabal builds your local projects, packages are registered into the local + in-place package database. + + :: - So your local packages get put in ``dist-newstyle`` instead of the store. + -- prefix ++ [store, in-place] - This flag manipulates the default prefix: ``[global, store]`` and accepts + This flag manipulates the default prefix: ``[global]`` and accepts paths, the special value ``global`` referring to the global package db, and ``clear`` which removes all prior entries. For example, :: - -- [global, store, foo] + -- prefix = [global, foo] package-dbs: foo - -- [foo] + -- prefix = [foo] package-dbs: clear, foo - -- [bar, baz] + -- prefix = [bar, baz] package-dbs: clear, foo, clear, bar, baz The command line variant of this flag is ``--package-db=DB`` which can be