Skip to content

Commit

Permalink
Apply local configuration to install targets
Browse files Browse the repository at this point in the history
The target of `cabal install` is not considered to be a local package,
which means local configuration (e.g. in cabal.project, or flags like
--enable-profiling) does not apply to it.

In 76670eb, we changed the behaviour to
applying the local flags to cabal install targets, but it used the
literal target string as a package name to which the flags were
additionally applied.

However, `cabal install` targets are NOT necessarily package names, so,
e.g., if we did `cabal install exe:mycomp`, the local flags would not
apply since "exe:mycomp" is not a recognized /package/.

The solution is to parse the target selectors first, and apply the local
flags to the package of the resolved targets.

Fixes haskell#7297, haskell#8909, the install part of haskell#7236, haskell#8529, haskell#7832
  • Loading branch information
alt-romes authored and mpickering committed Feb 6, 2024
1 parent 32d9319 commit 25e1283
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 9 deletions.
44 changes: 35 additions & 9 deletions cabal-install/src/Distribution/Client/CmdInstall.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
{-# LANGUAGE NamedFieldPuns #-}
{-# LANGUAGE RecordWildCards #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TupleSections #-}

-- | cabal-install CLI command: build
module Distribution.Client.CmdInstall
Expand Down Expand Up @@ -104,6 +105,7 @@ import Distribution.Client.Types
, PackageSpecifier (..)
, SourcePackageDb (..)
, UnresolvedSourcePackage
, pkgSpecifierTarget
)
import Distribution.Client.Types.OverwritePolicy
( OverwritePolicy (..)
Expand Down Expand Up @@ -371,7 +373,7 @@ installAction flags@NixStyleFlags{extraFlags = clientInstallFlags', ..} targetSt

-- First, we need to learn about what's available to be installed.
localBaseCtx <-
establishProjectBaseContext reducedVerbosity cliConfig InstallCommand
establishProjectBaseContext reducedVerbosity baseCliConfig InstallCommand
let localDistDirLayout = distDirLayout localBaseCtx
pkgDb <-
projectConfigWithBuilderRepoContext
Expand Down Expand Up @@ -432,7 +434,7 @@ installAction flags@NixStyleFlags{extraFlags = clientInstallFlags', ..} targetSt
withoutProject globalConfig = do
tss <- traverse (parseWithoutProjectTargetSelector verbosity) targetStrings'
let
projectConfig = globalConfig <> cliConfig
projectConfig = globalConfig <> baseCliConfig

ProjectConfigBuildOnly
{ projectConfigLogsDir
Expand Down Expand Up @@ -478,10 +480,17 @@ installAction flags@NixStyleFlags{extraFlags = clientInstallFlags', ..} targetSt

return (packageSpecifiers, uris, packageTargets, projectConfig)

(specs, uris, targetSelectors, config) <-
(specs, uris, targetSelectors, baseConfig) <-
withProjectOrGlobalConfig verbosity ignoreProject globalConfigFlag withProject withoutProject

-- We compute the base context again to determine packages available in the
-- project to be installed, so we can list the available package names when
-- the "all:..." variants of the target selectors are used.
localPkgs <- localPackages <$> establishProjectBaseContext verbosity baseConfig InstallCommand

let
config = addLocalConfigToPkgs baseConfig (map pkgSpecifierTarget specs ++ concatMap (targetPkgNames localPkgs) targetSelectors)

ProjectConfig
{ projectConfigBuildOnly =
ProjectConfigBuildOnly
Expand Down Expand Up @@ -631,8 +640,7 @@ installAction flags@NixStyleFlags{extraFlags = clientInstallFlags', ..} targetSt
globalFlags
flags{configFlags = configFlags'}
clientInstallFlags'
cliConfig = addLocalConfigToTargets baseCliConfig targetStrings
globalConfigFlag = projectConfigConfigFile (projectConfigShared cliConfig)
globalConfigFlag = projectConfigConfigFile (projectConfigShared baseCliConfig)

-- Do the install action for each executable in the install configuration.
traverseInstall :: InstallAction -> InstallCfg -> IO ()
Expand All @@ -641,17 +649,35 @@ installAction flags@NixStyleFlags{extraFlags = clientInstallFlags', ..} targetSt
actionOnExe <- action v overwritePolicy <$> prepareExeInstall cfg
traverse_ actionOnExe . Map.toList $ targetsMap buildCtx

-- | Treat all direct targets of install command as local packages: #8637
addLocalConfigToTargets :: ProjectConfig -> [String] -> ProjectConfig
addLocalConfigToTargets config targetStrings =
-- | Treat all direct targets of install command as local packages: #8637 and later #7297, #8909, #7236.
addLocalConfigToPkgs :: ProjectConfig -> [PackageName] -> ProjectConfig
addLocalConfigToPkgs config pkgs =
config
{ projectConfigSpecificPackage =
projectConfigSpecificPackage config
<> MapMappend (Map.fromList targetPackageConfigs)
}
where
localConfig = projectConfigLocalPackages config
targetPackageConfigs = map (\x -> (mkPackageName x, localConfig)) targetStrings
targetPackageConfigs = map (,localConfig) pkgs

targetPkgNames
:: [PackageSpecifier UnresolvedSourcePackage]
-- ^ The local packages, to resolve 'TargetAllPackages' selectors
-> TargetSelector
-> [PackageName]
targetPkgNames localPkgs = \case
TargetPackage _ pkgIds _ -> map pkgName pkgIds
TargetPackageNamed name _ -> [name]
TargetAllPackages _ -> map pkgSpecifierTarget localPkgs
-- Note how the target may select a component only, but we will always apply
-- the local flags to the whole package in which that component is contained.
-- The reason is that our finest level of configuration is per-package, so
-- there is no interface to configure options to a component only. It is not
-- trivial to say whether we could indeed support per-component configuration
-- because of legacy packages which we may always have to build whole.
TargetComponent pkgId _ -> [pkgName pkgId]
TargetComponentUnknown name _ -> [name]

-- | Verify that invalid config options were not passed to the install command.
--
Expand Down
25 changes: 25 additions & 0 deletions cabal-testsuite/PackageTests/Install/T7297-8909-7236/Main.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{-# LANGUAGE CPP #-}

#ifdef TEST1
main = putStrLn "hi1"
#endif

#ifdef TEST2
main = putStrLn "hi2"
#endif

#ifdef TEST3
main = putStrLn "hi3"
#endif

#ifdef TEST4
main = putStrLn "hi4"
#endif

#ifdef TEST5
main = putStrLn "hi5"
#endif

#ifdef TEST6
main = putStrLn "hi6"
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
packages: .
28 changes: 28 additions & 0 deletions cabal-testsuite/PackageTests/Install/T7297-8909-7236/cabal.test.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import Test.Cabal.Prelude

main = cabalTest $ do
env <- getTestEnv
recordMode DoNotRecord $ do
let
installdir = testPrefixDir env </> "bin"
commonOpts v = ["--ghc-options=-DTEST" ++ show v, "--overwrite-policy=always", "--installdir=" ++ installdir]
installWithTgt tgt v = do
cabal "install" (tgt:commonOpts v)
runInstalledExe' "my-exe" []
>>= assertOutputContains ("hi" ++ show v)

installExternalWithTgt tgt v = withRepo "repo" (installWithTgt tgt v)


cabal "install" (commonOpts 1) -- no target
runInstalledExe' "my-exe" []
>>= assertOutputContains "hi1"

installWithTgt "t7297-89097236a" 2
installWithTgt "exe:my-exe" 3
installWithTgt "my-exe" 4
installWithTgt "all" 5
installWithTgt "all:exes" 6

-- And test it works when installing from an external repo (think Hackage)
installExternalWithTgt "external-lib" 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Revision history for external-lib0100

## 0.1.0.0 -- YYYY-mm-dd

* First version. Released on an unsuspecting world.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{-# LANGUAGE CPP #-}

#ifdef TEST1
main = putStrLn "hi1"
#endif

#ifdef TEST2
main = putStrLn "hi2"
#endif

#ifdef TEST3
main = putStrLn "hi3"
#endif

#ifdef TEST4
main = putStrLn "hi4"
#endif

#ifdef TEST5
main = putStrLn "hi5"
#endif

#ifdef TEST6
main = putStrLn "hi6"
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
cabal-version: 3.0
name: external-lib
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

executable my-exe
import: warnings
main-is: Main.hs
build-depends: base
hs-source-dirs: .
default-language: Haskell2010
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name: t7297-89097236a
version: 1.0
build-type: Simple
cabal-version: >= 1.2

executable my-exe
main-is: Main.hs
build-depends: base

0 comments on commit 25e1283

Please sign in to comment.