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

Normalize package names with lowercase #460

Merged

Conversation

dsavineau
Copy link
Contributor

@dsavineau dsavineau commented Mar 29, 2023

When a python package is listed multiple time but with uppercase/lowercase changes then introspect sanitization won't handle it. In db9e74a, only the underscore/dash scenario has been solved. As a result, the assemble script will fail with a double requirement error.

ERROR: Double requirement given: Jinja2==3.0.3 (from -r /tmp/src/requirements.txt (line 40)) (already in jinja2>=2.8 (from -r /tmp/src/requirements.txt (line 12)), name='Jinja2')

While this isn't an issue for recent pip version, older versions with python 3.8 and 3.9 runtimes on RHEL 8 based distributions are affected by this.

This replaces the setuptools runtime usage by importlib.metadata which provides builtin method to normalize package names.

Unlike pkg_resources.safe_name, the dash character is changed to underscore.

Closes: #408

https://packaging.python.org/en/latest/specifications/name-normalization

@dsavineau dsavineau requested a review from a team as a code owner March 29, 2023 18:31
@github-actions github-actions bot added the needs_triage New item that needs to be triaged label Mar 29, 2023
Copy link
Member

@nitzmahone nitzmahone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need to decide what to do about the actual normalizing impl we're using- @Shrews / @Akasurde, any preference there?

ansible_builder/requirements.py Outdated Show resolved Hide resolved
@dsavineau dsavineau force-pushed the case_sensitive_double_requirement branch from c6262ae to 4d20d31 Compare March 29, 2023 19:24
When a python package is listed multiple time but with uppercase/lowercase
changes then introspect sanitization won't handle it.
In db9e74a, only the underscore/dash scenario has been solved.
As a result, the assemble script will fail with a double requirement error.

ERROR: Double requirement given: Jinja2==3.0.3 (from -r /tmp/src/requirements.txt (line 40))
(already in jinja2>=2.8 (from -r /tmp/src/requirements.txt (line 12)), name='Jinja2')

While this isn't an issue for recent pip version, older versions with python
3.8 and 3.9 runtimes on RHEL 8 based distributions are affected by this.

This replaces the setuptools runtime usage by importlib.metadata which
provides builtin method to normalize package names.

Unlike pkg_resources.safe_name, the dash character is changed to underscore.

Closes: ansible#408

https://packaging.python.org/en/latest/specifications/name-normalization

Signed-off-by: Dimitri Savineau <dsavinea@redhat.com>
@dsavineau dsavineau force-pushed the case_sensitive_double_requirement branch from 4d20d31 to 82285ac Compare April 6, 2023 20:30
@Akasurde Akasurde self-requested a review April 24, 2023 17:43
@nitzmahone nitzmahone merged commit 0c3aef8 into ansible:devel Apr 25, 2023
@dsavineau dsavineau deleted the case_sensitive_double_requirement branch April 25, 2023 21:09
@dsavineau
Copy link
Contributor Author

@nitzmahone @Akasurde Do you agree to backport this to 3.0 and 1.2 branches so current AAP 2.3 release and next AAP 2.4 will be fixed ?

@Akasurde Akasurde removed the needs_triage New item that needs to be triaged label Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERROR: Double requirement given: pyvmomi
3 participants