tomerfiliba / plumbum

Plumbum: Shell Combinators
https://plumbum.readthedocs.io
MIT License
2.81k stars 183 forks source link

DO NOT MERGE: trying setuptools PR #590

Closed henryiii closed 1 year ago

henryiii commented 2 years ago
coveralls commented 2 years ago

Coverage Status

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

henryiii commented 2 years ago

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 commented 2 years ago

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

abravalheri commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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!

abravalheri commented 2 years ago

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 :)