pipenv install add spurious "markers": "python_version == '3'" for a specific library since v2023.12.1, how to fix the library?

[bakert]

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 usable

Actual result

            "markers": "python_version == '3'",

appears in Pipfile.lock

Steps to replicate

$ pipenv install better-profanity
$ grep -A8 better-profanity Pipfile.lock
        "better-profanity": {
            "hashes": [
                "sha256:8a6fdc8606d7471e7b5f6801917eca98ec211098262e82f62da4f5de3a73145b",
                "sha256:bd4c529ea6aa2db1aaa50524be1ed14d0fe5c664f1fd88c8bc388c7e9f9f00e8"
            ],
            "index": "pypi",
            "markers": "python_version == '3'", <<<< this is the bad line
            "version": "==0.7.0"
        },

[bakert]

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.

[bakert]

My guess is in 6ac1451 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 :)

[matteius]

@bakert I greatly appreciate the amount of detail you have put into your analysis. Here is what I can recall:

• The conversion away from requirementslib was pretty good for the happy path, but then came the edge cases.
• One such edge case involved some weird thing requirementslib was doing to pull in hashes/markers for top level dependencies that don't match the system being resolved on.
• It was kind of a pickle, if I had more time I'd reference some of those issue reports, but I made some decision where sys_markers would continue to try resolving and if it could it would pull the right package data with markers, but if it couldn't then I'd advise users to move the sys markers to regular markers and then it would skip the package.
• This was a mediocre compromise (as was the original implementation), but it kind of resolved those issues, though we still have some issues about people confused by that change in behavior.
• As it related to this issue, with the markers, one thing I learned is the markers comparison logic that we use which I think is in packaging, is quite limited.
• Off the top of my head I know for sure packaging is don't basic integer comparison on semver, and anything else kind of breaks it.
• Quick check, look around here: https://github.com/pypa/pipenv/blob/main/pipenv/patched/pip/_vendor/packaging/markers.py#L222-L244

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".

[matteius]

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: #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.

[bakert]

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 :)