libiberty: new package by mwkrentel · Pull Request #8912 · spack/spack

mwkrentel · 2018-08-08T03:41:47Z

The libiberty.a library from GNU binutils. Libiberty provides
demangling and support functions for the GNU toolchain.

This package uses the binutils tarfile but only builds the libiberty
subdirectory. This is useful for other packages that want the
demangling functions without the rest of binutils.

Add variant 'fpic' to compile with -fPIC.

Addresses some issues raised in PR #8806.

The libiberty.a library from GNU binutils. Libiberty provides demangling and support functions for the GNU toolchain. This package uses the binutils tarfile but only builds the libiberty subdirectory. This is useful for other packages that want the demangling functions without the rest of binutils. Add variant 'fpic' to compile with -fPIC. Addresses some issues raised in PR spack#8806.

adamjstewart · 2018-08-08T15:00:03Z

var/spack/repos/builtin/packages/libiberty/package.py

+    # Configure and build just libiberty.
+    @property
+    def configure_directory(self):
+        return join_path(self.stage.source_path, 'libiberty')


This whole thing can be replace with:

configure_directory = 'libiberty'

http://spack.readthedocs.io/en/latest/build_systems/autotoolspackage.html#configure-script-in-a-sub-directory

Ok, I didn't realize that. I'll change it.

adamjstewart · 2018-08-08T15:00:50Z

var/spack/repos/builtin/packages/libiberty/package.py

+
+    # Set default cflags (-g -O2), add -fPIC if requested, and move to
+    # the configure line.
+    def flag_handler(self, name, flags):


Does this work for Autotools packages as well? I've only ever seen it used in CMake packages.

Yes, build_system_flags works for Autotools and Cmake. Any other
package type would have to implement flags_to_build_system_args() or
else use a different flag handler.

adamjstewart · 2018-08-08T15:01:11Z

var/spack/repos/builtin/packages/libiberty/package.py

+        return args
+
+    # Libiberty always installs libiberty.a into lib64, even with
+    # --libdir or --disable-multilib, so just give up and copy.


Is there something wrong with lib64?

I was worried about robustness and I thought it was simpler to always
put it in lib. I could ask if there's anything wrong with lib?

Is this the normal way spack handles this? That is, let the package
choose for itself whether to install into lib or lib64?

If that's how spack normally works, then yes, I'll change it. But if
that's the convention, then doesn't every package have to know if its
dependency libraries are in lib or lib64?

For autotools, you normally just use, '--with-pkg=prefix' and let the
package figure it out. But if you want the actual lib dir, then you
can't just assume that prefix.lib will always work, right?

Worse yet, I've seen other packages that install into lib on x86 and
lib64 on powerpc. Libunwind used to do this, I think they still do.
How does spack handle that?

(I don't mean to be sarcastic. I'm just asking, how does spack handle
this?)

I think in general Spack lets the package decide where to install its libraries. Note that we also need to support externally installed packages, where we have no control over where they install things. Most of the time, Autotools figures out where dependencies live, but in the case that we need to specify a directory, we can use the spec['foo'].libs attribute to automatically locate libraries.

adamjstewart · 2018-08-08T15:01:50Z

var/spack/repos/builtin/packages/libiberty/package.py

+    version('2.29.1', 'acc9cd826edb9954ac7cecb81c727793')
+    version('2.28.1', 'a3bf359889e4b299fce1f4cb919dc7b6')
+
+    variant('fpic', default=False, description='Compile with -fPIC.')


Call the variant pic. -fPIC is only the flag for some compilers, not all. The description should be Compile with position independent code.

You're right! Normally I grep through existing packages to see if
there's a standard name for these things. But I guess I was sloppy in
this case. I'll change it.

Btw, there is a 'Style guidelines for packages' in read-the-docs, but
it only mentions 'shared', 'static', 'mpi' and 'python' as standard
variant names.

No style guide yet. We could add one, but there are so many things to add...

mwkrentel · 2018-08-08T21:58:42Z

I guess I'll drop the lib64 to lib copy then.

Is there a reference for spec['foo'].libs?

I found this in ncurses/package.py:

    @property
    def libs(self):
        return find_libraries(
            ['libncurses', 'libncursesw'], root=self.prefix, recursive=True)

Is that what you meant? Is this a way for ncurses to announce to its
dependents how to find its libraries?

adamjstewart · 2018-08-08T22:11:26Z

Correct, see http://spack.readthedocs.io/en/latest/llnl.util.html#llnl.util.filesystem.LibraryList for more information.

By default, spec['libiberty'].libs would search spec['libiberty'].prefix.lib64, then spec['libiberty'].prefix.lib, then recursively search spec['libiberty'].prefix as a last resort. If you don't override anything, it will search for libiberty.so (or libiberty.dylib if on macOS) and return the LibraryList object described above. You can override this as you found in the ncurses package if the library is named differently or is in a weird location.

mwkrentel · 2018-08-08T22:19:46Z

Ok, I'll play around with this and update the patch.
Does this also search for .a archives (what libiberty uses)?

Thanks!

adamjstewart · 2018-08-08T22:22:52Z

If the package has a +shared variant, it searches for whichever you specify. If not, it will first search for shared object libraries and then archive libraries. See https://github.com/spack/spack/blob/develop/lib/spack/spack/spec.py#L702,L759 for the default libs implementation. Sorry this is so poorly documented, it's a fairly new feature.

Allow libiberty to install the library in lib64 and don't try to copy it to lib.

mwkrentel · 2018-08-09T05:54:45Z

Ok, I've made the changes as above, including the 'pic' name and
leaving the library in lib64.

Unless there's something else, I'm done.

I looked at the use of spec['...'].libs as you suggested. AFAICT, in
this case, there's no reason to use this for libiberty since the
default libs method finds the correct position of libiberty.a.
Is that right?

Thanks! I keep learning new things ....

* libiberty: new package The libiberty.a library from GNU binutils. Libiberty provides demangling and support functions for the GNU toolchain. This package uses the binutils tarfile but only builds the libiberty subdirectory. This is useful for other packages that want the demangling functions without the rest of binutils. Add variant 'fpic' to compile with -fPIC. Addresses some issues raised in PR spack#8806. * libiberty: change variant name to 'pic'. Allow libiberty to install the library in lib64 and don't try to copy it to lib.

mwkrentel mentioned this pull request Aug 8, 2018

dyninst: add dependency on libiberty #8806

Merged

adamjstewart added the new-package label Aug 8, 2018

adamjstewart reviewed Aug 8, 2018

View reviewed changes

libiberty: change variant name to 'pic'.

10efbc5

Allow libiberty to install the library in lib64 and don't try to copy it to lib.

adamjstewart approved these changes Aug 9, 2018

View reviewed changes

adamjstewart merged commit 393d3c6 into spack:develop Aug 9, 2018

mwkrentel mentioned this pull request Oct 22, 2018

Forced Binutils to use PIC with LibIberty #9570

Closed

mwkrentel mentioned this pull request Nov 13, 2018

elfutils: add version 0.174. #9812

Merged

Conversation

mwkrentel commented Aug 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mwkrentel commented Aug 8, 2018

Uh oh!

adamjstewart commented Aug 8, 2018

Uh oh!

mwkrentel commented Aug 8, 2018

Uh oh!

adamjstewart commented Aug 8, 2018

Uh oh!

mwkrentel commented Aug 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants