Skip to content

fix(markers): handle version pattern on LHS when marker variable is on RHS#1172

Closed
armorbreak001 wants to merge 1 commit into
pypa:mainfrom
armorbreak001:fix/marker-reversed-version-comparison
Closed

fix(markers): handle version pattern on LHS when marker variable is on RHS#1172
armorbreak001 wants to merge 1 commit into
pypa:mainfrom
armorbreak001:fix/marker-reversed-version-comparison

Conversation

@armorbreak001
Copy link
Copy Markdown

Summary

Fixes #934

When a marker expression has the version pattern (e.g. '3.12.*') on the left-hand side and the variable (e.g. python_full_version) on the right-hand side, evaluation produced incorrect results.

For example, "'3.12.*' == python_full_version" evaluated to False even when the Python version was 3.12.3, because the resolved env value ('3.12.3') was used as the specifier pattern while the literal ('3.12.*') — which is not a valid PEP 440 version — was passed to Specifier.contains(), which silently returned False.

Details

The fix in _eval_op(): after the normal spec.contains(lhs) attempt doesn't match, check whether lhs looks like a version pattern rather than a concrete version (by checking if it can be parsed as a Version). If it can't, try building the specifier from lhs instead and checking rhs against it.

The guard using Version(lhs) ensures we don't accidentally flip the meaning of asymmetric operators like < / > where both sides are concrete versions — only actual version patterns (wildcards, pre-release tags used as patterns, etc.) trigger the fallback.

Testing

All existing tests pass (2262/2262). Verified with the exact case from the issue:

  • "'3.12.*' == python_full_version"True (was False)
  • Normal-order markers unchanged
  • Asymmetric operators with concrete versions unaffected

…n RHS

When a marker like "'3.12.*' == python_full_version" is evaluated,
the variable is on the RHS so the resolved env value (e.g. '3.12.3')
was used as the specifier pattern while the literal ('3.12.*') was
treated as the item to check. Since '3.12.*' is not a valid PEP 440
version, Specifier.contains() silently returned False.

Now try building the specifier from lhs when the first attempt doesn't
match and lhs doesn't parse as a concrete version.
@henryiii
Copy link
Copy Markdown
Contributor

Duplicate of several others, notably #1126. I want to open a discussion before we merge one, to make sure people are happy with it. I can see if I can do that now, prompted by this.

@henryiii
Copy link
Copy Markdown
Contributor

I've started the discussion here: https://discuss.python.org/t/implementation-of-swapped-marker-order/107060

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

Successfully merging this pull request may close these issues.

Marker version comparison fails when the marker variable is on the RHS of a term.

2 participants