packaging icon indicating copy to clipboard operation
packaging copied to clipboard

Comparing marker string with marker string

Open konstin opened this issue 2 years ago • 7 comments

Currently comparing two marker string is legal in PEP 508, but fails in packaging 22.0:

from packaging.requirements import Requirement
Requirement('numpy; "a" <= "b"').marker.evaluate()
Traceback (most recent call last):
  File "/home/konsti/.local/share/JetBrains/Toolbox/apps/PyCharm-P/ch-0/223.7571.203/plugins/python/helpers/pydev/pydevconsole.py", line 364, in runcode
    coro = func()
  File "<input>", line 1, in <module>
  File "/home/konsti/pep508/.venv/lib/python3.8/site-packages/packaging/markers.py", line 241, in evaluate
    return _evaluate_markers(self._markers, current_environment)
  File "/home/konsti/pep508/.venv/lib/python3.8/site-packages/packaging/markers.py", line 148, in _evaluate_markers
    rhs_value = environment[environment_key]
KeyError: 'b'

The two options are to make this legal in packaging or to ban that from the Dependency specifiers aka PEP 508, imho it the better option would be to just ban it

konstin avatar Dec 08 '22 16:12 konstin

Did this work in an older version of packaging?

pradyunsg avatar Dec 08 '22 19:12 pradyunsg

no, it just throws a different error

konstin avatar Dec 08 '22 19:12 konstin

Could you post that error as well?

pradyunsg avatar Dec 08 '22 19:12 pradyunsg

Sure, i checked and in all versions down to 20.0 numpy; "a" <= "b" raises packaging.markers.UndefinedEnvironmentName: 'b' does not exist in evaluation environment.

konstin avatar Dec 08 '22 19:12 konstin

If I interpret PEP 508 correctly, these markers should be allowed:

"a" == "b"
"a" <= "b"

While these are forbidden:

"a" ~= "b"

Quote:

Comparisons in marker expressions are typed by the comparison operator. The <marker_op> operators that are not in <version_cmp> perform the same as they do for strings in Python. The <version_cmp> operators use the PEP 440 version comparison rules when those are defined (that is when both sides have a valid version specifier). If there is no defined PEP 440 behaviour and the operator exists in Python, then the operator falls back to the Python behaviour.

frostming avatar Dec 13 '22 08:12 frostming

Bah, this isn’t a logical thing to do and I’d much rather we amend the dependency specification format to disallow such string comparisons and reject these markers.

pradyunsg avatar Dec 13 '22 09:12 pradyunsg

I agree with @pradyunsg to change the spec. Imho the best thing would be to change the PEP 508 grammar to

marker_expr   = wsp* env_var wsp* marker_op wsp* python_str
              | wsp* python_str wsp* marker_op wsp* env_var
              | wsp* '(' marker wsp* ')'

This would also disallow other strange comparisons like extra == os_name and make parsing/evaluation easier.

(imho it should also be specified which env_var are PEP 440 and which are compared stringly, but that's a separate issue)

konstin avatar Dec 13 '22 09:12 konstin