Skip to content

Commit

Permalink
Fix project-local build flags being ignored.
Browse files Browse the repository at this point in the history
I noticed that running ‘cabal install’ with two separate sets of dynamic /
static build flags (e.g. one with none, and one with ‘--enable-shared
--enable-executable-dynamic --disable-library-vanilla’) produced packages with
the same hash, instead of different hashes.

After debugging this issue I found that this command (with no explicit cabal
project file) was resulting in these build configuration flags being ignored,
because in ProjectPlanning.hs, the sdist was not considered a local package, so
the (non-shared) local-package-only configuration was being dropped.

This fix ensures that these command-line arguments properly make it through to
where they belong in cases like this.

Additionally, adjust the ‘style’ attribute in plan.json so that globally
installed packages are designated as global even if they are local to the
project.  (Without this adjustment to ‘style2str’, the T5782Diamond test fails,
because it looks up ‘dist-dirs’ in plan.json, where ‘dist-dirs’ is absent from
the JSON.)

Finally, take into account elabDynExe and configDynExe to provide GHC with
‘-dynamic’ appropriately rather than going about it with static linking.
  • Loading branch information
bairyn committed Dec 22, 2022
1 parent df6c22b commit b547ead
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 8 deletions.
6 changes: 6 additions & 0 deletions cabal-install/src/Distribution/Client/Configure.hs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ configure verbosity packageDBs repoCtxt comp platform progdb
(fromFlagOrDefault
(useDistPref defaultSetupScriptOptions)
(configDistPref configFlags))
(fromFlagOrDefault
(setupConfigDynamic defaultSetupScriptOptions)
(configDynExe configFlags))
(chooseCabalVersion
configExFlags
(flagToMaybe (configCabalVersion configExFlags)))
Expand All @@ -167,6 +170,7 @@ configureSetupScript :: PackageDBStack
-> Platform
-> ProgramDb
-> FilePath
-> Bool
-> VersionRange
-> Maybe Lock
-> Bool
Expand All @@ -178,6 +182,7 @@ configureSetupScript packageDBs
platform
progdb
distPref
dynExe
cabalVersion
lock
forceExternal
Expand Down Expand Up @@ -209,6 +214,7 @@ configureSetupScript packageDBs
, useDependenciesExclusive = not defaultSetupDeps && isJust explicitSetupDeps
, useVersionMacros = not defaultSetupDeps && isJust explicitSetupDeps
, isInteractive = False
, setupConfigDynamic = dynExe
}
where
-- When we are compiling a legacy setup script without an explicit
Expand Down
1 change: 1 addition & 0 deletions cabal-install/src/Distribution/Client/Install.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,7 @@ performInstallations verbosity
platform
progdb
distPref
(fromFlagOrDefault (setupConfigDynamic defaultSetupScriptOptions) $ configDynExe configFlags)
(chooseCabalVersion configExFlags (libVersion miscOptions))
(Just lock)
parallelInstall
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/src/Distribution/Client/ProjectPlanOutput.hs
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,9 @@ encodePlanAsJson distDirLayout elaboratedInstallPlan elaboratedSharedConfig =
comp2str = prettyShow

style2str :: Bool -> BuildStyle -> String
style2str _ BuildAndInstall = "global"
style2str True _ = "local"
style2str False BuildInplaceOnly = "inplace"
style2str False BuildAndInstall = "global"

jdisplay :: Pretty a => a -> J.Value
jdisplay = J.String . prettyShow
Expand Down
39 changes: 36 additions & 3 deletions cabal-install/src/Distribution/Client/ProjectPlanning.hs
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,7 @@ rebuildInstallPlan verbosity
projectConfigAllPackages,
projectConfigLocalPackages,
projectConfigSpecificPackage,
projectPackagesNamed,
projectConfigBuildOnly
}
(compiler, platform, progdb) pkgConfigDB
Expand All @@ -692,6 +693,7 @@ rebuildInstallPlan verbosity
localPackages
sourcePackageHashes
defaultInstallDirs
projectPackagesNamed
projectConfigShared
projectConfigAllPackages
projectConfigLocalPackages
Expand Down Expand Up @@ -1350,6 +1352,7 @@ elaborateInstallPlan
-> [PackageSpecifier (SourcePackage (PackageLocation loc))]
-> Map PackageId PackageSourceHash
-> InstallDirs.InstallDirTemplates
-> [PackageVersionConstraint]
-> ProjectConfigShared
-> PackageConfig
-> PackageConfig
Expand All @@ -1361,6 +1364,7 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
solverPlan localPackages
sourcePackageHashes
defaultInstallDirs
extraPackages
sharedPackageConfig
allPackagesConfig
localPackagesConfig
Expand Down Expand Up @@ -2031,15 +2035,21 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
$ map packageId
$ SolverInstallPlan.reverseDependencyClosure
solverPlan
(map PlannedId (Set.toList pkgsLocalToProject))
(map PlannedId (Set.toList pkgsInplaceToProject))

isLocalToProject :: Package pkg => pkg -> Bool
isLocalToProject pkg = Set.member (packageId pkg)
pkgsLocalToProject

pkgsInplaceToProject :: Set PackageId
pkgsInplaceToProject =
Set.fromList (catMaybes (map shouldBeLocal localPackages))
--TODO: localPackages is a misnomer, it's all project packages
-- here is where we decide which ones will be local!

