cabal-install: guard the base >= 4.22 constraint on wired-in unit ids

[andreabedini]

Fixes #12328.

dependOnWiredIns adds two kinds of constraint: one installed-unit-id
constraint per wired-in unit of the compiler, and one base >= 4.22 version
constraint added in #12055.

The second only makes sense next to the first. The unit-id constraints pin the
new, reinstallable base; the version constraint then excludes the old,
non-reinstallable one. But it is added unconditionally, while the unit-id
constraints are empty for a compiler that reports no wired-in units. Such a
compiler is affected because --allow-boot-library-installs alone is enough to
reach this code.

This makes dependOnWiredIns a no-op for a compiler that reports no wired-in
unit ids, so the version constraint applies only where the unit-id constraints
do. Behaviour is unchanged for a compiler that reports wired-in unit ids.

Test

UnitTests.Distribution.Solver.Modular.Solver gets one test in the group
"Non-reinstallable base, template-haskell and ghc (GHC without wiredInUnitIds)".
It uses the existing dbBaseOld database, which holds base-1 alone, enables
--allow-boot-library-installs and expects a successful plan. Without the fix
the test fails.

QA notes

The script in #12328 exits non-zero on an unpatched cabal. Against this branch
it succeeds with GHC 9.10.3, 9.12.2 and 9.14.1.

Checklist

Template A: This PR modifies behaviour or interface

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
  • Is the change significant? No.
• The documentation has been updated, if necessary. No user guide change is necessary.
• Manual QA notes have been included.
• Tests have been added.

[Copilot]

Pull request overview

This PR fixes an unsound solver constraint interaction in cabal-install when --allow-boot-library-installs is enabled on compilers that do not report wired-in unit ids. It ensures the base >= 4.22 constraint introduced for reinstallable base is only applied when the corresponding wired-in unit-id pinning constraints are also in effect, and adds a regression test.

Changes:

• Guard the base >= 4.22 constraint in dependOnWiredIns on isJust (compilerInfoWiredInUnitIds compiler) so it is only applied when wired-in unit-id constraints are available.
• Add a unit test covering the “GHC without wiredInUnitIds + allow-boot-library-installs + old base” scenario and asserting the plan is solvable.
• Add a changelog entry documenting the fix and its motivation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
cabal-install/src/Distribution/Client/Dependency.hs	Guards the base >= 4.22 constraint so it’s only added when wired-in unit-id constraints are present.
cabal-install/tests/UnitTests/Distribution/Solver/Modular/Solver.hs	Adds a regression test ensuring old base plans remain solvable when no wired-in unit ids are reported.
changelog.d/base-422-wired-in-guard.md	Documents the solver fix and why the guard is necessary.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ffaf1]

QA notes

Those are not QA notes. QA notes are meant to add a human perspective in testing, challenging the assumption of the contributor. “Testsuite goes red without the patch” is good and expected, but not useful for QA.

[andreabedini]

Those are not QA notes. QA notes are meant to add a human perspective in testing, challenging the assumption of the contributor. “Testsuite goes red without the patch” is good and expected, but not useful for QA.

@ffaf1 Apologies, removed.

PS: To be honest, CONTRIBUTING.md does not make very clear what the are meant to be.

[TeofilC]

Could you also try to edit the PR description and commit message. Right now they are quite verbose and it's difficult for me to get the signal from the noise.

Also it would be helpful to create an issue.

[andreabedini]

@TeofilC

All we really need to say is that we've fixed a bug in allow-boot-library-installs which caused false solver failures when used with ghc>=9.14

This is backward, we fix a bug where using --allow-boot-library-installs fails with any ghc<9.14.

We have two cases:

cabal/cabal-install/src/Distribution/Client/Dependency.hs

Lines 919 to 921 in 5969ad6

	if isJust (compilerInfoWiredInUnitIds comp) || asBool (depResolverAllowBootLibInstalls params)
	then dependOnWiredIns comp params
	else dontInstallNonReinstallablePackages params

