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

[noetic] Restrict boost dependencies to components used #1871

Merged
merged 7 commits into from Mar 3, 2020

Conversation

@mikaelarguedas
Copy link
Member

mikaelarguedas commented Jan 26, 2020

To be targeted at the noetic-devel branch

depends on ros/rosdistro#23620 and ros/rosdistro#23622

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
@gavanderhoorn

This comment has been minimized.

Copy link
Contributor

gavanderhoorn commented Jan 26, 2020

Should this target a (to be created) noetic-devel?

<depend>boost</depend>
<depend>libboost-date-time-dev</depend>
<depend>libboost-filesystem-dev</depend>
<depend>libboost-program-options-dev</depend>
<depend>libboost-regex-dev</depend>
<depend>libboost-thread-dev</depend>
Comment on lines -23 to +27

This comment has been minimized.

Copy link
@gavanderhoorn

gavanderhoorn Jan 26, 2020

Contributor

Could we split this into exec_depend and the various build*_depend?

Did you not do this on purpose?

This comment has been minimized.

Copy link
@mikaelarguedas

mikaelarguedas Jan 26, 2020

Author Member

Purposeful 😬 , currently I just replaced the keys without changing the scope of the tags.

I figured that when splitting the depends tags, we should decide which to put in build_depend vs exec_depend vs build_export_depend which would require a closer audit of each package rather than just duplicating them all in build and exec depends.

It could be done in this PR though, it was just out of scope of my first pass

@mikaelarguedas

This comment has been minimized.

Copy link
Member Author

mikaelarguedas commented Jan 26, 2020

Should this target a (to be created) noetic-devel?

Indeed, I put it in the PR description but I should have put it in the title for more visibility, editing now

@mikaelarguedas mikaelarguedas changed the title Restrict boost dependencies to components used [noetic-devel] Restrict boost dependencies to components used Jan 26, 2020
@mikaelarguedas mikaelarguedas changed the title [noetic-devel] Restrict boost dependencies to components used [noetic] Restrict boost dependencies to components used Jan 26, 2020
@dirk-thomas

This comment has been minimized.

Copy link
Member

dirk-thomas commented Mar 2, 2020

@mikaelarguedas Please retarget the noetic-devel branch.

@mikaelarguedas mikaelarguedas changed the base branch from melodic-devel to noetic-devel Mar 2, 2020
@mikaelarguedas

This comment has been minimized.

Copy link
Member Author

mikaelarguedas commented Mar 2, 2020

Done,

Note: CI will still install all boost packages because some PRs on underlying packages are pending ros/rospack#115

@@ -37,6 +40,9 @@

<run_depend version_gte="0.3.17">cpp_common</run_depend>
<run_depend>message_runtime</run_depend>
<run_depend>libboost-chrono-dev</run_depend>
<run_depend>libboost-filesystem-dev</run_depend>
<run_depend>libboost-system-dev</run_depend>

This comment has been minimized.

Copy link
@mikepurvis

mikepurvis Mar 2, 2020

Member

Moving to format 2 would permit deduplicating these.

This comment has been minimized.

Copy link
@mikaelarguedas

mikaelarguedas Mar 3, 2020

Author Member

Agreed moving to format 2 for all packages would be a nice improvement.

Regarding deduplication: in the context of Generating ‘dev’ and runtime artefacts from ROS packages we're aiming for not using the <depend> tag to be able to generate pure runtime packages without bringing in all the dev dependencies.

@dirk-thomas

This comment has been minimized.

Copy link
Member

dirk-thomas commented Mar 3, 2020

CI will still install all boost packages because some PRs on underlying packages are pending ros/rospack#115

With rospack recently released and rebuilt with that change I will trigger another round of CI for this:

@ros-pull-request-builder retest this please

@dirk-thomas

This comment has been minimized.

Copy link
Member

dirk-thomas commented Mar 3, 2020

Thanks for the patch.

@dirk-thomas dirk-thomas merged commit 0b6c2c5 into ros:noetic-devel Mar 3, 2020
3 of 4 checks passed
3 of 4 checks passed
Mpr_ds__ros_comm__debian_stretch_amd64 Build finished.
Details
Mpr__ros_comm__ubuntu_bionic_amd64 Build finished.
Details
Npr__ros_comm__ubuntu_focal_amd64 Build finished.
Details
Npr_db__ros_comm__debian_buster_amd64 Build finished.
Details
@mikaelarguedas mikaelarguedas deleted the mikaelarguedas:restrict-boost-deps branch Mar 3, 2020
@mikaelarguedas

This comment has been minimized.

Copy link
Member Author

mikaelarguedas commented Mar 3, 2020

With rospack recently released and rebuilt with that change I will trigger another round of CI for this:

Note: The resulting debs are still pulling all the boost packages because this repo depends on pluginlib and ros/pluginlib#171 hasn't been merged and released.

@ros-discourse

This comment has been minimized.

Copy link

ros-discourse commented Mar 16, 2020

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/rfc-restricting-the-size-of-ros-docker-images/13252/1

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.