Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Add dependency on system poco. #18

Merged
merged 2 commits into from
Jun 19, 2018
Merged

Conversation

nuclearsandwich
Copy link
Member

If there is a suitable system version of poco, prefer to use it rather
than building poco locally. For bouncy the necessary version of poco is
available from the bionic repositories.

Necessary to bloom poco_vendor without patching.

If there is a suitable system version of poco, prefer to use it rather
than building poco locally. For bouncy the necessary version of poco is
available from the bionic repositories.
@nuclearsandwich nuclearsandwich self-assigned this Jun 19, 2018
@dirk-thomas dirk-thomas added the in progress Actively being worked on (Kanban column) label Jun 19, 2018
@nuclearsandwich nuclearsandwich added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jun 19, 2018
package.xml Outdated
@@ -13,6 +13,7 @@

<buildtool_export_depend>cmake</buildtool_export_depend>

<depend>libpoco-dev</depend>
<depend>pcre</depend>
<depend>zlib</depend>
Copy link
Member Author

@nuclearsandwich nuclearsandwich Jun 19, 2018

Choose a reason for hiding this comment

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

pcre and zlib are pulled in by the upstream poco package so these are only needed when building poco with this package.

My understanding is that they aren't build_depends because this package may build poco at runtime, that is, a downstream developer's build time if it is unavailable.

I think in order to build poco you'll also need libexpat1-dev and libsqlite3-dev. I can either add all build dependencies or remove them all.

@nuclearsandwich
Copy link
Member Author

Thanks for your review @mjcarroll I realized just after when checking this package.xml against the one from ardent that some additional dependencies are needed in the case that poco upstream is not available.

We're relying on libpoco-dev upstream for this package's release on
bionic. The removed dependencies were requirements to build poco but
this package should not need to do that in the released configuration.

The poco build dependencies have been added to the xenial setup
instructions on the ROS 2 wiki instead.
@nuclearsandwich
Copy link
Member Author

@mjcarroll @dirk-thomas ready for a second round of review with the poco build dependencies removed.

Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

lgtm

@nuclearsandwich nuclearsandwich merged commit 1b08a24 into master Jun 19, 2018
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Jun 19, 2018
@nuclearsandwich nuclearsandwich deleted the depend-on-system-poco branch June 19, 2018 21:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants