Skip to content

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

Merged
clalancette merged 1 commit intoros2:rollingfrom
traversaro:patch-1
Mar 11, 2024
Merged

On Windows fix installation directory of .dll files in tf2_eigen_kdl#657
clalancette merged 1 commit intoros2:rollingfrom
traversaro:patch-1

Conversation

@traversaro
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

ahcorde commented Mar 11, 2024

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

@clalancette
Copy link
Copy Markdown
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
@traversaro
Copy link
Copy Markdown
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.

3 participants