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

DO NOT MERGE: trying setuptools PR #590

Closed
wants to merge 7 commits into from

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Feb 9, 2022

  • chore: delete setup.py
  • DO NOT MERGE: trying PEP 621 directly
  • DO NOT MERGE: try full PEP 621

Copy link

@abravalheri abravalheri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @henryiii, thank you very much for testing this out!

I admit the errors are far from understandable... I have to review what is going on. Ideally the validation phase should tell you how to change pyproject.toml accordingly. (I have to revisit https://github.com/abravalheri/validate-pyproject, I suspect I forgot to forbid non-standardised keys to the project table, as was the case here with author and requires).

Could you try rebuilding with my suggestions? I am curious to see if the tests pass...

Something that might be interesting to you in future experiments: https://github.com/abravalheri/ini2toml can help to bootstrap pyproject.toml from setup.cfg (and then it can be further edited)

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Co-authored-by: Anderson Bravalheri <andersonbravalheri+github@gmail.com>
@coveralls
Copy link

coveralls commented Feb 10, 2022

Coverage Status

Coverage remained the same at 83.599% when pulling 110c22f on henryiii/experimental/setuptools621 into 3bca2d4 on master.

@henryiii
Copy link
Collaborator Author

Thanks, that seems to be it! It was being very picky about other things, so I assumed it would be catching things like that, so didn't check as carefully. It does check for extra keys in the tool.setuptools section, which is very helpful.

Something that might be interesting to you in future experiments: https://github.com/abravalheri/ini2toml

Yes, I noticed that after I did this, but this was a good experience (and looks like it might have been helpful!)

@henryiii
Copy link
Collaborator Author

Might be expected, but setuptools_scm is not working, 0.1.dev1 is the version.

@abravalheri
Copy link

looks like it might have been helpful!

Definitely! It helped to find a problem in validate-pyproject, thanks ❤️

Might be expected, but setuptools_scm is not working, 0.1.dev1 is the version.

I will try to have a look on this later this weekend, thank you very much for pointing it out. In theory it should be no different. I have to double check if all the plugin hooks in setuptools are being called...

@abravalheri
Copy link

Might be expected, but setuptools_scm is not working, 0.1.dev1 is the version.

Hi @henryiii, could you elaborate a bit more what is happening?
When I clone your fork in my machine and run python -m build, I get dist/plumbum-1.7.3.dev31+g110c22f-py3-none-any.whl.

I also can verify

$ unzip -c dist/plumbum-1.7.3.dev31+g110c22f-py3-none-any.whl plumbum/version.py                         
Archive:  dist/plumbum-1.7.3.dev31+g110c22f-py3-none-any.whl
  inflating: plumbum/version.py
# coding: utf-8
# file generated by setuptools_scm
# don't change, don't track in version control
version = '1.7.3.dev31+g110c22f'
version_tuple = (1, 7, 3, 'dev31', 'g110c22f')

and

$ python -m setuptools_scm
1.7.3.dev31+g110c22f

Is it supposed to work differently?

@henryiii
Copy link
Collaborator Author

henryiii commented Feb 10, 2022

I'm looking at https://github.com/tomerfiliba/plumbum/runs/5141354158?check_suite_focus=true

Ahhh! That version number is broken on master. Sorry for the noise! I hate "try not to break things" defaults... (I think it works on releases, just not random builds)

@abravalheri
Copy link

0.1.dev1 is the version

Now I see that the Dist job is creating a plumbum-0.1.dev1+g789344a-py3-none-any.whl file. So that explains the 0.1.dev1 😅

I wonder if that is happening because of a shallow clone?

Locally after cloning your fork and switching to the henryiii/experimental/setuptools621 branch, I can successfully run:

$ git tag '1.8'
$ python -m build
...
Successfully built plumbum-1.8.tar.gz and plumbum-1.8-py3-none-any.whl
$ unzip -c dist/plumbum-1.8-py3-none-any.whl plumbum/version.py
Archive:  dist/plumbum-1.8-py3-none-any.whl
  inflating: plumbum/version.py
# coding: utf-8
# file generated by setuptools_scm
# don't change, don't track in version control
version = '1.8'
version_tuple = (1, 8)

I also tried replicating the Dist job by running

$ rm -rf dist build plumbum.egg-info
$ pipx run build
...
Successfully built plumbum-1.8.tar.gz and plumbum-1.8-py3-none-any.whl

and the outcome is similar.

@abravalheri
Copy link

Ahhh! That version number is broken on master.

Perfect, thank you very much for the feedback!
Glad to see I didn't break a lot of stuff in my PR 😅

@henryiii
Copy link
Collaborator Author

Yeah, fix in #591 - checkout@v2 does shallow clones, unlike v1, and this seems to have been using v2 but assuming v1. It was probably not noticed because releases are on tags, and setuptools_scm is happy then. I'm guessing running this in act would have also had the shallow clones.

Glad to see I didn't break a lot of stuff in my PR

Well, there's still the pybind11 one. ;) There I'm working on cleaning up the Python 3 drop PR, then rebasing the attempt on that.

@abravalheri
Copy link

Well, there's still the pybind11 one. ;) There I'm working on cleaning up the Python 3 drop PR, then rebasing the attempt on that.

I promise I will have a look on that soon 😄
Thank you very much for the help in testing it!

pyproject.toml Outdated
@@ -1,11 +1,79 @@
[build-system]
requires = [
"setuptools>=42",
"setuptools @ git+https://github.com/abravalheri/setuptools@support-pyproject",
Copy link

@abravalheri abravalheri Feb 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@henryiii, you can also try git+https://github.com/pypa/setuptools@experimental/support-pyproject now.

However this branch also adds automatic discovery for packages (which supports src-layout without config) and considers implicit namespaces by default (PEP 420). So the experiments folder might end-up being included in the wheel (I suspect that previously it was not being included because setuptools predates PEP 420).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abravalheri
Copy link

Thank you very much @henryiii for updating the PR.

I think the problems in the CI are probably related to the fact setuptools supports Python >= 3.7 (and the vendored version of tomli uses features that don't exist in 3.6... The PyPy error seems a bit flaky isn't it?

I just pushed some changes to the experimental branch related to the feedback I got from Python's Discourse. Hopefully everything will keep working :)

@henryiii henryiii closed this Jan 2, 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.

None yet

3 participants