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.

Sign up for GitHub

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

Jump to bottom

Exporting LD_LIBRARY_PATH for interfaces #132

Closed
stonier opened this issue May 8, 2018 · 5 comments
Closed

Exporting LD_LIBRARY_PATH for interfaces #132

stonier opened this issue May 8, 2018 · 5 comments
Assignees
@dirk-thomas
Labels
bug Something isn't working

Comments

@stonier
Copy link

stonier commented May 8, 2018 •
edited

@clalancette noticed that if building in isolated mode, the LD_LIBRARY_PATH does not get set when only using ament_export_interfaces(...) and not ament_export_libraries(...).

See stonier/ecl_core#73 for context and discussion.

Seems redundant to have to additionally set the export_libraries, is this intended?
@dirk-thomas
Copy link
Contributor

If a package only exports dependencies but doesn't have any libraries why would it need to set the LD_LIBRARY_PATH? The assumption is that the dependency has defined the LD_LIBRARY_PATH itself.

@clalancette
Copy link
Contributor

I think there was a slight typo in @stonier initial comment. We were interested in whether LD_LIBRARY_PATH should get set when ament_export_interfaces(...) is called. We're not sure how ament_export_interfaces(...) and ament_export_libraries(...) are supposed to interact (if at all).

@stonier
Copy link
Author

stonier commented May 9, 2018 •
edited

Yes, indeed. Thanks @clalancette. Intended ament_export_interfaces and fixed appropriately in the description above.

@dirk-thomas dirk-thomas mentioned this issue May 9, 2018
Merged
@dirk-thomas dirk-thomas self-assigned this May 9, 2018
@dirk-thomas dirk-thomas added the in progress Actively being worked on (Kanban column) label May 9, 2018
@dirk-thomas
Copy link
Contributor

The problem should be fixed in #135. @stonier It would be great if you could confirm that it addresses your use case.

@dirk-thomas dirk-thomas added in review Waiting for review (Kanban column) bug Something isn't working in progress Actively being worked on (Kanban column) and removed in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) labels May 9, 2018
@dirk-thomas
Copy link
Contributor

@stonier I updated the patch in the referenced PR. You will have to change your code to actually pass the export name instead of the target as well as pass the new option added in the referenced PR. Please let me know if that works for your use case.

@dirk-thomas dirk-thomas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels May 10, 2018
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label May 18, 2018
@stonier stonier mentioned this issue Sep 16, 2018
Closed
stonier added a commit to stonier/ecl_core that referenced this issue Sep 27, 2018
@stonier
[infra] rever the workaround for interface library exports
663e768 
Hitherto would cause a problem on builds with isolated installs, so
you are supposed to explicitly state the target name, refer to

   ament/ament_cmake#132

Colcon/bouncy don't face any issues though. How to reproduce the error?
stonier added a commit to stonier/ecl_core that referenced this issue Sep 27, 2018
@stonier
[infra] revert workaround for exporting interfaces with libraries
0c18c9c 
As brought in, in PR 73

   #73

Reported in

    ament/ament_cmake#132

And fixed in

    https://github.com/ament/ament_cmake/pull/135/files
stonier added a commit to stonier/ecl_lite that referenced this issue Sep 27, 2018
@stonier
[infra] revert workaround for exporting interfaces with libraries
170d3ba 
As brought in, in PR 73

   stonier/ecl_core#73

Reported in

    ament/ament_cmake#132

And fixed in

    https://github.com/ament/ament_cmake/pull/135/files
@stonier stonier mentioned this issue Sep 27, 2018
Merged
stonier added a commit to stonier/ecl_core that referenced this issue Sep 27, 2018
@stonier
[infra] revert workaround for exporting interfaces with libraries (#81)
73d5f1d 
* [infra] rever the workaround (PR #73) for interface library exports

Hitherto would cause a problem on builds with isolated installs, so
you are supposed to explicitly state the target name, refer to

- ament/ament_cmake#132
- https://github.com/ament/ament_cmake/pull/135/files

Colcon/bouncy doesn't reproduce our earlier problems since it exports all of the appropriate paths regardless. This however, should be the right fix.
stonier added a commit to stonier/ecl_lite that referenced this issue Sep 27, 2018
@stonier
[infra] revert workaround for exporting interfaces with libraries (#24)
96391a1 
* [infra] rever the workaround (PR #73) for interface library exports

Hitherto would cause a problem on builds with isolated installs, so
you are supposed to explicitly state the target name, refer to

- ament/ament_cmake#132
- https://github.com/ament/ament_cmake/pull/135/files

Colcon/bouncy doesn't reproduce our earlier problems since it exports all of the appropriate paths regardless. This however, should be the right fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dirk-thomas dirk-thomas

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@clalancette @stonier @dirk-thomas