Group installed multilib packages

[sorki]

Before this patch, sub-libraries (either public or private) of installed
packages are treated as top level separate packages from solvers point of view.

This differs from how source packages are handled, which are treated as
a single package.

Sub-libraries of installed packages needed munging to prevent conflicts
like described in Note [Index conversion with internal libraries].

Due to package name munging, top level library we are trying to build,
cannot depend on public sub-libraries, more precisely - it can, but
the installed packages are never used. Instead they are always
rebuilt from source (as only source package index exposes public
sub-libs correctly).

The idea is to make packages coming from installed package index look
similar to packages coming from source - appearing as a single package
instead of multiple.

The approach we choose is adding a grouping pass to solvers installed package
index conversion. After the index is converted, we group installed
packages by name and version and adjust all dependencies to point to
newly built groups instead of split sub-lib packages. This way, solver
only sees a single package (similar to how source packages are treated).

This is done by means of adding

• InstGroup Loc to solver
• installedSublibs field to InstalledPackageInfo
• and finally expanding installedSublibs in ProjectPlanning

Now the sub-libs are handled correctly and build process
picks installed public sub-libs when available. It also
allows us to drop package name munging hacks from solver
completely.

See #6039

---

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
  • Is the change significant? If so, remember to add significance: significant in the changelog file.
• The documentation has been updated, if necessary.
• Manual QA notes have been included.
• Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

[ulysses4ever]

Would it be possible to add some prose about "the grouping approach" somewhere close to this PR (commit message or PR description or both)?

[sorki]

Sure! Adding a stripped before/after trace:

cabal freeze trace before

Resolving dependencies...

( PackageName "attoparsec"
, I
    ( mkVersion
        [ 0
        , 14
        , 4
        ]
    )
    ( Inst
        ( UnitId "attoparsec-0.14.4-3OIDNSUWze2AVr1XW1152M" )
    )
, PInfo
    [ <...>
    , Simple
        ( LDep
            ( DependencyReason
                ( PackageName "attoparsec" )
                ( fromList [] )
                ( fromList [] )
            )
            ( Dep
                ( PkgComponent
                    ( PackageName "z-attoparsec-z-attoparsec-internal" ) ( ExposedLib LMainLibName )
                )
                ( Fixed
                    ( I
                        ( mkVersion
                            [ 0
                            , 14
                            , 4
                            ]
                        )
                        ( Inst
                            ( UnitId "attoparsec-0.14.4-CbsDEOj99TyGhnIWKEfhEq-attoparsec-internal" )
                        )
                    )
                )
            )
        ) ComponentLib
    , <...>
    ]
    ( fromList
        [
            ( ExposedLib LMainLibName
            , ComponentInfo
                { compIsVisible = IsVisible True
                , compIsBuildable = IsBuildable True
                }
            )
        ]
    )
    ( fromList [] ) Nothing
)
( PackageName "z-attoparsec-z-attoparsec-internal"
, I
    ( mkVersion
        [ 0
        , 14
        , 4
        ]
    )
    ( Inst
        ( UnitId "attoparsec-0.14.4-CbsDEOj99TyGhnIWKEfhEq-attoparsec-internal" )
    )
, PInfo
    [ <...> ]
    ( fromList
        [
            ( ExposedLib LMainLibName
            , ComponentInfo
                { compIsVisible = IsVisible True
                , compIsBuildable = IsBuildable True
                }
            )
        ]
    )
    ( fromList [] ) Nothing
)
Error: [Cabal-7107]
Could not resolve dependencies:
[__0] trying: multilib-repro-0.1.0.0 (user goal)
[__1] next goal: io-classes (dependency of multilib-repro)
[__1] rejecting: io-classes-1.8.0.1/installed-33ddkjXAD1KJwOJZifrTm7 (does not contain library 'si-timers', which is required by multilib-repro)
[__1] fail (backjumping, conflict set: io-classes, multilib-repro)
After searching the rest of the dependency tree exhaustively, these were the goals I've had most trouble fulfilling: multilib-repro, io-classes

Note two libraries, both exposing LMainLibName

cabal freeze trace after

Resolving dependencies...
( PackageName "attoparsec"
, I
    ( mkVersion
        [ 0
        , 14
        , 4
        ]
    )
    ( InstGroup
        ( UnitId "attoparsec-0.14.4-3OIDNSUWze2AVr1XW1152M" )
        ( fromList
            [ UnitId "attoparsec-0.14.4-CbsDEOj99TyGhnIWKEfhEq-attoparsec-internal" ]
        )
    )
, PInfo
    [ <...>
    , Simple
        ( LDep
            ( DependencyReason
                ( PackageName "attoparsec" )
                ( fromList [] )
                ( fromList [] )
            )
            ( Dep
                ( PkgComponent
                    ( PackageName "attoparsec" )
                    ( ExposedLib
                        ( LSubLibName
                            ( UnqualComponentName "attoparsec-internal" )
                        )
                    )
                )
                ( Fixed
                    ( I
                        ( mkVersion
                            [ 0
                            , 14
                            , 4
                            ]
                        )
                        ( InstGroup
                            ( UnitId "attoparsec-0.14.4-3OIDNSUWze2AVr1XW1152M" )
                            ( fromList
                                [ UnitId "attoparsec-0.14.4-CbsDEOj99TyGhnIWKEfhEq-attoparsec-internal" ]
                            )
                        )
                    )
                )
            )
        ) ComponentLib
    , <...>
    ]
    ( fromList
        [
            ( ExposedLib LMainLibName
            , ComponentInfo
                { compIsVisible = IsVisible True
                , compIsBuildable = IsBuildable True
                }
            )
        ,
            ( ExposedLib
                ( LSubLibName
                    ( UnqualComponentName "attoparsec-internal" )
                )
            , ComponentInfo
                { compIsVisible = IsVisible True
                , compIsBuildable = IsBuildable True
                }
            )
        ]
    )
    ( fromList [] ) Nothing
)
Wrote freeze file: /home/srk/git/cabal/multilib-repro/cabal.project.freeze

