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

catkin plugin: use build-packages for compilers #2545

Merged

Conversation

kyrofa
Copy link
Contributor

@kyrofa kyrofa commented Apr 30, 2019

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit?

Currently, the Catkin plugin pulls compilers off to the side rather than requesting that they be installed on the host. The reason for this is historical at this point: before bases existed, one could build a ROS snap on any version of Ubuntu, even versions unsupported by the ROS distribution being used. In an effort to ensure this worked as well as possible, the Catkin plugin did as much as it could without utilizing build-packages to ensure all components were compatible.

With the introduction of bases (and the fact that the only ROS1 releases available correspond to LTS releases, and thus bases), this is no longer a concern. It's no longer possible to build a ROS snap on an unsupported Ubuntu version in a supported manner. Add to that the fact that there's a bug in the plugin's use of the compilers that prevents rebuilds from working properly, and it's time to get rid of them.

Rather than pulling compilers off to the side, this PR fixes LP: #1827148 by simply specifying them as required build-packages.

@kyrofa kyrofa force-pushed the bugfix/1827148/catkin_remove_compilers branch 3 times, most recently from 8fba20f to 28320ef Compare April 30, 2019 23:35
@kyrofa
Copy link
Contributor Author

kyrofa commented May 1, 2019

(#2548 is needed before tests will pass)

Currently, the Catkin plugin pulls compilers off to the side rather than
requesting that they be installed on the host. The reason for this is
historical at this point: before bases existed, one could build a ROS
snap on any version of Ubuntu, even versions unsupported by the ROS
distribution being used. In an effort to ensure this worked as well as
possible, the Catkin plugin did as much as it could without utilizing
`build-packages` to ensure all components were compatible.

With the introduction of bases (and the fact that the only ROS1 releases
available correspond to LTS releases, and thus bases), this is no longer
a concern. It's no longer possible to build a ROS snap on an unsupported
Ubuntu version in a supported manner. Add to that the fact that there's
a bug in the plugin's use of the compilers that prevents rebuilds from
working properly, and it's time to get rid of them.

Rather than pulling compilers off to the side, simply specify them as
required `build-packages`.

LP: #1827148

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
@kyrofa kyrofa force-pushed the bugfix/1827148/catkin_remove_compilers branch from 28320ef to df4a1a2 Compare May 1, 2019 21:53
@codecov-io
Copy link

codecov-io commented May 1, 2019

Codecov Report

Merging #2545 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2545      +/-   ##
=========================================
- Coverage   89.08%     89%   -0.09%     
=========================================
  Files         201     201              
  Lines       13657   13611      -46     
  Branches     2067    2067              
=========================================
- Hits        12166   12114      -52     
- Misses       1051    1057       +6     
  Partials      440     440
Impacted Files Coverage Δ
snapcraft/plugins/catkin.py 91.94% <100%> (-2.22%) ⬇️
snapcraft/plugins/catkin_tools.py 87.5% <100%> (-0.74%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50351d3...df4a1a2. Read the comment docs.

@sergiusens sergiusens merged commit ddb2d26 into canonical:master May 2, 2019
@kyrofa kyrofa deleted the bugfix/1827148/catkin_remove_compilers branch May 3, 2019 04:36
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.

3 participants