Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Requirement parser does not support arbitrary equality in legacy parentheses format #336

Closed
uranusjr opened this issue Oct 8, 2020 · 13 comments · Fixed by #624
Closed

Comments

@uranusjr
Copy link
Member

uranusjr commented Oct 8, 2020

These all work as expected:

>>> Requirement('foo==1.0')
<Requirement('foo==1.0')>
>>> Requirement('foo===1.0')
<Requirement('foo===1.0')>
>>> Requirement('foo (==1.0)')
<Requirement('foo==1.0')>

But not this:

>>> Requirement('foo (===1.0)')
Traceback (most recent call last):
  File "packaging/requirements.py", line 98, in __init__
    req = REQUIREMENT.parseString(requirement_string)
  File "pyparsing.py", line 1955, in parseString
    raise exc
  File "pyparsing.py", line 3814, in parseImpl
    raise ParseException(instring, loc, self.errmsg, self)
pyparsing.ParseException: Expected stringEnd, found '('  (at char 4), (line:1, col:5)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "packaging/requirements.py", line 100, in __init__
    raise InvalidRequirement(
packaging.requirements.InvalidRequirement: Parse error at "'(===1.0)'": Expected stringEnd
@di
Copy link
Sponsor Member

di commented Oct 8, 2020

The issue is that this operator matches everything after it that is not whitespace as the version:

>>> Requirement('foo===1.0)').specifier
<SpecifierSet('===1.0)')>

You'd need to do this instead::

>>> Requirement('foo (===1.0 )').specifier
===1.0
<SpecifierSet('===1.0')>

See here:

(?:
# The identity operators allow for an escape hatch that will
# do an exact string match of the version you wish to install.
# This will not be parsed by PEP 440 and we cannot determine
# any semantic meaning from it. This operator is discouraged
# but included entirely as an escape hatch.
(?<====) # Only match for the identity operator
\s*
[^\s]* # We just match everything, except for whitespace
# since we are only testing for strict identity.
)

I'm not sure this is a bug.

@uranusjr
Copy link
Member Author

uranusjr commented Oct 8, 2020

I’d say it is. foo (===1.0 is not a valid requirement, so The requirement parser should not pass in the closing parenthesis to the specifier parser if there’s an opening one. IOW for Requirement('foo (===1.0)'), Specifier should get ===1.0 instead of ===1.0).

@pradyunsg
Copy link
Member

The behavior @uranusjr's suggesting makes sense to me and IMO if the standards say otherwise, we should update them.

@gmmephisto
Copy link

I'm facing this issue while installing the wheel that built with arbitrary equal for dependency in requirements.txt. There is a traceback on pip install:

ERROR: Exception:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/pip/_internal/cli/base_command.py", line 228, in _main
    status = self.run(options, args)
  File "/usr/lib/python2.7/site-packages/pip/_internal/cli/req_command.py", line 182, in wrapper
    return func(self, options, args)
  File "/usr/lib/python2.7/site-packages/pip/_internal/commands/install.py", line 324, in run
    reqs, check_supported_wheels=not options.target_dir
  File "/usr/lib/python2.7/site-packages/pip/_internal/resolution/legacy/resolver.py", line 183, in resolve
    discovered_reqs.extend(self._resolve_one(requirement_set, req))
  File "/usr/lib/python2.7/site-packages/pip/_internal/resolution/legacy/resolver.py", line 437, in _resolve_one
    set(req_to_install.extras) - set(dist.extras)
  File "/usr/lib/python2.7/site-packages/pip/_vendor/pkg_resources/__init__.py", line 2978, in extras
    return [dep for dep in self._dep_map if dep]
  File "/usr/lib/python2.7/site-packages/pip/_vendor/pkg_resources/__init__.py", line 3023, in _dep_map
    self.__dep_map = self._compute_dependencies()
  File "/usr/lib/python2.7/site-packages/pip/_vendor/pkg_resources/__init__.py", line 3033, in _compute_dependencies
    reqs.extend(parse_requirements(req))
  File "/usr/lib/python2.7/site-packages/pip/_vendor/pkg_resources/__init__.py", line 3094, in parse_requirements
    yield Requirement(line)
  File "/usr/lib/python2.7/site-packages/pip/_vendor/pkg_resources/__init__.py", line 3103, in __init__
    raise RequirementParseError(str(e))
RequirementParseError: Parse error at "'(===1.1.'": Expected stringEnd

As I understand pep440 correctly, arbitrary equality is using not only for "escape hatch", but also for comparing versions without taking into account local versions (https://www.python.org/dev/peps/pep-0440/#arbitrary-equality) which may be useful and should not break pip install.

@uranusjr
Copy link
Member Author

Gental ping since this is surfaced in pip again recently. #353 is a reasonable fix to this IMO.

@IceTDrinker
Copy link

Hello, any news on this ?

@uranusjr
Copy link
Member Author

uranusjr commented Apr 7, 2021

I don’t think anyone is actively working on this. Feel free to dive into the parser code.

@IceTDrinker
Copy link

Well the question is related to the fact you indicate #353 is a reasonable fix for this so I was wondering if there were plans to merge it ?

@uranusjr
Copy link
Member Author

uranusjr commented Apr 7, 2021

I was wondering if there were plans to merge it?

Probably not, I guess? I don’t have permissions to merge PRs here and can’t definitely answer this.

The change is reasonable, but far from perfect. The correct logic should only detect the opening parenthesis after the package identifier, and remove the closing one only if there is an opening parenthesis at the correct location. This is likely doable with some parser logic changes, but I have not looked into actually trying it myself. This is very low priority, to be honest.

@IceTDrinker
Copy link

Ok, I might take a look at the open PR and try to iron it out

Cheers

@brettcannon
Copy link
Member

#353 is a PR to fix this. Does the approach seem reasonable to people?

@uranusjr
Copy link
Member Author

Now that we have a hand-crafted parser, I’d actually expect it be able to handle parenthesis-matching directly.

@pradyunsg
Copy link
Member

pradyunsg commented Dec 7, 2022

#624 does that updating, and makes it possible to use this format.

echo 'name(===arbitrarystring)' | python -c "from packaging.requirements import Requirement; print(vars(Requirement(input())))"
{'name': 'name', 'url': None, 'extras': set(), 'specifier': <SpecifierSet('===arbitrarystring')>, 'marker': None}
❯ echo ' name ( === arbitrarystring ) ' | python -c "from packaging.requirements import Requirement; print(vars(Requirement(input())))" 
{'name': 'name', 'url': None, 'extras': set(), 'specifier': <SpecifierSet('===arbitrarystring')>, 'marker': None}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants