Add and use a new dynamic-librariy-dirs field in the ghc-pkg info
ClosedPublic

Authored by ezyang on Oct 18 2016, 1:09 PM.

Details

Summary

Build systems / package managers want to be able to control the file
layout of installed libraries. In general they may want/need to be able
to put the static libraries and dynamic libraries in different places.
The ghc-pkg library regisrtation needs to be able to handle this.

This is already possible in principle by listing both a static lib dir
and a dynamic lib dir in the library-dirs field (indeed some previous
versions of Cabal did this for shared libs on ELF platforms).

The downside of listing both dirs is twofold. There is a lack of
precision, if we're not careful with naming then we could end up
picking up the wrong library. The more immediate problem however is
that if we list both directories then both directories get included
into the ELF and Mach-O shared object runtime search paths. On ELF this
merely slows down loading of shared libs (affecting prog startup time).
On the latest OSX versions this provokes a much more serious problem:
that there is a rather low limit on the total size of the section
containing the runtime search path (and lib names and related) and thus
listing any unnecessary directories wastes the limited space.

So the solution in this patch is fairly straightforward: split the
static and dynamic library search paths in the ghc-pkg db and its use
within ghc. This is a traditional solution: pkg-config has the same
static / dynamic split (though it describes in in terms of private and
public, but it translates into different behaviour for static and
dynamic linking).

Indeed it would make perfect sense to also have a static/dynamic split
for the list of the libraries to use i.e. to have dynamic variants of
the hs-libraries and extra-libraries fields. These are not immediately
required so this patch does not add it, but it is a reasonable
direction to follow.

To handle compatability, if the new dynamic-librariy-dirs field is not
specified then its value is taken from the librariy-dirs field.

Note that for the purpose of review this patch contains the necessary
Cabal changes, but these will be pushed upstream independently.

Test Plan

Run ./validate

Get christiaanb and carter to test it on OSX Sierra, in combination
with Cabal/cabal-install changes to the default file layout for
libraries.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
duncan updated this revision to Diff 9058.Oct 18 2016, 1:09 PM
duncan retitled this revision from to Add and use a new dynamic-librariy-dirs field in the ghc-pkg info.
duncan updated this object.
duncan edited the test plan for this revision. (Show Details)
duncan added reviewers: bgamari, carter, christiaanb.
duncan updated the Trac tickets for this revision.

Oh should have mentioned https://github.com/haskell/cabal/pull/3979. That was abandoned, but we decided actually it's the right approach, so we'll likely use a derivative of that.

christiaanb accepted this revision.Oct 18 2016, 1:25 PM
christiaanb edited edge metadata.
christiaanb added inline comments.
libraries/ghc-boot/GHC/PackageDb.hs
349–351

No matter how ugly alignment is, if it doesn't change the actual functionality don't mess up the history for this line

This revision is now accepted and ready to land.Oct 18 2016, 1:25 PM
Phyx added a subscriber: Phyx.Oct 18 2016, 1:39 PM

Doesn't https://github.com/ghc/ghc/blob/master/utils/ghc-cabal/Main.hs#L357 need to be updated as well. When RPATH is turned off (or unavailable such as on Windows) GHC should still work correctly.

duncan added inline comments.Oct 18 2016, 1:42 PM
libraries/ghc-boot/GHC/PackageDb.hs
349–351

Gah! I can't help it. It's soo offensive!

In D2611#75879, @Phyx wrote:

Doesn't https://github.com/ghc/ghc/blob/master/utils/ghc-cabal/Main.hs#L357 need to be updated as well. When RPATH is turned off (or unavailable such as on Windows) GHC should still work correctly.

So that bit of ghc-cabal is fidgeting with the library-dirs field of the rts package. We do not yet set the dynamic-library-dirs for any of the libraries bundled with ghc. If we do want to set the dynamic-library-dirs to be different from the library-dirs for the rts or other libs then at that point there'd need to be a number of build system changes. That bit of ghc-cabal and probably other bits would need to be changed at that point.

https://github.com/haskell/cabal/pull/3979 has been updated and these patches have been tested together by @christiaanb:

I've just build a dynamically linked cabal-install library
and it works :)
and it only has 1 RPATH for the libraries in the user package database
and the user package database entrie have a correct "dynamic-library-dirs" field

