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

Issue with filename for wheel with epoch version #6466

Closed
3 tasks done
wenkokke opened this issue Sep 9, 2022 · 10 comments
Closed
3 tasks done

Issue with filename for wheel with epoch version #6466

wenkokke opened this issue Sep 9, 2022 · 10 comments
Labels
kind/bug Something isn't working as expected

Comments

@wenkokke
Copy link

wenkokke commented Sep 9, 2022

  • I am on the latest Poetry version.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).
  • OS version and name: macOS 11
  • Poetry version: 1.2.0
  • Link of a Gist with the contents of your pyproject.toml file: n/a

Issue

When using a version number such as "1!1.0.0", the wheels generated by poetry have a filename with "1_1.0.0". This causes pip install to crash with the following error:
"ERROR: Could not build wheels for *, which is required to install pyproject.toml-based projects".

I believe that the following regular expression is at fault as it replaces the ! with _:

@property
def wheel_filename(self): # type: () -> str
return "{}-{}-{}.whl".format(
re.sub(r"[^\w\d.]+", "_", self._package.pretty_name, flags=re.UNICODE),
re.sub(r"[^\w\d.]+", "_", self._meta.version, flags=re.UNICODE),
self.tag,
)

@wenkokke wenkokke added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Sep 9, 2022
@dimbleby
Copy link
Contributor

dimbleby commented Sep 9, 2022

That seems to be what the spec says should be done: https://peps.python.org/pep-0427/#escaping-and-unicode

Each component of the filename is escaped by replacing runs of non-alphanumeric characters with an underscore

(I think you are confusing hyphens and underscores in your report, I don't think hyphens should be introduced)

@wenkokke
Copy link
Author

wenkokke commented Sep 9, 2022

Yeah, it's an underscore, that was a typo.

Unfortunately, that does break installing with the latest pip. Is there any way to make that work?

@dimbleby
Copy link
Contributor

dimbleby commented Sep 9, 2022

I dunno, I'm curious about whether PEP-427 is really the final word here. If it is then I guess you should report this to pip.

However I could believe that there's more to be said about this and that poetry might not be right.

Would be interesting to know what other tooling does when building a wheel with a version that contains an epoch, or to see any published examples on pypi of such a wheel...

@wenkokke
Copy link
Author

wenkokke commented Sep 9, 2022

I've also submitted this to pip, hopefully we can figure out which implementation is correct.

@wenkokke
Copy link
Author

wenkokke commented Sep 11, 2022

See pypa/pip#11443 (comment).

The rule you referenced from PEP 427 is outdated. The most up-to-date and canonical specification of wheel file names (among other things) can be found on packaging.python.org:

Version numbers should be normalised according to PEP 440. Normalised version numbers cannot contain -.

A wheel builder should retain the ! character, and Poetry’s current implementation is incorrect.

@dimbleby
Copy link
Contributor

Then I expect an MR fixing this would be welcome

I see you've linked above to some truly ancient code, the fix should now be over in poetry-core, something like this

@radoering
Copy link
Member

Version numbers should be normalised according to PEP 440. Normalised version numbers cannot contain -.

Sounds like we should just do return str(Version.parse(version)) in escape_version().

@dimbleby
Copy link
Contributor

I'm now pretty sure that the version being passed to escape_version() is already normalized and escape_version() is simply counterproductive and wants removing altogether.

neersighted pushed a commit that referenced this issue Sep 17, 2022
…#6476)

When you already have a `Version` in hand,
`normalize_version(version.text)` is a very roundabout way of calling
`version.to_string()`: it re-parses the version text to give you the
same `Version` you already had and then calls `to_string()` on that.

https://github.com/python-poetry/poetry-core/blob/37cee90a5dd4c7ee2c5ee836216ba813242b3ade/src/poetry/core/utils/helpers.py#L27-L28

Then calling `escape_version()` on such a version is actually
counter-productive, per #6466.

Similar changes can and should be made over in poetry-core, but it
should be safe to merge this before that is done.
poetry-bot bot pushed a commit that referenced this issue Sep 17, 2022
…#6476)

When you already have a `Version` in hand,
`normalize_version(version.text)` is a very roundabout way of calling
`version.to_string()`: it re-parses the version text to give you the
same `Version` you already had and then calls `to_string()` on that.

https://github.com/python-poetry/poetry-core/blob/37cee90a5dd4c7ee2c5ee836216ba813242b3ade/src/poetry/core/utils/helpers.py#L27-L28

Then calling `escape_version()` on such a version is actually
counter-productive, per #6466.

Similar changes can and should be made over in poetry-core, but it
should be safe to merge this before that is done.

(cherry picked from commit a14a93d)
neersighted pushed a commit that referenced this issue Sep 17, 2022
…#6476)

When you already have a `Version` in hand,
`normalize_version(version.text)` is a very roundabout way of calling
`version.to_string()`: it re-parses the version text to give you the
same `Version` you already had and then calls `to_string()` on that.

https://github.com/python-poetry/poetry-core/blob/37cee90a5dd4c7ee2c5ee836216ba813242b3ade/src/poetry/core/utils/helpers.py#L27-L28

Then calling `escape_version()` on such a version is actually
counter-productive, per #6466.

Similar changes can and should be made over in poetry-core, but it
should be safe to merge this before that is done.

(cherry picked from commit a14a93d)
@dimbleby
Copy link
Contributor

fixed in python-poetry/poetry-core#469, this can be closed

@mkniewallner mkniewallner removed the status/triage This issue needs to be triaged label Sep 18, 2022
Copy link

github-actions bot commented Mar 1, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

No branches or pull requests

5 participants