Skip to content

Prefer system packages over pip packages for Linux#593

Merged
christophebedard merged 1 commit intomasterfrom
christophebedard/apt-over-pip
Jul 18, 2023
Merged

Prefer system packages over pip packages for Linux#593
christophebedard merged 1 commit intomasterfrom
christophebedard/apt-over-pip

Conversation

@christophebedard
Copy link
Copy Markdown
Member

@christophebedard christophebedard commented Jul 9, 2023 •
edited
Loading

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 emersonknapp, gbiggs and jhdcs and removed request for a team July 9, 2023 22:11
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 9, 2023 •
edited
Loading

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
Copy Markdown
Member Author

christophebedard commented Jul 9, 2023 •
edited
Loading

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

@christophebedard christophebedard mentioned this pull request Jul 9, 2023
Merged
@christophebedard christophebedard force-pushed the christophebedard/apt-over-pip branch 2 times, most recently from 11e495a to 7375614 Compare July 13, 2023 19:56
@christophebedard
Prefer system packages over pip packages for Linux
d47b718 
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
@christophebedard christophebedard force-pushed the christophebedard/apt-over-pip branch from 7375614 to d47b718 Compare July 14, 2023 20:10
@christophebedard
Copy link
Copy Markdown
Member Author

christophebedard commented Jul 14, 2023 •
edited
Loading

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.

emersonknapp
emersonknapp approved these changes Jul 18, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@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
@christophebedard
Unpin mypy in GitHub CI
9538e0d 
No longer necessary after ros-tooling/setup-ros#593

Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
@christophebedard christophebedard mentioned this pull request Jul 18, 2023
Closed
@christophebedard christophebedard mentioned this pull request May 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emersonknapp emersonknapp emersonknapp approved these changes

@gbiggs gbiggs Awaiting requested review from gbiggs gbiggs is a code owner automatically assigned from ros-tooling/reviewers

@jhdcs jhdcs Awaiting requested review from jhdcs jhdcs is a code owner automatically assigned from ros-tooling/reviewers

Assignees

@christophebedard christophebedard

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Prefer system packages over pip packages

2 participants

@christophebedard @emersonknapp