GitLab

Sven Tennie pushed to branch wip/supersven/refactor-system-cxx-std-lib-hadrian-rules at Glasgow Haskell Compiler / GHC

Commits:

a9529a75

by Matthew Pickering at 2026-05-30T09:02:21+02:00

hadrian: Refactor system-cxx-std-lib rules

I noticed a few things wrong with the hadrian rules for
`system-cxx-std-lib` rules.

* For `text` there is an ad-hoc check to depend on `system-cxx-std-lib`
outside of `configurePackage`.
* The `system-cxx-std-lib` dependency is not read from cabal files.
* Recache is not called on the packge database after the `.conf` file is
generated, a more natural place for this rule is `registerRules`.

Treating this uniformly like other packages is complicated by it not
having any source code or a cabal file. However we can do a bit better
by reporting the dependency firstly in `PackageData` and then needing
the `.conf` file in the same place as every other package in
`configurePackage`.

This commit increases the `shakeVersion`, to provide backwards
compatibility to previous builds with different PackageData.

Fixes #25303

Co-authored-by: Sven Tennie <sven.tennie@gmail.com>

6 changed files:

+ changelog.d/hadrian-system-cxx-std-lib-25303
hadrian/src/Hadrian/Haskell/Cabal/Parse.hs
hadrian/src/Hadrian/Haskell/Cabal/Type.hs
hadrian/src/Main.hs
hadrian/src/Rules/Generate.hs
hadrian/src/Rules/Register.hs

Changes:

changelog.d/hadrian-system-cxx-std-lib-25303

 +section: packaging
 +synopsis: Fix Hadrian rules for system-cxx-std-lib package dependency
 +issues: #25303
 +mrs: !16013
 +description: {
 +  Hadrian's handling of the `system-cxx-std-lib` virtual package has been
 +  fixed and made more uniform.
++
 +  Previously, `text` had an ad-hoc rule outside of `configurePackage` to
 +  declare a dependency on `system-cxx-std-lib`, the dependency was not
 +  discovered from cabal files, and the package database was not recached after
 +  the `.conf` file was generated.
++
 +  The dependency is now read from cabal files via a new
 +  `dependsOnSystemCxxStdLib` field in `PackageData`, and the `.conf` file
 +  is needed inside `configurePackage` alongside all other package
 +  dependencies, consistent with how every other package is handled.
++
 +  `shakeVersion` has been bumped to ensure existing build databases are
 +  invalidated when upgrading, preventing binary deserialisation errors due to
 +  the changed `PackageData` type.
 +}

hadrian/src/Hadrian/Haskell/Cabal/Parse.hs

@@ -81,10 +81,11 @@ parsePackageData pkg = do
          sorted  = sort [ C.unPackageName p | C.Dependency p _ _ <- allDeps ]
          deps    = nubOrd sorted \\ [name]
          depPkgs = mapMaybe findPackageByName deps
 +        cxxStdLib = elem "system-cxx-std-lib" deps
      return $ PackageData name version
                           (C.fromShortText (C.synopsis pd))
                           (C.fromShortText (C.description pd))
 -                         depPkgs gpd
 +                         depPkgs cxxStdLib gpd
    where
      -- Collect an overapproximation of dependencies by ignoring conditionals
      collectDeps :: Maybe (C.CondTree v [C.Dependency] a) -> [C.Dependency]
@@ -138,7 +139,9 @@ configurePackage :: Context -> Action ()
  configurePackage context@Context {..} = do
      putProgressInfo $ "| Configure package " ++ quote (pkgName package)
      gpd     <- pkgGenericDescription package
 -    depPkgs <- packageDependencies <$> readPackageData package
 +    pd <- readPackageData package
 +    let depPkgs = packageDependencies pd
 +        needSystemCxxStdLib = dependsOnSystemCxxStdLib pd
      -- Stage packages are those we have in this stage.
      stagePkgs <- stagePackages stage
