Fix pre-existing public sublibraries from package DBs#1
Fix pre-existing public sublibraries from package DBs#1
Conversation
|
@copilot review |
Here is a code review of the PR. OverallThe fix is logically sound. The three-layer approach — fixing the solver index view, carrying the installed dep closure through the solver output, and materialising it in the elaborated plan — correctly handles the case where a public sublibrary depends on its own package's main library, which is never exposed as a separate solver node.
|
ac8dec4 to
77d53be
Compare
77d53be to
0c33f03
Compare
Problem
cabal v2-buildfails when a public sublibrary of a package is already present in an external GHC package DB and a consumer depends on that sublibrary.Minimal shape:
pkgand its public sublibrarypkg:sublibare both pre-installed in a GHC package DBpkg:sublibdepends onpkg's main libraryactive-repositories: :none(or otherwise cannot rebuild the dependency from source)Result before this patch: either the solver rejects the installed unit with
does not contain library ..., or install-plan construction fails because the chosen pre-existing units do not form a closed graph.This surfaces in practice in Nix developer shells that provision GHC package DBs with many pre-built dependencies — one concrete example is
cardano-wallet, which cannot runcabalinside itsnix developshell because of this bug.Fix
Two changes, in both the solver and the planner.
1. Solver sees public sublibraries as real components
Distribution.Solver.Modular.IndexConversion:LibraryNamerather than being flattened toLMainLibNameThis lets the solver pick
pkg:sublibas a pre-existing dependency.2. Install plan keeps the pre-existing unit closed
Picking the sublibrary alone isn't enough — its chosen installed unit transitively depends on other installed units that have no solver node.
Distribution.Solver.Modular.ConfiguredConversionrecords the installed dependency closure inInstSolverPackageDistribution.Client.ProjectPlanningmaterializes that closure when elaborating pre-existing packagesDistribution.Client.InstallPlandeduplicates byUnitIdso shared closure units don't collideTests
New regression under
cabal-testsuite/PackageTests/PackageDB/PublicSublibPreExisting:dep— main library + public sublibrary depending on itq— depends only on the sublibraryactive-repositories: :noneforces the external-DB pathDesign notes
InstSolverPackage? It's the handoff point where the selected installed unit is known and the installed index is still available.Files to review
cabal-install-solver/src/Distribution/Solver/Modular/IndexConversion.hscabal-install-solver/src/Distribution/Solver/Modular/ConfiguredConversion.hscabal-install-solver/src/Distribution/Solver/Types/InstSolverPackage.hscabal-install/src/Distribution/Client/ProjectPlanning.hscabal-install/src/Distribution/Client/InstallPlan.hscabal-testsuite/PackageTests/PackageDB/PublicSublibPreExisting/Additional verification
Beyond the new testsuite entry, the fix was validated against two reductions of the
cardano-walletfailure:cabal buildagainst it (fails on stockcabal, passes patched)Both harnesses are reductions rather than upstream-ready tests and live out of tree; they can be shared on request.