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

Error checking if package with dotted name is installed (pip==20.2) #8645

Closed
damb opened this issue Jul 29, 2020 · 13 comments · Fixed by #8659
Closed

Error checking if package with dotted name is installed (pip==20.2) #8645

damb opened this issue Jul 29, 2020 · 13 comments · Fixed by #8659
Labels
type: bug A confirmed bug or unintended behavior

Comments

@damb
Copy link

damb commented Jul 29, 2020

Hi,

I've got an issue while installing namespace distribution dependencies. While it worked fine with pip==20.1.1 resolving fails with pip==20.2.

  • Python: 3.8.2
  • Pip: 20.2
  • Platform:
$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 20.04 LTS
Release:	20.04
Codename:	focal

My namespace distributions follow PEP420. However, I use a slightly different naming scheme than proposed in https://github.com/pypa/sample-namespace-packages/tree/master/native.

To reproduce the issue the following steps need to be executed:

  1. Clone https://github.com/pypa/sample-namespace-packages
$ git clone https://github.com/pypa/sample-namespace-packages.git && cd sample-namespace-packages
  1. Apply the following diff:
$ git diff
diff --git a/native/pkg_a/setup.py b/native/pkg_a/setup.py
index 8021a7a..1238c0c 100644
--- a/native/pkg_a/setup.py
+++ b/native/pkg_a/setup.py
@@ -16,7 +16,7 @@ from setuptools import setup
 
 
 setup(
-    name='example_pkg_a',
+    name='example_pkg.a',
 
     version='1',
 
diff --git a/native/pkg_b/setup.py b/native/pkg_b/setup.py
index 872122a..1830dcc 100644
--- a/native/pkg_b/setup.py
+++ b/native/pkg_b/setup.py
@@ -16,7 +16,7 @@ from setuptools import setup
 
 
 setup(
-    name='example_pkg_b',
+    name='example_pkg.b',
 
     version='1',
 
@@ -27,6 +27,7 @@ setup(
     author_email='[email protected]',
 
     license='Apache Software License',
+    install_requires=['example_pkg.a==1'],
  1. Check pip version:
$ pip --version
pip 20.2 from /home/ubuntu/work/projects/sample-namespace-packages/venv/lib/python3.8/site-packages/pip (python 3.8)
  1. Install pkg_a:
$ pip install native/pkg_a/
Processing ./native/pkg_a
Using legacy 'setup.py install' for example-pkg.a, since package 'wheel' is not installed.
Installing collected packages: example-pkg.a
    Running setup.py install for example-pkg.a ... done
Successfully installed example-pkg.a-1
$ pip freeze
example-pkg.a==1
pkg-resources==0.0.0
  1. Try to install pkg_b which should fail:
$ pip install native/pkg_b/
Processing ./native/pkg_b
ERROR: Could not find a version that satisfies the requirement example_pkg.a==1 (from example-pkg.b==1) (from versions: none)
ERROR: No matching distribution found for example_pkg.a==1 (from example-pkg.b==1)

Though, when naming the namespace distributions as described in https://github.com/pypa/sample-namespace-packages/tree/master/native i.e. with a _ instead of a ., the distributions install correctly.

It took me a while until I noted that the issue is the naming convention, I didn't follow thoroughly.

Question: Is the change from pip==20.1.1 to pip==20.2 on purpose?

If you need any further details, let me know.

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Jul 29, 2020
@uranusjr
Copy link
Member

This seems like a normalisation issue. Dot, underscore, and dash are actually treated as equivalents when comparing package names, so pip is probably not doing that comparison correctly somewhere.

@damb
Copy link
Author

damb commented Jul 29, 2020

You refer to the snippet below?

_canonicalize_regex = re.compile(r"[-_.]+")
def canonicalize_name(name):
# type: (str) -> NormalizedName
# This is taken from PEP 503.
value = _canonicalize_regex.sub("-", name).lower()
return cast("NormalizedName", value)

@sbidoul
Copy link
Member

sbidoul commented Jul 29, 2020

@damb that canonicalize_name should be fine. It's the way pip uses it that has a bug since 20.2.

@sbidoul
Copy link
Member

sbidoul commented Jul 29, 2020

Here is a reproducer for a variant of what is probably the same bug:

#!/bin/bash
set -ex
cd $(mktemp -d)
python3 -m venv venv
. venv/bin/activate
pip install -U "pip>=20.2" wheel py3o.formats
echo "from setuptools import setup; setup(name='myproject', install_requires=['py3o.formats'])" > setup.py
pip install --no-index -e .

@sbidoul sbidoul added the type: bug A confirmed bug or unintended behavior label Jul 29, 2020
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Jul 29, 2020
@sbidoul
Copy link
Member

sbidoul commented Jul 29, 2020

So the bug is in check_if_exists and has been introduced in #8054.

Apparently, passing a canonical name with dots replaced by dashes is not compatible with pkg_resources.get_distribution()

Example:

$ pip install zope.inferface
$ python -c "import pkg_resources as pr; print(pr.get_distribution('zope.interface'))"
zope.interface 5.1.0
$ python -c "import pkg_resources as pr; print(pr.get_distribution('zope-interface'))"
...
pkg_resources.DistributionNotFound: The 'zope-interface' distribution was not found and is required by the application

@uranusjr
Copy link
Member

You beat me to it 😛 Relevent code:

def safe_name(name):
"""Convert an arbitrary string to a standard distribution name
Any runs of non-alphanumeric/. characters are replaced with a single '-'.
"""
return re.sub('[^A-Za-z0-9.]+', '-', name)

We’ll probably need to ask setuptools maintainers whether this is an oversight or a delibrate decision, and whether we can patch this logic ourselves.

@uranusjr
Copy link
Member

Or alternatively, we can rollback #8054 (the root issue it tries to resolve is exactly pkg_resources does not handle dot in names well), and migrate wholesale to importlib.metadata.

@sbidoul sbidoul changed the title Namespace package dependency cannot be resolved (pip==20.2) Error checking if package with dotted name is installed (pip==20.2) Jul 30, 2020
@pradyunsg
Copy link
Member

Looks like we'd need a bugfix release.

Or alternatively, we can rollback #8054

I like this plan. Anyone wanna file a PR for this?

@sbidoul
Copy link
Member

sbidoul commented Jul 30, 2020

I wanted to verify why check_if_exists still needs to call pkg_resources.get_distribution while we have our own misc.get_distribution which seems to do the right thing. I will probably not have time for that until the week-end or early next week though.

@uranusjr
Copy link
Member

Oh, good point! IIRC that was planned as a follow-up PR (since the refactoring would be difficult to review if committed together with the get_distribution() change), but it never happened. I’ll try rewriting that and see if that’s enough to get this issue resolved.

@deveshks
Copy link
Contributor

deveshks commented Jul 30, 2020

I had filed an issue to remove the last remaining usage of pkg_resources.get_distribution (#8550) as a follow-up of #8054, but I was waiting on #8114 to get merged first before proceeding on it further.

@uranusjr
Copy link
Member

I experiemented a bit, and it seems like switching from calling pkg_resources.get_distribution() to pip’s own would indeed solve this particular issue.

Since this is a quite significant issue (there are many popular packages preferring to be referred by a dot in the name, e.g. coverage.py), I’m going to do the one required replacement in a separate PR, so we can get a bugfix version out as soon as possible. #8114 and the other replacements can happen afterwards.

@uranusjr
Copy link
Member

I opened #8659 to fix check_if_exists.

bors bot referenced this issue in duckinator/emanate Aug 5, 2020
158: Update pip to 20.2.1 r=duckinator a=pyup-bot


This PR updates [pip](https://pypi.org/project/pip) from **20.2** to **20.2.1**.



<details>
  <summary>Changelog</summary>
  
  
   ### 20.2.1
   ```
   ===================

Features
--------

- Ignore require-virtualenv in ``pip list`` (`8603 &lt;https://github.com/pypa/pip/issues/8603&gt;`_)

Bug Fixes
---------

- Correctly find already-installed distributions with dot (``.``) in the name
  and uninstall them when needed. (`8645 &lt;https://github.com/pypa/pip/issues/8645&gt;`_)
- Trace a better error message on installation failure due to invalid ``.data``
  files in wheels. (`8654 &lt;https://github.com/pypa/pip/issues/8654&gt;`_)
- Fix SVN version detection for alternative SVN distributions. (`8665 &lt;https://github.com/pypa/pip/issues/8665&gt;`_)
- New resolver: Correctly include the base package when specified with extras
  in ``--no-deps`` mode. (`8677 &lt;https://github.com/pypa/pip/issues/8677&gt;`_)
- Use UTF-8 to handle ZIP archive entries on Python 2 according to PEP 427, so
  non-ASCII paths can be resolved as expected. (`8684 &lt;https://github.com/pypa/pip/issues/8684&gt;`_)

Improved Documentation
----------------------

- Add details on old resolver deprecation and removal to migration documentation. (`8371 &lt;https://github.com/pypa/pip/issues/8371&gt;`_)
- Fix feature flag name in docs. (`8660 &lt;https://github.com/pypa/pip/issues/8660&gt;`_)
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pip
  - Changelog: https://pyup.io/changelogs/pip/
  - Homepage: https://pip.pypa.io/
</details>



Co-authored-by: pyup-bot <[email protected]>
damb pushed a commit to EIDA/eidaws that referenced this issue Nov 16, 2020
This commit partly reverts ccfa788
which took into account pypa/pip#8645.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants