fix(markers): only parse versions on certain keys#939
Conversation
Liam-DeVoe
left a comment
There was a problem hiding this comment.
Replaces: #932 (this is a different approach)
I don't think this supersedes #932, because it's still possible to hit InvalidVersion with manual use of SpecifierSet (eg the example in #767). FWIW, the issue in this pull would actually be fixed by #932, though I think the explicit specification of markers which require a version here is good to have regardless.
|
I think the spec needs to change. It states the type depends on the operator used, not on that table (not sure why that table is there, then!), and |
933ef8b to
7bc058b
Compare
|
I've updated this to follow the spec change update in pypa/packaging.python.org#1971. |
96bc364 to
5194f57
Compare
862b640 to
f80a3fc
Compare
According to the [environment markers](https://packaging.python.org/en/latest/specifications/dependency-specifiers/#environment-markers), most markers are strings, with only a small subset being use to handle versions. This commit ensures that only those keys which _are_ versions get compared as versions, and all other keys are compared as string literals. Signed-off-by: JP-Ellis <josh@jpellis.me>
Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
Comparisons relying on order are always False. Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
f80a3fc to
a172dd0
Compare
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [packaging](https://github.com/pypa/packaging) | packages | major | `==25.0` → `==26.0` | --- ### Release Notes <details> <summary>pypa/packaging (packaging)</summary> ### [`v26.0`](https://github.com/pypa/packaging/releases/tag/26.0) [Compare Source](pypa/packaging@25.0...26.0) Read about the performance improvements here: <https://iscinumpy.dev/post/packaging-faster>. #### What's Changed Features: - PEP 751: support pylock by [@​sbidoul](https://github.com/sbidoul) in [#​900](pypa/packaging#900) - PEP 794: import name metadata by [@​brettcannon](https://github.com/brettcannon) in [#​948](pypa/packaging#948) - Support writing metadata by [@​henryiii](https://github.com/henryiii) in [#​846](pypa/packaging#846) - Support `__replace__` for `Version` by [@​henryiii](https://github.com/henryiii) in [#​1003](pypa/packaging#1003) - Support positional pattern matching for `Version` and `Specifier` by [@​henryiii](https://github.com/henryiii) in [#​1004](pypa/packaging#1004) Behavior adaptations: - PEP 440 handling of prereleases for `Specifier.contains`, `SpecifierSet.contains`, and `SpecifierSet.filter` by [@​notatallshaw](https://github.com/notatallshaw) in [#​897](pypa/packaging#897) - Handle PEP 440 edge case in `SpecifierSet.filter` by [@​notatallshaw](https://github.com/notatallshaw) in [#​942](pypa/packaging#942) - Adjust arbitrary equality intersection preservation in `SpecifierSet` by [@​notatallshaw](https://github.com/notatallshaw) in [#​951](pypa/packaging#951) - Return `False` instead of raising for `.contains` with invalid version by [@​Liam-DeVoe](https://github.com/Liam-DeVoe) in [#​932](pypa/packaging#932) - Support arbitrary equality on arbitrary strings for `Specifier` and `SpecifierSet`'s `filter` and `contains` method. by [@​notatallshaw](https://github.com/notatallshaw) in [#​954](pypa/packaging#954) - Only try to parse as `Version` on certain marker keys, return `False` on unequal ordered comparsions by [@​JP-Ellis](https://github.com/JP-Ellis) in [#​939](pypa/packaging#939) Fixes: - Update `_hash` when unpickling `Tag()` by [@​dholth](https://github.com/dholth) in [#​860](pypa/packaging#860) - Correct comment and simplify implicit prerelease handling in `Specifier.prereleases` by [@​notatallshaw](https://github.com/notatallshaw) in [#​896](pypa/packaging#896) - Use explicit `_GLibCVersion` `NamedTuple` in `_manylinux` by [@​cthoyt](https://github.com/cthoyt) in [#​868](pypa/packaging#868) - Detect invalid license expressions containing `()` by [@​bwoodsend](https://github.com/bwoodsend) in [#​879](pypa/packaging#879) - Correct regex for metadata `'name'` format by [@​di](https://github.com/di) in [#​925](pypa/packaging#925) - Improve the message around expecting a semicolon by [@​pradyunsg](https://github.com/pradyunsg) in [#​833](pypa/packaging#833) - Support nested parens in license expressions by [@​Liam-DeVoe](https://github.com/Liam-DeVoe) in [#​931](pypa/packaging#931) - Add space before at symbol in `Requirements` string by [@​henryiii](https://github.com/henryiii) in [#​953](pypa/packaging#953) - A root logger use found by ruff LOG, use `packaging` logger instead by [@​henryiii](https://github.com/henryiii) in [#​965](pypa/packaging#965) - Better support for subclassing `Marker` and `Requirement` by [@​henryiii](https://github.com/henryiii) in [#​1022](pypa/packaging#1022) - Normalize all extras, not just if it comes first by [@​henryiii](https://github.com/henryiii) in [#​1024](pypa/packaging#1024) - Don't produce a broken repr if `Marker` fails to construct by [@​henryiii](https://github.com/henryiii) in [#​1033](pypa/packaging#1033) Performance: - Avoid recompiling regexes in the tokenizer for a 3x speedup by [@​hauntsaninja](https://github.com/hauntsaninja) in [#​1019](pypa/packaging#1019) - Improve performance in `_manylinux.py` by [@​cthoyt](https://github.com/cthoyt) in [#​869](pypa/packaging#869) - Minor cleanups to `Version` by [@​bearomorphism](https://github.com/bearomorphism) in [#​913](pypa/packaging#913) - Skip redundant creation of `Version`s in specifier comparison by [@​notatallshaw](https://github.com/notatallshaw) in [#​986](pypa/packaging#986) - Cache `Specifier`'s Version by [@​notatallshaw](https://github.com/notatallshaw) in [#​985](pypa/packaging#985) - Make `Version` a little faster by [@​henryiii](https://github.com/henryiii) in [#​987](pypa/packaging#987) - Minor `Version` regex cleanup by [@​henryiii](https://github.com/henryiii) in [#​990](pypa/packaging#990) - Faster regex on Python 3.11.5+ by [@​henryiii](https://github.com/henryiii) in [#​988](pypa/packaging#988) and [#​1055](pypa/packaging#1055) - Lazily calculate `_key` in `Version` by [@​notatallshaw](https://github.com/notatallshaw) in [#​989](pypa/packaging#989) and regression for `packaging_legacy` fixed by [@​henryiii](https://github.com/henryiii) in [#​1048](pypa/packaging#1048) - Faster `canonicalize_version` by [@​henryiii](https://github.com/henryiii) in [#​993](pypa/packaging#993) - Use `fullmatch` in a couple more places by [@​henryiii](https://github.com/henryiii) in [#​992](pypa/packaging#992) - Use `fullmatch` for markers too by [@​henryiii](https://github.com/henryiii) in [#​1029](pypa/packaging#1029) - Use `map` instead of generator by [@​henryiii](https://github.com/henryiii) in [#​996](pypa/packaging#996) - Deprecate `._version` (`_Version`, a `NamedTuple`) by [@​henryiii](https://github.com/henryiii) in [#​995](pypa/packaging#995) and [#​1062](pypa/packaging#1062) - Avoid duplicate `Version` creation in `canonicalize_version` by [@​henryiii](https://github.com/henryiii) in [#​994](pypa/packaging#994) - Add `__slots__` to `Version` by [@​henryiii](https://github.com/henryiii) in [#​1001](pypa/packaging#1001) - Add `__slots__` to `Specifier`s by [@​henryiii](https://github.com/henryiii) in [#​1002](pypa/packaging#1002) - Add `__slots__` to `Node`s by [@​henryiii](https://github.com/henryiii) in [#​1032](pypa/packaging#1032) - Use `version.__replace__` in specifier comparison by [@​notatallshaw](https://github.com/notatallshaw) in [#​999](pypa/packaging#999) - Use `_get_spec_version` in more places in `Specifier` by [@​notatallshaw](https://github.com/notatallshaw) in [#​1005](pypa/packaging#1005) - Pull `set` construction out of function by [@​henryiii](https://github.com/henryiii) in [#​1012](pypa/packaging#1012) - Letter normalization dict for prereleases and the like by [@​henryiii](https://github.com/henryiii) in [#​1014](pypa/packaging#1014) - Avoid normalizing extras again when comparing by [@​henryiii](https://github.com/henryiii) in [#​1028](pypa/packaging#1028) - Speed up `Version.__str__` by about 10% by [@​henryiii](https://github.com/henryiii) in [#​997](pypa/packaging#997) - Increase the performance of `canonicalize_name` by avoiding a regex by [@​henryiii](https://github.com/henryiii) in [#​1030](pypa/packaging#1030), [#​1047](pypa/packaging#1047), and [#​1064](pypa/packaging#1064) - Faster zero stripping by [@​henryiii](https://github.com/henryiii) in [#​1058](pypa/packaging#1058) Type annotations: - Fix a type annotation by [@​brettcannon](https://github.com/brettcannon) in [#​907](pypa/packaging#907) - Fix tags return type in `parse_wheel_filename` docs by [@​ncoghlan](https://github.com/ncoghlan) in [#​973](pypa/packaging#973) - Add type hint for `_version` in `.version.Version` by [@​brettcannon](https://github.com/brettcannon) in [#​927](pypa/packaging#927) - Changed static type annotations in prereleases setter method in `specifier.py` by [@​subhajitsaha01](https://github.com/subhajitsaha01) in [#​930](pypa/packaging#930) - Statically type the tests by [@​henryiii](https://github.com/henryiii) in [#​982](pypa/packaging#982) Internal: - Test and declare support Python 3.14 by [@​henryiii](https://github.com/henryiii) in [#​901](pypa/packaging#901) - Modernize and speed up tests on Python 3.14 by [@​henryiii](https://github.com/henryiii) in [#​903](pypa/packaging#903) - Change our license metadata to use an SPDX license expression by [@​cdce8p](https://github.com/cdce8p) in [#​881](pypa/packaging#881) - No need for `license-files` by [@​DimitriPapadopoulos](https://github.com/DimitriPapadopoulos) in [#​924](pypa/packaging#924) - Update mypy by [@​hauntsaninja](https://github.com/hauntsaninja) in [#​891](pypa/packaging#891) - Some config updates by [@​henryiii](https://github.com/henryiii) in [#​902](pypa/packaging#902) - Add spell check and rst check by [@​henryiii](https://github.com/henryiii) in [#​904](pypa/packaging#904) - Clean up ruff ignores by [@​henryiii](https://github.com/henryiii) in [#​905](pypa/packaging#905) - Update example for env marker `python_version` by [@​trim21](https://github.com/trim21) in [#​908](pypa/packaging#908) - Move codespell configuration into pyproject.toml by [@​yarikoptic](https://github.com/yarikoptic) in [#​910](pypa/packaging#910) - Check warning a little more precisely by [@​henryiii](https://github.com/henryiii) in [#​837](pypa/packaging#837) - Speed up mypy a little by [@​henryiii](https://github.com/henryiii) in [#​836](pypa/packaging#836) - Apply ruff/flake8-pyi rules (PYI) by [@​DimitriPapadopoulos](https://github.com/DimitriPapadopoulos) in [#​835](pypa/packaging#835) - Better local runs for codespell by [@​henryiii](https://github.com/henryiii) in [#​911](pypa/packaging#911) - Remove outdated/confusing Gist link by [@​stefan6419846](https://github.com/stefan6419846) in [#​921](pypa/packaging#921) - Fix docs and docs ci after [#​897](pypa/packaging#897) landed by [@​notatallshaw](https://github.com/notatallshaw) in [#​926](pypa/packaging#926) - Run twine-check on push in CI by [@​EpicWink](https://github.com/EpicWink) in [#​922](pypa/packaging#922) - `ruff` was renamed `ruff-check` in pre-commit by [@​henryiii](https://github.com/henryiii) in [#​933](pypa/packaging#933) - Fix incorrectly implicitly concatenated string in specifiers test by [@​notatallshaw](https://github.com/notatallshaw) in [#​946](pypa/packaging#946) - Simplify conditional by [@​ofek](https://github.com/ofek) in [#​949](pypa/packaging#949) - Modernize nox, use dependency-groups for tests by [@​henryiii](https://github.com/henryiii) in [#​952](pypa/packaging#952) - Add more checks that don't affect anything by [@​henryiii](https://github.com/henryiii) in [#​957](pypa/packaging#957) - Enable Ruff ISC rule by [@​henryiii](https://github.com/henryiii) in [#​959](pypa/packaging#959) - Ruff code FLY by [@​henryiii](https://github.com/henryiii) in [#​963](pypa/packaging#963) - pytest `log_level` is better than `log_cli_level` by [@​henryiii](https://github.com/henryiii) in [#​956](pypa/packaging#956) - Ruff code TRY by [@​henryiii](https://github.com/henryiii) in [#​961](pypa/packaging#961) - Add the ruff PL checks by [@​henryiii](https://github.com/henryiii) in [#​964](pypa/packaging#964) - Enable Ruff ARG rules by [@​henryiii](https://github.com/henryiii) in [#​958](pypa/packaging#958) - Ruff PT code (pytest) by [@​henryiii](https://github.com/henryiii) in [#​960](pypa/packaging#960) - Add ruff DTZ by [@​henryiii](https://github.com/henryiii) in [#​968](pypa/packaging#968) - Add ruff BLE by [@​henryiii](https://github.com/henryiii) in [#​967](pypa/packaging#967) - Add the ruff SIM checks by [@​henryiii](https://github.com/henryiii) in [#​966](pypa/packaging#966) - Adding ruff PERF by [@​henryiii](https://github.com/henryiii) in [#​969](pypa/packaging#969) - Move some config into coverage config by [@​henryiii](https://github.com/henryiii) in [#​971](pypa/packaging#971) - Check ruff C4 by [@​henryiii](https://github.com/henryiii) in [#​962](pypa/packaging#962) - Adding ruff T20 by [@​henryiii](https://github.com/henryiii) in [#​972](pypa/packaging#972) - Add a tests pass job by [@​henryiii](https://github.com/henryiii) in [#​977](pypa/packaging#977) - Add ruff TC by [@​henryiii](https://github.com/henryiii) in [#​980](pypa/packaging#980) - Adding part of ruff RET by [@​henryiii](https://github.com/henryiii) in [#​979](pypa/packaging#979) - Reorder mypy check by [@​henryiii](https://github.com/henryiii) in [#​983](pypa/packaging#983) - Enable ruff ALL by [@​henryiii](https://github.com/henryiii) in [#​984](pypa/packaging#984) - Link back to repo/source in furo by [@​henryiii](https://github.com/henryiii) in [#​991](pypa/packaging#991) - Add case insensitivity tests for arbitrary equality by [@​notatallshaw](https://github.com/notatallshaw) in [#​975](pypa/packaging#975) - Synchronize documentation and code for markers by [@​zahlman](https://github.com/zahlman) in [#​1008](pypa/packaging#1008) - Use `partition` in `_parse_project_urls` by [@​henryiii](https://github.com/henryiii) in [#​1013](pypa/packaging#1013) - auto-skip the dependabot PRs in the release changelog generation by [@​henryiii](https://github.com/henryiii) in [#​1016](pypa/packaging#1016) - Update unreleased section in changelog by [@​henryiii](https://github.com/henryiii) in [#​1017](pypa/packaging#1017) - Fix PR role to match extlinks by [@​hugovk](https://github.com/hugovk) in [#​1020](pypa/packaging#1020) - Mention new parts in README by [@​henryiii](https://github.com/henryiii) in [#​1023](pypa/packaging#1023) - Replace a couple of asserts with else by [@​henryiii](https://github.com/henryiii) in [#​1027](pypa/packaging#1027) - Simplify and/or check a little more by [@​henryiii](https://github.com/henryiii) in [#​1031](pypa/packaging#1031) - Use slim runner for all check by [@​henryiii](https://github.com/henryiii) in [#​1021](pypa/packaging#1021) - Use typos instead of codespell by [@​henryiii](https://github.com/henryiii) in [#​1015](pypa/packaging#1015) - Update changelog with recent additions by [@​henryiii](https://github.com/henryiii) in [#​1034](pypa/packaging#1034) - Publish to PyPI via GitHub CI by [@​EpicWink](https://github.com/EpicWink) in [#​893](pypa/packaging#893) - Use prek for faster pre-commit lint step by [@​henryiii](https://github.com/henryiii) in [#​1037](pypa/packaging#1037) - Add help text to noxfile by [@​henryiii](https://github.com/henryiii) in [#​1038](pypa/packaging#1038) - Update licenses to 3.27 by [@​henryiii](https://github.com/henryiii) in [#​1036](pypa/packaging#1036) - Use relative import in `packaging.licenses` by [@​notatallshaw](https://github.com/notatallshaw) in [#​1039](pypa/packaging#1039) - Add zizmor and tighten up CI by [@​henryiii](https://github.com/henryiii) in [#​1035](pypa/packaging#1035) - Fix release script by [@​henryiii](https://github.com/henryiii) in [#​1040](pypa/packaging#1040) - Fix using a dev version (again) by [@​henryiii](https://github.com/henryiii) in [#​1041](pypa/packaging#1041) - Fix type hint of function used with `contextlib.contextmanager` by [@​SpecLad](https://github.com/SpecLad) in [#​1046](pypa/packaging#1046) - Always run tests by [@​henryiii](https://github.com/henryiii) in [#​1044](pypa/packaging#1044) - Fix a changelog number by [@​henryiii](https://github.com/henryiii) in [#​1042](pypa/packaging#1042) - Fix the publish job by [@​henryiii](https://github.com/henryiii) in [#​1043](pypa/packaging#1043) - Get the correct tag on publish by [@​henryiii](https://github.com/henryiii) in [#​1045](pypa/packaging#1045) - Test on first public release of CPython 3.11 and newer by [@​henryiii](https://github.com/henryiii) in [#​1056](pypa/packaging#1056) - Fix publication job (again) by [@​henryiii](https://github.com/henryiii) in [#​1051](pypa/packaging#1051) - Use `actionlint` to check CI workflows by [@​miketheman](https://github.com/miketheman) in [#​1052](pypa/packaging#1052) - Fix formatting of distribution types in metadata.rst by [@​brettcannon](https://github.com/brettcannon) in [#​1053](pypa/packaging#1053) #### New Contributors - [@​cdce8p](https://github.com/cdce8p) made their first contribution in [#​881](pypa/packaging#881) - [@​dholth](https://github.com/dholth) made their first contribution in [#​860](pypa/packaging#860) - [@​trim21](https://github.com/trim21) made their first contribution in [#​908](pypa/packaging#908) - [@​yarikoptic](https://github.com/yarikoptic) made their first contribution in [#​910](pypa/packaging#910) - [@​cthoyt](https://github.com/cthoyt) made their first contribution in [#​868](pypa/packaging#868) - [@​bwoodsend](https://github.com/bwoodsend) made their first contribution in [#​879](pypa/packaging#879) - [@​stefan6419846](https://github.com/stefan6419846) made their first contribution in [#​921](pypa/packaging#921) - [@​bearomorphism](https://github.com/bearomorphism) made their first contribution in [#​913](pypa/packaging#913) - [@​EpicWink](https://github.com/EpicWink) made their first contribution in [#​922](pypa/packaging#922) - [@​Liam-DeVoe](https://github.com/Liam-DeVoe) made their first contribution in [#​932](pypa/packaging#932) - [@​subhajitsaha01](https://github.com/subhajitsaha01) made their first contribution in [#​930](pypa/packaging#930) - [@​ncoghlan](https://github.com/ncoghlan) made their first contribution in [#​973](pypa/packaging#973) - [@​zahlman](https://github.com/zahlman) made their first contribution in [#​1008](pypa/packaging#1008) - [@​JP-Ellis](https://github.com/JP-Ellis) made their first contribution in [#​939](pypa/packaging#939) #### Since last RC Fixes: - Restore `._version` as a compat shim by [@​henryiii](https://github.com/henryiii) in [#​1062](pypa/packaging#1062) Performance: - Dual replace by [@​henryiii](https://github.com/henryiii) in [#​1064](pypa/packaging#1064) Documentaiton: - Prepare for 26.0 final by [@​henryiii](https://github.com/henryiii) in [#​1063](pypa/packaging#1063) **Full Changelog**: <pypa/packaging@26.0rc3...26.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4zLjYiLCJ1cGRhdGVkSW5WZXIiOiI0My4zLjYiLCJ0YXJnZXRCcmFuY2giOiJtYXN0ZXIiLCJsYWJlbHMiOltdfQ==--> Reviewed-on: https://kubara.git.onstackit.cloud/STACKIT/kubara/pulls/280
According to the environment markers, most markers are strings, with only a small subset being use to handle versions. As such, this PR changes the behaviour to use version comparison only on those keys which are dealing with versions.
Note that the choice of only doing version comparison on keys which are defined as being versions results in one failure for a custom key:
From my reading of the specification, it is unclear whether the use of version comparison in this case is meant to be supported or not.
I can see a few ways of moving forward
If version comparisons are only meant to be used with the defined set of keys, then this PR is ready to go, requiring only that the one test be changed to expect failure.
If version comparison should be the default, and only some keys are exempted from the version comparison, then we can invert the logic in
_eval_opfrom anif key in MARKERS_REQUIRING_VERSION:to anif key in MARKERS_REQUIRING_STRING_COMPARISON. If this is the case, then we would need to define what those keys are.An intermediate approach would be to inspect the operator and, for example, using version comparison for
<,<=,>=,>,~=and===, and string comparison for==and!=. I don't like this approach as it muddies the logic, and I think it would be better to be explicit about which keys are to be treated as versions.Resolves: #938
Replaces: #932 (this is a different approach)