Handle pip package versions like pip does [rev2]

[peci1]

Continuation of #901.

This would come handy e.g. with package m2r added in ros/rosdistro#35827 , where the newest released version on pip is python3-only, but it is also offered on python2 pip on bionic.

[danielcranston]

I came across ros/rosdistro#46946 (comment) which made me remember this issue. The linked comment talks about a con with adding custom rosdep entries:

Cons: still effectively limits you to "one version" as the rosdep pip keys don't (truly) support versioning. (but if you put the like ==1.2.3 in the package name for pip it will install that version... it just breaks rosdeps ability to tell that it's already installed and skip reinstallation when you run)

My understanding is that this PR addresses this exact thing, and gives rosdep the ability to "(truly) support versioning" of pip packages.

I made some commits on top of this branch a while back, mainly to resolve merge conflicts. I've been using it with custom rosdep keys targeting explicit pip package versions, and it's working well. https://github.com/danielcranston/rosdep/tree/handle-pip-module-versions. @peci1 mind having a look at the commits? If they seem good to you feel free to adopt them here.

@emersonknapp do you have any thoughts on this?

[peci1]

Thanks, I'll have a look. Not sure about the timeframe, though.

[codecov]

Codecov Report

❌ Patch coverage is 87.87879% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.09%. Comparing base (4cb03bb) to head (574076d).
⚠️ Report is 42 commits behind head on master.

Files with missing lines	Patch %	Lines
src/rosdep2/platforms/pip.py	87.87%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #909      +/-   ##
==========================================
- Coverage   74.22%   72.09%   -2.14%     
==========================================
  Files          44       44              
  Lines        3376     3519     +143     
  Branches        0      696     +696     
==========================================
+ Hits         2506     2537      +31     
+ Misses        870      802      -68     
- Partials        0      180     +180     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[peci1]

Thanks @danielcranston , I've integrated your changes.

[fujitatomoya]

@tfoote @sloretz @cottsay

either of you could take a look at this, related to ros2/ros2_documentation#6187

this enables rosdep to properly handle pip package version specifiers (like package==1.2.3 or package>=1.0). Currently, rosdep doesn't "truly" support pip versioning. if you specify a version in the package name, rosdep can't detect that the versioned package is already installed and will try to reinstall it every time.

Note: this PR came up in ROS APAC Developer Meeting

[cottsay]

Taking a broader view of our package management ecosystem, the main reason I hesitate to support version data in the rosdep database is that we already have version information on dependencies as part of package.xml files. Each declared dependency may specify version requirements for that specific dependency.

It's worth calling out that rosdep (as a tool) doesn't do anything interesting with this version information right now. When the rosdep database is used by Bloom, it translates the version requirements to the system package metadata format so the values are used, just not here in rosdep. So debian/rpm rules already have and use version metadata coming from the package manifests and not the rosdep database, and I see this as precedent.

IMO, the only reasons that rosdep (the tool) doesn't use the manifest's version constraints today stem from:

1. Nobody has been motivated enough to champion it. Maybe that has now changed.
2. Ambiguity on how to properly handle a situation where a package is present but not at a compatible version. This is really just about establishing a policy or playbook for how we expect the underlying package managers to act.
3. Ambiguity on how to properly handle a situation where one operation calls for an impossible combination of version numbers.
4. Inability to make the underlying package managers do what we want. For example, I'm not sure it's possible to ask apt to perform an installation operation with a version range or even an explicit version that doesn't also contain the debian-specific parts of the version.

[peci1]

I understand your concerns and I agree that enforcing versions on APT is almost impossible (because most public repos like Debian/Ubuntu only keep the last version).

But package (non-)managers like pip are a completely different story. They usually keep all versions, so it is easy to ask for any version you like. Please note the initial motivation for this change, which was a pip package whose latest version on pypi stopped supporting Python 2 but some older version did support it.

Without being able to specify version constraints, this package could just be thrown away for Bionic because the latest version is just not compatible. Or it could be left in rosdep, but that would be even worse because it would lead to impossible install instructions for pip (or to broken package, I don't remember which one).

Given the fact that pip packages can be updated any time, it can quite easily happen that some normally working rosdep key will stop working for some platform at any time (e.g. require a too new Python version).

This is why I think pip packages should have the possibility of having version constraints in rosdep, too. The ones from package.xml are not used for anything for Python/pip packages.

Maybe the solution could be to add a parameter to the installers that would tell whether version constraints are supported? Or maybe even directly a version constraint validation function (which could just return False on APT).

[danielcranston]

Thanks for reviving this conversation!

[...] I hesitate to support version data in the [official] rosdep database [because ...]

I agree with those reasons, and for clarity I wouldn't equate this PR to any implication that we should start versioning the pip packages in the official rosdep database.

This PR just extends rosdep (the tool) to support versioned pip packages, where the only place you'd probably ever see them (for now, or maybe forever) would be in custom defined rosdep rules.

Note

Just to clarify the value of being able to do this: it unlocks the possibility

• To only rely on rosdep to install the dependencies of your project [which requires very specific pip package versions]
  • (no conda/venvs/etc to force/pin those pip versions)
• To specify all your package dependencies in your package.xml files
  • (no requirements.txt, no manual pip install invocations alongside rosdep install)

At the same time I understand this is a bit off the beaten path of regular rosdep usage, and it's somewhat in contention with the package.xml spec since this adds a separate way to specify version (beyond the package.xml spec way. But again, rosdep currently is not using package.xml-declared version requirements and doing so requires answering @cottsay's bullet points above).

---

@cottsay (and other maintainers) do you mind sharing your thoughts on whether the "value" explained in the Note above is enough to warrant moving forward with this PR as-is? I'm expecting "no" 😄 (which is fine, I just want to confirm that you see the value proposition)

As for tackling your bullet points, I'm definitely interested in helping out, though I think there's some things I'd need to brush up on.

[cottsay]

...do you mind sharing your thoughts on whether the "value" explained in the Note above is enough to warrant moving forward with this PR as-is?

For clarity, I'm still opposed to including any version information in the official rosdep database, but I've been sufficiently convinced that this change provides value.

[cottsay]

The output of these commands is very similar. What advantage is there to using pip list over pip freeze?

Does this change the minimum version of pip required?

[peci1]

It works on pip 20.0.2 from Ubuntu 20.04/Python 3.8. Do we need to support even older?

[cottsay]

I'm not sure that returning just the names is the right move here. For example, if pkgs contains mutually exclusive specifiers (for example, empy<4 and empy>=4), the function will simply return ['empy'] and rosdep will report success, even though only one of the specifiers was actually detected.

I'm not sure what the "right" behavior is here, but I don't think that rosdep should indicate success.

[peci1]

What if this function returned a dict instead where each pkg would either have a value saying which pip package satisfies it, or None, which would mean it is not satisfied?

Or, maybe a better and simpler solution: what if we checked pkgs for conflicts explicitly? If there is a conflict like in your example, I think rosdep should fail anyways, shouldn't it?

[peci1]

@cottsay I've responded to your inline questions.

[peci1]

It works on pip 20.0.2 from Ubuntu 20.04/Python 3.8. Do we need to support even older?

[peci1]

What if this function returned a dict instead where each pkg would either have a value saying which pip package satisfies it, or None, which would mean it is not satisfied?

Or, maybe a better and simpler solution: what if we checked pkgs for conflicts explicitly? If there is a conflict like in your example, I think rosdep should fail anyways, shouldn't it?