search

ros / catkin

A CMake-based build system that is used to build all packages in ROS.
http://wiki.ros.org/catkin
BSD 3-Clause "New" or "Revised" License
321 stars 280 forks source link

[noetic-devel][Windows] Fix some devel space is not captured in the context of catkin_make run_tests #1111

Closed seanyen closed 4 years ago

seanyen commented 4 years ago

The Problem

This is manifesting when running catkin_make run_tests and I saw some shared libraries are not found when rostest runs the ROS nodes in my workspace. It tuned out that <catkin_ws>\devel\bin (where the DLL lives) should be captured by catkin/python/catkin/environment_cache.py in setup_cached.bat and later pass to the processes running rostest, but it didn't.

So, as I trace upward, I found when generate_cached_setup.py gets invoked, at that moment devel space is empty but only contains the common files (env.* and setup.*) added by catkin. Therefore, as this particular logic gets exercised inside the call stack from generate_cached_setup.py, it will skip adding the bin and lib subfolder to the %PATH%, because those subfolders doesn't yet exist. As a result, the <catkin_ws>\devel\bin doesn't get added and passed to rostest later.

Why is it not a problem to Linux? On Linux, there is a concept or RPATH\RUNPATH ,which is backed inside the binaries, for the loader to know where to search the shared objects. On Windows, the shared library search-order system is different and by the convention we used in ROS, the search path needs to be added to %PATH%.

Proposal Solution

I believe there is a reason to keep this check, so I go another route to force the basic layout to be created ahead in the devel space, so later they will be added by _setup_util.py.

And please consider to back-port to kinetic-devel. Thanks!

dirk-thomas commented 4 years ago

This change would add each of the directories to the environment variables even if they aren't being populated. I don't think that is desired.

Did you run catkin_make run_tests as the first command without a normal build before?

seanyen commented 4 years ago

Did you run catkin_make run_tests as the first command without a normal build before?

I am running catkin_make first and then catkin_make run_tests.

The thing I observed is that the setup_cached.bat gets generated fairly earlier even before any files get copied to devel\bin when catkin_make is invoked. And the later catkin_make run_tests won't trigger setup_cached.bat to be regenerated neither.

Btw, removing the check added in https://github.com/ros/catkin/pull/777 helps fix this problem, but it is probably going to have some side-effects (roswtf tests?).

dirk-thomas commented 4 years ago

The goal is to only add paths to environment variable where necessary. So removing the previous check wouldn't be good since it add non-existing directories and this patch only creates the directories unconditionally (basically the check) and adding unnecessary paths in some cases.

I would prefer a solution which still maintain the original goal to only add paths to environment variable when necessary.

seanyen commented 4 years ago

Closing this pull request first before I (or anyone) figure out a better solution.