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

Prefer system packages over pip packages for Linux #593

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

christophebedard
Copy link
Member

@christophebedard christophebedard commented Jul 9, 2023

Closes #535

This updates the list of installed packages on Linux to match the installation instructions, which are all pretty much the same for Humble, Iron, and Rolling, which are all currently on Ubuntu 22.04:

Note that we used to install a lot of packages using pip. Some of those are included in ros-dev-tools with apt, and others were simply changed to be installed with apt instead of pip (like python3-colcon-*), but some were left out. While we could keep some of them, we can also just let rosdep install them if they're really needed (by action-ros-ci).

Everything stays the same for non-Linux platforms and Focal/non-Jammy.

Finally, I did some cleanup and removed old workarounds/comments that don't really apply anymore.

Once this is merged, I'll bump the minor version (v0.6 -> v0.7).

@christophebedard christophebedard requested a review from a team as a code owner July 9, 2023 22:11
@christophebedard christophebedard self-assigned this Jul 9, 2023
@christophebedard christophebedard requested review from gbiggs, jhdcs and emersonknapp and removed request for a team July 9, 2023 22:11
@codecov
Copy link

codecov bot commented Jul 9, 2023

Codecov Report

Patch coverage: 89.65% and project coverage change: -0.71 ⚠️

Comparison is base (e8569de) 93.18% compared to head (d975c7b) 92.47%.

❗ Current head d975c7b differs from pull request most recent head d47b718. Consider uploading reports for the commit d47b718 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #593      +/-   ##
==========================================
- Coverage   93.18%   92.47%   -0.71%     
==========================================
  Files           8        8              
  Lines         176      186      +10     
  Branches       17       22       +5     
==========================================
+ Hits          164      172       +8     
- Misses         12       14       +2     
Impacted Files Coverage Δ
src/package_manager/pip.ts 94.44% <85.71%> (-5.56%) ⬇️
src/setup-ros-linux.ts 96.15% <90.47%> (-1.58%) ⬇️
src/package_manager/apt.ts 93.75% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@christophebedard
Copy link
Member Author

christophebedard commented Jul 9, 2023

I'll bump the minor version when releasing this, but it probably needs to be tested manually with real repos first.

@christophebedard christophebedard force-pushed the christophebedard/apt-over-pip branch 2 times, most recently from 11e495a to 7375614 Compare July 13, 2023 19:56
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
@christophebedard
Copy link
Member Author

christophebedard commented Jul 14, 2023

I'll bump the minor version when releasing this, but it probably needs to be tested manually with real repos first.

Seems to work fine. In the above PR, I removed the workaround (using pip to pin mypy to the version available from apt) and there are no mypy test failures, as expected.

Copy link
Contributor

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

LGTM

@christophebedard christophebedard merged commit 0827768 into master Jul 18, 2023
@christophebedard christophebedard deleted the christophebedard/apt-over-pip branch July 18, 2023 19:38
christophebedard added a commit to ros2/ros2_tracing that referenced this pull request Jul 18, 2023
No longer necessary after ros-tooling/setup-ros#593

Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
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.

Prefer system packages over pip packages
2 participants