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

WIP: Make safe_name compliant to PEP 503 and behaviour of pip #1492

Closed
wants to merge 2 commits into from

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Sep 16, 2018

This PR revives the work begun in #1324 (merged and reverted).

torsava and others added 2 commits September 16, 2018 12:04
According to PEP 503, a "normalized" project name has all runs of the
characters ., - and _ replaced with a single - character. [0]

Similarly, since version 8.1.2, that is the behaviour of pip as well. [1]

However, Setuptools still allows a . in the normalized name, which
is causing trouble down the line.

[0] https://www.python.org/dev/peps/pep-0503/#normalized-names
[1] pypa/pip#3666
@jaraco
Copy link
Member Author

jaraco commented Sep 16, 2018

@torsava: Can you work on the failing tests please?

@jaraco jaraco changed the title Make safe_name compliant to PEP 503 and behaviour of pip WIP: Make safe_name compliant to PEP 503 and behaviour of pip Sep 16, 2018
@jaraco jaraco added the draft label Sep 16, 2018
@torsava
Copy link
Contributor

torsava commented Sep 17, 2018

@torsava: Can you work on the failing tests please?

Sadly I'm completely swamped right now and won't get to it in the foreseeable future. :/

@pganssle pganssle added Needs Triage Issues that need to be evaluated for severity and status. and removed Needs Triage Issues that need to be evaluated for severity and status. labels Oct 19, 2018
@hroncok
Copy link
Contributor

hroncok commented Jan 16, 2020

This has risen from the dead and it bites again, see https://bugzilla.redhat.com/show_bug.cgi?id=1791530

@jaraco
Copy link
Member Author

jaraco commented Feb 4, 2020

This problem is much more insidious than this PR purports.

It's not obvious from this PR what safe_name is used for, so it's not clear that PEP 503 even applies. PEP 503 is about normalizing names in a package index, such that these URLs are equivalent:

  • /jaraco-functools, /jaraco.functools
  • /CherryPy, cherrypy

PEP 503 does not declare the scope of these normalized names. It merely declares a normalization routine for use in the context of a package index. It is clear, however, that package names are allowed to have dots and capital characters in them.

However, safe-name is intentionally doing something different. It's generating a safe name for a project from an unsafe name. And the tests for safe name explicitly allow for capital letters and the dot character and specifically disallow other characters like $ and 🐍.

The current behavior is more desirable because it's more restrictive (only ascii letters and select symbols) but also more descriptive (allowing for namespaced packages and camel-cased names), even if these descriptive elements are normalized downstream to further constrain the namespace. Setuptools is focused on building safe, descriptive names while pip and PyPI are focused on resolving descriptive names from less-descriptive, normalized ones. They're different operations.

This one tiny change causes 4/5 of the prior assertions (and all non-degenerate assertions) in testSafeName to fail in addition to four other test failures.

I believe the solution to the (largely underdefined) problem here is going to require quite a bit more than mutating a function. You'll need to more clearly define the problem and express what change should address that problem, and if that change involves constraining the namespace for Python packages, that's probably going to require a separate PEP and some extensive transition effort.

In the meantime, Setuptools is working as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants