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

Store interfaces files separately from library object files #3982

Closed
wants to merge 3 commits into from

Conversation

christiaanb
Copy link
Collaborator

@christiaanb christiaanb commented Oct 14, 2016

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

--binlibsubdir: The subdirectory of --libdir where the binary 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.

After several attempts (#3955,#3968,#3979), this should be the definitive solution for: https://ghc.haskell.org/trac/ghc/ticket/12479

EDIT: --binlibsubdir was originally called --commonsubdir when this PR was first created.

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, @hvr and @sopvop to be potential reviewers.

@christiaanb christiaanb mentioned this pull request Oct 14, 2016
@23Skidoo
Copy link
Member

/cc @dcoutts

@ilovezfs
Copy link
Collaborator

@christiaanb seems to work

==> Summary
🍺  /usr/local/Cellar/git-annex/6.20161012: 105 files, 87.4M, built in 21 minutes 39 seconds

and

All 281 tests passed (192.81s)

@ilovezfs
Copy link
Collaborator

This appears to fail with new-build https://gist.github.com/ilovezfs/ff5539ab4b6b16ab9a832ff7c9476284

@christiaanb
Copy link
Collaborator Author

@ilovezfs what you mean by "it fails on new-build" is "this patch doesn't solve the problems reported in GHC issue 12497 when using new-build"? Or that things are actually broken by this patch that used to work before?

@ilovezfs
Copy link
Collaborator

ilovezfs commented Oct 15, 2016

I mean that this PR is an incomplete solution to GHC 12479 https://ghc.haskell.org/trac/ghc/ticket/12479 not that it causes a regression.

Is there no hope of a common solution that will address non-new-build and new-build alike?

If there is a prospect for a common solution, then it should be understood that the change in this PR may need to be backed out in favor of that common solution at some point in the near future.

If there is no such prospect, then no harm, no foul in treating them as separate problems even though GHC issue 12479 is the underlying cause for both.

CC @cartazio @UniqMartin

@23Skidoo
Copy link
Member

23Skidoo commented Oct 15, 2016

the change in this PR may need to be backed out in favor of that common solution at some point in the near future.

If it's true, I'd prefer to get it right the first time, since backing this change out will result in more backwards compatibility cruft.

@phadej
Copy link
Collaborator

phadej commented Oct 15, 2016

IMHO new-build is more or less cabal-install issue (or even ghc-pkg?). It's a bit like trying to fix commercialhaskell/stack#2577 also in this PR.

I'd like to hear @dcoutts or someone who knows on how new-build store works, to comment on this.

EDIT it seems that those flags will affect where stuff goes also in new-build and stack. So they just need to be taught to use them. Am I correct?

@christiaanb
Copy link
Collaborator Author

christiaanb commented Oct 15, 2016

@phadej: yes these flags allow you to place library objectfiles (.so/.a/.dll/.dylib) in another directory than the interface files (.hi). That's all this PR basically does:

  • Add flags so we can put .hi files somewhere else than .so/.a/.dll/.dylib files
  • By default, put .so/.a/.dll/.dylib in a shared directory (per package database).

So yes, to fix new-build and stack with regards to https://ghc.haskell.org/trac/ghc/ticket/12479 should just be to teach them these new flags, and make sure that library object files end up in a shared directory.

@ezyang
Copy link
Contributor

ezyang commented Oct 15, 2016

Yeah, the fix for new-build should not be too difficult. Just passing the new flag.

@cartazio
Copy link
Contributor

  1. do we want to put the object code in these locations, or merely symlinks
    (ignoring the complication that symlinks don't exist on windows I think?)

  2. I thought Duncan had a concern with having different dylib builds put
    into a shared directory

  3. possibly doesn't matter: but I seem to recall that many file systems
    have ad hoc limits to the number of files/folders stored in single
    directory? Or would that require several orders of magnitude more dynamic
    libraries than all of hackage built by a particular ghc version?

On Saturday, October 15, 2016, Edward Z. Yang notifications@github.com
wrote:

Yeah, the fix for new-build should not be too difficult. Just passing the
new flag.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3982 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAQwq8jIJuhPZ3EeQVYB2IX6u0ED8XZks5q0RXWgaJpZM4KW9eM
.

@23Skidoo
Copy link
Member

23Skidoo commented Oct 15, 2016

Or would that require several orders of magnitude more dynamic libraries than all of hackage built by a particular ghc version?

Not exactly, but yes. See here and here. HFS+ and FAT32 have the lowest limits on the number of files per directory (32K and 65K, respectively), for other commonly used filesystems it is practically unlimited.

Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

This is a lovely patchset for a complicated problem. I think the only major question is whether or not we want to bikeshed the name commonlibdir any further (sorry, probably a bit my fault). The reason why commonlibdir may not be a good name is because it doesn't describe what gets installed to the directory; it describes a "mode of use" of the directory (which may or may not be true.) Perhaps binlibdir? Another possibility is to add ANOTHER parameter so we have both dynlibdir and staticlibdir (and then staticlibdir could be set equal to hidir, since rpath considerations don't apply to static libraries.)


, option "" ["libsubdir"]
("subdirectory of libdir in which libs are installed." ++
"Only has an effect on Setup files build against Cabal < 1.25"
Copy link
Contributor

Choose a reason for hiding this comment

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

s/build/built/

"subdirectory of libdir in which libs are installed"
libsubdir (\v flags -> flags { libsubdir = v })
, option "" ["commonlibdir"]
"subdirectory of libdir in which the object files of libs are installed"
Copy link
Contributor

@ezyang ezyang Oct 16, 2016

Choose a reason for hiding this comment

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

The object files aren't installed here, right? Only the .so/.a/.dll/.dylib libraries.

@@ -690,6 +698,22 @@ path options:
``$pkg``, ``$version``, ``$compiler``, ``$os``, ``$arch``, ``$abi``,
``$abitag``

.. option:: --libsubdir=dir

For use with Setup.hs files build against a version of Cabal prior to 1.25.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/build/built/


For use with Setup.hs files build against a version of Cabal prior to 1.25.
For later versions of Cabal, this flag is basically deprecated, and you
should use ``--commonlibdir=dir``.
Copy link
Contributor

@ezyang ezyang Oct 16, 2016

Choose a reason for hiding this comment

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

With later versions of Cabal, you should prefer :option:--commonlibdir and :option:--hidir, which let you separately specify where binary libraries and interface files get installed, so that binary libraries can be installed to a shared directory.

``/usr/local/lib/mypkg-0.2/ghc-6.4``.
default *libdir* is ``/usr/local/lib``, and *commonlibdir* contains the
ABI, e.g. ``x86_64-linux-8.0.1``, so libraries would be installed in
``/usr/local/lib/x86_64-linux-8.0.1``.
Copy link
Contributor

Choose a reason for hiding this comment

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

It is recommended that a single, common directory to be used to store all installed libraries (as opposed to using $pkgid or similar variables to create a directory per installed library), as this helps reduce the size of rpath in executables built against dynamic libraries. See #3982 for more details.

@dcoutts
Copy link
Contributor

dcoutts commented Oct 16, 2016

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.

I don't understand this bit.

Surely the situation is that in older Cabal lib versions the $libdir/$libsubdir is used for both library files {a,so,dynlib,dll} and also for .hi files. Nothing we can do now can change that. But in new versions we can control them independently. To do that we have to introduce at minimum one new flag, e.g. $hidir for the .hi files, leaving the $libdir/$libsubdir for the .{a,so,dynlib,dll} files. But why would we need to introduce two new flags?

Isn't building Setup.hs against older Cabal lib versions a red herring? Since that doesn't let us control them independently anyway. We know what version of the Cabal Setup.hs CLI we're talking to, so we can set the flags accordingly.

What am I missing?

@dcoutts
Copy link
Contributor

dcoutts commented Oct 16, 2016

So as I see it, there's a few parts to this change:

  • Introducing a --hidir flag, independent from --libdir / --libsubdir (but defaulting to $libdir/$libsubdir so that there is no change in behaviour yet). This on its own is I think completely uncontroversial since it just provides more control without changing anything if it's not actively used.

One or both of:

  • Changing the default in Cabal for $libdir and $hidir
  • Changing the default in cabal-install/stack for --libdir and making use of --hidir.

The advantage of changing the default in Cabal is that it affects Linux distros that install libraries globally, for those Linux distros that don't override things already and just go with the defaults.

The disadvantage of changing the default in Cabal is that it adds extra version-dependent behaviour changes, that downstream tools need to be aware of and allow for or override (e.g. cabal-install already overrides all the flags in new-build at least, though it obviously does not yet override new flags like --hidir). In principle the file layout should be fully controlled by the callers, cabal-install/stack and not leave any of it to changing defaults in Cabal.

So an advantage of only changing the defaults in cabal-install/stack is that it may be simpler to do there, since they have to do it anyway.

@dcoutts
Copy link
Contributor

dcoutts commented Oct 16, 2016

This is to help me understand...

So the original layout scheme was:

libdir=$prefix/lib
libsubdir=$abi/$libname
includedir=$libdir/$libsubdir/include

The .hi files and the .a .so .dll .dynlib files all get installed under $libdir/$libsubdir.

The $abi is like x86_64-linux-ghc-7.10.1, while the $libname is things like tar-0.4.2.1-3r9cNWLCzNmJa5oTzjaqTP.

Note that the libname is the package id plus a hash that is unique locally to a linked app/lib. The hash includes the names and versions of all deps, but no more. It is not a nix hash including sources and configuration.

The layout proposed in this patch (and please correct me if I'm wrong) is:

libdir=$prefix/lib
libsubdir=$abi/$libname
commonlibdir=$abi
hidir=$libname
includedir=$hidir/include

The .hi files get installed under $hidir. The .a .so .dll .dynlib files all get installed under $libdir/$commonlibdir.

So this introduces both commonlibdir and hidir and changes the default location for both .hi files and .a .so .dll .dynlib files. The .hi files move from $prefix/lib/$abi/$libname to $prefix/lib/$libname, ie dropping the $abi (it's not clear if this is intentional). The .a .so .dll .dynlib files move from $prefix/lib/$abi/$libname to $prefix/lib/$abi, i.e. dropping the $libname (which is the original motivation of the patch). The $libsubdir and --libsubdir remains but they are now not used for anything at all.

So a few things I'm concerned about:

  • I'm still not clear why we should introduce a new $commonlibdir and leave $libsubdir unused.
  • The hidir should be explicitly relative to something like $libdir, not implicitly. The tradition (going back to classic gnu ./configure) is that the default for all these variables are relative to each other, and ultimately relative to $prefix. The only reason we have $libsubdir is as a hack because distros are used to using --libdir=/usr/lib and we needed a way to implicitly re-interpret that. But we don't need to do that for any other new vars, so we can use the normal explicitly-relative default scheme.
  • We still don't have any way to control the location of dynamic libs vs static libs. The scheme we want (I think) still places the static libs in the versioned dirs. This is relevant because we do not currently (afaik) use the $libname hash in the names of the static libs.

@christiaanb
Copy link
Collaborator Author

christiaanb commented Oct 16, 2016

So let's assume we only add --hidir (which is what I did in #3968), and chose this new layout scheme:

libdir=$prefix/lib
libsubdir=$abi
hidir=$libdir/$abi/$libname

This all works fine for cabal-install and for Setup.hs files that can be build against Cabal-1.25. But then, we have a custom Setup.hs that can only be build against an older version of Cabal that does not know --hidir. What is going to happen then? Well cabal-install runs:

./Setup configure --libdir=$prefix/lib --libsubdir=$abi

And so what happens is that the Setup will put all library files .hi and the .a/.so/.dll/.dylib files in $prefix/lib/$abi. Something that I think is highly undesired to happen by default. Now, you might say: well, if we can detect that Setup does not know the --hidir flag, then we can just revert to the old install layout of --libsubdir=$abi/$libname.
But, we can't! Because a user might have explicitly set libsubdir=$abi. And detecting whether the user explicitly set libsubdir to be the same as the Cabal-1.25 default is just extremely fragile.

And that is the reason I had to add another flag, --commonlibdir (name up for bikeshedding) to determine where the binary files (.a/.dll/.so/.dylib) go in Cabal-1.25 while still keeping around --libsubdir for pre-1.25 Cabal.

@christiaanb
Copy link
Collaborator Author

@dcoutts

The proposed layout is:

libdir=$prefix/lib
libsubdir=$abi/$libname
commonlibdir=$abi
hidir=$libdir/$abi/$libname
includedir=$hidir/include

@christiaanb
Copy link
Collaborator Author

@dcoutts Currently in this PR, --hidir can be set to a directory that is completely independent of --libdir. However, by default, --hidir is set to $libdir/$abi/$libname.

But I'm not really sure of your comments: is this something you find desirable? or do you want --hidir to always be implicitly be prefixed by $libdir?

@cartazio
Copy link
Contributor

Could we not use symlinks to collect all the libs Into a flattened
directory? At least outside of windows?

On Sunday, October 16, 2016, Christiaan Baaij notifications@github.com
wrote:

@dcoutts https://github.com/dcoutts Currently in this PR, --hidir can
be set to a directory that is completely independent of --libdir.
However, by default, --hidir is set to $libdir/$abi/$libname.

But I'm not really sure of your comments: is this something you find
desirable? or do you want --hidir to always be implicitly be prefixed
by $libdir?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3982 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAQwoVfZfjLlzHw8Y35zuYvjq3dhWQKks5q0huxgaJpZM4KW9eM
.

@christiaanb
Copy link
Collaborator Author

@cartazio If we did, where would be "store" this location so that GHC and Cabal know where to find this symlinked directory? Would we update the package database with an extra field?

We need to know the location of the symlinked libraries so that GHC and Cabal can tell the linker which RPATHs to add.

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

dcoutts commented Oct 16, 2016

But I'm not really sure of your comments: is this something you find desirable? or do you want --hidir to always be implicitly be prefixed by $libdir?

I was wrong actually. The hidir is exactly following the scheme I meant. It's hidir=$libdir/$abi/$libname as you said.

@dcoutts
Copy link
Contributor

dcoutts commented Oct 18, 2016

Reopened #3979 since we're pursuing that approach. So I'll take the liberty of closing this PR.

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.

8 participants