-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
pkg_resources: Correctly handle normalized project names #4855
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much @di for having a look on this.
I was working on a regression test.
Would you consider adding the following to the test_pkg_resources
?
import inspect
# ...
class TestWorkdirRequire:
def fake_site_packages(self, tmp_path, monkeypatch, dist_files):
site_packages = tmp_path / "site-packages"
site_packages.mkdir()
for file, content in self.FILES.items():
path = site_packages / file
path.parent.mkdir(exist_ok=True, parents=True)
path.write_text(inspect.cleandoc(content), encoding="utf-8")
monkeypatch.setattr(sys, "path", [site_packages])
return os.fspath(site_packages)
FILES = {
"pkg1_mod-1.2.3.dist-info/METADATA": """
Metadata-Version: 2.4
Name: pkg1.mod
Version: 1.2.3
""",
"pkg2.mod-0.42.dist-info/METADATA": """
Metadata-Version: 2.1
Name: pkg2.mod
Version: 0.42
""",
"pkg3_mod.egg-info/PKG-INFO": """
Name: pkg3.mod
Version: 1.2.3
""",
"pkg4.mod.egg-info/PKG-INFO": """
Name: pkg4.mod
Version: 0.42
""",
}
@pytest.mark.parametrize(
("name", "version", "req"),
[
("pkg1.mod", "1.2.3", "pkg1.mod>=1"),
("pkg2.mod", "0.42", "pkg2.mod>=0.4"),
("pkg3.mod", "1.2.3", "pkg3.mod<=2"),
("pkg4.mod", "0.42", "pkg4.mod>0.2,<1"),
],
)
def test_require_normalised_name(self, tmp_path, monkeypatch, name, version, req):
# https://github.com/pypa/setuptools/issues/4853
site_packages = self.fake_site_packages(tmp_path, monkeypatch, self.FILES)
ws = pkg_resources.WorkingSet([site_packages])
[dist] = ws.require(req)
assert dist.version == version
assert dist.project_name == name
assert os.path.commonpath([dist.location, site_packages]) == site_packages
@krpatter-intc, would you be able to test if this PR solves your problem? I believe that can use If it does not work, could you then please have a look on #4856 ( |
Yep, I can confirm this fixes
Thanks for the quick turnaround! EDIT: For clarity, this was done using: https://github.com/di/setuptools/archive/refs/heads/fix/4853.zip |
Thank you very much @krpatter-intc. My initial idea was to go with this PR, but there are some tests failing, so we might go instead with #4856 (I think it will also solve your problem). |
Summary of changes
This fixes an issue where
pkg_resources.require
treats project names with periods differently than other project names.Before:
After:
The simplest fix would be a one-character update to
safe_name
, which currently does not normalize periods when producing a "safe" project name:setuptools/pkg_resources/__init__.py
Lines 1551 to 1556 in 56c055b
However, since this is a public API, changes here might be more disruptive, so I opted to change
pkg_resources
usage ofsafe_name
internally instead.Closes #4853.
Pull Request Checklist
newsfragments/
.(See documentation for details)