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
318 stars 279 forks source link

Consider adding support for generator expression in pkgConfig.cmake.in #1154

Open FederAndInk opened 2 years ago

FederAndInk commented 2 years ago

urdfdom is providing libraries with generator expression in it, which is not supported by the pkgConfig.cmake.in template as it will call find_library on it: https://github.com/ros/urdf/issues/37

Maybe it could just add the library if it looks like a generator expression as suggested by https://github.com/ros/urdf/issues/37#issuecomment-939183824

https://github.com/ros/catkin/blob/085e8950cafa3eb979edff1646b9e3fe55a7053a/cmake/templates/pkgConfig.cmake.in#L122 https://github.com/ros/catkin/blob/085e8950cafa3eb979edff1646b9e3fe55a7053a/cmake/templates/pkgConfig.cmake.in#L171

j-rivero commented 2 years ago

Following the problem, I can dedicate a bit of time to patch the issue. All I need would be a reproducible instructions (or a Dockerfile) to get a source build failing with the problem described. Thanks!

FederAndInk commented 2 years ago

Here is a Dockerfile that demonstrate the issue, the last RUN won't work because it will cause the issue on configure:

FROM ubuntu

ENV TZ=Europe/Paris
RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

RUN apt update && apt install -y \
    build-essential \
    g++ \
    git \
    curl \
    vim \
    gnupg2 \
    cmake \
    lsb-release

# to simplify your life:
WORKDIR /root
RUN apt install -y zsh zsh-syntax-highlighting zsh-autosuggestions vim bat wget
RUN echo 'source /usr/share/zsh-autosuggestions/zsh-autosuggestions.zsh' >~/.zshrc.local && \
    echo 'source /usr/share/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh' >~/.zshrc.local
RUN wget -O ~/.zshrc https://git.grml.org/f/grml-etc-core/etc/zsh/zshrc
RUN git clone --depth 1 https://github.com/junegunn/fzf.git ~/.fzf && \
    yes | ~/.fzf/install

# ROS
RUN sh -c 'echo "deb http://packages.ros.org/ros/ubuntu $(lsb_release -sc) main" >/etc/apt/sources.list.d/ros-latest.list' && \
    curl -s https://raw.githubusercontent.com/ros/rosdistro/master/ros.asc | apt-key add - && \
    apt update && \
    apt install -y ros-noetic-catkin \
    ros-noetic-cmake-modules \
    ros-noetic-hardware-interface \
    ros-noetic-rostest \
    # just to get the dependencies:
    liburdfdom-dev \
    liburdfdom-sensor \
    ros-noetic-urdf && \
    apt remove -y ros-noetic-urdf

RUN git clone https://github.com/ros-controls/ros_control && \
    git clone https://github.com/ros/urdf && \
    git clone https://github.com/ros/urdfdom

SHELL [ "/bin/bash", "-c" ]

RUN cd urdfdom && \
    mkdir build && \
    cd build && \
    cmake .. -DCMAKE_INSTALL_PREFIX=/opt/ros/noetic && \
    make -j && \
    make install

RUN cd urdf/urdf_parser_plugin && \
    mkdir build && \
    cd build && \
    source /opt/ros/noetic/setup.bash && \
    cmake .. -DCATKIN_BUILD_BINARY_PACKAGE=ON -DCMAKE_INSTALL_PREFIX=/opt/ros/noetic && \
    make -j && \
    make install

RUN cd urdf/urdf && \
    mkdir build && \
    cd build && \
    source /opt/ros/noetic/setup.bash && \
    cmake .. -DCATKIN_BUILD_BINARY_PACKAGE=ON -DCMAKE_INSTALL_PREFIX=/opt/ros/noetic && \
    make -j && \
    make install

# or with zsh: source /opt/ros/noetic/setup.zsh

RUN cd ros_control/joint_limits_interface && \
    mkdir build && \
    cd build && \
    source /opt/ros/noetic/setup.bash && \
    cmake .. -DCATKIN_BUILD_BINARY_PACKAGE=ON -DCMAKE_INSTALL_PREFIX=/opt/ros/noetic && \
    make && \
    make install
ivanpauno commented 2 years ago

if("${library}" MATCHES "^(debug|optimized|general)$")

Just in case there's a confusion, that line is matching configuration keywords, which are not the same as generator expressions.

Maybe it could just add the library if it looks like a generator expression as suggested by ros/urdf#37 (comment)

I don't think that's going to work. e.g. the error reported there was caused by the line $<$<NOT:$<CONFIG:Debug>>:/usr/lib/liburdfdom_sensor.so>. In that case, you only want to try to find that library for non-debug builds.

