unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] build: cmake: Add input libraries to the rpath.
@ 2014-04-25  7:13 Andreas Enge
  2014-04-25 11:31 ` Ludovic Courtès
  2014-04-25 17:44 ` Eric Bavier
  0 siblings, 2 replies; 7+ messages in thread
From: Andreas Enge @ 2014-04-25  7:13 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

In a discussion we had yesterday, Ludovic mentioned the need to pass a
special flag to the cmake configure phase to modify the rpath of installed
libraries, as done for the package slim. I then noticed I needed the same
flag for clucene. The attached patch applies it globally in the cmake build
system. This should also avoid the need for the add-libs-to-runpath phase
in the gmsh package Eric Bavier posted yesterday.

In slim, there is another flag:
    ;; Don't build libslim.so, because then the build
    ;; system is unable to set the right RUNPATH on the
    ;; 'slim' binary.
    "-DBUILD_SHARED_LIBS=OFF"
I wonder if we should instead use another of the rpath setting variables
given at
   http://www.cmake.org/Wiki/CMake_RPATH_handling

Moreover, libclucene-core.so needs to be linked to libclucene-shared.so.1
from the same package. Here we usually employ patchelf, but maybe yet again
a cmake flag could solve the problem directly.

Comments from cmake specialists are very welcome!

Andreas


