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

Improve detection of unparsable ASTs #5357

Closed

Conversation

albertoandreottiATgmail

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 #.

@oz123
Copy link
Contributor

oz123 commented Sep 15, 2022

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

@matteius
Copy link
Member

@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
Copy link
Member

Also this: #5167

@albertoandreottiATgmail
Copy link
Author

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

@matteius
Copy link
Member

@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 matteius mentioned this pull request Aug 5, 2023
2 tasks
@matteius
Copy link
Member

matteius commented Aug 7, 2023

@albertoandreottiATgmail This branch outdates this PR and should also make the desired improvement, if you have spare cycles and could check: #5793

@matteius matteius closed this Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants