pip 20.3+ compatibility: use wheelhouse versions on install #193
Conversation
pip 20.3.4 and higher fail to install the packages specified in the wheelhouse if versions are not used directly. This patch uses the 'package==version' method of specifying a versio for a package when installing the packages from the wheelhouse (which ought to be all of them).
|
I've tested this with the aodh and pacemaker-remote charms (built on xenial) on trusty, xenial, bionic, focal, groovy and hirsute. |
lib/charms/layer/basic.py
Outdated
| return _s[:-len(ending)] | ||
| return _s | ||
|
|
||
| def _maybe_make_version(k): |
There was a problem hiding this comment.
I don't understand why this is called "maybe"? Shouldn't it be called "add_version_remove_extension" or "enforce_with_version_without_extension" or something like that?
There was a problem hiding this comment.
It's called _maybe* as it might make the key k into a versioned package==version. However, I could probably do with better names.
lib/charms/layer/basic.py
Outdated
| return _s[:-len(ending)] | ||
| return _s | ||
|
|
||
| def _maybe_make_version(k): |
There was a problem hiding this comment.
This is a bit hard to read on the overall, because of the one-char var names and the missing docstrings+types. I know docstrings aren't mandatory here but they would really help me and my weak brain.
So I think k is a package name that may or may not have an extension and may or may not have a version number, correct?
There was a problem hiding this comment.
Yes, sometimes a function can be a bit opaque. The function would be better named as _maybe_add_version_to_pkg as that is what it is doing (i.e. if the pkg has a version in the versions{} then it will add it, otherwise it will leave it alone.
lib/charms/layer/basic.py
Outdated
| @@ -278,6 +284,29 @@ def _load_wheelhouse_versions(): | |||
| return versions | |||
|
|
|||
|
|
|||
| def _add_back_versions(pkgs, _versions): | |||
There was a problem hiding this comment.
I think it would help to describe (e.g. with a docstring) that _versions is supposed to be a dictionary where the keys are elements of pkgs
There was a problem hiding this comment.
I'm a bit confused; the docstring is immediately below? Do you mean, list out the params and types in the docstring (which isn't currently the library's style)?
| # <package_name>==<version>. | ||
| # This ensures that pip 20.3.4+ will install the packages from the | ||
| # wheelhouse without (erroneously) flagging an error. | ||
| pkgs = _add_back_versions(_pkgs, _versions) |
There was a problem hiding this comment.
So apparently if I understand correctly this is not only adding the version back but also removing extensions?
There was a problem hiding this comment.
Yes; you can't ask pip install ==[.tar.gz|.zip] as it will throw an error. Unfortunately, LooseVersion() wrapping the version also includes the extension and so it needs to be removed.
Thanks for the review; I'll walk through the changes against your comments and then try to update the code to make more sense. |
Fixes pip install wheelhouse failure on Hirsute. canonical/layer-basic#193 Also pin ops!=1.2.0 due to that version of the ops (operator) library not working with xenial [1]. [1] canonical/operator#517 Change-Id: Ia8194247d5d553e72e694b4e2c649bd418d2adf4
* Update charm-vault from branch 'master'
to c4f0c880a3d11a860fb5f408e0ce1150e4426bdb
- Merge "Rebuild for picking up layer-basic#193"
- Rebuild for picking up layer-basic#193
Fixes pip install wheelhouse failure on Hirsute.
canonical/layer-basic#193
Also pin ops!=1.2.0 due to that version of the ops (operator)
library not working with xenial [1].
[1] canonical/operator#517
Change-Id: Ia8194247d5d553e72e694b4e2c649bd418d2adf4
pip 20.3.4 and higher fail to install the packages specified in the
wheelhouse if versions are not used directly. This patch uses the
'package==version' method of specifying a versio for a package when
installing the packages from the wheelhouse (which ought to be all of
them).