-
Notifications
You must be signed in to change notification settings - Fork 365
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
does not build on macOS when using isolated install with colcon #1270
Comments
Given that the only direct dependency of Cyclone is to Indeed, there is no
For reference:
@dkroenke any idea why there is no rpath entry? |
@eboasson we do not set the rpath and there is an issue to change this but we did not yet have time to implement it. eclipse-iceoryx/iceoryx#1287 We can give it a higher priority and fix it the next days if necessary. |
It's not super high priority for me, but I wanted to give a thorough bug report. For now I'm fixing the lib manually with install name tool. I agree with you @eboasson, but the logic about extending the dyld library path I linked to is still suspect/fragile to me. |
@wjwwood I'm just wondering why we do not have this problem on the ROS CI. Do you have a newer/older colcon or cmake than on the CI? |
We no longer run macOS CI. Also, it's possible we use/used |
@wjwwood we are on it but it make take a few more days eclipse-iceoryx/iceoryx#1357 |
Thanks for the update, I'm not blocked by this at the moment. 👍 |
@wjwwood it would be great if you could also check if the branch from ApexAI:iox-#1287-setting-rpath-for-all-iceoryx-targets works for you. We checked it on several platforms but we couldn't reproduce the problem with your instructions in the first place and had to trigger it by different means. If this also works for you we would merge the branch to master. |
I'm testing it now, but as a tangent, I recommend not using |
It fails because the libraries already have an
|
Hi, Is there any update on it. I am getting same error building on jetson docker. Thanks! |
@automech-rb it appears not ... but I think I found the problem: it seems CMake either simply sets the run-time link path based to the contents of I always left as many CMake things to others as I could because CMake and I don't go well together, assuming that the way it set I don't know how fragile the (Cyclone DDS) patch below is, but "it seems to work for me today" ™️. If it works for you, too, I'll turn it into a PR as better than nothing ... diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 2c7ee5e0..fa08094a 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -57,6 +57,10 @@ if(ENABLE_SHM)
endif()
find_package(iceoryx_binding_c ${iceoryx_required})
set(ENABLE_SHM ${iceoryx_binding_c_FOUND} CACHE STRING "" FORCE)
+ if(ENABLE_SHM AND APPLE)
+ get_filename_component(iceoryx_libdir "${ICEORYX_LIB}" DIRECTORY)
+ set(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_RPATH};${iceoryx_libdir}")
+ endif()
endif()
# ones that linger in the sources There's got to be a better way, though ... |
@automech-rb Scratch that ... it is useful (see #1353) but it doesn't fix the colcon build problem, which really is internal to Iceoryx and so unaffected by my little patch. The problem is libiceoryx_binding_c not finding libiceoryx_posh, not Cyclone not finding libiceoryx_binding_c. I mixed them up in my head because I have suffered from the latter quite a bit, patching rpaths after builds many times. But it is a different problem. For me |
@eboasson @automech-rb we are not sure why it doesn't work since we cannot reproduce the problem. There was this PR eclipse-iceoryx/iceoryx#1357 which fixed it for all scenarios we could reproduce. Do you have more information regarding the issue. |
@elBoberido there's only one thing that comes to mind: macOS' SIP, which among other things likes to strip out environment variables with names starting with
Another thing to check would be whether in your case there are RPATHS embedded in the libraries, e.g. using something like I don't know what difference there could be that would plausibly result in these being set differently on one machine then on another while using the same build scripts. The least implausible that springs to mind is a change somewhere in macOS and its toolchain. ROS 2 used to support macOS 10.12, I am on (10.)15 and stuff definitely changes from time to time, like Apple's adding the locations it tried for finding the library to the error message. Indeed, that tells me that @wjwwood copy-pasted from an older version of macOS. |
Actually, this PR prepared everything to fix this issue but in the end did not set the cc: @mossmaurice, @dkroenke since you review the PR. |
@elfenpiff |
This issue is now outdated and no more complaints reached us, it seems things are working (also, the last remaining issue was on the iceoryx side). I'm going to close this, please open a new issue if any problems persist. |
When building with just
colcon build
on macOS I get:I believe this is because of a naive assumption that
libiceoryx_binding_c.dylib
is installed to the same place as all of its dependencies, likelibiceoryx_posh.dylib
, which is not the case in an isolated install.This failure is coming from trying to run
idlc
here:cyclonedds/cmake/Modules/Generate.cmake
Lines 135 to 139 in e52fda8
And I guess it is linked against iceoryx.
There is some logic to try and handle this for starting the daemon when testing, but it is similarly flawed as it only extends the
DYLD_LIBRARY_PATH
with the place thaticeoryx_binding_c
is installed, but not it's dependencies:cyclonedds/src/core/ddsc/tests/CMakeLists.txt
Lines 197 to 209 in e52fda8
I suppose it may fail there too.
Steps to reproduce:
mkdir -p cyclone_ws/src
cd cyclone_ws
cyclone.repos
file with these contents:vcs import ./src < cyclone.repos
colcon build
Note: You can work around this by using
colcon build --merge-install
, but that is not a satisfactory solution for my situation unfortunately.I guess this issue may be related, though it's hard to tell from the description which is pretty vague: #1198
The text was updated successfully, but these errors were encountered: