[GHC] #14381: Consider making ghc-pkg fill in abi-depends based on depends

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: (none) Type: bug | Status: new Priority: high | Milestone: Component: ghc-pkg | Version: 8.2.1 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- 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 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: thoughtpolice Type: bug | Status: new Priority: high | Milestone: 8.2.2 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by thoughtpolice): * owner: (none) => thoughtpolice * milestone: => 8.2.2 Comment: 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. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: thoughtpolice Type: bug | Status: new Priority: high | Milestone: 8.4.1 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.2.2 => 8.4.1 Comment: It seems unlikely that this will happen for 8.2. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: thoughtpolice Type: bug | Status: new Priority: high | Milestone: 8.2.3 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Changes (by thoughtpolice): * differential: => Phab:D4159 * milestone: 8.4.1 => 8.2.3 Comment: 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. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: thoughtpolice Type: bug | Status: new Priority: high | Milestone: 8.2.3 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Changes (by ntc2): * cc: ntc2 (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: thoughtpolice Type: bug | Status: new Priority: high | Milestone: 8.2.3 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Comment (by juhpetersen): I am going to try this patch on 8.2.2, for Fedora 28 development. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: thoughtpolice Type: bug | Status: new Priority: high | Milestone: 8.2.3 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Comment (by juhpetersen): So far seems to be working great on 8.2.2, thanks! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: thoughtpolice Type: bug | Status: new Priority: highest | Milestone: 8.4.2 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Changes (by bgamari): * priority: high => highest * milestone: 8.2.3 => 8.4.2 Comment: I have encountered this on quite a few occasions. Bumping priority. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: new Priority: highest | Milestone: 8.4.2 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Changes (by bgamari): * owner: thoughtpolice => tdammers Comment: Tobias will have a look at this. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends
-------------------------------------+-------------------------------------
Reporter: ezyang | Owner: tdammers
Type: bug | Status: new
Priority: highest | Milestone: 8.4.2
Component: ghc-pkg | Version: 8.2.1
Resolution: | Keywords:
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: |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: new Priority: highest | Milestone: 8.4.2 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Comment (by tdammers): OK, I have a patch incoming that should do the trick. One issue though is that there is a theoretical possibility of the same package database being mentioned multiple times; this would sabotage the "get packages below the current one in the stack" logic, because right now, two package databases are considered "the same" iff their locations are equal, and the "get packages below this one in the stack" logic consists of popping all package db's off the stack that are not the same as the "current" one. So if we have a stack like this one: {{{#!hs [ "foo/bar" , "baz/quux" , "foo/bar" , "pizza/olives" ] }}} ...and we're trying to get all the package databases below the second occurrence of "foo/bar", we would want to get just {{{ ["foo/bar", "pizza/olives"]}}}, but we will in fact get the entire stack. Question is whether this is ever going to be a problem in practice - I would assume that having the same package database in the stack more than once would pose all sorts of problems anyway. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: new Priority: highest | Milestone: 8.4.2 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Comment (by tdammers): As discussed in person: this can produce a lot of warnings that may not be very useful in practice. `ghc-pkg` is essentially always doing the "right thing" here, overriding declared dependencies with inferred ones, and we are more confident on the inferred ones than the declared ones (the latter essentially being subject to human error). The way it works now, we will also see a lot of unnecessary warnings: when the declared and inferred dependencies agree, `ghc-pkg` will still do the overriding and issue a warning, but that warning will be pointless, because we aren't making any effective changes. Because of this, we will only raise this warning in debug mode, which should also take care of the now-failing tests. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: new Priority: highest | Milestone: 8.4.2 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Comment (by bgamari): I have reverted the patch in comment:9 on `master` as the warning caused validation issues. That being said, we will ship it in 8.4.3 to allow distributions to drop their patches. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: new Priority: highest | Milestone: 8.4.2 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Comment (by juhpetersen): Thanks a lot this sounds good to me. Do you want to set the milestone to 8.4.3 then? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: new Priority: highest | Milestone: 8.4.3 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.4.2 => 8.4.3 Comment: Indeed I do. Good catch. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: patch Priority: highest | Milestone: 8.4.3 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | Phab:D4729 -------------------------------------+------------------------------------- Changes (by tdammers): * status: new => patch * differential: Phab:D4159 => Phab:D4159 Phab:D4729 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: patch Priority: highest | Milestone: 8.4.3 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | Phab:D4729 -------------------------------------+------------------------------------- Comment (by bgamari): Phab:D4159 has been merged to GHC 8.4.3 as a stop-gap with 51abb1c88b53e2989a2a8c2939ac4abc04bef194. We will merge the more sensible Phab:D4729 to `master` shortly. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: patch Priority: highest | Milestone: 8.4.3 Component: ghc-pkg | Version: 8.2.1 Resolution: | Keywords: 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: | Phab:D4729 -------------------------------------+------------------------------------- Comment (by phadej): The warning is indeed noisy, and gives an impression cabal-install (or GHC-8.4.3) is broken. I'd hope for 8.4.4 with a proper fix. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:17 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends
-------------------------------------+-------------------------------------
Reporter: ezyang | Owner: tdammers
Type: bug | Status: patch
Priority: highest | Milestone: 8.4.3
Component: ghc-pkg | Version: 8.2.1
Resolution: | Keywords:
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: | Phab:D4729
-------------------------------------+-------------------------------------
Comment (by Ben Gamari

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: closed Priority: highest | Milestone: 8.6.1 Component: ghc-pkg | Version: 8.2.1 Resolution: fixed | Keywords: 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: | Phab:D4729 -------------------------------------+------------------------------------- Changes (by bgamari): * status: patch => closed * resolution: => fixed * milestone: 8.4.3 => 8.6.1 Comment: Fixed correctly in 8.6.1. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:19 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14381: Consider making ghc-pkg fill in abi-depends based on depends -------------------------------------+------------------------------------- Reporter: ezyang | Owner: tdammers Type: bug | Status: closed Priority: highest | Milestone: 8.4.4 Component: ghc-pkg | Version: 8.2.1 Resolution: fixed | Keywords: 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: | Phab:D4729 -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.6.1 => 8.4.4 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14381#comment:20 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC