Skip to content

install renamed scripts as symlinks if --symlink-install flag is set#673

Open
heuristicus wants to merge 3 commits intobdaiinstitute:mainfrom
ori-goals:fix-symlink-install
Open

install renamed scripts as symlinks if --symlink-install flag is set#673
heuristicus wants to merge 3 commits intobdaiinstitute:mainfrom
ori-goals:fix-symlink-install

Conversation

@heuristicus
Copy link
Contributor

Change Overview

Fixes #672, which could be confusing if users build spot_driver with the --symlink-install flag of colcon build.

Testing Done

Run colcon build --symlink-install --packages-select spot_driver and verify that spot_ros and spot_alerts are now symlinks. Behaviour without symlink flag should be the same.

@heuristicus
Copy link
Contributor Author

Seems that just changing to symlink may not be sufficient as launch complains about not being able to find an executable spot_ros2.

Signed-off-by: Michal Staniaszek <m.staniaszek@gmail.com>
Copy link
Collaborator

@mhidalgo-rai mhidalgo-rai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heuristicus would it be simpler to just rename the scripts? Executable permissions and a shebang line should do.

@heuristicus
Copy link
Contributor Author

That might be simpler but I'm not sure how it would affect module discoverability and other things if the .py extension is removed. Doing it in the cmake file seems preferable to me. I'm not very familiar with the recommended way of working with mixed python-C++ packages. This seems more or less a question of conventions.

Copy link
Collaborator

@tcappellari-bdai tcappellari-bdai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im ok with this change. Please rebase and I'll get it merged in

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Building with --symlink-install does not symlink renamed scripts

3 participants