From 533cbc1e6f1915a1eca240b68b0b2ea281309963 Mon Sep 17 00:00:00 2001 From: Mikolaj Konarski Date: Wed, 8 Feb 2023 17:08:09 +0100 Subject: [PATCH 1/2] Revert "Fix project-local build flags being ignored." This reverts commit b547ead58bf09bb838c13f02afb2f1042ad1bc7c from https://github.com/haskell/cabal/pull/8623. Unexpected side-effect has been found, so these code improvements have to be done differently. The other commit in the PR is a test and it's retained. --- .../src/Distribution/Client/Configure.hs | 6 --- .../src/Distribution/Client/Install.hs | 1 - .../Distribution/Client/ProjectPlanOutput.hs | 2 +- .../Distribution/Client/ProjectPlanning.hs | 39 ++----------------- .../src/Distribution/Client/SetupWrapper.hs | 15 ++----- cabal-install/tests/IntegrationTests2.hs | 2 +- changelog.d/pr-8623 | 7 ---- 7 files changed, 8 insertions(+), 64 deletions(-) delete mode 100644 changelog.d/pr-8623 diff --git a/cabal-install/src/Distribution/Client/Configure.hs b/cabal-install/src/Distribution/Client/Configure.hs index 554785ff847..2cbe16096a4 100644 --- a/cabal-install/src/Distribution/Client/Configure.hs +++ b/cabal-install/src/Distribution/Client/Configure.hs @@ -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))) @@ -170,7 +167,6 @@ configureSetupScript :: PackageDBStack -> Platform -> ProgramDb -> FilePath - -> Bool -> VersionRange -> Maybe Lock -> Bool @@ -182,7 +178,6 @@ configureSetupScript packageDBs platform progdb distPref - dynExe cabalVersion lock forceExternal @@ -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 diff --git a/cabal-install/src/Distribution/Client/Install.hs b/cabal-install/src/Distribution/Client/Install.hs index a53c7ded1aa..2baa8af9e49 100644 --- a/cabal-install/src/Distribution/Client/Install.hs +++ b/cabal-install/src/Distribution/Client/Install.hs @@ -1059,7 +1059,6 @@ performInstallations verbosity platform progdb distPref - (fromFlagOrDefault (setupConfigDynamic defaultSetupScriptOptions) $ configDynExe configFlags) (chooseCabalVersion configExFlags (libVersion miscOptions)) (Just lock) parallelInstall diff --git a/cabal-install/src/Distribution/Client/ProjectPlanOutput.hs b/cabal-install/src/Distribution/Client/ProjectPlanOutput.hs index fde7ea8b97a..c9243c310e0 100644 --- a/cabal-install/src/Distribution/Client/ProjectPlanOutput.hs +++ b/cabal-install/src/Distribution/Client/ProjectPlanOutput.hs @@ -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 diff --git a/cabal-install/src/Distribution/Client/ProjectPlanning.hs b/cabal-install/src/Distribution/Client/ProjectPlanning.hs index 4ec141037b7..8d8947ae8a8 100644 --- a/cabal-install/src/Distribution/Client/ProjectPlanning.hs +++ b/cabal-install/src/Distribution/Client/ProjectPlanning.hs @@ -672,7 +672,6 @@ rebuildInstallPlan verbosity projectConfigAllPackages, projectConfigLocalPackages, projectConfigSpecificPackage, - projectPackagesNamed, projectConfigBuildOnly } (compiler, platform, progdb) pkgConfigDB @@ -698,7 +697,6 @@ rebuildInstallPlan verbosity localPackages sourcePackageHashes installDirs - projectPackagesNamed projectConfigShared projectConfigAllPackages projectConfigLocalPackages @@ -1363,7 +1361,6 @@ elaborateInstallPlan -> [PackageSpecifier (SourcePackage (PackageLocation loc))] -> Map PackageId PackageSourceHash -> InstallDirs.InstallDirTemplates - -> [PackageVersionConstraint] -> ProjectConfigShared -> PackageConfig -> PackageConfig @@ -1375,7 +1372,6 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB solverPlan localPackages sourcePackageHashes defaultInstallDirs - extraPackages sharedPackageConfig allPackagesConfig localPackagesConfig @@ -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! @@ -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) @@ -3430,8 +3398,7 @@ setupHsScriptOptions (ReadyPackage elab@ElaboratedConfiguredPackage{..}) useWin32CleanHack = False, --TODO: [required eventually] forceExternalSetupMethod = isParallelBuild, setupCacheLock = Just cacheLock, - isInteractive = False, - setupConfigDynamic = elabDynExe + isInteractive = False } diff --git a/cabal-install/src/Distribution/Client/SetupWrapper.hs b/cabal-install/src/Distribution/Client/SetupWrapper.hs index 239e1a37908..e4885ed07c6 100644 --- a/cabal-install/src/Distribution/Client/SetupWrapper.hs +++ b/cabal-install/src/Distribution/Client/SetupWrapper.hs @@ -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 @@ -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 @@ -277,8 +272,7 @@ defaultSetupScriptOptions = SetupScriptOptions { useWin32CleanHack = False, forceExternalSetupMethod = False, setupCacheLock = Nothing, - isInteractive = False, - setupConfigDynamic = False + isInteractive = False } workingDir :: SetupScriptOptions -> FilePath @@ -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 diff --git a/cabal-install/tests/IntegrationTests2.hs b/cabal-install/tests/IntegrationTests2.hs index 6b81643fe0b..90d272aacae 100644 --- a/cabal-install/tests/IntegrationTests2.hs +++ b/cabal-install/tests/IntegrationTests2.hs @@ -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" diff --git a/changelog.d/pr-8623 b/changelog.d/pr-8623 deleted file mode 100644 index 29c9c3bb2c5..00000000000 --- a/changelog.d/pr-8623 +++ /dev/null @@ -1,7 +0,0 @@ -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`. -} From dbcaa76374d2feec837feda4ca9e9b60d0690666 Mon Sep 17 00:00:00 2001 From: Mikolaj Konarski Date: Wed, 8 Feb 2023 18:16:26 +0100 Subject: [PATCH 2/2] Mark the test we are retaining as expected broken --- .../PackageTests/LinkerOptions/NonignoredConfigs/cabal.test.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cabal-testsuite/PackageTests/LinkerOptions/NonignoredConfigs/cabal.test.hs b/cabal-testsuite/PackageTests/LinkerOptions/NonignoredConfigs/cabal.test.hs index 9da924366f4..2e8dac23a20 100644 --- a/cabal-testsuite/PackageTests/LinkerOptions/NonignoredConfigs/cabal.test.hs +++ b/cabal-testsuite/PackageTests/LinkerOptions/NonignoredConfigs/cabal.test.hs @@ -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’