Fail gracefully at parsing setup.py with no deps.

[bennati]

Raise an exception in 'get_requirements_from_python_manifest' only if dependencies are defined in 'setup.py'.

Signed-off-by: Stefano Bennati [email protected]

Mentions: #116

[TG1999]

@bennati please update your branch

[bennati]

@TG1999 I rebased the branch, is this what you were asking?
The tests complain about the signed-off message, but that looks fine to me. Any idea of what is wrong?

[AyanSinhaMahapatra]

@bennati see https://github.com/nexB/python-inspector/pull/117/checks?check_run_id=10682725875:

Expected "Bennati, Stefano [[email protected]](mailto:[email protected])", but got "Stefano Bennati [[email protected]](mailto:[email protected])".

which is very minor, setting it to pass.

Also the failing tests are from pycodestyle, can you run make check locally and verify?

[shricodev]

@bennati I also had this issue yesterday. Running these included the following sign-off message in my earlier commit:
HEAD~1 signs off the previous commit.

git rebase HEAD~1 --signoff
git push --force-with-lease origin <your_branch_name>

[bennati]

I ran isort but tests keep failing with
ERROR: /home/vsts/work/1/s/tests/data/setup/no-direct-dependencies-setup.py Imports are incorrectly sorted and/or formatted..
Any suggestions?

[TG1999]

@bennati just run make valid and this will fix your failing tests.

[TG1999]

@bennati please add some tests to showcase the changes you have done fixes the said issue

[bennati]

Is there anything left before we can merge this?

[TG1999]

@bennati please remove the unrelated changes and update your tests.

[pombredanne]

Thanks!
I wonder if we could do better wrt. using the AST parsing instead?

[bennati]

Can we please merge this one?? It's been open for more than a month now.

[pombredanne]

@bennati Thaanks for using an AST... I reviewed this in details Friday and did not post a comment then. I realized that there may be a lot of similarities (and opportunities to reuse code and update the code as needed) with https://github.com/nexB/python-inspector/blob/main/src/_packagedcode/pypi_setup_py.py that's already doing some of the basic features and is used in https://github.com/nexB/python-inspector/blob/9e765ec50ed4c30eca20ef56686c9bcbe07fb89e/src/_packagedcode/pypi.py#L1557 ?

[bennati]

@pombredanne Great that there is some code already for processing the AST. I'd be happy to refactor this code to maximize reuse, as part of a separate PR. Can we merge this? Thanks!

[pombredanne]

You are missing one case which is when there are install_requires that are processed dynamically commonly either loaded from a requirements file (that we do not handle and needs "insecure loading") or from a variable (that we handle and that would be regression See https://github.com/nexB/python-inspector/blob/9e765ec50ed4c30eca20ef56686c9bcbe07fb89e/src/_packagedcode/pypi_setup_py.py#L155 )

But I ran a test with this setup.py and it works, so there is no issue:

from distutils.core import setup

reqs = ["boolean.py"]

setup(
    name="foo",
    version="0.3.0",
    install_requires=reqs,
)

[pombredanne]

LGTM!
(just a reminder for the future to make the commit messages a bit more explicit or to squash them).
I am merging this now. Thanks!

[bennati]

@pombredanne Thanks! Yeah I meant to squash the commits, next time I'll improve the commit messages.