[-- Attachment #2: 0001-build-cmake-Add-input-libraries-to-the-rpath.patch --]
[-- Type: text/plain, Size: 870 bytes --]

From f5089b7ca46ac8199aa89f582efc04f12add8cbc Mon Sep 17 00:00:00 2001
From: Andreas Enge <andreas@enge.fr>
Date: Fri, 25 Apr 2014 09:00:03 +0200
Subject: [PATCH] build: cmake: Add input libraries to the rpath.

* guix/build/cmake-build-system.scm (configure): Add flag.
---
 guix/build/cmake-build-system.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/guix/build/cmake-build-system.scm b/guix/build/cmake-build-system.scm
index 7599856..fb07b99 100644
--- a/guix/build/cmake-build-system.scm
+++ b/guix/build/cmake-build-system.scm
@@ -48,6 +48,7 @@
 
     (let ((args `(,srcdir
                   ,(string-append "-DCMAKE_INSTALL_PREFIX=" out)
+                  "-DCMAKE_SKIP_BUILD_RPATH=ON"
                   ,@configure-flags)))
       (setenv "CMAKE_LIBRARY_PATH" (getenv "LIBRARY_PATH"))
       (setenv "CMAKE_INCLUDE_PATH" (getenv "CPATH"))
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: cmake: Add input libraries to the rpath.
  2014-04-25  7:13 [PATCH] build: cmake: Add input libraries to the rpath Andreas Enge
@ 2014-04-25 11:31 ` Ludovic Courtès
  2014-04-25 17:44 ` Eric Bavier
  1 sibling, 0 replies; 7+ messages in thread
From: Ludovic Courtès @ 2014-04-25 11:31 UTC (permalink / raw)
  To: Andreas Enge; +Cc: guix-devel

Andreas Enge <andreas@enge.fr> skribis:

> In a discussion we had yesterday, Ludovic mentioned the need to pass a
> special flag to the cmake configure phase to modify the rpath of installed
> libraries, as done for the package slim. I then noticed I needed the same
> flag for clucene. The attached patch applies it globally in the cmake build
> system. This should also avoid the need for the add-libs-to-runpath phase
> in the gmsh package Eric Bavier posted yesterday.
>
> In slim, there is another flag:
>     ;; Don't build libslim.so, because then the build
>     ;; system is unable to set the right RUNPATH on the
>     ;; 'slim' binary.
>     "-DBUILD_SHARED_LIBS=OFF"
> I wonder if we should instead use another of the rpath setting variables
> given at
>    http://www.cmake.org/Wiki/CMake_RPATH_handling

Yes, that would be better.

> Moreover, libclucene-core.so needs to be linked to libclucene-shared.so.1
> from the same package. Here we usually employ patchelf, but maybe yet again
> a cmake flag could solve the problem directly.
>
> Comments from cmake specialists are very welcome!

[...]

> --- a/guix/build/cmake-build-system.scm
> +++ b/guix/build/cmake-build-system.scm
> @@ -48,6 +48,7 @@
>  
>      (let ((args `(,srcdir
>                    ,(string-append "-DCMAKE_INSTALL_PREFIX=" out)
> +                  "-DCMAKE_SKIP_BUILD_RPATH=ON"
>                    ,@configure-flags)))

So that means that CMake won’t pass -rpath flag when building, nor when
installing, right?  (Which may be compensated by what ld-wrapper does.)

Does it actually work with the packages you looked at, and for SLiM and
and libssh (try building guile-ssh to check)?

At any rate, that’ll deserve a comment because even after discussing it
I still have to think twice what the option really does.  :-)

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: cmake: Add input libraries to the rpath.
  2014-04-25  7:13 [PATCH] build: cmake: Add input libraries to the rpath Andreas Enge
  2014-04-25 11:31 ` Ludovic Courtès
@ 2014-04-25 17:44 ` Eric Bavier
  2014-04-26  8:52   ` Ludovic Courtès
  2014-04-27  8:59   ` Andreas Enge
  1 sibling, 2 replies; 7+ messages in thread
From: Eric Bavier @ 2014-04-25 17:44 UTC (permalink / raw)
  To: Andreas Enge; +Cc: guix-devel

Andreas Enge <andreas@enge.fr> writes:

> In a discussion we had yesterday, Ludovic mentioned the need to pass a
> special flag to the cmake configure phase to modify the rpath of installed
> libraries, as done for the package slim. I then noticed I needed the same
> flag for clucene. The attached patch applies it globally in the cmake build
> system.

We can't set CMAKE_SKIP_BUILD_RPATH=OFF as it prevents tests from
working, since the executables and libraries would not have references
to libraries in the build tree (I ran the lapack build e.g. with your
patch, and all the tests fail).

> This should also avoid the need for the add-libs-to-runpath phase
> in the gmsh package Eric Bavier posted yesterday.
>
> In slim, there is another flag:
>     ;; Don't build libslim.so, because then the build
>     ;; system is unable to set the right RUNPATH on the
>     ;; 'slim' binary.
>     "-DBUILD_SHARED_LIBS=OFF"
> I wonder if we should instead use another of the rpath setting variables
> given at
>    http://www.cmake.org/Wiki/CMake_RPATH_handling
>
> Moreover, libclucene-core.so needs to be linked to libclucene-shared.so.1
> from the same package. Here we usually employ patchelf, but maybe yet again
> a cmake flag could solve the problem directly.
>
> Comments from cmake specialists are very welcome!

I wouldn't necessarily count myself as a cmake specialist, but I work
with it a bit in my day job.

Your post prompted me to look into this matter a bit more.  I found for
the gmsh package I posted yesterday that I could add the following to
#:configuration-flags instead of using the add-libs-to-runpath phase::

    "-DCMAKE_INSTALL_RPATH_USE_LINK_PATH:BOOL=ON"
     ,(string-append "-DCMAKE_INSTALL_RPATH="
                     (assoc-ref %outputs "out")
                     "/lib")

I would defer to the cmake wiki page you linked above for a full
explanation, but briefly: The CMAKE_INSTALL_RPATH_USE_LINK_PATH tells
cmake to include in the installed rpath all of the directories of
libraries it has linked into the library or executable, and the
CMAKE_INSTALL_RPATH tells cmake that in addition there are libraries in
the current project whose directory also needs to be added to the rpath.

I tried substituting the above two flags in for the lapack, gmsh, and
slim builds, and the RUNPATHS seem to be at least as good as when using
the manual augment-rpath method.

-- 
`~Eric

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: cmake: Add input libraries to the rpath.
  2014-04-25 17:44 ` Eric Bavier
@ 2014-04-26  8:52   ` Ludovic Courtès
  2014-04-27  8:59   ` Andreas Enge
  1 sibling, 0 replies; 7+ messages in thread
From: Ludovic Courtès @ 2014-04-26  8:52 UTC (permalink / raw)
  To: Eric Bavier; +Cc: guix-devel

Eric Bavier <ericbavier@gmail.com> skribis:

> Andreas Enge <andreas@enge.fr> writes:
>
>> In a discussion we had yesterday, Ludovic mentioned the need to pass a
>> special flag to the cmake configure phase to modify the rpath of installed
>> libraries, as done for the package slim. I then noticed I needed the same
>> flag for clucene. The attached patch applies it globally in the cmake build
>> system.
>
> We can't set CMAKE_SKIP_BUILD_RPATH=OFF as it prevents tests from
> working, since the executables and libraries would not have references
> to libraries in the build tree (I ran the lapack build e.g. with your
> patch, and all the tests fail).

OK, that’s what I feared.

> Your post prompted me to look into this matter a bit more.  I found for
> the gmsh package I posted yesterday that I could add the following to
> #:configuration-flags instead of using the add-libs-to-runpath phase::
>
>     "-DCMAKE_INSTALL_RPATH_USE_LINK_PATH:BOOL=ON"
>      ,(string-append "-DCMAKE_INSTALL_RPATH="
>                      (assoc-ref %outputs "out")
>                      "/lib")

[...]

> I tried substituting the above two flags in for the lapack, gmsh, and
> slim builds, and the RUNPATHS seem to be at least as good as when using
> the manual augment-rpath method.

Good, it looks like this is what ‘cmake-build-system’ should do by
default.

One concern though: if a package installs libraries in a place other
than $out/lib, like $out/lib/PACKAGE, this will break.

In practice, do CMake packages always install libraries in $libdir?

Thanks for helping out,
Ludo’.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: cmake: Add input libraries to the rpath.
  2014-04-25 17:44 ` Eric Bavier
  2014-04-26  8:52   ` Ludovic Courtès
@ 2014-04-27  8:59   ` Andreas Enge
  2014-04-27  9:57     ` Andreas Enge
  1 sibling, 1 reply; 7+ messages in thread
From: Andreas Enge @ 2014-04-27  8:59 UTC (permalink / raw)
  To: Eric Bavier; +Cc: guix-devel

On Fri, Apr 25, 2014 at 12:44:32PM -0500, Eric Bavier wrote:
> We can't set CMAKE_SKIP_BUILD_RPATH=OFF as it prevents tests from
> working, since the executables and libraries would not have references
> to libraries in the build tree (I ran the lapack build e.g. with your
> patch, and all the tests fail).

Yes, I noticed the same problem with lapack.

> Your post prompted me to look into this matter a bit more.  I found for
> the gmsh package I posted yesterday that I could add the following to
> #:configuration-flags instead of using the add-libs-to-runpath phase::
>     "-DCMAKE_INSTALL_RPATH_USE_LINK_PATH:BOOL=ON"
>      ,(string-append "-DCMAKE_INSTALL_RPATH="
>                      (assoc-ref %outputs "out")
>                      "/lib")
> 
> I would defer to the cmake wiki page you linked above for a full
> explanation, but briefly: The CMAKE_INSTALL_RPATH_USE_LINK_PATH tells
> cmake to include in the installed rpath all of the directories of
> libraries it has linked into the library or executable, and the
> CMAKE_INSTALL_RPATH tells cmake that in addition there are libraries in
> the current project whose directory also needs to be added to the rpath.

Excellent, so we could drop all patchelf occurrences!

On Sat, Apr 26, 2014 at 10:52:52AM +0200, Ludovic Courtès wrote:
> One concern though: if a package installs libraries in a place other
> than $out/lib, like $out/lib/PACKAGE, this will break.

I think that Eric's following sentence gives the answer:
> I tried substituting the above two flags in for the lapack, gmsh, and
> slim builds, and the RUNPATHS seem to be at least as good as when using
> the manual augment-rpath method.

There is still the possibility of adding an augment-rpath phase manually
if a problem occurs, or maybe simply adding a second flag
"-DCMAKE_INSTALL_RPATH".

I will give it a try and commit a corresponding patch if everything works.

Andreas

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: cmake: Add input libraries to the rpath.
  2014-04-27  8:59   ` Andreas Enge
@ 2014-04-27  9:57     ` Andreas Enge
  2014-04-27 17:17       ` Ludovic Courtès
  0 siblings, 1 reply; 7+ messages in thread
From: Andreas Enge @ 2014-04-27  9:57 UTC (permalink / raw)
  To: Eric Bavier; +Cc: guix-devel

On Sun, Apr 27, 2014 at 10:59:27AM +0200, Andreas Enge wrote:
> I will give it a try and commit a corresponding patch if everything works.

Done. Thanks a lot for your help!

Andreas

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: cmake: Add input libraries to the rpath.
  2014-04-27  9:57     ` Andreas Enge
@ 2014-04-27 17:17       ` Ludovic Courtès
  0 siblings, 0 replies; 7+ messages in thread
From: Ludovic Courtès @ 2014-04-27 17:17 UTC (permalink / raw)
  To: Andreas Enge; +Cc: guix-devel

Andreas Enge <andreas@enge.fr> skribis:

> On Sun, Apr 27, 2014 at 10:59:27AM +0200, Andreas Enge wrote:
>> I will give it a try and commit a corresponding patch if everything works.
>
> Done. Thanks a lot for your help!

Nice, thanks to both of you!

Ludo’.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-04-27 17:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-25  7:13 [PATCH] build: cmake: Add input libraries to the rpath Andreas Enge
2014-04-25 11:31 ` Ludovic Courtès
2014-04-25 17:44 ` Eric Bavier
2014-04-26  8:52   ` Ludovic Courtès
2014-04-27  8:59   ` Andreas Enge
2014-04-27  9:57     ` Andreas Enge
2014-04-27 17:17       ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).