pkgsLocalToProject :: Set PackageId
pkgsLocalToProject =
Set.fromList (catMaybes (map shouldBeLocal localPackages))
Set.fromList (catMaybes (map (isInLocal extraPackages) localPackages))
--TODO: localPackages is a misnomer, it's all project packages
-- here is where we decide which ones will be local!

Expand Down Expand Up @@ -2108,6 +2118,28 @@ shouldBeLocal (SpecificSourcePackage pkg) = case srcpkgSource pkg of
LocalUnpackedPackage _ -> Just (packageId pkg)
_ -> Nothing

-- Used to determine which packages are affected by local package configuration
-- flags like ‘--enable-shared --enable-executable-dynamic --disable-library-vanilla’.
isInLocal :: [PackageVersionConstraint] -> PackageSpecifier (SourcePackage (PackageLocation loc)) -> Maybe PackageId
isInLocal _ NamedPackage{} = Nothing
isInLocal _extraPackages (SpecificSourcePackage pkg) = case srcpkgSource pkg of
LocalUnpackedPackage _ -> Just (packageId pkg)
-- LocalTarballPackage is matched here too, because otherwise ‘sdistize’
-- produces for ‘localPackages’ in the ‘ProjectBaseContext’ a
-- LocalTarballPackage, and ‘shouldBeLocal’ will make flags like
-- ‘--disable-library-vanilla’ have no effect for a typical
-- ‘cabal install --lib --enable-shared enable-executable-dynamic --disable-library-vanilla’,
-- as these flags would apply to local packages, but the sdist would
-- erroneously not get categorized as a local package, so the flags would be
-- ignored and produce a package with an unchanged hash.
LocalTarballPackage _ -> Just (packageId pkg)
-- TODO: the docs say ‘extra-packages’ is implemented in cabal project
-- files. We can fix that here by checking that the version range matches.
--RemoteTarballPackage _ -> _
--RepoTarballPackage _ -> _
--RemoteSourceRepoPackage _ -> _
_ -> Nothing

-- | Given a 'ElaboratedPlanPackage', report if it matches a 'ComponentName'.
matchPlanPkg :: (ComponentName -> Bool) -> ElaboratedPlanPackage -> Bool
matchPlanPkg p = InstallPlan.foldPlanPackage (p . ipiComponentName) (matchElabPkg p)
Expand Down Expand Up @@ -3387,7 +3419,8 @@ setupHsScriptOptions (ReadyPackage elab@ElaboratedConfiguredPackage{..})
useWin32CleanHack = False, --TODO: [required eventually]
forceExternalSetupMethod = isParallelBuild,
setupCacheLock = Just cacheLock,
isInteractive = False
isInteractive = False,
setupConfigDynamic = elabDynExe
}


Expand Down
15 changes: 12 additions & 3 deletions cabal-install/src/Distribution/Client/SetupWrapper.hs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import Distribution.Simple.BuildPaths
import Distribution.Simple.Command
( CommandUI(..), commandShowOptions )
import Distribution.Simple.Program.GHC
( GhcMode(..), GhcOptions(..), renderGhcOptions )
( GhcMode(..), GhcDynLinkMode(..), GhcOptions(..), renderGhcOptions )
import qualified Distribution.Simple.PackageIndex as PackageIndex
import Distribution.Simple.PackageIndex (InstalledPackageIndex)
import qualified Distribution.InstalledPackageInfo as IPI
Expand Down Expand Up @@ -249,7 +249,12 @@ data SetupScriptOptions = SetupScriptOptions {
-- | Is the task we are going to run an interactive foreground task,
-- or an non-interactive background task? Based on this flag we
-- decide whether or not to delegate ctrl+c to the spawned task
isInteractive :: Bool
isInteractive :: Bool,

-- Also track build output artifact configuration.

-- | Pass `-dynamic` to `ghc` for dynamic rather than static linking.
setupConfigDynamic :: Bool
}

defaultSetupScriptOptions :: SetupScriptOptions
Expand All @@ -272,7 +277,8 @@ defaultSetupScriptOptions = SetupScriptOptions {
useWin32CleanHack = False,
forceExternalSetupMethod = False,
setupCacheLock = Nothing,
isInteractive = False
isInteractive = False,
setupConfigDynamic = False
}

workingDir :: SetupScriptOptions -> FilePath
Expand Down Expand Up @@ -840,6 +846,9 @@ getExternalSetupMethod verbosity options pkg bt = do
-- --ghc-option=-v instead!
ghcOptVerbosity = Flag (min verbosity normal)
, ghcOptMode = Flag GhcModeMake
, ghcOptDynLinkMode = case setupConfigDynamic options'' of
True -> Flag GhcDynamicOnly
False -> Flag GhcStaticOnly
, ghcOptInputFiles = toNubListR [setupHs]
, ghcOptOutputFile = Flag setupProgFile
, ghcOptObjDir = Flag setupDir
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/tests/IntegrationTests2.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1591,7 +1591,7 @@ testProgramOptionsLocal config0 = do
(Just [ghcFlag])
(getProgArgs localPackages "q")
assertEqual "p"
Nothing
(Just [ghcFlag])
(getProgArgs localPackages "p")
where
testdir = "regression/program-options"
Expand Down
7 changes: 7 additions & 0 deletions changelog.d/pr-8623
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
synopsis: Fix project-local flags being ignored
packages: cabal-install
prs: #8623
description: {
Fix some cases of configuration flags being dropped, e.g. with `v2-install`
and `--enable-shared --enable-executable-dynamic --disable-library-vanilla`.
}

0 comments on commit b547ead

Please sign in to comment.