bgamari accepted this revision.Oct 19 2016, 4:57 PM
bgamari edited edge metadata.

Looks good to me. Thanks @duncan and @christiaanb !

libraries/ghc-boot/GHC/PackageDb.hs
349–351

Meh, I don't mind the occasional unrelated style fix so much; with a reasonable git frontend it's not that hard to get blame from more than one commit in the past.

duncan added a subscriber: ezyang.Oct 20 2016, 4:44 PM

@bgamari so the Cabal changes have been merged into Cabal master and 1.24 branches.

I'm not sure what branch you're tracking on ghc master, but on ghc-8.0 I assume you're tracking the Cabal 1.24 branch. So if you update the Cabal repo and then in the patch above, just omit/update the cabal submodule change, then it should be ok.

BTW, if you're on an a much older Cabal master on the ghc master branch then that could be awkward since there's been a lot of changes, but perhaps @ezyang has been keeping things up to date there too, coordinating backpack changes. Just a heads up.

GHC HEAD is tracking Cabal master, so there shouldn't be any problems there.

I'm commandeering this patch for GHC HEAD. I'll let bgamari handle 8.0 though.

This patch doesn't validate on master; there seem to be build system problems related to installing. Error is:

Installing library in /home/hs01/ezyang/ghc-validate/bindisttest/install
dir/lib/ghc-8.1.20161021/random-1.1
"/home/hs01/ezyang/ghc-validate/bindisttest/install   dir/lib/ghc-8.1.20161021/bin/ghc-pkg" --f
orce --global-package-db "/home/hs01/ezyang/ghc-validate/bindisttest/install   dir/lib/ghc-8.1.20161021/package.conf.d" update rts/dist/package.conf.install                                  /home/hs01/ezyang/ghc-validate/bindisttest/install   dir/lib/ghc-8.1.20161021/bin/ghc-pkg: erro
r while loading shared libraries: libHSterminfo-0.4.0.2-ghc8.1.20161021.so: cannot open shared 
object file: No such file or directory
make[3]: *** [ghc.mk:987: install_packages] Error 127
make[2]: *** [Makefile:51: install] Error 2
make[1]: *** [bindisttest/ghc.mk:33: test_bindist] Error 2
make: *** [Makefile:127: test_bindist] Error 2
ezyang commandeered this revision.Oct 21 2016, 7:19 PM
ezyang added a reviewer: duncan.

OK this patch needs some more stuff:

diff --git a/utils/ghc-cabal/Main.hs b/utils/ghc-cabal/Main.hs
index e72e46c..4b6b496 100644
--- a/utils/ghc-cabal/Main.hs
+++ b/utils/ghc-cabal/Main.hs
@@ -248,6 +248,10 @@ updateInstallDirTemplates relocatableBuild myPrefix myLibdir myDocdir idts
                           if relocatableBuild
                           then "$topdir"
                           else myLibdir,
+          dynlibdir = toPathTemplate $
+                          (if relocatableBuild
+                          then "$topdir"
+                          else myLibdir) </> "$libname",
           libsubdir = toPathTemplate "$libname",
           docdir    = toPathTemplate $
                           if relocatableBuild
diff --git a/utils/ghc-pkg/Main.hs b/utils/ghc-pkg/Main.hs
index f07f5e1..063a806 100644
--- a/utils/ghc-pkg/Main.hs
+++ b/utils/ghc-pkg/Main.hs
@@ -1634,7 +1634,7 @@ checkPackageConfig pkg verbosity db_stack
   checkExposedModules db_stack pkg
   checkOtherModules pkg
   let has_code = Set.null (openModuleSubstFreeHoles (Map.fromList (instantiatedWith pkg)))
-  when has_code $ mapM_ (checkHSLib verbosity (libraryDirs pkg)) (hsLibraries pkg)
+  when has_code $ mapM_ (checkHSLib verbosity (libraryDirs pkg ++ libraryDynDirs pkg)) (hsLibraries pkg)
   -- ToDo: check these somehow?
   --    extra_libraries :: [String],
   --    c_includes      :: [String],

Macro imcommandeeringthisdiff:

This revision was automatically updated to reflect the committed changes.

Gah, pushing it to HEAD convinced Phab that this one should be closed. I'll reopen for 8.0