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

Hashin support for not distinguishing between underscored/hyphenated #116

Closed
caphrim007 opened this issue Jun 9, 2020 · 10 comments
Closed

Comments

@caphrim007
Copy link
Contributor

pypi appears to follow guidelines outlined in PEP8 around package names preferring to hyphenate them as opposed to underscore them

https://www.python.org/dev/peps/pep-0008/#package-and-module-names

"""
Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged.
"""

Note there is a distinction between a module and a package. hashin is operating on packages.

Regardless of the naming choices, pypi appears to support both underscores and hyphens in packages. This is likely being done so-as to prevent major breaking changes across existing software.

Consider, for example, if pypi drew a hard line on this, switched all package names to hyphens, and disallowed underscores; a tidal wave of broken software would exist based on this decision.

So it looks like pypi supports both and (may?) support both forever. However, when you use hashin to get the package of choice, you're always returned the "correct" one; ie, the hyphenated version.

There exists a case where a user may have an existing (underscored) package definition, and hashin should behave to correct this to use the hyphenated version.

For instance, if the user's requirements file has the following (truncated sha's)

python_memcached==1.59 \
    --hash=sha256:4dac64916871bd35502 \
    --hash=sha256:a2e28637be13ee0bf1a8

Then upon running either of the following

  • hashin python_memcached==1.6.0 -r requirements.txt
  • hashin python-memcached==1.6.0 -r requirements.txt

The expected change to the requirements file would be

python-memcached==1.60 \
    --hash=sha256:x00c64916871bd35502 \
    --hash=sha256:x02e28637be13ee0bf1a

Today, however, the behavior is that hashin will do the following.

  • Update, in place, if python_memcached is hashed
  • Append if python-memcached is hashed

This issue proposes that hashin normalize this behavior to prefer (in all cases) the hyphenated name. Therefore,

  • If python_memcached is hashed, replace python_memcached with python-memcached, and update hashes in place for python-memcached
  • If python-memcached is hashed, update hashes in place for python-memcached
@caphrim007
Copy link
Contributor Author

@peterbe lemme know if this captures what we were talking about over here renovatebot/renovate#2444

@peterbe
Copy link
Owner

peterbe commented Jun 9, 2020

I know about python-memcached but are there any packages in PyPI that appears listed there with an underscore. E.g if there was a https://pypi.org/project/some_lib/ and our users use hashin some-lib.

Perhaps, from reading your description that does not exist.

@peterbe
Copy link
Owner

peterbe commented Jun 9, 2020

I'm not convinced we should change the names of the packages if the version isn't different. It feels rather disruptive. E.g. You had python_memcached==1.59 ... in your requirements.txt what should happen if you run hashin python-memcached? (because as of today, version 1.59, is already the latest version).

I'm thinking we only correct (the name of the package in) requirements.txt IF the version changes. That's going to cause less git diffs for people.

However, and I think we're in agreement, if you find out that there's a new version of python-memcached that is version 1.60, running hashin -u -i or hashin python-memcached or hashin python_memcached the new requirements.txt should become:

-python_memcached==1.59 \
-    --hash=sha256:y10c64916871bd3550a \
-    --hash=sha256:y12e28637be13ee0bf1b
+python-memcached==1.60 \
+    --hash=sha256:x00c64916871bd35502 \
+    --hash=sha256:x02e28637be13ee0bf1a

@caphrim007
Copy link
Contributor Author

ahh. good point. I agree. if the version changes, then we would upgrade the name. otherwise, leaving it alone would be sufficient.

@caphrim007
Copy link
Contributor Author

Some other packages which are like this are readme_renderer and websocket_client.

@peterbe
Copy link
Owner

peterbe commented Jun 10, 2020

@caphrim007 Do you have the intention to take a stab at writing a pull request for this?

@caphrim007
Copy link
Contributor Author

@peterbe I do.

I spent some time looking at it the other day and think I can provide something for review within the next day or two. Happy to adjust it per your review.

Upon first looks, my guess is that the majority of the change is within the amend_requirements_content function since this is is called only after hashin has determined that changed-ness happens

caphrim007 added a commit to caphrim007/hashin that referenced this issue Jun 11, 2020
This patch changes hashin such that,

If the package specified has underscores in its name, but the new package
has hyphens, the following occurs

* If there is a new version, hashin updates the package in requirements to
  be the hyphenated name
* If there is no new version, the underscored name remains in place

Prior to this patch, the 1st case about would not occur. Instead, hashin
would append the new version in its hyphenated form to the requirements file
and leave the old version in its underscored form in the requirements file.
@caphrim007
Copy link
Contributor Author

@peterbe ready for review

@caphrim007
Copy link
Contributor Author

@peterbe ping

@peterbe
Copy link
Owner

peterbe commented Jun 15, 2020

@peterbe ping

Back from 2 days of vacation and Mondays is my meetings day.

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

No branches or pull requests

2 participants