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

Remove interface install from CMakeLists.txt #7

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

timonegk
Copy link
Contributor

It is not required and leads to problems with symlink install (see ament/ament_cmake#412).

It is not required and leads to problems with symlink install (see ament/ament_cmake#412).
@alsora
Copy link
Collaborator

alsora commented Dec 9, 2022

Thank you!
I wonder though if a more correct fix would be to do

ament_export_targets(foo-export)
install(TARGETS foo EXPORT foo-export)

and remove the part

install(DIRECTORY include/
  DESTINATION include/${PROJECT_NAME}
)

@timonegk
Copy link
Contributor Author

timonegk commented Dec 9, 2022

I just tested your suggestion, but it does not seem to install the contents of the include directory at all. From what I can see in the cmake documentation, manually installing the include directory is still required.

@nightduck
Copy link

Fixes #9

@alsora
Copy link
Collaborator

alsora commented Dec 13, 2022

Thank you for testing this; I want to do a couple extra tests before merging it, but it should be ready soon

@timonegk
Copy link
Contributor Author

@alsora any progress on the tests?

timonegk added a commit to bit-bots/bitbots_main that referenced this pull request Feb 10, 2023
timonegk added a commit to bit-bots/bitbots_main that referenced this pull request Feb 10, 2023
@Flova
Copy link

Flova commented Feb 11, 2023

Any updates on this?

I did not notice any issues with the pr until now.

@Flova
Copy link

Flova commented Apr 26, 2023

Ping @alsora

@alsora alsora merged commit 1cd0be4 into irobot-ros:main Apr 26, 2023
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.

4 participants