Case 1: compilerInfoWiredInUnitIds is Just _ OR allow-boot-library-installs is true.
In this case we use the new dependOnWiredIns policy.

Case 2: compilerInfoWiredInUnitIds is Nothing AND allow-boot-library-installs is false.
In this case we use the old dontInstallNonReinstallablePackages policy.

The issue is that dependOnWiredIns hardcodes base >= 4.22 regardless of the compiler version, since allow-boot-library-installs can be used with any version of the compiler.

Therefore, someone using GHC 9.12.2 with --allow-boot-library-installs won't be able to use the installed base-4.21.0.0 (because of the constraint) and won't be able to use base-4.22.0.0 either (because it fails).

I will rewrite the description. @TeofilC Do you want me to open an issue?

[TeofilC]

fails with any ghc<9.14.

Indeed it's < rather than >=.

I will rewrite the description. @TeofilC Do you want me to open an issue?

Yes please. It's always helpful to have issues for history

[andreabedini]

@TeofilC I created #12328 and rewritten the PR and the commit description.

[TeofilC]

Thanks

[philderbeast]

The indentation change makes the diff look larger than it is but the indentation glitch is due to fourmolu. Remove the | isJust ... line and fourmolu decides it wants more indentation 🤷.

[philderbeast]

I see dependOnWiredIns has two calls to compilerInfoWiredInUnitIds compiler. This could a single wiredInUnitIds binding.

wiredInUnitIds.patch

$ git diff
diff --git a/cabal-install/src/Distribution/Client/Dependency.hs b/cabal-install/src/Distribution/Client/Dependency.hs
index 3e5da48be..dde549d6c 100644
--- a/cabal-install/src/Distribution/Client/Dependency.hs
+++ b/cabal-install/src/Distribution/Client/Dependency.hs
@@ -1,3 +1,5 @@
+{-# LANGUAGE ViewPatterns #-}
+
 -----------------------------------------------------------------------------
 
 -- |
@@ -445,13 +447,14 @@ setSolverVerbosity verbosity params =
     }
 
 dependOnWiredIns :: CompilerInfo -> DepResolverParams -> DepResolverParams
-dependOnWiredIns compiler params = addConstraints extraConstraints params
+dependOnWiredIns (compilerInfoWiredInUnitIds -> wiredInUnitIds) params =
+  addConstraints extraConstraints params
   where
     extraConstraints =
       [ LabeledPackageConstraint
         (PackageConstraint (ScopeAnyQualifier pkgName) (PackagePropertyInstalledSpecificUnitId unitId))
         ConstraintSourceNonReinstallablePackage
-      | (pkgName, unitId) <- fromMaybe [] $ compilerInfoWiredInUnitIds compiler
+      | (pkgName, unitId) <- fromMaybe [] wiredInUnitIds
       ]
         ++
         -- Old versions of `base` must be excluded from build plans still as they do not depend on any version of a wired-in unit.
@@ -460,7 +463,7 @@ dependOnWiredIns compiler params = addConstraints extraConstraints params
         [ LabeledPackageConstraint
           (PackageConstraint (ScopeAnyQualifier $ mkPackageName "base") (PackagePropertyVersion (orLaterVersion (mkVersion [4, 22]))))
           ConstraintSourceNonReinstallablePackage
-        | isJust (compilerInfoWiredInUnitIds compiler)
+        | isJust wiredInUnitIds
         ]
 
 -- | Some packages are specific to a given compiler version and should never be

[TeofilC]

Or use a case statement over compilerInfoWiredInUnitIds compiler and only do anything in the Just case?

[andreabedini]

Went with the case statement. It also drops the second compilerInfoWiredInUnitIds call, so it covers @philderbeast's point too. Behaviour is unchanged: with Nothing the function was already adding no constraints at all.