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 #535

Closed
christophebedard opened this issue Jan 26, 2023 · 6 comments · Fixed by #593
Closed

Prefer system packages over pip packages #535

christophebedard opened this issue Jan 26, 2023 · 6 comments · Fixed by #593
Assignees
Labels
enhancement New feature or request

Comments

@christophebedard
Copy link
Member

christophebedard commented Jan 26, 2023

Description

The goal is to more closely match the packages installed:

Related Issues

Otherwise, when using setup-ros, we might get a newer version of a package from pip. This can lead to issues like ros2/ros2_tracing#27, where tests fail with setup-ros but not with ci.ros2.org

Completion Criteria

** What needs to be true before we can call this "Done"? Bullet lists are appropriate. **

Implementation Notes / Suggestions

Compare with ci.ros2.org, e.g.: https://ci.ros2.org/job/ci_linux/17993/consoleFull#console-section-4

Testing Notes / Suggestions

Nothing new or different to test.

@Flova
Copy link

Flova commented Feb 11, 2023

I currently face the same issue. I require python3-transforms3d via rosdep. This is compatible to python3-numpy, but the CI upgrades NumPy to a version where np.float is removed and replaced with float. This makes it incompatible with the python3-transforms3d version in the system packages. Locally everything works well, but the CI does not pass.

@christophebedard
Copy link
Member Author

I'll take care of this once the bump to Node.js v16 is done (#521).

@christophebedard
Copy link
Member Author

christophebedard commented Feb 18, 2023

I've looked into this more and have a proposal.

If you follow the docs for a source build, you:

  1. Install a fairly limited number of packages needed for development
    1. The Rolling instructions here for 22.04 only use apt, but the Rolling instructions for 20.04 use pip for some dependencies, possibly to get versions that are newer and closer/equal to the ones from apt on 22.04. Foxy on 20.04 also uses pip, possibly just to get newer versions.
  2. Clone the ROS 2 repos
  3. Then install dependencies using rosdep, which prefers apt

The list of packages we currently install using pip is much longer than the list of packages installed during step 1:

const pip3Packages: string[] = [
"argcomplete",
"colcon-bash==0.4.2",
"colcon-cd==0.1.1",
"colcon-cmake==0.2.27",
"colcon-common-extensions==0.3.0",
"colcon-core==0.11.0",
"colcon-coveragepy-result==0.0.8",
"colcon-defaults==0.2.7",
"colcon-lcov-result==0.5.0",
"colcon-library-path==0.2.1",
"colcon-meson==0.4.2",
"colcon-metadata==0.2.5",
"colcon-mixin==0.2.2",
"colcon-notification==0.2.15",
"colcon-output==0.2.12",
"colcon-package-information==0.3.3",
"colcon-package-selection==0.2.10",
"colcon-parallel-executor==0.2.4",
"colcon-pkg-config==0.1.0",
"colcon-powershell==0.3.7",
"colcon-python-setup-py==0.2.7",
"colcon-recursive-crawl==0.2.1",
"colcon-ros==0.3.23",
"colcon-test-result==0.3.8",
"coverage",
"cryptography",
"empy",
"flake8<3.8",
"flake8-blind-except",
"flake8-builtins",
"flake8-class-newline",
"flake8-comprehensions",
"flake8-deprecated",
"flake8-docstrings",
"flake8-import-order",
"flake8-quotes",
"ifcfg",
"importlib-metadata==2.*",
"importlib-resources",
"lark-parser",
"mock",
"mypy",
"nose",
"numpy",
"pep8",
"pydocstyle",
"pyopenssl",
"pyparsing",
"pytest",
"pytest-cov",
"pytest-mock",
"pytest-repeat",
"pytest-rerunfailures",
"pytest-runner",
"setuptools<60.0",
"wheel",
];
This is probably because Windows and macOS use pip instead of apt (although they use choco and brew for some dependencies), so doing (mostly) the same steps for all platforms is easier. However, as we can see from the issues mentioned above, it doesn't work well.

On the other hand, ci.ros2.org's equivalent to step 1 (before the repos are even cloned) installs more packages, like python3-mypy, although this is most likely done for common dependencies to speed up builds with Docker layer caching.

I'd go with copying the official source build instructions for Ubuntu and leave Windows and macOS as-is. That means Ubuntu installs would have distro-specific apt and pip packages lists and wouldn't use the "universal" pip list. I wouldn't bother installing additional common dependencies like ci.ros2.org does, because there's no real benefit since we're not using Docker here.

The only risk is if this breaks existing configs/code, e.g., if someone changed their code to appease setup-ros's newer version of mypy, their CI might break if setup-ros switches to using an older mypy version. However, one could argue that it's already sort of broken. I would at least bump the minor version, of course. I implemented this for Jammy (https://github.com/ros-tooling/setup-ros/tree/christophebedard/ubuntu-prefer-system-packages) and it works as expected: ros2/ros2_tracing#8.

@emersonknapp @ijnek @Flova any thoughts?

@emersonknapp
Copy link
Contributor

I think the state as it is currently was based on an older version of the setup instructions. But it may have been a copy of the ci.ros2.org approach instead. I have no problem changing things to match the current source build instructions.

@ijnek
Copy link
Contributor

ijnek commented Feb 23, 2023

@christophebedard Agree with your suggestions.

e.g., if someone changed their code to appease setup-ros's newer version of mypy, their CI might break if setup-ros switches to using an older mypy version.

Personally, I wouldn't push changes that don't work locally so I think if this happens, it would only affect very few users, if any (assuming users aren't installing their dependencies through pip too).

christophebedard added a commit to ros2/ros2_tracing that referenced this issue Mar 24, 2023
See ros-tooling/setup-ros#535

Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
christophebedard added a commit to ros2/ros2_tracing that referenced this issue Mar 29, 2023
See ros-tooling/setup-ros#535

Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
christophebedard added a commit to ros2/ros2_tracing that referenced this issue Mar 29, 2023
See ros-tooling/setup-ros#535

Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
@christophebedard
Copy link
Member Author

FYI @ijnek @Flova this was implemented in #593. It was released as v0.7/0.7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants