[InvalidRequirement] Invalid requirement, parse error at "'extra =='" #2326

simonepri · 2020-04-20T14:59:09Z

I am on the latest Poetry version.
I have searched the issues of this repo and believe that this is not a duplicate.
If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).

OS version and name: maxOS 10.15
Poetry version: 1.0.5

Issue

When I run:

poetry add git+https://github.com/huggingface/transformers.git#a21d4fa410dc3b4c62f93aa0e6bbe4b75a101ee9 -vvv

The following exception is thrown:

[InvalidRequirement]
Invalid requirement, parse error at "'extra =='"

Traceback (most recent call last):
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/vendor/lib/python3.7/site-packages/clikit/console_application.py", line 131, in run
    status_code = command.handle(parsed_args, io)
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/vendor/lib/python3.7/site-packages/clikit/api/command/command.py", line 120, in handle
    status_code = self._do_handle(args, io)
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/vendor/lib/python3.7/site-packages/clikit/api/command/command.py", line 171, in _do_handle
    return getattr(handler, handler_method)(args, io, self)
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/vendor/lib/python3.7/site-packages/cleo/commands/command.py", line 92, in wrap_handle
    return self.handle()
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/lib/python3.7/site-packages/poetry/console/commands/add.py", line 89, in handle
    packages, allow_prereleases=self.option('allow-prereleases')
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/lib/python3.7/site-packages/poetry/console/commands/init.py", line 293, in _determine_requirements
    requires = self._parse_requirements(requires)
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/lib/python3.7/site-packages/poetry/console/commands/init.py", line 381, in _parse_requirements
    "git", url.url, reference=pair.get("rev")
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/lib/python3.7/site-packages/poetry/puzzle/provider.py", line 204, in get_package_from_vcs
    package = cls.get_package_from_directory(tmp_dir, name=name)
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/lib/python3.7/site-packages/poetry/puzzle/provider.py", line 412, in get_package_from_directory
    dep = dependency_from_pep_508(req)
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/lib/python3.7/site-packages/poetry/packages/__init__.py", line 39, in dependency_from_pep_508
    req = Requirement(name)
  File "/usr/local/Cellar/poetry/1.0.5_1/libexec/lib/python3.7/site-packages/poetry/version/requirements.py", line 212, in __init__
    requirement_string[e.loc : e.loc + 8]

If I try to add the package manually to the pyproject.toml file as follows:

transformers = {git = "https://github.com/huggingface/transformers.git", rev = "a21d4fa410dc3b4c62f93aa0e6bbe4b75a101ee9"}

The following exception is thrown instead:

[SolverProblemError]
Because text2error depends on transformers (*) which doesn't exist, version solving failed.

Ref huggingface/transformers#3982

The text was updated successfully, but these errors were encountered:

abn · 2020-04-20T22:01:56Z

@simonepri I was able to use the add command without errors on a linux machine. I suspect this might be an issue with transformer setup.py failing and poetry falling back to the ast parser. The ast parser is known to have issues with complex setup.py files.

Could you see if you can execute python setup.py egg_info within a clone of transformers? This should help isolate the problem, more so confirm the above.

simonepri · 2020-04-21T07:05:45Z

I was able to use the add command without errors on a linux machine.

I tried on Ubuntu and it produces the same error: https://colab.research.google.com/drive/1xUMhXL1iPiJjAgLMAN8OzJk21SGGhGQR

Could you see if you can execute python setup.py egg_info within a clone of transformers? This should help isolate the problem, more so confirm the above.

On macOS running:

git clone https://github.com/huggingface/transformers
cd transformers
git checkout a21d4fa410dc3b4c62f93aa0e6bbe4b75a101ee9
python setup.py egg_info

gives the following:

running egg_info
creating src/transformers.egg-info
writing src/transformers.egg-info/PKG-INFO
writing dependency_links to src/transformers.egg-info/dependency_links.txt
writing requirements to src/transformers.egg-info/requires.txt
writing top-level names to src/transformers.egg-info/top_level.txt
writing manifest file 'src/transformers.egg-info/SOURCES.txt'
reading manifest file 'src/transformers.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'src/transformers.egg-info/SOURCES.txt'

simonepri · 2020-04-24T10:25:55Z

git+git://github.com/timothycrosley/isort.git@e63ae06ec7d70b06df9e528357650281a3d3ec22; extra == "dev"

This is the value of requirement_string for which the REQUIREMENT pyparsing grammar fails.

poetry/poetry/version/requirements.py

Lines 207 to 214 in e8b6e15

    
           try: 
        
               req = REQUIREMENT.parseString(requirement_string) 
        
           except ParseException as e: 
        
               raise InvalidRequirement( 
        
                   'Invalid requirement, parse error at "{0!r}"'.format( 
        
                       requirement_string[e.loc : e.loc + 8] 
        
                   ) 
        
               )

simonepri · 2020-04-24T11:34:41Z

It seems that when a url is provided the REQUIREMENT grammar (mistakenly?) expects a space before the ;.

cc: @sdispater

abn · 2020-04-25T13:46:31Z

@simonepri most of that was copied over from the packaging project.

I believe it is a side-effect of using egg-info fallback here. When poetry parses the requires.txt generated; it converts the following

[dev]
...
isort@ git+git://github.com/timothycrosley/isort.git@e63ae06ec7d70b06df9e528357650281a3d3ec22#egg=isort
...

To this;

isort@ git+git://github.com/timothycrosley/isort.git@e63ae06ec7d70b06df9e528357650281a3d3ec22#egg=isort; extra == "dev"

As per PEP-508;

The "extra" variable is special. It is used by wheels to signal which specifications apply to a given extra in the wheel METADATA file, but since the METADATA file is based on a draft version of PEP-426, there is no current specification for this. Regardless, outside of a context where this special handling is taking place, the "extra" variable should result in an error like all other unknown variables.

The wheel METADATA will contain this;

Requires-Dist: isort @ git+git://github.com/timothycrosley/isort.git@e63ae06ec7d70b06df9e528357650281a3d3ec22#egg=isort ; extra == 'dev'

The fix here, I think should be to ensure we add a whitespace prefix for the semicolon as expected.

https://github.com/python-poetry/core/blob/8f360360913e0a935bdcf04e0c36b5faee33491b/poetry/core/utils/helpers.py#L83-L85

simonepri · 2020-04-25T14:53:09Z

The fix here, I think should be to ensure we add a whitespace prefix for the semicolon as expected.

After the fix you suggested, the parse_requires function returns:

isort@ git+git://github.com/timothycrosley/isort.git@e63ae06ec7d70b06df9e528357650281a3d3ec22#egg=isort ; extra == "dev"

But then this string is fed into dependency_from_pep_508 which changes it to

isort@ git+git://github.com/timothycrosley/isort.git@e63ae06ec7d70b06df9e528357650281a3d3ec22; extra == "dev"

Thus we also need to change this:

https://github.com/python-poetry/core/blob/6d0f8fdd8cde3959cd347cbd0060fdede6e097d1/poetry/core/packages/__init__.py#L66-L67

simonepri · 2020-04-25T15:24:13Z

@abn I noticed that all the tests strings do not have a space before the ;.

https://github.com/python-poetry/core/blob/3ccd963c724121939a3f11aa73dc543ea1293fad/tests/packages/test_main.py

Shall I change them as well together with the new PR?

The reason why they all pass is that the REQUIREMENT grammar does not need the space before the ; if what comes before is not an URL.

It seems weird to me that we have a grammar that allows for both
requests==2.18.0; extra == "foo" and requests==2.18.0 ; extra == "foo", but it does not allow for django-utils @ https://example.com/django-utils-1.0.0.tar.gz; extra == "foo".

simonepri · 2020-04-25T15:28:03Z

Also, I couldn't find any tests for the parse_requires function.
Are there any?

abn · 2020-04-25T15:33:49Z

I agree that the grammar could be improved, but at the moment I am reluctant to make changes to that module since I am not entirely sure if we should replicate the logic and would like to look at reusing package.requirements instead, but this is something that need to discussed elsewhere.

For now, I am inclined to keep the grammar as is until we decide on how we should proceed on the broader case.

As for tests, I would retain the existing tests, but also add extra ones to ensure that a whitespace is also supported.

The parse_requires function might not already have coverage; I'd recommend creating a requires.txt fixture with a few different cases. I know this might be more than what we need here, but it is a good opportunity to expand the test suite. I'd add this only for the core PR as the tests.packages.test_main suite should be sufficient for the bug fix in master.

simonepri · 2020-04-25T15:44:03Z

but at the moment I am reluctant to make changes to that module since I am not entirely sure if we should replicate the logic and would like to look at reusing package.requirements instead

I agree that reusing their code makes a lot of sense.

Anyway, I think is worth opening an issue on their side to ask wether that is intentional or not.

If you look at their test they indeed add a space for urls:
https://github.com/pypa/packaging/blob/61672bf9f507f38e84ce2786a1c42f55fa0a3153/tests/test_requirements.py#L26

But then in the __str__ method they do not add the space back after the marker.
https://github.com/pypa/packaging/blob/61672bf9f507f38e84ce2786a1c42f55fa0a3153/packaging/requirements.py#L139

abn · 2020-04-25T16:04:46Z

But then in the __str__ method they do not add the space back after the marker.
https://github.com/pypa/packaging/blob/61672bf9f507f38e84ce2786a1c42f55fa0a3153/packaging/requirements.py#L139

Our fix is simply working around the issue. The root issue is definitely the grammar, the regex is a bit too greedy and should be fixed eventually. Let's see what happens with the issue upstream.

simonepri · 2020-04-25T16:20:25Z

The root issue is definitely the grammar

I think the reason why the grammar behaves like that is because of the following example, that should be a valid url:

poetry @ git+https://github.com/python-poetry/poetry.git@b;ar; ; extra == "foo;"

But then in the str method they do not add the space back after the marker.
https://github.com/pypa/packaging/blob/61672bf9f507f38e84ce2786a1c42f55fa0a3153/packaging/requirements.py#L139

I was actually wrong, they do add the space when they stringify the object, the thing is that they only do that for URLs.

https://github.com/pypa/packaging/blob/61672bf9f507f38e84ce2786a1c42f55fa0a3153/packaging/requirements.py#L136

simonepri · 2020-04-25T17:10:15Z

The parse_requires function might not already have coverage; I'd recommend creating a requires.txt fixture with a few different cases. I know this might be more than what we need here, but it is a good opportunity to expand the test suite. I'd add this only for the core PR as the tests.packages.test_main suite should be sufficient for the bug fix in master.

It is tested in python-poetry. Should we add the tests to core as well?
https://github.com/python-poetry/poetry/blob/299a885e880afce613f76927165dce3b1f55591c/tests/utils/test_helpers.py

abn · 2020-04-25T17:31:45Z

Yes please.

Prior to this fix, when formatting PEP 508 dependency specification for remote url references, the quoted marker was not prefixed with required whitespace. This change ensures that the format adheres to the specified grammar. Resolves: python-poetry/poetry#2326

Prior to this fix, when formatting PEP 508 dependency specification for remote url references, the quoted marker was not prefixed with required whitespace. This change ensures that the format adheres to the specified grammar. Resolves: #2326

github-actions · 2024-03-03T00:17:30Z

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

simonepri added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Apr 20, 2020

simonepri closed this as completed Apr 24, 2020

simonepri reopened this Apr 24, 2020

simonepri mentioned this issue Apr 24, 2020

Fix requirement string parsing for URLs #2344

Closed

This was referenced Apr 25, 2020

Fix git dependencies python-poetry/poetry-core#30

Merged

Fix git dependencies #2349

Merged

Can't install transformers from sources using poetry huggingface/transformers#3982

Closed

abn closed this as completed in python-poetry/poetry-core#30 May 10, 2020

abn removed the status/triage This issue needs to be triaged label Sep 25, 2020

github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[InvalidRequirement] Invalid requirement, parse error at "'extra =='" #2326

[InvalidRequirement] Invalid requirement, parse error at "'extra =='" #2326

simonepri commented Apr 20, 2020 •

edited

Loading

abn commented Apr 20, 2020

simonepri commented Apr 21, 2020

simonepri commented Apr 24, 2020 •

edited

Loading

simonepri commented Apr 24, 2020

abn commented Apr 25, 2020

simonepri commented Apr 25, 2020

simonepri commented Apr 25, 2020 •

edited

Loading

simonepri commented Apr 25, 2020

abn commented Apr 25, 2020

simonepri commented Apr 25, 2020

abn commented Apr 25, 2020

simonepri commented Apr 25, 2020

simonepri commented Apr 25, 2020

abn commented Apr 25, 2020

github-actions bot commented Mar 3, 2024

[InvalidRequirement] Invalid requirement, parse error at "'extra =='" #2326

[InvalidRequirement] Invalid requirement, parse error at "'extra =='" #2326

Comments

simonepri commented Apr 20, 2020 • edited Loading

Issue

abn commented Apr 20, 2020

simonepri commented Apr 21, 2020

simonepri commented Apr 24, 2020 • edited Loading

simonepri commented Apr 24, 2020

abn commented Apr 25, 2020

simonepri commented Apr 25, 2020

simonepri commented Apr 25, 2020 • edited Loading

simonepri commented Apr 25, 2020

abn commented Apr 25, 2020

simonepri commented Apr 25, 2020

abn commented Apr 25, 2020

simonepri commented Apr 25, 2020

simonepri commented Apr 25, 2020

abn commented Apr 25, 2020

github-actions bot commented Mar 3, 2024

simonepri commented Apr 20, 2020 •

edited

Loading

simonepri commented Apr 24, 2020 •

edited

Loading

simonepri commented Apr 25, 2020 •

edited

Loading