tesseract-robotics / tesseract_ext

This contains external dependencies for Tesseract
1 stars 11 forks source link

Registration of Taskflow in CMake package registry breaks workspace isolation #18

Open mpowelson opened 4 years ago

mpowelson commented 4 years ago

When I build test_ext with this directory structure it fails.

When I change it to this it works

Here is the error

Errors     << taskflow:cmake /home/matthew/workspaces/test_ext/logs/taskflow/build.cmake.000.log                                  
CMake Error at /home/matthew/workspaces/tesseract/build/tesseract_ext/Taskflow-build/TaskflowConfig.cmake:11 (message):
  File or directory /home/matthew/workspaces/tesseract/include referenced by
  variable Taskflow_INCLUDE_DIR does not exist !
Call Stack (most recent call first):
  /home/matthew/workspaces/tesseract/build/tesseract_ext/Taskflow-build/TaskflowConfig.cmake:27 (set_and_check)
  CMakeLists.txt:4 (find_package)
mpowelson commented 4 years ago

I imagine this is something wrong with my setup, but I thought it sufficiently bizarre that I would post it in case anyone else runs into it.

gavanderhoorn commented 4 years ago

Ah, so this is something we've also run into (@IKapitaniuk).

This actually is not caused by names of workspaces, but by the CMake Package registry.

@IKapitaniuk: could you perhaps add what you found out?

gavanderhoorn commented 4 years ago

@mpowelson: I hope you don't mind I updated the title of the issue.

gavanderhoorn commented 4 years ago

@IKapitaniuk: could you try disabling searching the registry for Taskflow using the variables mentioned here: Disabling the Package Registry?

It might be sufficient to set(..)-ing CMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY here:

https://github.com/ros-industrial-consortium/tesseract_ext/blob/21e196e7aad724e0d3efcc26d97f99b172f59287/taskflow/CMakeLists.txt#L3-L4

before the find_package(..).

@Levi-Armstrong: I'm not sure the package registry would be compatible with the isolation a Catkin/Colcon workspace provides/requires. Thoughts?


Edit: you could apparently also add NO_CMAKE_PACKAGE_REGISTRY to the find_package(..) call itself.


Edit 2: apparently this line registers Taskflow in the user package registry (docs).

None of the other projects in tesseract_ext do this. At least Eigen also does this and orocos_kdl also.

Levi-Armstrong commented 4 years ago

I know this does not resolve this issue but we have released taskflow as a debian.

https://launchpad.net/~ros-industrial/+archive/ubuntu/ppa

Levi-Armstrong commented 4 years ago

@gavanderhoorn I agree this should be disabled. I was not aware of this CMake feature until recently, but I believe this is something catkin/colcon should be disabling?

Other references found with similar issues.

  1. https://github.com/conan-io/conan/issues/3070
  2. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=868584
gavanderhoorn commented 4 years ago

Colcon has helpers for specific build systems, so this could be something to suggest adding as a default. The helper for CMake is colcon/colcon-cmake. We could open an issue there to discuss.

For Catkin, this could potentially be added, but I'm not sure that will happen.

I was going to suggest opening issues on ros/catkin and catkin/catkin_tools, but it seems you already did that/are doing that.

Levi-Armstrong commented 4 years ago

I opened one on catkin. I will create one on colcon too.

gavanderhoorn commented 4 years ago

Disabling it blanketly could make it harder for users which would like CMake to find a package they're building from source to get it to notice it.

