Skip to content

Commit

Permalink
Merge pull request #8744 from Mikolaj/revert-8623
Browse files Browse the repository at this point in the history
Revert "Fix project-local build flags being ignored."
  • Loading branch information
mergify[bot] authored Feb 14, 2023
2 parents 7d4b33f + dbcaa76 commit fd09059
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 65 deletions.
6 changes: 0 additions & 6 deletions cabal-install/src/Distribution/Client/Configure.hs
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,6 @@ configure verbosity packageDBs repoCtxt comp platform progdb
(fromFlagOrDefault
(useDistPref defaultSetupScriptOptions)
(configDistPref configFlags))
(fromFlagOrDefault
(setupConfigDynamic defaultSetupScriptOptions)
(configDynExe configFlags))
(chooseCabalVersion
configExFlags
(flagToMaybe (configCabalVersion configExFlags)))
Expand All @@ -170,7 +167,6 @@ configureSetupScript :: PackageDBStack
-> Platform
-> ProgramDb
-> FilePath
-> Bool
-> VersionRange
-> Maybe Lock
-> Bool
Expand All @@ -182,7 +178,6 @@ configureSetupScript packageDBs
platform
progdb
distPref
dynExe
cabalVersion
lock
forceExternal
Expand Down Expand Up @@ -214,7 +209,6 @@ 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: 0 additions & 1 deletion cabal-install/src/Distribution/Client/Install.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,6 @@ 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: 3 additions & 36 deletions cabal-install/src/Distribution/Client/ProjectPlanning.hs
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,6 @@ rebuildInstallPlan verbosity
projectConfigAllPackages,
projectConfigLocalPackages,
projectConfigSpecificPackage,
projectPackagesNamed,
projectConfigBuildOnly
}
(compiler, platform, progdb) pkgConfigDB
Expand All @@ -698,7 +697,6 @@ rebuildInstallPlan verbosity
localPackages
sourcePackageHashes
installDirs
projectPackagesNamed
projectConfigShared
projectConfigAllPackages
projectConfigLocalPackages
Expand Down Expand Up @@ -1363,7 +1361,6 @@ elaborateInstallPlan
-> [PackageSpecifier (SourcePackage (PackageLocation loc))]
-> Map PackageId PackageSourceHash
-> InstallDirs.InstallDirTemplates
-> [PackageVersionConstraint]
-> ProjectConfigShared
-> PackageConfig
-> PackageConfig
Expand All @@ -1375,7 +1372,6 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
solverPlan localPackages
sourcePackageHashes
defaultInstallDirs
extraPackages
sharedPackageConfig
allPackagesConfig
localPackagesConfig
Expand Down Expand Up @@ -2046,21 +2042,15 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
$ map packageId
$ SolverInstallPlan.reverseDependencyClosure
solverPlan
(map PlannedId (Set.toList pkgsInplaceToProject))
(map PlannedId (Set.toList pkgsLocalToProject))

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 (isInLocal extraPackages) localPackages))
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!

Expand Down Expand Up @@ -2129,28 +2119,6 @@ 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 @@ -3430,8 +3398,7 @@ setupHsScriptOptions (ReadyPackage elab@ElaboratedConfiguredPackage{..})
useWin32CleanHack = False, --TODO: [required eventually]
forceExternalSetupMethod = isParallelBuild,
setupCacheLock = Just cacheLock,
isInteractive = False,
setupConfigDynamic = elabDynExe
isInteractive = False
}


Expand Down
15 changes: 3 additions & 12 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(..), GhcDynLinkMode(..), GhcOptions(..), renderGhcOptions )
( GhcMode(..), 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,12 +249,7 @@ 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,

-- Also track build output artifact configuration.

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

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

workingDir :: SetupScriptOptions -> FilePath
Expand Down Expand Up @@ -846,9 +840,6 @@ 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"
(Just [ghcFlag])
Nothing
(getProgArgs localPackages "p")
where
testdir = "regression/program-options"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ linkConfigFlags Dynamic =
lrun :: [Linking]
lrun = [Static, Dynamic, Static, Dynamic]

main = cabalTest $ do
main = cabalTest . expectBroken 8744 $ do
-- Skip if on Windows, since my default Chocolatey Windows setup (and the CI
-- server setup at the time, presumably) lacks support for dynamic builds
-- since the base package appears to be static only, lacking e.g. ‘.dyn_o’
Expand Down
7 changes: 0 additions & 7 deletions changelog.d/pr-8623

This file was deleted.

0 comments on commit fd09059

Please sign in to comment.