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

Support arch-specific lib and cmake folders with catkin_make_isolated #545

Closed
scpeters opened this issue Oct 25, 2013 · 4 comments
Closed

Comments

@scpeters
Copy link
Contributor

In order to get into debian, they recommend using GNUInstallDirs for installing architecture-dependent libraries. For example, sdformat is in the process of this change, which on my machine causes libraries to be installed to {prefix}/lib/x86_64-linux-gnu/ and the cmake config file to be installed to {prefix}/lib/x86_64-linux-gnu/cmake/sdformat/.

There was some discussion about whether GNUInstallDirs should be used only for packaging debians or for all source installs. In the end, sdformat chose to use it always by default, and I think other packages may do the same. It would greatly enhance the utility of catkin_make_isolated to support this behavior, both for setting the CMAKE_PREFIX_PATH and for setting the LD_LIBRARY_PATH in the setup.sh scripts.

I had been using catkin_make_isolated to build sdformat and gazebo together by manually adding my own package.xml files. gazebo already uses GNUInstallDirs, so I have had to manually edit the LD_LIBRARY_PATH in setup.sh so that the gazebo binaries can find the gazebo libraries. Now with the sdformat change, gazebo cannot find_package sdformat because the cmake folder is in a subfolder of lib rather than share.

One clue for implementing this would be to parse the contents of /etc/ld.so.conf.d/:

$ cat /etc/ld.so.conf.d/x86_64-linux-gnu.conf 
# Multiarch support
/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu
@scpeters
Copy link
Contributor Author

An alternative to parsing /etc/ld.so.conf.d/ is to include the GNUInstallDirs module in a sandbox to see if there are arch-specific lib folders for the given platform. An example CMakeLists.txt:

cmake_minimum_required(VERSION 2.8 FATAL_ERROR)
project (CMAKE_GNU_DIRS_TEST)
include(GNUInstallDirs)
message (STATUS "CMAKE_INSTALL_LIBDIR ${CMAKE_INSTALL_LIBDIR}")

and its output on Ubuntu:

...
-- CMAKE_INSTALL_LIBDIR lib/x86_64-linux-gnu
...

@scpeters
Copy link
Contributor Author

