Skip to content

Allow Python projects that only have a setup.py#5324

Closed
DanielNoord wants to merge 1 commit intodependabot:mainfrom
DanielNoord:patch-1
Closed

Allow Python projects that only have a setup.py#5324
DanielNoord wants to merge 1 commit intodependabot:mainfrom
DanielNoord:patch-1

Conversation

@DanielNoord
Copy link

Closes #4483
Based on the solution provided by @jurre in #4483 (comment).

As I said in #4483 (comment) if I could get some guidance I'd happily provide tests or additional information, but I have no experience with Ruby so I don't really know how to.

However, as the Python ecosystem is moving away from setup.py I really think this change should be landed. Hopefully by providing the PR we can get some traction on the issue. If this is not the preferred way to do so, please feel free to close.

@DanielNoord DanielNoord requested a review from a team as a code owner June 29, 2022 07:55
@jurre
Copy link
Member

jurre commented Jul 14, 2022

Thanks @DanielNoord, I can look into adding a test for this ^^

@jurre
Copy link
Member

jurre commented Jul 14, 2022

While trying to reproduce this in order to write a test, and I can't seem to 😅 do yo have a repo handy where this is happening?

@DanielNoord
Copy link
Author

DanielNoord commented Jul 14, 2022

Shameless plug of my own repo:

https://github.com/DanielNoord/pydocstringformatter 😄

The base repositories of astroid and pylint which can be found on my profile also showed this behaviour whenever you remove setup.py.

Edit: @jurre Not sure if you have need any privileges to get access to logs of dependabot runs. But if so, please give a shout. I'll gladly add you to get this resolved 😄

@jurre
Copy link
Member

jurre commented Jul 15, 2022

Thanks! I can just point a local version dependabot at those public repo's to debug further, this should get me unstuck 👍

@jurre
Copy link
Member

jurre commented Jul 15, 2022

@DanielNoord should the title of this PR be: Allow Python projects that have a pyproject.toml + setup.cfg? That seems to be what causes the error I think? Only a pyproject is fine, only a setup.cfg is fine also, but the combination causes an issue?

@DanielNoord
Copy link
Author

DanielNoord commented Jul 15, 2022

@DanielNoord should the title of this PR be: Allow Python projects that have a pyproject.toml + setup.cfg? That seems to be what causes the error I think? Only a pyproject is fine, only a setup.cfg is fine also, but the combination causes an issue?

I haven't tested that, but that could be the case.

To give some background. The ecosystem is moving towards putting everything in pyproject.toml but many of the build tools don't fully support that yet. The traditional setup was to have setup.py or setup.cfg or both. Because of the slow move towards pyproject.toml I think now all 6 possible combinations of the three files should be allowed.

The title should probably be Allow Python projects that do not have a "setup.py" as 1) pyproject.toml, 2) setup.cfg and 3) pyproject.toml + setup.cfg should all be allowed. I'm not sure about the current support of case 1 and 2, but I think after this PR all 3 should be okay.

@jurre
Copy link
Member

jurre commented Jul 15, 2022

Thanks for that context! It seems like only a pyproject.toml and only a setup.cfg are already working as expected, there's tests for those cases as well:

context "with only a pyproject.toml" do

context "with only a setup.cfg file" do

@jurre
Copy link
Member

jurre commented Jul 15, 2022

Ah ok so for your repo it seems Dependabot wants to fetch a setup.py because there is a requirements-test.txt requirements file, and it assumes this needs a setup.py, if I understand correctly this is not or no longer true?

@DanielNoord
Copy link
Author

Ah ok so for your repo it seems Dependabot wants to fetch a setup.py because there is a requirements-test.txt requirements file, and it assumes this needs a setup.py, if I understand correctly this is not or no longer true?

No! I think there is no case where setup.py is necessary any more.

See:
https://setuptools.pypa.io/en/latest/userguide/quickstart.html#setup-py

This is the tool that originally created the need for setup.py. They actively discourage creating a setup.py for any new project and only recommend using setup.py whenever you need "editable installs". This last case is almost moot as well because of this beta feature that is almost ready for release: https://discuss.python.org/t/help-testing-pep-660-support-in-setuptools/16904/

Dependabot shouldn't really care about whether somebody wants "editable installs" so as far as I can see there is no need for any type of checking whether a setup.py file exists. Based on my own experience and the documentation there is no case where it is required for dependency resolvement or installation of a package.

@jurre
Copy link
Member

jurre commented Jul 15, 2022

there is no need for any type of checking whether a setup.py file exists

Yeah I agree, however this code is currently lumped in with the code that pulls in path dependencies, so it's a little tricky to get it to always behave correctly, and I don't think my proposed fix really cuts it, because it assumes there is always a setup.py or a setup.cfg when there is a requirements.*.txt present, and if I understand correctly this is also not correct?

@DanielNoord
Copy link
Author

there is no need for any type of checking whether a setup.py file exists

Yeah I agree, however this code is currently lumped in with the code that pulls in path dependencies, so it's a little tricky to get it to always behave correctly, and I don't think my proposed fix really cuts it, because it assumes there is always a setup.py or a setup.cfg when there is a requirements.*.txt present, and if I understand correctly this is also not correct?

No, that's indeed not correct. I'm trying to find an example of a package that doesn't have this but I can't quickly find one. Probably because many project have kept either of those files around to make sure they keep getting dependabot updates 😄

@jurre
Copy link
Member

jurre commented Jul 19, 2022

@DanielNoord I had to dig a bit deeper and eventually was able to fix it here: #5392

Would you mind taking a look at this, especially interested if I've described the problem correctly and if my assumptions on the pip side are correct.

@DanielNoord
Copy link
Author

I'll close this as it is superseded by #5392.

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.

Dependabot trying to download setup.py after move to pyproject.toml

2 participants