@@ -157,7 +160,12 @@ configurePackage context@Context {..} = do
      -- We'll need those packages in our package database.
      deps <- sequence [ pkgConfFile (context { package = pkg, iplace = forceBaseAfterGhcInternal pkg })
                       | pkg <- depPkgs, pkg `elem` stagePkgs ]
 -    need $ extraPreConfigureDeps ++ deps
 +    -- system-cxx-std-lib is magic.. it doesn't have a cabal file or source code, so we have
 +    -- to treat it specially as `pkgConfFile` uses `readPackageData` to compute the version.
 +    systemCxxStdLib <- sequence [ systemCxxStdLibConfPath (PackageDbLoc stage iplace) | needSystemCxxStdLib ]
 +    need $ extraPreConfigureDeps
 +            ++ deps
 +            ++ systemCxxStdLib
      -- Figure out what hooks we need.
      let configureFile = replaceFileName (pkgCabalFile package) "configure"

hadrian/src/Hadrian/Haskell/Cabal/Type.hs

@@ -30,6 +30,7 @@ data PackageData = PackageData
      , synopsis                  :: String
      , description               :: String
      , packageDependencies       :: [Package]
 +    , dependsOnSystemCxxStdLib  :: Bool
      , genericPackageDescription :: GenericPackageDescription
      } deriving (Eq, Generic, Show)

hadrian/src/Main.hs

@@ -63,7 +63,13 @@ main = do
      shakeColor <- shouldUseColor
      let options :: ShakeOptions
          options = shakeOptions
 -            { shakeChange   = ChangeModtimeAndDigest
 +            { -- Bump shakeVersion whenever a type stored in the Shake oracle
 +              -- changes its Binary representation (e.g. fields added/removed
 +              -- from PackageData or other oracle value types). This forces
 +              -- Shake to wipe the stale database instead of crashing on
 +              -- deserialisation.
 +              shakeVersion  = "2"
 +            , shakeChange   = ChangeModtimeAndDigest
              , shakeFiles    = buildRoot -/- Base.shakeFilesDir
              , shakeProgress = Progress.hadrianProgress cwd
              , shakeRebuild  = rebuild

hadrian/src/Rules/Generate.hs

@@ -242,9 +242,6 @@ copyRules = do
          prefix -/- "html/**"           <~ return "utils/haddock/haddock-api/resources"
          prefix -/- "latex/**"          <~ return "utils/haddock/haddock-api/resources"
 -        forM_ [Inplace, Final] $ \iplace ->
 -          root -/- relativePackageDbPath (PackageDbLoc stage iplace) -/- systemCxxStdLibConf %> \file -> do
 -            copyFile ("mk" -/- "system-cxx-std-lib-1.0.conf") file
  generateRules :: Rules ()
  generateRules = do

hadrian/src/Rules/Register.hs

@@ -6,7 +6,6 @@ module Rules.Register (
  import Base
  import Context
 -import Flavour
  import Oracles.Setting
  import Hadrian.BuildPath
  import Hadrian.Expression
@@ -48,14 +47,6 @@ configurePackageRules = do
            isGmp <- (== "gmp") <$> interpretInContext ctx getBignumBackend
            when isGmp $
              need [buildP -/- "include/ghc-gmp.h"]
 -        when (pkg == text) $ do
 -          simdutf <- textWithSIMDUTF <$> flavour
 -          when simdutf $ do
 -            -- This is required, otherwise you get Error: hadrian:
 -            -- Encountered missing or private dependencies:
 -            -- system-cxx-std-lib ==1.0
 -            cxxStdLib <- systemCxxStdLibConfPath $ PackageDbLoc stage Inplace
 -            need [cxxStdLib]
          Cabal.configurePackage ctx
      root -/- "**/autogen/cabal_macros.h" %> \out -> do
@@ -105,6 +96,12 @@ registerPackageRules rs stage iplace = do
              target (Context stage compiler vanilla iplace) (GhcPkg Recache stage) [] []
          writeFileLines stamp []
 +    -- Special rule for registering system-cxx-std-lib
 +    root -/- relativePackageDbPath (PackageDbLoc stage iplace) -/- systemCxxStdLibConf %> \file -> do
 +        copyFile ("mk" -/- "system-cxx-std-lib-1.0.conf") file
 +        buildWithResources rs $
 +            target (Context stage compiler vanilla iplace) (GhcPkg Recache stage) [] []
++
      -- Register a package.
      root -/- relativePackageDbPath (PackageDbLoc stage iplace) -/- "*.conf" %> \conf -> do
          historyDisable