The issue should be solved by making sure that PKG_CONFIG_LIBRARIES has a list of actual libraries, with all generator expressions already (somehow) resolved.

PKG_CONFIG_LIBRARIES is generated here. Once you get the generators expressions in the pkgConfig file, I don't think it's possible to do something reasonable with them (though there might be a way to fix the issue there).

xlla commented 2 years ago

can we add extra filter to extract pure library name from those regex expression in where that (https://github.com/ros/catkin/blob/085e8950cafa3eb979edff1646b9e3fe55a7053a/cmake/templates/pkgConfig.cmake.in#L119) generated ?

by the way, why urdf will cause something to generate that ugly expression, I have build hundred ros packages, only urdf's cmake file raise those errors.

-- check library: urdf
-- check library: $<$>:/usr/local/lib/liburdfdom_sensor.dylib>
CMake Error at /Users/xlla/ros_catkin_ws/install_isolated/share/urdf/cmake/urdfConfig.cmake:174 (message):
  Project 'gazebo_plugins' tried to find library
  '$<$>:/usr/local/lib/liburdfdom_sensor.dylib>'.  The
  library is neither a target nor built/installed properly.  Did you compile
  project 'urdf'? Did you find_package() it before the subdirectory
  containing its code is included?
Call Stack (most recent call first):
  /Users/xlla/ros_catkin_ws/install_isolated/share/catkin/cmake/catkinConfig.cmake:76 (find_package)
  CMakeLists.txt:7 (find_package)

original log

errors
ivanpauno commented 2 years ago

by the way, why urdf will cause something to generate that ugly expression, I have build hundred ros packages, only urdf's cmake file raise those errors.

Those are cmake generator expressions. See was it was added here https://github.com/ros2/urdfdom/pull/16. If you use modern cmake targets and they automatically generated export file, they also generate the same kind of generator expressions.

I'm rereading this thread and https://github.com/ros/urdf/issues/37, and I think the suggestion here is fine https://github.com/ros/urdf/issues/37#issuecomment-939183824. If a generator expression is detected, add the library to the list unconditionally. That should fix the issue.

can we add extra filter to extract pure library name from those regex expression in where that (

I'm not sure what you're suggesting. Is your suggestion the same as https://github.com/ros/urdf/issues/37#issuecomment-939183824? If yes, that sounds fine. If not, could you clarify a bit? Thanks!

xlla commented 2 years ago

I'm not sure what you're suggesting.

hi @ivanpauno , I have tried the solutions on https://github.com/ros/urdf/issues/37#issuecomment-939183824 , manual remove those regex expressions actually work but trivial and bring mistake sometimes.

AchmadFathoni's sed regex command just remove the prefix of lib regex expression and leave suffix ">" there.

the filter I mean is like this

filter

sorry, I don't know how to avoid html to render these special symbols so I have to post a picture to demonstrate it.

ivanpauno commented 2 years ago

I'm not sure what are you trying to do. You don't need to filter, but instead add all generator expressions unconditionally without checking.

i.e. you would need to replace this https://github.com/ros/catkin/blob/7fa3eb3508ba12c34d85b8e54bbdf4bbdc60a5c1/cmake/templates/pkgConfig.cmake.in#L122-L123 with:

 if("${library}" MATCHES "^\\$<.*>$") 
   list(APPEND @PROJECT_NAME@_LIBRARIES ${library})
 elseif("${library}" MATCHES "^(debug|optimized|general)$") 
   list(APPEND @PROJECT_NAME@_LIBRARIES ${library}) 

The regex expression I proposed my need to be adjusted. I don't have time to test it now, but I think something like that should fix the issue.

xlla commented 2 years ago

I just want to pass the build of the package that depend on urdfdom lib. Those cmake-generator-expressions does not got eval or parsed while pass to function find_library, and they even goes into ros install_isolated/lib/pkgconfig/urdf.pc , and cause many build problems.

ivanpauno commented 2 years ago

Those cmake-generator-expressions does not got eval or parsed while pass to function find_library

Yes, but it never should reach this find_library() line with the fix I mentioned.

and they even goes into ros install_isolated/lib/pkgconfig/urdf.pc , and cause many build problems.

That's bad. I'm not sure how that could be fixed. The thing is that generator expressions cannot be evaluated at configuration time, only at build time (because of things like https://cmake.org/cmake/help/latest/prop_gbl/GENERATOR_IS_MULTI_CONFIG.html). Maybe using https://cmake.org/cmake/help/v3.13/command/file.html#generate instead of will help https://github.com/ros/catkin/blob/7fa3eb3508ba12c34d85b8e54bbdf4bbdc60a5c1/cmake/em_expand.cmake#L5 But getting that right is going to be tricky.