Skip to content

Fix pre-existing public sublibraries from package DBs#1

Open
paolino wants to merge 2 commits intomasterfrom
fix/public-sublib-installed-solver
Open

Fix pre-existing public sublibraries from package DBs#1
paolino wants to merge 2 commits intomasterfrom
fix/public-sublib-installed-solver

Conversation

@paolino
Copy link
Copy Markdown

@paolino paolino commented Apr 17, 2026

Problem

cabal v2-build fails 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:

  • Package pkg and its public sublibrary pkg:sublib are both pre-installed in a GHC package DB
  • pkg:sublib depends on pkg's main library
  • A consumer is built against the DB with active-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 run cabal inside its nix develop shell 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:

  • Installed public sublibraries keep their source package name instead of the munged name
  • They expose their actual LibraryName rather than being flattened to LMainLibName
  • Installed-to-installed edges target the real exposed component
  • Same-package self-edges are dropped from the solver view (reintroduced later from the installed closure)

This lets the solver pick pkg:sublib as 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.ConfiguredConversion records the installed dependency closure in InstSolverPackage
  • Distribution.Client.ProjectPlanning materializes that closure when elaborating pre-existing packages
  • Distribution.Client.InstallPlan deduplicates by UnitId so shared closure units don't collide

Tests

New regression under cabal-testsuite/PackageTests/PackageDB/PublicSublibPreExisting:

  • dep — main library + public sublibrary depending on it
  • q — depends only on the sublibrary
  • active-repositories: :none forces the external-DB path

Design notes

  • Why not the solver alone? Once the solver picks the right unit, the failure moves to install-plan construction. The installed closure must travel from where the installed index is still in scope (solver-side conversion) to where the plan is built (planner-side elaboration) — hence changes on both sides.
  • Why store the closure on InstSolverPackage? It's the handoff point where the selected installed unit is known and the installed index is still available.
  • Not a source-planning workaround. This fixes the installed-package-DB reuse path directly, rather than routing consumers back to source builds.

Files to review

  • cabal-install-solver/src/Distribution/Solver/Modular/IndexConversion.hs
  • cabal-install-solver/src/Distribution/Solver/Modular/ConfiguredConversion.hs
  • cabal-install-solver/src/Distribution/Solver/Types/InstSolverPackage.hs
  • cabal-install/src/Distribution/Client/ProjectPlanning.hs
  • cabal-install/src/Distribution/Client/InstallPlan.hs
  • cabal-testsuite/PackageTests/PackageDB/PublicSublibPreExisting/

Additional verification

Beyond the new testsuite entry, the fix was validated against two reductions of the cardano-wallet failure:

  • a one-file harness that provisions a package DB from Hackage sources and runs cabal build against it (fails on stock cabal, passes patched)
  • the same harness with CHaP sources (the actual origin of the wallet failure) — fails stock, passes patched

Both harnesses are reductions rather than upstream-ready tests and live out of tree; they can be shared on request.

@paolino paolino added the bug Something isn't working label Apr 17, 2026
@paolino paolino self-assigned this Apr 17, 2026
@paolino
Copy link
Copy Markdown
Author

paolino commented Apr 17, 2026

@copilot review

Copy link
Copy Markdown

Copilot AI commented Apr 17, 2026

@copilot review

Here is a code review of the PR.


Overall

The 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.


IndexConversion.hs

isPublicSubLibrary predicate is duplicated in InstSolverPackage.hs.
The check IPI.libVisibility ipi == LibraryVisibilityPublic && IPI.sourceLibName ipi /= LMainLibName appears as the named helper isPublicSubLibrary here, but is copy-pasted inline into the Package instance in InstSolverPackage.hs (lines 49–50). Extracting it to a shared module (e.g. a small Distribution.Solver.Types.InstalledPkgUtils) would eliminate the drift risk.

convIPId self-edge filter uses packageId (source package ID) comparison.

| packageId current == packageId ipi -> Right Nothing

This correctly drops all intra-source-package deps because packageId for InstalledPackageInfo returns sourcePackageId. Fine as-is, but worth a one-line comment explaining why packageId (not installedUnitId) is the right key here, since the new installedPackageName indirection could confuse a future reader.


ConfiguredConversion.hsinstalledDepsClosure

The BFS is correct but the rootUnitId is never added to the initial seen set. In a well-formed installed DB this cannot cause a cycle, but adding Set.singleton rootUnitId as the seed would make the termination argument self-evident:

go (Set.singleton rootUnitId) (IPI.depends rootIpi)

InstallPlan.hsdistinctUnits

Map.fromList keeps the last value for duplicate keys. All duplicates are guaranteed identical units, so the behaviour is correct, but the semantics are the reverse of what you probably intend ("first registered wins"). Map.fromListWith const makes the first-wins intent explicit and avoids any future surprise if the list ordering changes.


ProjectPlanning.hspreExistingUnitClosure

The solverPreExistingUnitIds guard correctly avoids re-adding units that already have their own solver node. distinctUnits in InstallPlan.hs then catches any remaining duplicates across multiple pre-existing nodes. The two-pass deduplication is necessary and correct.


configureInstallPlan (legacy v1 path) — not updated

configureInstallPlan in InstallPlan.hs (used by cabal install / Configure.hs) still yields only instSolverPkgIPI pkg without the closure. If the legacy path ever encounters this shape it will fail in the same way. This is probably an acceptable known limitation for a v1 code path, but it is worth a comment or a follow-up issue.


Test

The fixture is well-targeted and directly exercises the failing shape. One small note: the ghcPkg' "field" [...] assertions check that the units are present in the DB — good for early diagnosis — but the actual regression is caught by the runPlanExe' that follows, which would panic or error if the plan were still broken.


Summary

The core logic is correct. The main actionable points are:

  1. Extract isPublicSubLibrary to a shared location to remove the duplication.
  2. Seed installedDepsClosure with Set.singleton rootUnitId.
  3. Use Map.fromListWith const in distinctUnits for clarity.
  4. Note (or track) the unresolved gap in configureInstallPlan.

@paolino paolino force-pushed the fix/public-sublib-installed-solver branch 2 times, most recently from ac8dec4 to 77d53be Compare April 18, 2026 11:41
@paolino paolino force-pushed the fix/public-sublib-installed-solver branch from 77d53be to 0c33f03 Compare April 18, 2026 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants