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

Fix the "safe_name" attribute of PackageFile for backwards compatibility #745

Merged
merged 2 commits into from Mar 17, 2021

Conversation

@pablogsal
Copy link
Contributor

@pablogsal pablogsal commented Mar 16, 2021

Commit 0bd26af introduced a regression
that was causing some namespace packages with dots in them fail to
upload to PyPI. The reason is because
packaging.utils.canonicalize_name() is not equivalent to
pkg_resources.safe_name() as the former transforms the "." into "-",
while the later only does it for non-alphanumeric/. characters.

Closes: #743

@pablogsal pablogsal force-pushed the issue-743 branch 2 times, most recently from e5641cf to 57a19f3 Mar 16, 2021
@pablogsal
Copy link
Contributor Author

@pablogsal pablogsal commented Mar 16, 2021

Loading

@pablogsal pablogsal force-pushed the issue-743 branch 3 times, most recently from 0027889 to 92c78cb Mar 16, 2021
Commit 0bd26af introduced a regression
that was causing some namespace packages with dots in them fail to
upload to PyPI. The reason is because
`packaging.utils.canonicalize_name()` is not equivalent to
`pkg_resources.safe_name()` as the former transforms the "." into "-",
while the later only does it for non-alphanumeric/. characters.

Closes: pypa#743
Copy link
Contributor

@bhrutledge bhrutledge left a comment

Thanks for the fix, test, and changelog! One small suggestion for the docstring, and I'd still like to get sign-off from the Twine maintainer that made the original change.

Loading

twine/package.py Outdated Show resolved Hide resolved
Loading
Co-authored-by: Brian Rutledge <brian@bhrutledge.com>
jaraco
jaraco approved these changes Mar 16, 2021
Copy link
Member

@jaraco jaraco left a comment

Looks good to me. This was my original instinct before converting to canonicalize name. Does this change remove the last dependency on packaging?

Loading

@pablogsal
Copy link
Contributor Author

@pablogsal pablogsal commented Mar 16, 2021

Does this change remove the last dependency on packaging?

It seems so. I would recommend doing the removal in a different change to keep things clean but I can push some commit to remove packaging from the install_requires and other places if you wish

Loading

@bhrutledge
Copy link
Contributor

@bhrutledge bhrutledge commented Mar 17, 2021

Does this change remove the last dependency on packaging?

It seems so. I would recommend doing the removal in a different change to keep things clean but I can push some commit to remove packaging from the install_requires and other places if you wish

Thanks @pablogsal. I'm going to merge this, then follow up with the removal of packaging, then release 3.4.1.

Loading

@bhrutledge bhrutledge merged commit 7dc99ad into pypa:master Mar 17, 2021
23 checks passed
Loading
bhrutledge added a commit to bhrutledge/twine that referenced this issue Mar 17, 2021
bhrutledge added a commit to bhrutledge/twine that referenced this issue Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants