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

CFG_EXTRAS doesnt support directories #805

Closed
asmodehn opened this issue Jun 3, 2016 · 4 comments
Closed

CFG_EXTRAS doesnt support directories #805

asmodehn opened this issue Jun 3, 2016 · 4 comments

Comments

@asmodehn
Copy link
Contributor

@asmodehn asmodehn commented Jun 3, 2016

using:

catkin_package(
  CFG_EXTRAS
  directory/file.cmake
)

and using:

catkin_package(
  CFG_EXTRAS
  file.cmake
)

will both finally install the file.cmake in the same location.

The directory should not be removed from the path...

I had the issue with this version : pyros-dev/catkin_pip@78eaaae

@asmodehn

This comment has been minimized.

Copy link
Contributor Author

@asmodehn asmodehn commented Jun 3, 2016

This leads to package requiring these extra to not find them. In my case :

CMake Error at /opt/ros/indigo/share/catkin_pip/cmake/catkin_pipConfig.cmake:190 (include):

  include could not find load file:

    /opt/ros/indigo/share/catkin_pip/cmake/test/pytest.cmake
asmodehn added a commit to pyros-dev/catkin_pip that referenced this issue Jun 3, 2016
@dirk-thomas

This comment has been minimized.

Copy link
Member

@dirk-thomas dirk-thomas commented Jun 3, 2016

While keeping a relative path would be possible the passed filename can also be absolute and then it would be more difficult to still ensure that multiple files don't collide. Therefore catkin simplifies the process and uses just the basename of the file.

While I understand your use case I am not convinced that this feature is necessary. The benefit is minimal and the effort to implement this robustly for cases like absolute paths is significant. Maybe it is enough to clarify the documentation (

# :param CFG_EXTRAS: a CMake file containing extra stuff that should
# be accessible to users of this package after
# ``find_package``\ -ing it. This file must live in the
# subdirectory ``cmake`` or be an absolute path. Various additional
# file extension are possible:
# for a plain cmake file just ``.cmake``, for files expanded using
# CMake's ``configure_file()`` use ``.cmake.in`` or for files expanded
# by empy use ``.cmake.em``. The templates can distinguish between
# devel- and installspace using the boolean variables ``DEVELSPACE``
# and ``INSTALLSPACE``. For templated files it is also possible to
# use the extensions ``.cmake.develspace.(in|em)`` or
# ``.cmake.installspace.(em|in)`` to generate the files only for a
# specific case.
# If the global variable ${PROJECT_NAME}_CFG_EXTRAS is set it will be
# prepended to the explicitly passed argument.
# :type CFG_EXTRAS: string
)?

@asmodehn

This comment has been minimized.

Copy link
Contributor Author

@asmodehn asmodehn commented Jun 3, 2016

I agree it might not be a really needed feature, I did my implementation this way because it worked in devel space.

So I think what got me there, is the difference between the devel space and the install space...
From what I can recall, in the devel space the relative path is kept. But on install space (when building package) it is removed.

The most important would be to get devel and install space behaving consistently, to avoid surprises.

This issue might need more detailed confirmed behavior, I cant recall exactly all details with all changes I did recently...

dirk-thomas added a commit that referenced this issue Jun 3, 2016
dirk-thomas added a commit that referenced this issue Jun 3, 2016
@dirk-thomas

This comment has been minimized.

Copy link
Member

@dirk-thomas dirk-thomas commented Jun 3, 2016

devel and install space intentionally behave different. In the case where your extra file is not a template the file is not being copied to the devel space but used from the source space directly. In the install space case the file must be copied though.

I have updated the API documentation to mention that all basenames need to be unique:

@dirk-thomas dirk-thomas closed this Jun 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.