pipenv icon indicating copy to clipboard operation
pipenv copied to clipboard

Improve detection of unparsable ASTs

Open albertoandreottiATgmail opened this issue 3 years ago • 5 comments

Thank you for contributing to Pipenv!

The issue

While running pipenv lock, I got this,

File "/usr/local/lib/python3.7/site-packages/pipenv/vendor/requirementslib/models/setup_info.py", line 198, in caller
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/pipenv/vendor/requirementslib/models/setup_info.py", line 366, in _find_install_requires
    return [el.s for el in variable.elts]
  File "/usr/local/lib/python3.7/site-packages/pipenv/vendor/requirementslib/models/setup_info.py", line 366, in <listcomp>
    return [el.s for el in variable.elts]
AttributeError: 'BinOp' object has no attribute 's'

It seems that some situations in which the AST cannot be parsed are passing undetected.

Always consider opening an issue first to describe your problem, so we can discuss what is the best way to amend it. Note that if you do not describe the goal of this change or link to a related issue, the maintainers may close the PR without further review.

If your pull request makes a non-insignificant change to Pipenv, such as the user interface or intended functionality, please file a PEEP.

https://github.com/pypa/pipenv/blob/master/peeps/PEEP-000.md

The fix

How does this pull request fix your problem? Did you consider any alternatives? Why is this the best solution, in your opinion?

The checklist

  • [ ] Associated issue
  • [ ] A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

albertoandreottiATgmail avatar Sep 15 '22 21:09 albertoandreottiATgmail

Thank you! This PR should be opened against https://github.com/sarugaku/requirementslib.

oz123 avatar Sep 15 '22 21:09 oz123

@oz123 is correct and there is a ticket over there too for this already. I started a PR once around it, but never finished it. We should alert the user when we were unable to parse the AST (likely because it used string interpolation).

matteius avatar Sep 15 '22 22:09 matteius

Also this: https://github.com/pypa/pipenv/issues/5167

matteius avatar Sep 15 '22 22:09 matteius

Hello, thank you so much for the comments. Any advice on how to avoid running into this problem?

albertoandreottiATgmail avatar Sep 16 '22 00:09 albertoandreottiATgmail

@albertoandreottiATgmail Yes -- the problem is the requirementslib setup.py parser is designed to not execute any code, so likely in the install_requires, or other string being parsed, its an f-string or using format string, which is interpolation and would require executing arbitrary code to evaluate. If the affected library could change to use a basic string without interpolation then you would not get this error. In requirementslib we should detect it and alert the user as to this cause. Once we do that, we will vendor in the new requirementslib and pipenv would output the more reasonable error.

matteius avatar Sep 16 '22 03:09 matteius

@albertoandreottiATgmail This branch outdates this PR and should also make the desired improvement, if you have spare cycles and could check: https://github.com/pypa/pipenv/pull/5793

matteius avatar Aug 07 '23 01:08 matteius