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

test_lockfile_from_pipfile_meta and test_hash_from_hash due to string case #8

Open
jayvdb opened this issue Mar 2, 2019 · 5 comments

Comments

@jayvdb
Copy link

jayvdb commented Mar 2, 2019

I'm packaging this for openSUSE, and ran into these two errors only on openSUSE Leap 42.3 's Python 2.7, which is 2.7.13 .. it looks like it might even be that the Python hashlib is giving uppercase.

[   40s] _______________________ test_lockfile_from_pipfile_meta ________________________
[   40s] 
[   40s]     def test_lockfile_from_pipfile_meta():
[   40s]         pipfile = Pipfile({
[   40s]             "source": [
[   40s]                 {
[   40s]                     "name": "pypi",
[   40s]                     "url": "https://pypi.org/simple",
[   40s]                     "verify_ssl": True,
[   40s]                 },
[   40s]             ],
[   40s]             "requires": {
[   40s]                 "python_version": "3.7",
[   40s]             }
[   40s]         })
[   40s]         pipfile_hash_value = pipfile.get_hash().value
[   40s]         lockfile = Lockfile.with_meta_from(pipfile)
[   40s]     
[   40s]         pipfile.requires._data["python_version"] = "3.8"
[   40s]         pipfile.sources.append({
[   40s]             "name": "devpi",
[   40s]             "url": "http://localhost/simple",
[   40s]             "verify_ssl": True,
[   40s]         })
[   40s]     
[   40s] >       assert lockfile.meta.hash._data == {"sha256": pipfile_hash_value}
[   40s] E       AssertionError: assert {'SHA256': '7...baa518a65489'} == {'sha256': '7e...baa518a65489'}
[   40s] E         Left contains more items:
[   40s] E         {u'SHA256': '7e7ef69da7248742e869378f8421880cf8f0017f96d94d086813baa518a65489'}
[   40s] E         Right contains more items:
[   40s] E         {u'sha256': '7e7ef69da7248742e869378f8421880cf8f0017f96d94d086813baa518a65489'}
[   40s] E         Use -v to get the full diff
[   40s] 
[   40s] tests/test_lockfiles.py:114: AssertionError
[   40s] _____________________________ test_hash_from_hash ______________________________
[   40s] 
[   40s]     def test_hash_from_hash():
[   40s]         v = hashlib.md5(b"foo")
[   40s]         h = models.Hash.from_hash(v)
[   40s] >       assert h.name == "md5"
[   40s] E       AssertionError: assert 'MD5' == 'md5'
[   40s] E         - MD5
[   40s] E         + md5
[   40s] 
@jayvdb
Copy link
Author

jayvdb commented Mar 2, 2019

Confirming that on this platform, hashlib is giving uppercase

python2 -c 'import hashlib; print(hashlib.md5(b"foo").name)'
MD5

Randomly searching, I find https://aecilius.cecs.anu.edu.au/mu/mu-client-pypy/commit/1812ddb62ec35bffe0c1f65118d41a3d232e851f?view=parallel

It is supposed to always be lower case, but wasnt formally specified until 3.4.
https://docs.python.org/3/library/hashlib.html#hashlib.hash.name

Is this a deal breaker for plette? Will the name case cause other problems?
Can plette normalise the name internally, so its own objects dont suffer the same bug?

@uranusjr
Copy link
Member

uranusjr commented Mar 2, 2019

Normalisation is reasonable. The key is eventually passed to the Simple API, where it should always be lowercase anyway. Patch welcome!

@uranusjr
Copy link
Member

uranusjr commented Mar 2, 2019

With that said, I’m a little surprised Python does not do it automatically. Maybe this is worth raising as a bug in CPython as well?

@uranusjr
Copy link
Member

uranusjr commented Mar 2, 2019

@jayvdb
Copy link
Author

jayvdb commented Mar 3, 2019

Nice find.
I think the CPython docs should be more clear that it was using lowercase uppercase, and users should be accepting of that if they want to support 2.7.* and 3.3 or earlier.

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