Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ghc-cabal in GHC bindist broken due to inplace library expectation #3633

Closed
ezyang opened this issue Jul 28, 2016 · 4 comments
Closed

ghc-cabal in GHC bindist broken due to inplace library expectation #3633

ezyang opened this issue Jul 28, 2016 · 4 comments
Assignees
Milestone

Comments

@ezyang
Copy link
Contributor

ezyang commented Jul 28, 2016

While working on convenience libraries, I refactored how the inplace package database worked so that it was created at configure time (rather than when we do a build). This had the knock-on effect of causing us to pass the path to the inplace package database to GHC for all invocations, not just during builds.

It turns out this breaks ghc-cabal in GHC binary distributions, in the following way:

  1. When GHC packages up libraries for the binary distribution, it packs up the configuration (LocalBuildInfo), interfaces and objects, but NOT the inplace database (who needs that!)
  2. To install a package, we register it into the final database. register operates by running ghc --abi-hash, which following the aforementioned changes now points to the inplace package database. But the inplace package database doesn't exist, so GHC chokes.

I'll fix this, but I'm trying to decide between a few options:

  1. Remove the -package-db flag when calling --abi-hash (they are not needed for GHC to operate correctly)
  2. Compute ABI hash as part of build and stash it in a file somewhere, which register reads out later (I'd teach GHC's bindist to package up this file then)
  3. Change GHC's bindist to copy the inplace DBs over and distribute them

There were also a number of things that would have made this easier to debug:

  1. ghc-cabal really needs to grow a verbose flag (OK, not a Cabal bug)
  2. The error message I got was:
ghc-cabal: '/home/hs01/ezyang/ghc-validate3/bindisttest/install
dir/lib/ghc-8.1.20160727/bin/ghc' exited with an error:
ghc: can't find a package database at dist-install/package.conf.inplace

