colcon / colcon-core

Command line tool to build sets of software packages
http://colcon.readthedocs.io
Apache License 2.0
99 stars 44 forks source link

Why can data_files destinations not be absolute for ament_python packages? #616

Open PatrickSowinski opened 5 months ago

PatrickSowinski commented 5 months ago

This is about building an ament_python package with setup.py.

When I try to install my config folder into an absolute path (in my case /data/config), I get an AssertionError from the build system, because the path is absolute.

This is coming from this line in the code: https://github.com/colcon/colcon-core/blob/master/colcon_core/task/python/__init__.py#L38

for data_file in data_files:
        if isinstance(data_file, tuple):
            assert len(data_file) == 2
            dest = data_file[0]
            assert not os.path.isabs(dest)  <------- HERE
            sources = data_file[1]
            assert isinstance(sources, list)
            for source in sources:
                assert not os.path.isabs(source), \
                    f"'data_files' must be relative, '{source}' is absolute"
                mapping[source] = os.path.join(dest, os.path.basename(source))
        else:
            assert not os.path.isabs(data_file)
            mapping[data_file] = os.path.basename(data_file)
    return mapping

Why should the destination path for an installed file not be absolute? With CMakeLists.txt, this is possible with something like:

install(DIRECTORY config
  DESTINATION /data/config
)

Maybe there is a good reason for enforcing non-absolute source paths, but for destinations?

mikepurvis commented 5 months ago

That seems like a reasonable restriction. A package should always be installing files relative to its colcon-assigned installspace location (ideally using GNUInstallDirs though that is still under discussion), not spraying them out into arbitrary absolute locations on the filesystem, where the user invoking colcon may not even necessarily have write permissions.

PatrickSowinski commented 5 months ago

We want to "export" the default config files to an absolute path that is mounted onto the Docker container running ROS2/colcon. This would allow us to distribute these default configs outside the container and later to mount modified versions of them without requiring a rebuild (for production images where the source is not available anymore). If we just install the configs to the install directory, then modifying them later requires mounting to the correct install location, which is messy. Also distributing them outside the Docker image would require some other export step.

PatrickSowinski commented 5 months ago

Maybe we can add an optional argument/flag that enables absolute install paths?

mikepurvis commented 5 months ago

A more conventional approach would be just to have a --data-path flag you pass it that specifies this global location at runtime. There's no need to bake it into the packaging machinery.