Skip to content

Fix arm64 build #303

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

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Fix arm64 build #303

merged 1 commit into from
Aug 30, 2024

Conversation

Rayman
Copy link
Contributor

@Rayman Rayman commented Aug 30, 2024

The cmake module was located in another folder.

The cmake module was located in another folder.
@Timple
Copy link
Contributor

Timple commented Aug 30, 2024

That actually does make a lot of sense!

I was under the (wrong) impression that openvdb wasn't apt installable there...

@SteveMacenski SteveMacenski merged commit 5c57eed into SteveMacenski:ros2 Aug 30, 2024
SteveMacenski pushed a commit that referenced this pull request Aug 30, 2024
The cmake module was located in another folder.
@SteveMacenski
Copy link
Owner

I'm backporting and releasing jazzy again

Copy link

@tfoote tfoote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be good to do conditionally or with a variable substitution so as not to pollute the path with unused elements.

@Rayman Rayman deleted the fix/arm64 branch September 2, 2024 07:05
@Rayman
Copy link
Contributor Author

Rayman commented Sep 2, 2024

@tfoote can you give me some pointers on which conditions this should be enabled.

Alternatively, what about searching for OpenVDBConfig.cmake in all paths under /usr and adding the parent folder?

@tfoote
Copy link

tfoote commented Sep 2, 2024

In this case you should only be appending the path for the current architecture. Right now you're appending the path for both architectures, which probably will be ok, but could give some very weird errors if someone happens to be doing some multi-arch builds and it picks up the wrong architecture from their system.

I don't have the exact commands but GNUInstallDirs is commonly used for picking the installation targets like this: https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html#result-variables so I presume it should be able to help set the search path.

Otherwise you could simply check the arch in your CMake manually and then extend the CMAKE_MODULE_PATH conditionally.

@SteveMacenski
Copy link
Owner

Good idea.

Otherwise you could simply check the arch in your CMake manually and then extend the CMAKE_MODULE_PATH conditionally.

Do you have an example of best practices for this? I see ways of doing it with CMAKE_SYSTEM_PROCESSOR which seems a little fragile since its OS dependent, 32/64 bit dependent, and even Linux flavor dependent. If that's SOP, I'll do that, but otherwise I'd love to know if there's a better solution that some ROS core packages currently leverage

@Rayman
Copy link
Contributor Author

Rayman commented Sep 4, 2024

In the cmake these variables are available that could be of use:

-- CMAKE_CXX_LIBRARY_ARCHITECTURE=x86_64-linux-gnu
-- CMAKE_C_IMPLICIT_LINK_DIRECTORIES=/usr/lib/gcc/x86_64-linux-gnu/13;/usr/lib/x86_64-linux-gnu;/usr/lib;/lib/x86_64-linux-gnu;/lib
-- CMAKE_C_LIBRARY_ARCHITECTURE=x86_64-linux-gnu
-- CMAKE_LIBRARY_ARCHITECTURE=x86_64-linux-gnu

A solution could be to append /usr/lib/${CMAKE_LIBRARY_ARCHITECTURE}/cmake/OpenVDB?

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

Successfully merging this pull request may close these issues.

4 participants