This is not very useful. What would be more useful is if I had the full current working directory (that would have informed me WHICH library was failing), and if I had the FULL command line invocation to GHC (which would have notified me that this was due to an --abi-hash invocation.

@ezyang ezyang added this to the Cabal 2.0 milestone Jul 28, 2016
@ezyang ezyang self-assigned this Jul 28, 2016
@23Skidoo
Copy link
Member

Do I understand correctly that option 1 will be specific to the GHC bindist process?

This is not very useful. What would be more useful is if I had the full current working directory (that would have informed me WHICH library was failing), and if I had the FULL command line invocation to GHC (which would have notified me that this was due to an --abi-hash invocation.

-v would give you that, but I agree that the default could be better.

@ezyang
Copy link
Contributor Author

ezyang commented Jul 28, 2016

I mean, it is specific to GHC bindist insofar as much as that is the only case I know of someone deleting the inplace package database, and then expecting to be able to register a package.

@23Skidoo
Copy link
Member

Then I vote for option 1, since option 2 adds more complexity, and option 3 will force GHC to ship stuff it doesn't really need (which I don't think we should do).

@ezyang
Copy link
Contributor Author

ezyang commented Aug 21, 2016

I did (1) in #3639.

@ezyang ezyang closed this as completed Aug 21, 2016
@ezyang ezyang modified the milestones: Cabal 2.0, 2.0 (planned for next feature release) Sep 6, 2016
bgamari pushed a commit to ghc/ghc that referenced this issue Nov 12, 2018
Summary:
For holes, its necessary to "see through" the instantiation
of the hole to get accurate family instance dependencies.
For example, if B imports <A>, and <A> is instantiated with
F, we must grab and include all of the dep_finsts from
F to have an accurate transitive dep_finsts list.

However, we MUST NOT do this for regular modules.
First, for efficiency reasons, doing this
bloats the the dep_finsts list, because we *already* had
those modules in the list (it wasn't a hole module, after
all). But there's a second, more important correctness
consideration: we perform module renaming when running
--abi-hash.  In this case, GHC's contract to the user is that
it will NOT go and read out interfaces of any dependencies
(haskell/cabal#3633); the point of
--abi-hash is just to get a hash of the on-disk interfaces
for this *specific* package.  If we go off and tug on the
interface for /everything/ in dep_finsts, we're gonna have a
bad time.  (It's safe to do do this for hole modules, though,
because the hmap for --abi-hash is always trivial, so the
interface we request is local.  Though, maybe we ought
not to do it in this case either...)

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Test Plan: validate

Reviewers: alexbiehl, goldfire, bgamari

Subscribers: ppk, shlevy, rwbarton, carter

GHC Trac Issues: #15594

Differential Revision: https://phabricator.haskell.org/D5123
angerman pushed a commit to input-output-hk/ghc that referenced this issue Jun 22, 2020
Summary:
For holes, its necessary to "see through" the instantiation
of the hole to get accurate family instance dependencies.
For example, if B imports <A>, and <A> is instantiated with
F, we must grab and include all of the dep_finsts from
F to have an accurate transitive dep_finsts list.

However, we MUST NOT do this for regular modules.
First, for efficiency reasons, doing this
bloats the the dep_finsts list, because we *already* had
those modules in the list (it wasn't a hole module, after
all). But there's a second, more important correctness
consideration: we perform module renaming when running
--abi-hash.  In this case, GHC's contract to the user is that
it will NOT go and read out interfaces of any dependencies
(haskell/cabal#3633); the point of
--abi-hash is just to get a hash of the on-disk interfaces
for this *specific* package.  If we go off and tug on the
interface for /everything/ in dep_finsts, we're gonna have a
bad time.  (It's safe to do do this for hole modules, though,
because the hmap for --abi-hash is always trivial, so the
interface we request is local.  Though, maybe we ought
not to do it in this case either...)

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Test Plan: validate

Reviewers: alexbiehl, goldfire, bgamari

Subscribers: ppk, shlevy, rwbarton, carter

GHC Trac Issues: #15594

Differential Revision: https://phabricator.haskell.org/D5123

Signed-off-by: Moritz Angermann <moritz.angermann@gmail.com>
angerman pushed a commit to input-output-hk/ghc that referenced this issue Jun 22, 2020
Summary:
For holes, its necessary to "see through" the instantiation
of the hole to get accurate family instance dependencies.
For example, if B imports <A>, and <A> is instantiated with
F, we must grab and include all of the dep_finsts from
F to have an accurate transitive dep_finsts list.

However, we MUST NOT do this for regular modules.
First, for efficiency reasons, doing this
bloats the the dep_finsts list, because we *already* had
those modules in the list (it wasn't a hole module, after
all). But there's a second, more important correctness
consideration: we perform module renaming when running
--abi-hash.  In this case, GHC's contract to the user is that
it will NOT go and read out interfaces of any dependencies
(haskell/cabal#3633); the point of
--abi-hash is just to get a hash of the on-disk interfaces
for this *specific* package.  If we go off and tug on the
interface for /everything/ in dep_finsts, we're gonna have a
bad time.  (It's safe to do do this for hole modules, though,
because the hmap for --abi-hash is always trivial, so the
interface we request is local.  Though, maybe we ought
not to do it in this case either...)

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Test Plan: validate

Reviewers: alexbiehl, goldfire, bgamari

Subscribers: ppk, shlevy, rwbarton, carter

GHC Trac Issues: #15594

Differential Revision: https://phabricator.haskell.org/D5123

(cherry picked from commit 13ff0b7)
Signed-off-by: Moritz Angermann <moritz.angermann@gmail.com>
angerman pushed a commit to input-output-hk/ghc that referenced this issue Sep 22, 2020
Summary:
For holes, its necessary to "see through" the instantiation
of the hole to get accurate family instance dependencies.
For example, if B imports <A>, and <A> is instantiated with
F, we must grab and include all of the dep_finsts from
F to have an accurate transitive dep_finsts list.

However, we MUST NOT do this for regular modules.
First, for efficiency reasons, doing this
bloats the the dep_finsts list, because we *already* had
those modules in the list (it wasn't a hole module, after
all). But there's a second, more important correctness
consideration: we perform module renaming when running
--abi-hash.  In this case, GHC's contract to the user is that
it will NOT go and read out interfaces of any dependencies
(haskell/cabal#3633); the point of
--abi-hash is just to get a hash of the on-disk interfaces
for this *specific* package.  If we go off and tug on the
interface for /everything/ in dep_finsts, we're gonna have a
bad time.  (It's safe to do do this for hole modules, though,
because the hmap for --abi-hash is always trivial, so the
interface we request is local.  Though, maybe we ought
not to do it in this case either...)

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Test Plan: validate

Reviewers: alexbiehl, goldfire, bgamari

Subscribers: ppk, shlevy, rwbarton, carter

GHC Trac Issues: #15594

Differential Revision: https://phabricator.haskell.org/D5123
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants