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

CMAKE_INSTALL_PREFIX in toolchain breaks compilation #17

Open
asherikov opened this issue Sep 15, 2021 · 3 comments
Open

CMAKE_INSTALL_PREFIX in toolchain breaks compilation #17

asherikov opened this issue Sep 15, 2021 · 3 comments
Assignees

Comments

@asherikov
Copy link

I am setting CMAKE_INSTALL_PREFIX in a cmake toolchain for certain technical reason. In this case toolchain overrides command line parameters, so
https://github.com/ament/google_benchmark_vendor/blob/main/CMakeLists.txt#L66 has no effect and the installation logic below https://github.com/ament/google_benchmark_vendor/blob/main/CMakeLists.txt#L74 fails.

Is there a reason for installing external project to a temporary directory? It looks like an unnecessary step --CMAKE_INSTALL_PREFIX can be passed directly to the external project without extra install command. This would work just fine in my environment as well.

@hidmic
Copy link
Contributor

hidmic commented Sep 16, 2021

@cottsay I think you're the best person to answer this.

@cottsay cottsay self-assigned this Sep 16, 2021
@cottsay
Copy link
Contributor

cottsay commented Sep 17, 2021

I'm not convinced that setting CMAKE_INSTALL_PREFIX in a toolchain file is a supported scenario. The CMAKE_INSTALL_PREFIX should contain the runtime installation directory, while toolchain files are used to direct the build process for binaries. Clearly CMake lets you set arbitrary variables in the toolchain file, but overriding CMake projects' internal build mechanisms unconditionally is probably a recipe for problems...like this one.

Is there a reason for installing external project to a temporary directory?

The primary reason is staging. Build and install are separate phases, and ExternalProject_Add is only part of the build phase. When we build ROS with colcon, the install phase always follows the build phase so the lines are a little blurred, but when we build debs and RPMs, the install phase is separate and the staging directory for the package build isn't even passed to the build phase - it gets sent to the install phase via the DESTDIR argument to make. So this might work for you when you use CMAKE_INSTALL_PREFIX to specify the runtime installation directory, but it breaks all scenarios that use DESTDIR.

@asherikov
Copy link
Author

IMO setting CMAKE_INSTALL_PREFIX this way is legitimate (and in my case necessary) -- this parameter is supposed to be specified globally and should be respected by packages. The source of the problem is misuse of external project (https://cmake.org/cmake/help/latest/module/ExternalProject.html):

The external project's own install step is invoked as part of the main project's build.

You are hacking around this feature by playing with install prefix. There is probably a way to do it properly by adjusting external project steps and adding install dependencies, see https://stackoverflow.com/questions/37838786/how-to-not-make-install-step-when-building-external-project-with-cmake and https://cmake.org/cmake/help/latest/module/ExternalProject.html, but I would generally advise against using external project at all -- it is unreliable in many ways including the necessity to fetch sources during builds.
In my experience the best way to approach such situations is to embed upstream code in your repo and use add_subdirectory: for example in https://github.com/asherikov/ariles/tree/pkg_catkin_2 the main branch is embedded in ariles subdirectory with git read-tree (https://github.com/asherikov/ariles/blob/pkg_catkin_2/.make/ariles.mk#L14) and then included in ROS wrapper packages with add_subdirectory (https://github.com/asherikov/ariles/blob/pkg_catkin_2/.cmake/common.cmake#L30)

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

No branches or pull requests

3 participants