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

On Windows fix installation directory of .dll files in tf2_eigen_kdl #657

Merged
merged 1 commit into from Mar 11, 2024

Conversation

traversaro
Copy link
Contributor

For consistency with the rest of geometry2 :

On Windows it is important to install the .dll libraries (RUNTIME in CMake jargon) in <prefix>/bin, as otherwise the Windows dynamic loader is not able to find them.

Cross-ref downstream issue:

Signed-off-by: Silvio Traversaro <silvio@traversaro.it>
@traversaro traversaro changed the title Fix installation directory of .dll files in tf2_eigen_kdl On Windows fix installation directory of .dll files in tf2_eigen_kdl Mar 11, 2024
@traversaro
Copy link
Contributor Author

traversaro commented Mar 11, 2024

The CI error (from https://build.ros2.org/job/Rpr__geometry2__ubuntu_noble_amd64/2/console) is:

01:42:34 Traceback (most recent call last):
01:42:34   File "/tmp/ros_buildfarm/scripts/devel/create_devel_task_generator.py", line 21, in <module>
01:42:34     run_module('ros_buildfarm.scripts.devel.create_devel_task_generator', run_name='__main__')
01:42:34   File "<frozen runpy>", line 229, in run_module
01:42:34   File "<frozen runpy>", line 88, in _run_code
01:42:34   File "/tmp/ros_buildfarm/ros_buildfarm/scripts/devel/create_devel_task_generator.py", line 304, in <module>
01:42:34     sys.exit(main())
01:42:34              ^^^^^^
01:42:34   File "/tmp/ros_buildfarm/ros_buildfarm/scripts/devel/create_devel_task_generator.py", line 140, in main
01:42:34     get_binary_package_versions(apt_cache, debian_pkg_names))
01:42:34     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
01:42:34   File "/tmp/ros_buildfarm/ros_buildfarm/common.py", line 178, in get_binary_package_versions
01:42:34     assert len(prov) == 1
01:42:34 AssertionError

that seems unrelated to the PR content.

@traversaro
Copy link
Contributor Author

traversaro commented Mar 11, 2024

As a kind of OT comment, as soon as we can require CMake 3.14, all the DESTINATION boilerplate in install(TARGETS <...>) commands can be removed, as the default values match what we always set: https://cmake.org/cmake/help/latest/release/3.14.html#commands and https://cmake.org/cmake/help/v3.14/command/install.html#targets .

@ahcorde
Copy link
Contributor

ahcorde commented Mar 11, 2024

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

As a kind of OT comment, as soon as we can require CMake 3.14,

For Rolling, at least, we now require CMake 3.20 due to some (unrelated) changes. So we can do this if you want.

That said, it is a different effort, so I'm still going to merge this one as-is.

@clalancette clalancette merged commit 3c50932 into ros2:rolling Mar 11, 2024
1 of 2 checks passed
@traversaro
Copy link
Contributor Author

As a kind of OT comment, as soon as we can require CMake 3.14,

For Rolling, at least, we now require CMake 3.20 due to some (unrelated) changes. So we can do this if you want.

That said, it is a different effort, so I'm still going to merge this one as-is.

Sure, I just realized this while writing this PR, so I wanted to write it down somehow. Just bumping the cmake_minimum_required version can have unintended consequences, so for sure it is better to open a PR just for that.

@traversaro traversaro deleted the patch-1 branch March 11, 2024 21:53
CursedRock17 added a commit to CursedRock17/geometry2 that referenced this pull request Apr 4, 2024
CursedRock17 added a commit to CursedRock17/geometry2 that referenced this pull request Apr 19, 2024
CursedRock17 added a commit to CursedRock17/geometry2 that referenced this pull request Apr 19, 2024
…_kdl (ros2#657)""

This reverts commit 0ed5d79cecd0e3bb10ef08a6471241b9d47ae315.
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.

None yet

3 participants