Don't use installedPkgs for internal library library dirs. #4033
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ezyang, thanks for your PR! By analyzing the history of the files in this pull request, we identified @christiaanb, @dcoutts and @23Skidoo to be potential reviewers. |
In 1.24, installedPkgs contained only references to the external package database, not any internal libraries. In particular, when we built a dynamically linked executable, installedPkgs did NOT have a reference to the internal library; instead, depLibraryPaths has a special case (hasInternalDeps) to add the final libdir to the RPATH. In HEAD, after 0d15ede, we started adding internal libraries (with the INPLACE registrations) to installedPkgs to fix haskell#2971. But depLibraryPaths was not updated, which means that the inplace registrations got picked up and added to the RPATH, resulting in bad temporary directories showing up in RPATHs. In this commit, we just filter out internal entries from installedPkgs and compute the library dirs for them from scratch (this code already existed, so no loss!) Fixes haskell#4025. Signed-off-by: Edward Z. Yang <[email protected]>
|
CC @trofi |
|
Last call for CRs otherwise I'm merging this sucker. |
|
Looks reasonable to me. |
|
Tried to apply it on top of ghc-HEAD and it's Cabal submodule and it worked! Thank you! I have another question though (see below) :) Cabal is supposed to install dyn libs to the same shared location. Worth a separate bug report? |
|
Yeah, that also sounds like a bug. |
|
Filed #4046 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In 1.24, installedPkgs contained only references to the external package
database, not any internal libraries. In particular, when we built a
dynamically linked executable, installedPkgs did NOT have a reference to
the internal library; instead, depLibraryPaths has a special case
(hasInternalDeps) to add the final libdir to the RPATH.
In HEAD, after 0d15ede, we started adding internal libraries (with the
INPLACE registrations) to installedPkgs to fix #2971. But depLibraryPaths
was not updated, which means that the inplace registrations got picked
up and added to the RPATH, resulting in bad temporary directories
showing up in RPATHs.
In this commit, we just filter out internal entries from installedPkgs
and compute the library dirs for them from scratch (this code already
existed, so no loss!)
Fixes #4025.
Signed-off-by: Edward Z. Yang [email protected]