Skip to content

Backport "Store interfaces files separately from library object files #3982" to 1.24#3983

Closed
christiaanb wants to merge 2 commits intohaskell:1.24from
christiaanb:commonlibdir-1.24
Closed

Backport "Store interfaces files separately from library object files #3982" to 1.24#3983
christiaanb wants to merge 2 commits intohaskell:1.24from
christiaanb:commonlibdir-1.24

Conversation

@christiaanb
Copy link
Collaborator

These two flags indicate where the two distinct aspects of a
Haskell library end up.

--commonlibdir: The subdirectory of --libdir where the object
library files (.so/.a/.dll/.dylib) get installed.
--hidir: The directory where the interface files (.hi)
get installed.

The reason we want to do this is because we want dynamic libraries
to all end up in a shared directory to reduce start-up times of
the run-time linker. However, we still want the .hi to end up
in one directory per package.

We cannot repurpose --libsubdir to take over the role of what
--commonlibdir is doing because then we would run into trouble
with Setup.hs files build against an older Cabal. If we were
to repurpose --libsubdir, then all the object and interface
files would end up in a single shared directory for all
packages when using an older Cabal.

This PR backports #3982 to the 1.24 branch in order to solve https://ghc.haskell.org/trac/ghc/ticket/12479

These two flags indicate where the two distinct aspects of a
Haskell library end up.

--commonlibdir: The subdirectory of --libdir where the object
                library files (.so/.a/.dll/.dylib) get installed.
--hidir:        The directory where the interface files (.hi)
                get installed.

The reason we want to do this is because we want dynamic libraries
to all end up in a shared directory to reduce start-up times of
the run-time linker. However, we still want the .hi to end up
in one directory per package.

We cannot repurpose --libsubdir to take over the role of what
--commonlibdir is doing because then we would run into trouble
with Setup.hs files build against an older Cabal. If we were
to repurpose --libsubdir, then all the object and interface
files would end up in a single shared directory for all
packages when using an older Cabal.
@mention-bot
Copy link

@christiaanb, thanks for your PR! By analyzing the history of the files in this pull request, we identified @dcoutts, @23Skidoo and @ezyang to be potential reviewers.

@ilovezfs
Copy link
Collaborator

@phadej
Copy link
Collaborator

phadej commented Oct 15, 2016

I think it's unfortunate that 1.24 new-build won't work on Sierra, but it shouldn't block this going in.

@phadej
Copy link
Collaborator

phadej commented Oct 15, 2016

ping @dcoutts about new-build and where to place dylibs in the store. Should we make a separate issue of it?

@23Skidoo
Copy link
Member

Do I understand correctly that new-build doesn't currently work on Sierra & 1.24 anyway?

@ilovezfs
Copy link
Collaborator

No, new-build works just fine on Sierra, as long as you don't run into the bug that this PR is fixing for non-new-build.

@ilovezfs
Copy link
Collaborator

Half a loaf is better than no loaf, but it would be good if a comprehensive solution shipped before 8.0.2 so that Sierra isn't rendered a second class GHC citizen.

The question is whether there is a possibility of a single fix that will address the problem for both non-new-build and new-build. If so, this PR may end up having to be backed out.

If there is no reasonable prospect of having a common fix, then there's a case for "pretending" like they're different problems, though they actually aren't, and shipping this PR.

@23Skidoo
Copy link
Member

For new-build, we currently recommend using cabal-install HEAD, since it's more stable. Will this fix break HEAD as well?

@phadej
Copy link
Collaborator

phadej commented Oct 15, 2016

@23Skidoo this doesn't.

Yet the store layout is decided only in cabal-install, so we can fix it with a point release.
I hope that flags here are enough to make it work.

@ilovezfs
Copy link
Collaborator

For new-build, we currently recommend using cabal-install HEAD, since it's more stable. Will this fix break HEAD as well?

The bug I referred to is not caused by this PR. So no it won't break HEAD or anything else.

@christiaanb
Copy link
Collaborator Author

OK, just to be absolutely 100% clear: This PR does not break anything.

What is going on, is that this PR does currently not solve the issues mentioned in https://ghc.haskell.org/trac/ghc/ticket/12479 when we use new-build. It does, however, solve the issues mentioned in https://ghc.haskell.org/trac/ghc/ticket/12479 when we use "old" build / install.

I believe that the changes in this PR to Cabal (the library) are sufficient to eventually solve the issues as reported in https://ghc.haskell.org/trac/ghc/ticket/12479 when we use new-build. However, since this PR is holding up the GHC 8.0.2 release, I think this issue with new-build should be solved at a later time in another PR.

The tests on this PR are all green, so I'm just waiting for the reviewers to give the go-ahead for a merge.

@christiaanb
Copy link
Collaborator Author

Also, regardless of whether this PR solves https://ghc.haskell.org/trac/ghc/ticket/12479, it allows us to place library object files (.dll/.so/.a/.dylib) in a directory seperate from interface files .hi. And this might be desirable for reason other than solving https://ghc.haskell.org/trac/ghc/ticket/12479. For example, on Linux, startup times can be significantly reduced by limiting the number of RPATHs. One way to do this is by placing all the dynamic libraries in a single directory, which is what this PR does.

Copy link
Member

@23Skidoo 23Skidoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look okay, I'm just slightly uneasy about the proliferation of different libdir flags. Would be nice if @dcoutts could comment.

libsubdir :: dir,
commonlibdir :: dir,
hidir :: dir,
libsubdir :: dir, -- This field is basically deprecated by the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/basically deprecated/deprecated/

hidir :: dir,
libsubdir :: dir, -- This field is basically deprecated by the
-- introduction of commonlibdir and hidir. However,
-- We must keep it so that we can still work with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/We/we/


`--commonlibdir=`_dir_
: For use with Setup.hs files build against a version of Cabal prior to 1.24.1.
For later versions of Cabal, this flag is basically deprecated, and you
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For later versions of Cabal, this flag is basically deprecated

I think you were talking about libsubdir here.

`--binlibsubdir` more properly describes what this flag does:
it indicates the directory in which the binary libraries go,
and it is a subdir of `--libdir`.
@23Skidoo
Copy link
Member

Closing in favour of #3979, context: #3982 (comment).

@23Skidoo 23Skidoo closed this Oct 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants