Ensure consistent extras formatting in output

AndydeCleyre commented 10 months ago

This aims to resolve the discussion in #2004, ~~and is a WIP~~ (all contributions or alternatives are welcome).

~~_compat~~ utils now exports install_req_from_line ~~and canonicalize_ireq~~ for other modules to use. This uses our copy_install_requirement which is updated to canonicalize extra strings.

~~As of now, canonicalize_ireq only affects the extras attributes, as that's all we need. So it may be poorly named 🤷🏼~~

~~No tests yet.~~

To avoid circular imports, this relocates PIP_VERSION from _compat to utils, as IMO utils should be importable by as many other piptools modules as possible.

Contributor checklist

[ ] Included tests for the changes.
[ ] PR title is short, clear, and ready to be included in the user-facing changelog.

Maintainer checklist

[ ] Verified one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
[ ] Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

AndydeCleyre commented 10 months ago

I wonder if this logic ought to be shoved into copy_install_requirement?

webknjaz commented 10 months ago

copy_install_requirement

No preference here.

AndydeCleyre commented 10 months ago

copy_install_requirement

No preference here.

@atugushev Do you think this logic belongs in or out of copy_install_requirement?

AndydeCleyre commented 6 months ago

I've just been rebasing this periodically. Are any changes needed? Is the PR still wanted?

@atugushev @webknjaz

tboddyspargo commented 6 months ago

Is the PR still wanted?

Thanks for keeping this updated, @AndydeCleyre. I, for one, am still interested in this PR!

AndydeCleyre commented 6 months ago

OK, here's a problem I could use help with:

reqs.in:

sentry_sdk[pure_eval]

$ pip-compile --no-header reqs.in

certifi==2024.2.2
    # via sentry-sdk
sentry-sdk[pure-eval]==1.40.5
    # via -r reqs.in
urllib3==2.2.1
    # via sentry-sdk

$ pip-sync /dev/null
$ pip install --dry-run --quiet --report=- -r reqs.in | jq -r '.install[].metadata.name'

urllib3
asttokens
certifi
executing
pure-eval
sentry-sdk
six

I'm demoting this to WIP again until this gets resolved.

In its current state, this branch has resolver_result missing items:

--- pip-tools-main.txt  2024-02-26 14:40:10.004827841 -0500
+++ pip-tools-2013.txt  2024-02-26 14:39:52.064453749 -0500
@@ -3,19 +3,12 @@
         'sentry-sdk[pure-eval]': ExtrasCandidate(base=LinkCandidate('https://files.pythonhosted.org/packages/f6/21/e41b8dd0da432a06b8bf10b7ba634d6e62e60b9a79aba15146a2581265cc/sentry_sdk-1.40.5-py2.py3-none-any.whl (from https://pypi.org/simple/sentry-sdk/)'),
         extras=frozenset({'pure-eval'})),
         'urllib3': LinkCandidate('https://files.pythonhosted.org/packages/a2/73/a68704750a7679d0b6d3ad7aa8d4da8e14e151ae82e6fee774e6e0d05ec8/urllib3-2.2.1-py3-none-any.whl (from https://pypi.org/simple/urllib3/) (requires-python:>=3.8)'),
-        '<Python from Requires-Python>': <pip._internal.resolution.resolvelib.candidates.RequiresPythonCandidate object at 0x75b8142a9af0>,
-        'asttokens': LinkCandidate('https://files.pythonhosted.org/packages/45/86/4736ac618d82a20d87d2f92ae19441ebc7ac9e7a581d7e58bbe79233b24a/asttokens-2.4.1-py2.py3-none-any.whl (from https://pypi.org/simple/asttokens/)'),
+        '<Python from Requires-Python>': <pip._internal.resolution.resolvelib.candidates.RequiresPythonCandidate object at 0x743e650b7b60>,
         'certifi': LinkCandidate('https://files.pythonhosted.org/packages/ba/06/a07f096c664aeb9f01624f858c3add0a4e913d6c96257acb4fce61e7de14/certifi-2024.2.2-py3-none-any.whl (from https://pypi.org/simple/certifi/) (requires-python:>=3.6)'),
-        'executing': LinkCandidate('https://files.pythonhosted.org/packages/80/03/6ea8b1b2a5ab40a7a60dc464d3daa7aa546e0a74d74a9f8ff551ea7905db/executing-2.0.1-py2.py3-none-any.whl (from https://pypi.org/simple/executing/) (requires-python:>=3.5)'),
-        'pure-eval': LinkCandidate('https://files.pythonhosted.org/packages/2b/27/77f9d5684e6bce929f5cfe18d6cfbe5133013c06cb2fbf5933670e60761d/pure_eval-0.2.2-py3-none-any.whl (from https://pypi.org/simple/pure-eval/)'),
-        'sentry-sdk': LinkCandidate('https://files.pythonhosted.org/packages/f6/21/e41b8dd0da432a06b8bf10b7ba634d6e62e60b9a79aba15146a2581265cc/sentry_sdk-1.40.5-py2.py3-none-any.whl (from https://pypi.org/simple/sentry-sdk/)'),
-        'six': LinkCandidate('https://files.pythonhosted.org/packages/d9/5a/e7c31adbe875f2abbb91bd84cf2dc52d792b5a01506781dbcf25c91daf11/six-1.16.0-py2.py3-none-any.whl (from https://pypi.org/simple/six/) (requires-python:>=2.7,
-        !=3.0.*,
-        !=3.1.*,
-        !=3.2.*)')
+        'sentry-sdk': LinkCandidate('https://files.pythonhosted.org/packages/f6/21/e41b8dd0da432a06b8bf10b7ba634d6e62e60b9a79aba15146a2581265cc/sentry_sdk-1.40.5-py2.py3-none-any.whl (from https://pypi.org/simple/sentry-sdk/)')
     },
-    graph=<pip._vendor.resolvelib.structs.DirectedGraph object at 0x75b8143a0a10>,
-    criteria={'sentry-sdk[pure-eval]': Criterion((SpecifierRequirement('sentry_sdk[pure_eval]'),
+    graph=<pip._vendor.resolvelib.structs.DirectedGraph object at 0x743e658c9af0>,
+    criteria={'sentry-sdk[pure-eval]': Criterion((SpecifierRequirement('sentry_sdk[pure-eval]'),
     via=None)),
     'sentry-sdk': Criterion((ExplicitRequirement(LinkCandidate('https://files.pythonhosted.org/packages/f6/21/e41b8dd0da432a06b8bf10b7ba634d6e62e60b9a79aba15146a2581265cc/sentry_sdk-1.40.5-py2.py3-none-any.whl (from https://pypi.org/simple/sentry-sdk/)')),
     via=ExtrasCandidate(base=LinkCandidate('https://files.pythonhosted.org/packages/f6/21/e41b8dd0da432a06b8bf10b7ba634d6e62e60b9a79aba15146a2581265cc/sentry_sdk-1.40.5-py2.py3-none-any.whl (from https://pypi.org/simple/sentry-sdk/)'),
@@ -30,26 +23,8 @@
     extras=frozenset({'pure-eval'}))),
     (SpecifierRequirement('urllib3>=1.26.11; python_version >= "3.6"'),
     via=LinkCandidate('https://files.pythonhosted.org/packages/f6/21/e41b8dd0da432a06b8bf10b7ba634d6e62e60b9a79aba15146a2581265cc/sentry_sdk-1.40.5-py2.py3-none-any.whl (from https://pypi.org/simple/sentry-sdk/)'))),
-    'pure-eval': Criterion((SpecifierRequirement('pure-eval; extra == "pure_eval"'),
-    via=ExtrasCandidate(base=LinkCandidate('https://files.pythonhosted.org/packages/f6/21/e41b8dd0da432a06b8bf10b7ba634d6e62e60b9a79aba15146a2581265cc/sentry_sdk-1.40.5-py2.py3-none-any.whl (from https://pypi.org/simple/sentry-sdk/)'),
-    extras=frozenset({'pure-eval'})))),
-    'executing': Criterion((SpecifierRequirement('executing; extra == "pure_eval"'),
-    via=ExtrasCandidate(base=LinkCandidate('https://files.pythonhosted.org/packages/f6/21/e41b8dd0da432a06b8bf10b7ba634d6e62e60b9a79aba15146a2581265cc/sentry_sdk-1.40.5-py2.py3-none-any.whl (from https://pypi.org/simple/sentry-sdk/)'),
-    extras=frozenset({'pure-eval'})))),
-    'asttokens': Criterion((SpecifierRequirement('asttokens; extra == "pure_eval"'),
-    via=ExtrasCandidate(base=LinkCandidate('https://files.pythonhosted.org/packages/f6/21/e41b8dd0da432a06b8bf10b7ba634d6e62e60b9a79aba15146a2581265cc/sentry_sdk-1.40.5-py2.py3-none-any.whl (from https://pypi.org/simple/sentry-sdk/)'),
-    extras=frozenset({'pure-eval'})))),
     '<Python from Requires-Python>': Criterion((RequiresPythonRequirement('>=3.8'),
     via=LinkCandidate('https://files.pythonhosted.org/packages/a2/73/a68704750a7679d0b6d3ad7aa8d4da8e14e151ae82e6fee774e6e0d05ec8/urllib3-2.2.1-py3-none-any.whl (from https://pypi.org/simple/urllib3/) (requires-python:>=3.8)')),
     (RequiresPythonRequirement('>=3.6'),
-    via=LinkCandidate('https://files.pythonhosted.org/packages/ba/06/a07f096c664aeb9f01624f858c3add0a4e913d6c96257acb4fce61e7de14/certifi-2024.2.2-py3-none-any.whl (from https://pypi.org/simple/certifi/) (requires-python:>=3.6)')),
-    (RequiresPythonRequirement('>=3.5'),
-    via=LinkCandidate('https://files.pythonhosted.org/packages/80/03/6ea8b1b2a5ab40a7a60dc464d3daa7aa546e0a74d74a9f8ff551ea7905db/executing-2.0.1-py2.py3-none-any.whl (from https://pypi.org/simple/executing/) (requires-python:>=3.5)')),
-    (RequiresPythonRequirement('!=3.0.*,!=3.1.*,!=3.2.*,>=2.7'),
-    via=LinkCandidate('https://files.pythonhosted.org/packages/d9/5a/e7c31adbe875f2abbb91bd84cf2dc52d792b5a01506781dbcf25c91daf11/six-1.16.0-py2.py3-none-any.whl (from https://pypi.org/simple/six/) (requires-python:>=2.7,
-    !=3.0.*,
-    !=3.1.*,
-    !=3.2.*)'))),
-    'six': Criterion((SpecifierRequirement('six>=1.12.0'),
-    via=LinkCandidate('https://files.pythonhosted.org/packages/45/86/4736ac618d82a20d87d2f92ae19441ebc7ac9e7a581d7e58bbe79233b24a/asttokens-2.4.1-py2.py3-none-any.whl (from https://pypi.org/simple/asttokens/)')))
+    via=LinkCandidate('https://files.pythonhosted.org/packages/ba/06/a07f096c664aeb9f01624f858c3add0a4e913d6c96257acb4fce61e7de14/certifi-2024.2.2-py3-none-any.whl (from https://pypi.org/simple/certifi/) (requires-python:>=3.6)')))
 })

AndydeCleyre commented 6 months ago

I thought that pip was handling "canonicalized" extras, but:

$ pip-sync /dev/null
$ pip install --dry-run --quiet --report=- 'sentry_sdk[pure_eval]' | jq -r '.install[].metadata.name'

urllib3
asttokens
certifi
executing
pure-eval
sentry-sdk
six

$ pip install --dry-run --quiet --report=- 'sentry_sdk[pure-eval]' | jq -r '.install[].metadata.name'

urllib3
certifi
sentry-sdk

So I need to step back and request a sanity check about what we should be doing here. @atugushev ?

webknjaz commented 6 months ago

Sorry, this fell off of my radar. Sounds like a bug in pip to fix as well?

AndydeCleyre commented 6 months ago

Sounds like a bug in pip to fix as well?

Maybe these:

AndydeCleyre commented 6 months ago

So I'll take this back out of draft but maybe we should consider it blocked until pip is fixed.

AndydeCleyre commented 4 months ago

Sounds like a bug in pip to fix as well?

Maybe these:

https://github.com/pypa/pip/issues/11445

https://github.com/pypa/pip/issues/11715

These were just marked as resolved! I haven't retested with pip from git yet but I'm optimistic.

EDIT: Yes, this seems to be working fine with pip's main branch.

In fact it might be proper to undo some of these changes, if we can require the next pip release as a minimum. I'll add a commit undoing changes as long as the test still passes and see what everyone thinks/tests.

chrysle commented 4 months ago

CI is fixed on main now. Could you rebase?

AndydeCleyre commented 4 months ago

I'm getting a failure with test_diff_should_not_uninstall, but on main as well.

AndydeCleyre commented 4 months ago

I rebased onto main, but it looks like the recently merged PR #2087 introduced flake8 and mypy violations in test_pip_compat.py and pip_compat.py, so those are now replicated here.

chrysle commented 4 months ago

Hmm yes, I also wondered whether the one or other import was necessary, but trusted that pre-commit would catch that. I think that check should be marked as required to pass before merging, seems like it isn't.

chrysle commented 4 months ago

The linters ought to be satisfied, now.

AndydeCleyre commented 4 months ago

Thanks. I won't have access to a computer for a day but feel free to rebase again.

chrysle commented 4 months ago

I'll wait for you. Happy time out!

chrysle commented 4 months ago

Thanks!

jazzband / pip-tools

Ensure consistent extras formatting in output #2013

Contributor checklist

Maintainer checklist