[sebright]

@sorki Thanks for working on finishing #6039!

I haven't read the code yet, but it sounds like this PR doesn't currently add a way to check whether two installed libraries with the same package name and version were installed together. I'm concerned that that could cause the solver to err in the unsafe direction and allow inconsistent dependencies. For example, a source package (A-1.0) could depend on one installed sub-library from B-2.0 configured with one set of flags and a second installed sub-library from B-2.0 configured with a different set of flags. This inconsistency in dependencies could lead to subtle and confusing bugs. There is some discussion of how to group libraries by instance starting at #6039 (comment). Would it be possible to add a check in this PR, even if it's initially too conservative and needs to be loosened later?

Here are a few ideas for how to implement the check:

• Here is an overly conservative approach that can be implemented completely within the solver: Only group libraries when they depend on each other by their UnitIds. It is safe, because, if two libraries within a package depend on each other, then they must have been installed together. This is probably the easiest to implement.
• Implement InstanceUnitId from Make the cabal-install solver multilibs-aware #6039 (comment). However, I'm not sure that we decided whether the UnitId captures enough information to be correct on its own.
• Add a new field to InstalledPackageInfo that is a random unique identifier for an instance. This may be too conservative, though.

[sorki]

@sorki Thanks for working on finishing #6039!

I haven't read the code yet, but it sounds like this PR doesn't currently add a way to check whether two installed libraries with the same package name and version were installed together. I'm concerned that that could cause the solver to err in the unsafe direction and allow inconsistent dependencies. For example, a source package (A-1.0) could depend on one installed sub-library from B-2.0 configured with one set of flags and a second installed sub-library from B-2.0 configured with a different set of flags. This inconsistency in dependencies could lead to subtle and confusing bugs. There is some discussion of how to group libraries by instance starting at #6039 (comment). Would it be possible to add a check in this PR, even if it's initially too conservative and needs to be loosened later?

Thanks for the feedback! This indeed only helps in simple nix-shell case, where a separate package db containing no duplicates is created.

Here are a few ideas for how to implement the check:

Here is an overly conservative approach that can be implemented completely within the solver: Only group libraries when they depend on each other by their UnitIds. It is safe, because, if two libraries within a package depend on each other, then they must have been installed together. This is probably the easiest to implement.

I've tried this but quickly found issues:

• io-classes:testlib only depends on io-classes:strict-mvar which depends on main io-classes library
  which means computing transitive closure as a simple check like sub lib depends on main lib won't suffice.
• worse one, package has a public sub-library that has no dependency on main lib - this leads to a problem that was solved by munging the private sub-libraries so picking both sub and main won't create a solver conflict.

* Implement `InstanceUnitId` from [Make the cabal-install solver multilibs-aware #6039 (comment)](https://github.com/haskell/cabal/issues/6039#issuecomment-1091146249). However, I'm not sure that we decided whether the `UnitId` captures enough information to be correct on its own.
* Add a new field to `InstalledPackageInfo` that is a random unique identifier for an instance. This may be too conservative, though.

Trying this approach now - grabbing UnitId of the main library (if it exists or some other component if not) and assigning to all other components, propagated to InstalledPackageInfo. Getting a bit tricky as I need to rebuild GHC as well so ghc-pkg is aware of the changes.

[sebright]

Here is an overly conservative approach that can be implemented completely within the solver: Only group libraries when they depend on each other by their UnitIds. It is safe, because, if two libraries within a package depend on each other, then they must have been installed together. This is probably the easiest to implement.

I've tried this but quickly found issues:

* `io-classes:testlib` only depends on `io-classes:strict-mvar` which depends on main `io-classes` library
  which means computing transitive closure as a simple check like sub lib depends on main lib won't suffice.

* worse one, package has a public sub-library that has no dependency on main lib - this leads to a problem that was solved by munging the private sub-libraries so picking both sub and main won't create a solver conflict.

I'm not sure I understand. I would expect the situation in the second bullet point to lead to a conflict, because the solver can't choose two instances for the same package name. I would expect it to have to choose one instance, either the one with the main library or the one with the sub-library. So there would be no solution if a source package depended on both libraries.

I know that this approach isn't perfect, but I suggested it because I think that it only fails in the safe direction (failing to find a solution when there is one), so it could be a good temporary solution. It would allow us to merge this PR quickly without increasing the scope too much. Then the full solution, such as InstanceUnitId, could be implemented later.