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

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

Open
PatrickSowinski opened this issue Feb 15, 2024 · 4 comments

Comments

@PatrickSowinski
Copy link

PatrickSowinski commented Feb 15, 2024

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
Copy link
Contributor

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
Copy link
Author

PatrickSowinski commented Feb 16, 2024

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
Copy link
Author

PatrickSowinski commented Feb 16, 2024

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

@mikepurvis
Copy link
Contributor

mikepurvis commented Feb 16, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants