Opened 4 months ago

Last modified 3 weeks ago

#14381 new bug

Consider making ghc-pkg fill in abi-depends based on depends

Reported by: ezyang Owned by: thoughtpolice
Priority: high Milestone: 8.2.3
Component: ghc-pkg Version: 8.2.1
Keywords: Cc: ntc2
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D4159
Wiki Page:

Description

In GHC 8.2, we introduced abi-depends to solve #12485. Following the same pattern as all ghc-pkg fields, this field is to be fllled by whoever is performing the registration.

I now suspect designing it this way was a mistake. In https://github.com/haskell/cabal/issues/4728 we have a bug where Cabal is writing nonsense data for abi-depends, ghc-pkg isn't noticing it, and GHC is rejecting the package (with a "shadow" warning) when it gets to the end. The problem is the Cabal aggressively caches the contents of the package database (ostensibly because it is expensive to query ghc-pkg); this means that it is easy to get into a situation where its understanding of the ABIs of its dependencies is out-of-date (because it is not re-reading the database in order to get newer information).

The insult to the injury is, in most cases, ghc-pkg already knows what the ABIs are supposed to be: they're whatever the ABIs of the packages pointed at by 'depends' already in the database are. So ghc-pkg could have computed the abi dependency itself, and prevented this stale data situation from ever happening. This sounds quite attractive to me.

What do people think? Here is one possible proposal (but it is just one in the space):

  • ghc-pkg will be modified to ignore the abi-depends field (perhaps with a warning), to prevent itself from being poisoned by buggy versions of Cabal which are giving bad ABI information
  • Instead, ghc-pkg generates the abi-depends field by looking up dependency IDs from the database. If an ID is not found, it omits the dep from abi-depends (this is equivalent to suspending ABI checking in GHC, so this won't break anything; it will just make ABI checking less robust)
  • Possibly, introduce a new "virtual" field, which can be used to override ghc-pkg default

Change History (6)

comment:1 Changed 4 months ago by thoughtpolice

Milestone: 8.2.2
Owner: set to thoughtpolice

This issue is causing us a large amount of pain at work, and we're currently blocked on a major upgrade to GHC 8.2.1 because of it. So I'm going to be taking a look at fixing this later this week (at least "Part 1" and "Part 2" of the above proposal).

For our case, we're using Nix to control and keep everything working, including GHC -- so we can limp by with approaches that aren't fully upstream-worthy for now, it's easy enough to apply a temporary patch. (I'll of course see it through until it lands in HEAD, just a fore-warning on immediate time constraints.)

Ben, I'm tentatively marking this as slated for 8.2.2, although admittedly I don't know what its current schedule looks like. If it doesn't make it this isn't world-ending for us, but I imagine it would make several people happy anyway. Feel free to push it out if needed.

comment:2 Changed 4 months ago by bgamari

Milestone: 8.2.28.4.1

It seems unlikely that this will happen for 8.2.

comment:3 Changed 4 months ago by thoughtpolice

Differential Rev(s): Phab:D4159
Milestone: 8.4.18.2.3

It'd be nice to get this into the 8.2 branch and the backport is fairly easy; so if there's an 8.2.3, I'd like to queue this patch for its release.

comment:4 Changed 3 months ago by ntc2

Cc: ntc2 added

comment:5 Changed 3 weeks ago by juhpetersen

I am going to try this patch on 8.2.2, for Fedora 28 development.

comment:6 Changed 3 weeks ago by juhpetersen

So far seems to be working great on 8.2.2, thanks!

Note: See TracTickets for help on using tickets.