InvalidRequirement: name@ git+...@branch; (python_version=="...") and extra == "..."

[hroncok]

Hello. I found a requirement that packaging deems invalid but I am not sure why.

This is happening on 20.4, 20.9 and main branch @ 7350746.

I see what I believe is a proper requirement generated by https://github.com/sqlobject/sqlobject/blob/3.9.1/setup.py#L118-L121 as:

[oursql:python_version=="2.7"]
oursql@ git+https://github.com/sqlobject/oursql.git@master

[oursql:python_version>="3.4"]
oursql3@ git+https://github.com/sqlobject/oursql.git@py3k

Parsed by importlib.metadata.PathDistribution.requires as:

oursql@ git+https://github.com/sqlobject/oursql.git@master; (python_version=="2.7") and extra == "oursql"
oursql3@ git+https://github.com/sqlobject/oursql.git@py3k; (python_version>="3.4") and extra == "oursql"

Failing with:

>>> from packaging.requirements import Requirement
>>> Requirement('oursql@ git+https://github.com/sqlobject/oursql.git@master; (python_version=="2.7") and extra == "oursql"')
Traceback (most recent call last):
  File "/home/churchyard/Dokumenty/pypa/packaging/packaging/requirements.py", line 102, in __init__
    req = REQUIREMENT.parseString(requirement_string)
  File "/usr/lib/python3.9/site-packages/pyparsing.py", line 1955, in parseString
    raise exc
  File "/usr/lib/python3.9/site-packages/pyparsing.py", line 3814, in parseImpl
    raise ParseException(instring, loc, self.errmsg, self)
pyparsing.ParseException: Expected stringEnd, found '('  (at char 60), (line:1, col:61)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/churchyard/Dokumenty/pypa/packaging/packaging/requirements.py", line 104, in __init__
    raise InvalidRequirement(
packaging.requirements.InvalidRequirement: Parse error at "'(python_'": Expected stringEnd

What's wrong with this requirement?

[uranusjr]

Looks like the current parsing definition does not allow you to put parentheses around a marker expression without a boolean operator (and or or):

packaging/packaging/markers.py

Lines 134 to 136 in 7350746

	MARKER_EXPR = Forward()
	MARKER_ATOM = MARKER_ITEM | Group(LPAREN + MARKER_EXPR + RPAREN)
	MARKER_EXPR << MARKER_ATOM + ZeroOrMore(BOOLOP + MARKER_EXPR)

This is wrong because it is allowed by PEP 508:

url_req       = (name:n wsp* extras?:e wsp* urlspec:v (wsp+ | end) quoted_marker?:m
                 -> (n, e or [], v, m))
quoted_marker = ';' wsp* marker
marker        = marker_or
marker_or     = marker_and wsp* 'or' marker_and
              | marker_and
marker_and    = marker_expr wsp* 'and' marker_expr
              | marker_expr
marker_expr   = marker_var marker_op marker_var
              | wsp* '(' marker wsp* ')'

where

; (python_version=="2.7") and extra == "oursql"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ quoted_marker
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ marker -> marker_or -> marker_and
  ^^^^^^^^^^^^^^^^^^^^^^^                       marker_expr
   ^^^^^^^^^^^^^^^^^^^^^                        marker -> marker_or -> marker_and -> marker_expr
   ^^^^^^^^^^^^^--^^^^^
   marker_var   ^ marker_var
                |
             marker_op

So this is a bug in packaging’s parser. MARKER_ATOM should allow LPAREN + MARKER_ITEM + RPAREN.

[pradyunsg]

oursql@ git+https://github.com/sqlobject/oursql.git@master; (python_version=="2.7") and extra == "oursql"
oursql3@ git+https://github.com/sqlobject/oursql.git@py3k; (python_version>="3.4") and extra == "oursql"

These aren't valid requirements. In #624, these error out appropriately as:

packaging.requirements.InvalidRequirement: Expected end or semicolon (after URL and whitespace)
    oursql@ git+https://github.com/sqlobject/oursql.git@master; (python_version=="2.7") and extra == "oursql"
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^

Again, with #624, Adding whitespace after the URL correctly parses this as:

{'name': 'oursql', 'url': 'git+https://github.com/sqlobject/oursql.git@master', 'extras': set(), 'specifier': <SpecifierSet('')>, 'marker': <Marker('python_version == "2.7" and extra == "oursql"')>}

[uranusjr]

So I think this is no longer relevant after the parser rewrite?

[pradyunsg]

It is -- #624 makes the additional changes needed to fix this issue.