But then again: those users should probably not be building such a package in a Catkin/Colcon workspace (as that implies they'd like to exploit the isolation it offers).

Levi-Armstrong commented 4 years ago

It is unclear to me how this is leveraged within cmake. When you perform a find_package does it first search the user package registry, then system registry, the look for a Config.cmake? Or does it only look in the registry if it is unable to find the Config.cmake?

gavanderhoorn commented 4 years ago

The list of locations searched is documented in the find_package(..) docs. I can't link to it directly, but if you search for "The set of installation prefixes is constructed using the following steps" you should find it.

Registries are searched almost last (only locations passed using PATHS are later).

Given this, you'd expect CMake to be able to find Taskflow in the workspace before looking in the registry ..

Is there perhaps something wrong with the way Taskflow is installed into the workspace? I haven't checked what options you pass the external project macro.

Levi-Armstrong commented 4 years ago

So we did find they were installing it into a incorrect location which was fixed in this PR. I assumed since this was created after that fix that this was not the issue, but not certain.

Levi-Armstrong commented 4 years ago

@mpowelson When you experienced this were you using a branch that had the install location fix for taskflow?

IKapitaniuk commented 4 years ago

It's a bit late, but here are some particular details regarding the issue.

The CMake package registry is located in ~/.cmake ; particularly we got the following structure

➜  ~ tree .cmake 
.cmake
└── packages
    └── Taskflow
        └── 2be66d93a89e1968194e1931a8750d05

where

➜  ~ cat ~/.cmake/packages/Taskflow/2be66d93a89e1968194e1931a8750d05 
/home/yuri/tesseract_ws2/build/tesseract_ext/Taskflow-build

In a normal situation, CMake searches in the following places:

Warnings   << taskflow:cmake /home/yuri/tesseract_ws2/logs/taskflow/build.cmake.000.log    
Checking prefix [/home/yuri/tesseract_ws2/devel/]
Checking file [/home/yuri/tesseract_ws2/devel/TaskflowConfig.cmake]
Checking file [/home/yuri/tesseract_ws2/devel/taskflow-config.cmake]
Checking prefix [/opt/ros/noetic/]
Checking file [/opt/ros/noetic/TaskflowConfig.cmake]
Checking file [/opt/ros/noetic/taskflow-config.cmake]
Checking prefix [/home/yuri/.local/]
Checking file [/home/yuri/.local/TaskflowConfig.cmake]
Checking file [/home/yuri/.local/taskflow-config.cmake]
Checking prefix [/usr/local/]
Checking file [/usr/local/TaskflowConfig.cmake]
Checking file [/usr/local/taskflow-config.cmake]
Checking prefix [/usr/]
Checking file [/usr/TaskflowConfig.cmake]
Checking file [/usr/taskflow-config.cmake]
Checking prefix [/]
Checking file [/TaskflowConfig.cmake]
Checking file [/taskflow-config.cmake]
Checking prefix [/usr/games/]
Checking file [/usr/games/TaskflowConfig.cmake]
Checking file [/usr/games/taskflow-config.cmake]
Checking prefix [/usr/local/games/]
Checking file [/usr/local/games/TaskflowConfig.cmake]
Checking file [/usr/local/games/taskflow-config.cmake]
Checking prefix [/snap/]
Checking file [/snap/TaskflowConfig.cmake]
Checking file [/snap/taskflow-config.cmake]
Checking prefix [/usr/X11R6/]
Checking prefix [/usr/pkg/]
Checking prefix [/opt/]
Checking file [/opt/TaskflowConfig.cmake]
Checking file [/opt/taskflow-config.cmake]

When we run from another workspace we got this:

Errors     << taskflow:cmake /home/yuri/tesseract_ws1/logs/taskflow/build.cmake.000.log
Checking prefix [/home/yuri/tesseract_ws1/devel/]
Checking file [/home/yuri/tesseract_ws1/devel/TaskflowConfig.cmake]
Checking file [/home/yuri/tesseract_ws1/devel/taskflow-config.cmake]
Checking prefix [/opt/ros/noetic/]
Checking file [/opt/ros/noetic/TaskflowConfig.cmake]
Checking file [/opt/ros/noetic/taskflow-config.cmake]
Checking prefix [/home/yuri/.local/]
Checking file [/home/yuri/.local/TaskflowConfig.cmake]
Checking file [/home/yuri/.local/taskflow-config.cmake]
Checking prefix [/usr/local/]
Checking file [/usr/local/TaskflowConfig.cmake]
Checking file [/usr/local/taskflow-config.cmake]
Checking prefix [/usr/]
Checking file [/usr/TaskflowConfig.cmake]
Checking file [/usr/taskflow-config.cmake]
Checking prefix [/]
Checking file [/TaskflowConfig.cmake]
Checking file [/taskflow-config.cmake]
Checking prefix [/usr/games/]
Checking file [/usr/games/TaskflowConfig.cmake]
Checking file [/usr/games/taskflow-config.cmake]
Checking prefix [/usr/local/games/]
Checking file [/usr/local/games/TaskflowConfig.cmake]
Checking file [/usr/local/games/taskflow-config.cmake]
Checking prefix [/snap/]
Checking file [/snap/TaskflowConfig.cmake]
Checking file [/snap/taskflow-config.cmake]
Checking prefix [/home/yuri/tesseract_ws2/build/tesseract_ext/Taskflow-build/]
Checking file [/home/yuri/tesseract_ws2/build/tesseract_ext/Taskflow-build/TaskflowConfig.cmake]
CMake Error at /home/yuri/tesseract_ws2/build/tesseract_ext/Taskflow-build/TaskflowConfig.cmake:11 (message):
  File or directory /home/yuri/tesseract_ws2/include referenced by variable
  Taskflow_INCLUDE_DIR does not exist !
Call Stack (most recent call first):
  /home/yuri/tesseract_ws2/build/tesseract_ext/Taskflow-build/TaskflowConfig.cmake:27 (set_and_check)
  CMakeLists.txt:4 (find_package)

@gavanderhoorn I've just tested it using set(CMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY 1) - It works, I've managed to build Taskflow.

mpowelson commented 4 years ago

@mpowelson When you experienced this were you using a branch that had the install location fix for taskflow?

I should have been. I posted this on the day that I had it, so that fix should have been in master.

mpowelson commented 4 years ago

@Levi-Armstrong We ran into this again. Should we just add set(CMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY 1) to the cmake? That seems to fix it.

Levi-Armstrong commented 4 years ago

I believe catkin should be passing these when building. Are you asking if we should add these to every package?

Levi-Armstrong commented 4 years ago

If so, do they only need to be added to the packages in tesseract_ext?

mpowelson commented 4 years ago

Taskflow seems to be the only one causing problems.

Levi-Armstrong commented 4 years ago

I created PR #20 so it does not export to the registry.