Open bakert opened 5 months ago
I ended up investigating this in some detail.
I isolated the problem away from my specific scenario. Now I'm just trying to pipenv install
the better-profanity library in a new project:
[dent ~] mkdir test
[dent ~] cd test
[dent test] python3.12 -m venv .
[dent test] source bin/activate
[dent test] pip install pipenv && source bin/activate # So that "pipenv" points at our venv's version from now on
(test) [dent test] for VERSION in 2023.12.1 2023.9.7 2023.8.28 2023.8.26 2023.8.25 2023.8.23 2023.8.22 2023.8.21 2023.8.20 2023.8.19 2023.7.23 2023.7.11 2023.7.9 2023.7.4 2023.7.3; do {\rm Pipfile*; yes | pip uninstall pipenv && pip install pipenv==$VERSION && pipenv install better-profanity && (grep -A8 better-profanity Pipfile.lock | grep -q "python_version == '3'" && echo "$VERSION bad" || echo "$VERSION good") || echo "$VERSION FAIL"} 2>/dev/null | egrep "(good|bad|FAIL)"; done
This gives me:
2023.12.1 bad
2023.9.7 FAIL
2023.8.28 FAIL
2023.8.26 FAIL
2023.8.25 FAIL
2023.8.23 FAIL
2023.8.22 FAIL
2023.8.21 FAIL
2023.8.20 FAIL
2023.8.19 FAIL
2023.7.23 good
2023.7.11 good
2023.7.9 good
2023.7.4 good
2023.7.3 good
The FAIL cases all give me an error like this:
File "/Users/bakert/test/lib/python3.12/site-packages/pipenv/utils/resolver.py", line 812, in venv_resolve_deps
return prepare_lockfile(
^^^^^^^^^^^^^^^^^
File "/Users/bakert/test/lib/python3.12/site-packages/pipenv/utils/locking.py", line 143, in prepare_lockfile
lockfile_entry = get_locked_dep(project, dep, pipfile, current_entry)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/bakert/test/lib/python3.12/site-packages/pipenv/utils/locking.py", line 126, in get_locked_dep
lockfile_entry = clean_resolved_dep(project, dep, is_top_level, current_entry)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/bakert/test/lib/python3.12/site-packages/pipenv/utils/dependencies.py", line 300, in clean_resolved_dep
current_entry.update(lockfile)
^^^^^^^^^^^^^^^^^^^^
AttributeError: 'String' object has no attribute 'update'
Probably needless to say a library like requests installs fine against any of these versions.
So pipenv's handling of something funky about better-profanity changed in 2023.8.19 to the point of crashing pipenv install
. And then that was fixed up in some way in 2023.12.1 that meant that the bad line now gets added to Pipfile.lock.
I updated the first comment here for a clearer/simpler repro case as a result.
My guess is in https://github.com/pypa/pipenv/commit/6ac1451ec86d2949855e557109c2eb92b61930fc by @matteius exposed an issue in whatever faulty metadata better-profanity is issuing. Before it was being swallowed/ignored in some way. That change caused it to instead crash pipenv. And then some change in v2023.12.1 stopped the crash, but now causes the bad line to be added to Pipefile.lock.
What might be the cause on the better-profanity side? I'm not sure as I don't really understand how pipenv works. I am guessing that something in the setup.py is not right:
classifiers=[
"Programming Language :: Python :: 3",
"License :: OSI Approved :: MIT License",
"Operating System :: OS Independent",
],
python_requires="==3.*",
https://github.com/snguyenthanh/better_profanity/blob/master/setup.py
Maybe the wildcard isn't allowed. Or maybe a bare "3" with no minor version is causing the issue.
Maybe @matteius would be kind enough to take a quick look at this and help out an ignoramus :)
@bakert I greatly appreciate the amount of detail you have put into your analysis. Here is what I can recall:
I wish I could put a bit more time into now, but keep probing and I may be able to provide additional details. There are other issues as it relates to those marker evaluations and dev dependency versions, so it could use more TLC.
I think my priorities are wrapping up the install refactor and some other things and getting the first pipenv 3000 semver release finalized; and if I had even more time I'd like to look at the pythonfinder bug reports/regressions I helped cause (but a proper solution).
Sorry to steal this post a minute for a general update, but I feel that the code base is finally to a point where we can support more Individual Contributions towards improving things -- there were a lot of hurdle to get over with pip-shims, requirementslib being its own repo that had ballooned into a mess, and not deterministic behavior from referencing the system pip prior--all of that has been resolved, and it should be easier than ever to contribute to pipenv, but I don't have a ton of time as of late to take on "everything".
I guess the "good" cases mean something else might have been happening in requirementslib, so looking back on that code base may provide more clues on possible improvements to pipenv. I will say that we abandoned the circular classes hierarchy of requirementslib in favor of a more functional utils approach that tries to fulfill this vision: https://github.com/pypa/pipenv/discussions/5818
I think its easier to work with now and fix things, but does still require some mental overhead of understanding the different InstallRequirements and how/why they are handled differently (vcs vs file, vs pypi, etc), which largely relates to the internal libraries we pull from.
Thanks for your comments @matteius . I think the one question I have now is … "is ==3.*
a valid python version specifier?" What follows is a big discussion of that, but if you just know the answer to that question that would be great!
I am coming around to the idea that this might be a pipenv bug rather than an issue with better-profanity's metadata.packaging
seems to think ==3.*
is a valid specifier, although that code is pretty permissive and not a strict check of the spec I don't think:
>>> from packaging.specifiers import Specifier
>>> "3.1" in Specifier("==3.*")
True
>>> "3.10" in Specifier("==3.*")
True
>>> "2.7" in Specifier("==3.*")
False
I've been debugging with a local copy of pipenv today and I've found that in more than one place during resolving of dependencies we take version specifiers and strip off any trailing .*
. Notably in markers.normalize_specifier_set
but also in markers._tuplize_version
and maybe elsewhere too.
markers.normalize_specifier_set
actually has this comment:
.. note:: This function exists largely to deal with ``pyzmq`` which handles
the ``requires_python`` specifier incorrectly, using ``3.7*`` rather than
the correct form of ``3.7.*``. This workaround can likely go away if
we ever introduce enforcement for metadata standards on PyPI.
I guess it's much more normal to specify something like ==3.1.*
or (more likely) >=3.1.*
than it is to specify ==3.*
as better-profanity does. But nothing I read in https://packaging.python.org/en/latest/specifications/version-specifiers/#version-specifiers suggests that ==3.*
is not legal. There is even an example that involves ==2.*
.
If we decide that ==3.*
IS a valid identifier that we want to support I can look at making the necessary changes now that I'm passingly familiar with the code. But I guess we should rule on whether we even want that before I do that :)
Issue description
pipenv install better-profanity
adds"markers": "python_version == '3'",
to Pipfile.lock. This started happening with pipenv v2023.12.1. The library has not changed, so this comes from a pipenv change. I would like to understand what to fix in better-profanity to avoid this behavior.Expected result
pipenv install better-profanity
makes an entry in Pipfile.lock that is usableActual result
appears in Pipfile.lock
Steps to replicate