Note that this will be an issue for console_bridge when 0.2.5 is released (currently it's on 0.2.4).

This is when GNUInstallDirs was introduced into console_bridge, and it's listed as 0.2.5:
ros/console_bridge@3830f26

scpeters added a commit to scpeters/catkin that referenced this issue Oct 28, 2013
Workaround hack for ros#545, by setting LD_LIBRARY_PATH
for x86_64-linux-gnu
@scpeters
Copy link
Contributor Author

scpeters commented Nov 1, 2013

I've been using a hardcoded workaround on my fork for the past week or so, and it's been successful. I just have to add the arch-specific folder to LD_LIBRARY_PATH and PKG_CONFIG_PATH. Now we just need to automatically identify the name of the arch-specific library folders.

@@ -498,11 +498,12 @@ def build_cmake_package(
     if new_setup_path != last_setup_env:
         subs = {}
         subs['cmake_prefix_path'] = install_target + ":"
-        subs['ld_path'] = os.path.join(install_target, 'lib') + ":"
+        subs['ld_path'] = os.path.join(install_target, 'lib') + ":" + \
+                          os.path.join(install_target, 'lib', 'x86_64-linux-gnu') + ":"
         pythonpath = os.path.join(install_target, get_python_install_dir())
         subs['pythonpath'] = pythonpath + ':'
-        subs['pkgcfg_path'] = os.path.join(install_target, 'lib', 'pkgconfig')
-        subs['pkgcfg_path'] += ":"
+        subs['pkgcfg_path'] = os.path.join(install_target, 'lib', 'pkgconfig') + ":" + \
+                              os.path.join(install_target, 'lib', 'x86_64-linux-gnu', 'pkgconfig') + ":"
         subs['path'] = os.path.join(install_target, 'bin') + ":"
         if not os.path.exists(os.path.dirname(new_setup_path)):
             os.mkdir(os.path.dirname(new_setup_path))

@scpeters
Copy link
Contributor Author

scpeters commented Nov 3, 2013

So my previous testing only involved devel builds; I tried catkin_make_isolated --install and it didn't work. I made another hard-coded hack in scpeters@5597b58 that fixes install builds.

scpeters added a commit to scpeters/catkin that referenced this issue Apr 28, 2014
This is an attempt at multiarch support (ros#545), supercedes
(ros#604). The dpkg-architecture command is used to identify
an appropriate lib folder suffix to use for LD_LIBRARY_PATH
and PKG_CONFIG_PATH. builder.py and
catkin_generate_environment.cmake are modified
accordingly, though there's still a problem with isolated
devel spaces.
scpeters added a commit to scpeters/catkin that referenced this issue Apr 30, 2014
commit 0f2d1c7
Author: Jose Luis Rivero <[email protected]>
Date:   Wed Apr 30 02:36:11 2014 +0200

    Improve the method of detecting multiarch

    Run a two step approach using gcc -print-multiarch and, if failed,
    fallback to use dpkg-architecture.

commit 6039d93
Author: Jose Luis Rivero <[email protected]>
Date:   Tue Apr 29 19:21:44 2014 +0200

    Fix call to get_multiarch by removing the parameter

commit dcbc8f3
Author: Jose Luis Rivero <[email protected]>
Date:   Tue Apr 29 19:08:27 2014 +0200

    Replace dpkg-architecture by gcc -print-multiarch.

    Silent the error on cmake if the flag is not present. Checking the
    content of the OUTPUT_VARIABLE for empty should be enough to detect
    errors on multiarch detection.

commit c1c02af
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 17:07:42 2014 -0700

    Fix test by escaping single quotes

commit ae19d1b
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 16:20:22 2014 -0700

    Starting to fix broken test

commit 232d56b
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:55:27 2014 -0700

    Use temporary path variable in loop per @dirk-thomas 's suggestion

commit 324ac93
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:46:22 2014 -0700

    Remove trailing whitespace from dpkg-architecture call

commit 90f56a4
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:41:22 2014 -0700

    Use string with spaces for subprocess command

commit 948eb7d
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:32:02 2014 -0700

    Fix subprocess invocation per suggestion from @wjwwood

commit e114c9d
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:24:38 2014 -0700

    Use CATKIN_GLOBAL_LIB_DESTINATION and os.path.join

commit 6f89efe
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:01:52 2014 -0700

    Attempt at multiarch support (ros#545)

    This is an attempt at multiarch support (ros#545), supercedes
    (ros#604). The dpkg-architecture command is used to identify
    an appropriate lib folder suffix to use for LD_LIBRARY_PATH
    and PKG_CONFIG_PATH. builder.py and
    catkin_generate_environment.cmake are modified
    accordingly, though there's still a problem with isolated
    devel spaces.
scpeters added a commit to scpeters/catkin that referenced this issue Apr 30, 2014
Squashed commit of the following:

commit 0f2d1c7
Author: Jose Luis Rivero <[email protected]>
Date:   Wed Apr 30 02:36:11 2014 +0200

    Improve the method of detecting multiarch

    Run a two step approach using gcc -print-multiarch and, if failed,
    fallback to use dpkg-architecture.

commit 6039d93
Author: Jose Luis Rivero <[email protected]>
Date:   Tue Apr 29 19:21:44 2014 +0200

    Fix call to get_multiarch by removing the parameter

commit dcbc8f3
Author: Jose Luis Rivero <[email protected]>
Date:   Tue Apr 29 19:08:27 2014 +0200

    Replace dpkg-architecture by gcc -print-multiarch.

    Silent the error on cmake if the flag is not present. Checking the
    content of the OUTPUT_VARIABLE for empty should be enough to detect
    errors on multiarch detection.

commit c1c02af
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 17:07:42 2014 -0700

    Fix test by escaping single quotes

commit ae19d1b
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 16:20:22 2014 -0700

    Starting to fix broken test

commit 232d56b
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:55:27 2014 -0700

    Use temporary path variable in loop per @dirk-thomas 's suggestion

commit 324ac93
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:46:22 2014 -0700

    Remove trailing whitespace from dpkg-architecture call

commit 90f56a4
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:41:22 2014 -0700

    Use string with spaces for subprocess command

commit 948eb7d
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:32:02 2014 -0700

    Fix subprocess invocation per suggestion from @wjwwood

commit e114c9d
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:24:38 2014 -0700

    Use CATKIN_GLOBAL_LIB_DESTINATION and os.path.join

commit 6f89efe
Author: Steven Peters <[email protected]>
Date:   Mon Apr 28 15:01:52 2014 -0700

    Attempt at multiarch support (ros#545)

    This is an attempt at multiarch support (ros#545), supercedes
    (ros#604). The dpkg-architecture command is used to identify
    an appropriate lib folder suffix to use for LD_LIBRARY_PATH
    and PKG_CONFIG_PATH. builder.py and
    catkin_generate_environment.cmake are modified
    accordingly, though there's still a problem with isolated
    devel spaces.
scpeters added a commit to scpeters/catkin that referenced this issue May 6, 2014
commit 7bad3da
Author: Steven Peters <[email protected]>
Date:   Mon May 5 16:55:04 2014 -0700

    Implement multiarch support in catkin

    commit e99145a
    Author: Steven Peters <[email protected]>
    Date:   Mon May 5 16:45:19 2014 -0700

        Updates per comments by @dirk-thomas

        Remove shell keyword and call Popen with list of strings.
        Remove extra : from PATH variable packing.
        Fix indentation.
        Use " to fix escaped single quotes.

    commit a0eb2a0
    Author: Steven Peters <[email protected]>
    Date:   Mon May 5 15:11:28 2014 -0700

        Don't check multiarch on non-Linux systems

    commit 655012a
    Author: Steven Peters <[email protected]>
    Date:   Mon May 5 14:45:47 2014 -0700

        Updates per comments from @wjwwood

        Renamed the CATKIN_MULTIARCH_*_DESTINATION variables to
        CATKIN_*_ENVIRONMENT_PATHS.
        Use isinstance to verify that variables are lists.
        Check Popen return code instead of std_err.
        Fix logic error in catkin_generate_environment.cmake

commit 35d9827
Author: Steven Peters <[email protected]>
Date:   Tue Apr 29 18:10:43 2014 -0700

    Implement multiarch support in catkin

    Squashed commit of the following:

    commit 0f2d1c7
    Author: Jose Luis Rivero <[email protected]>
    Date:   Wed Apr 30 02:36:11 2014 +0200

        Improve the method of detecting multiarch

        Run a two step approach using gcc -print-multiarch and, if failed,
        fallback to use dpkg-architecture.

    commit 6039d93
    Author: Jose Luis Rivero <[email protected]>
    Date:   Tue Apr 29 19:21:44 2014 +0200

        Fix call to get_multiarch by removing the parameter

    commit dcbc8f3
    Author: Jose Luis Rivero <[email protected]>
    Date:   Tue Apr 29 19:08:27 2014 +0200

        Replace dpkg-architecture by gcc -print-multiarch.

        Silent the error on cmake if the flag is not present. Checking the
        content of the OUTPUT_VARIABLE for empty should be enough to detect
        errors on multiarch detection.

    commit c1c02af
    Author: Steven Peters <[email protected]>
    Date:   Mon Apr 28 17:07:42 2014 -0700

        Fix test by escaping single quotes

    commit ae19d1b
    Author: Steven Peters <[email protected]>
    Date:   Mon Apr 28 16:20:22 2014 -0700

        Starting to fix broken test

    commit 232d56b
    Author: Steven Peters <[email protected]>
    Date:   Mon Apr 28 15:55:27 2014 -0700

        Use temporary path variable in loop per @dirk-thomas 's suggestion

    commit 324ac93
    Author: Steven Peters <[email protected]>
    Date:   Mon Apr 28 15:46:22 2014 -0700

        Remove trailing whitespace from dpkg-architecture call

    commit 90f56a4
    Author: Steven Peters <[email protected]>
    Date:   Mon Apr 28 15:41:22 2014 -0700

        Use string with spaces for subprocess command

    commit 948eb7d
    Author: Steven Peters <[email protected]>
    Date:   Mon Apr 28 15:32:02 2014 -0700

        Fix subprocess invocation per suggestion from @wjwwood

    commit e114c9d
    Author: Steven Peters <[email protected]>
    Date:   Mon Apr 28 15:24:38 2014 -0700

        Use CATKIN_GLOBAL_LIB_DESTINATION and os.path.join

    commit 6f89efe
    Author: Steven Peters <[email protected]>
    Date:   Mon Apr 28 15:01:52 2014 -0700

        Attempt at multiarch support (ros#545)

        This is an attempt at multiarch support (ros#545), supercedes
        (ros#604). The dpkg-architecture command is used to identify
        an appropriate lib folder suffix to use for LD_LIBRARY_PATH
        and PKG_CONFIG_PATH. builder.py and
        catkin_generate_environment.cmake are modified
        accordingly, though there's still a problem with isolated
        devel spaces.
scpeters added a commit to scpeters/catkin that referenced this issue May 6, 2014
This implements multiarch support (ros#545), and supercedes
(ros#604). A two step approach uses gcc -print-multiarch and,
if failed, fallback to use dpkg-architecture to identify
an appropriate lib folder suffix to use for LD_LIBRARY_PATH
and PKG_CONFIG_PATH. builder.py and
catkin_generate_environment.cmake are modified
accordingly.

Thanks to @j-rivero for his contributions, which are hidden in
this squashed commit.
dirk-thomas added a commit that referenced this issue May 6, 2014
Attempt at multiarch support (#545)
@wjwwood wjwwood closed this as completed May 14, 2014
@dirk-thomas dirk-thomas removed this from the untargeted milestone May 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants