Conversation
|
@christiaanb, thanks for your PR! By analyzing the history of the files in this pull request, we identified @dcoutts, @luite and @23Skidoo to be potential reviewers. |
|
Is this supposed to be 1.24-only? |
|
@23Skidoo I think it should ultimately be master, but I just wanted to fix 8.0 on OS X Sierra first, so I picked the cabal version that is currently shipped with ghc8. |
|
Thanks! I haven't dug into the sordid details, but if GHC HQ is behind this plan we'll take these patches. |
|
Milestoned for 1.24.0.1 since Christiaan told me he wants this in 1.24. |
|
Once travis-ci is finished, I feel that this PR is ready to be merged into 1.24 |
| UHC -> "$pkgid" | ||
| _other -> "$abi" </> "$libname", | ||
| _other -> case buildOS of | ||
| OSX -> "$abi" -- OSX libs go into a single directory |
There was a problem hiding this comment.
Needs a link to that GHC trac ticket.
There was a problem hiding this comment.
Is there any particular reason to make this conditional on the operating system?
Placing dynamic libraries in per-package directories is actually a rather severe performance problem in general since the dynamic linker (on Linux at least) performs a linear scan through RPATH entries when looking for each library. For this reason just running ghc --help on Linux performs over 750 open system calls. This was first documented in GHC #11587 and the obvious solution is to move all dynamic libraries into a single directory, just as you've done here.
There was a problem hiding this comment.
It was needed to fix OS X given time/manpower constraints. I, however, didn't know whether doing it for all operating systems was something all involved parties could agree on. If you, as GHC maintainer, are saying to do it for all operating systems, and Cabal maintainers agree, then it is an easy fix/patch.
There was a problem hiding this comment.
I think that it makes sense if it improves start-up performance.
|
So both PRs look good modulo some minor comments. |
|
Correction, it should not be merged yet. Currently, |
|
You can also call it |
23Skidoo
added
blocked: info-needed
blocked: action-required
and removed
blocked: info-needed
labels
Add a setting to indicate the directory for .hi interface files
|
There are some test failures in #3968, so I'd like to see them resolved first before merging this. |
|
On the whole this looks good to me. However, as noted inline, I would like to see this single-directory layout used across all operating systems. Placing each package's shared object in a separate directory is quite bad for dynamic linker performance. |
|
I'm closing this PR due to the problems mentioned in #3968. I will submit a new PR soon, which reinstates |
|
A bit late to the party, but just ran into this issue. Would using the latest Cabal to set a single dir for lib files also work on GHC 7.10.3? I gave it a try and either I missed a crucial point or it doesn't seem to work. Any thoughts on a solution under GHC 7.10.3? |
|
As the final fix involved modifications to GHC, I think it would take a bit more work to fix this for GHC 7.10. |
|
I suspected as much, thanks. Any chance a bug fix release can be accommodated for 7.10 given how new the 8.0 release is? I can only imagine this will trip up folks as 7.10 will remain relevant for a while.
As one data point, the problem is quite severe for me - it has already blocked the builds of 3 separate projects I deal with day to day. One of them is a medium size project at best.
I'd gladly offer to contribute but I'm really quite far from all the low level linker/ghc internals, sadly. Same goes for the folks I work with unfortunately.
… On Nov 27, 2016, at 11:55 AM, Edward Z. Yang ***@***.***> wrote:
As the final fix involved modifications to GHC, I think it would take a bit more work to fix this for GHC 7.10.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@ozataman I think you could use a cabal tree that incorporates the changes in #3968 -- that wasn't a complete solution, but if you don't have the corner-cases that ran it aground, it should work on 7.10.3... so if you have projects stalled by needing to be on the latest OS but use ghc 7.10, you could build a cabal fork based on that patchset, I suspect? |
|
@gbaz I may not be following you 100% - I've tried the latest cabal release (1.24.1.0) that incorporates the fix on top of GHC 7.10.3 and the issue is still there. It seems we'd need to patch GHC 7.10 to go with it as @ezyang has pointed out. Or perhaps I'm falsely assuming that using a cabal-install based on the latest Cabal to do the installs is enough to get around the issue... My situation is a bit unfortunate: There are other, non-Haskell related reasons my OS had to be upgraded. However, none of my mid+ sized projects build anymore after the bump and it'll be months before they all get on GHC 8. The machine I'm on is basically disabled for dev work at the moment - thanks Apple! |
|
@ozataman as I understand it, the PR I linked is not in the latest cabal release. It was an attempt at a patch that didn't require GHC changes, but which was not adopted because of corner cases with certain Setup scripts. Instead, a more invasive but more complete change was made which did require GHC changes. So if you use that branch instead, then, potentially, I would imagine that unlike the released cabal, it might be compatible with older ghcs... |
|
@gbaz Thank you for pointing that out! I missed that change of direction completely in the few issue threads that were ultimately involved. I will give that a try and report back. |
7 participants
This pull request:
.dylib/.a) in a single shared directory in order to solve https://ghc.haskell.org/trac/ghc/ticket/12479