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

build: Migrate from setup.py to pyproject.toml #566

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

adrienverge
Copy link
Owner

@adrienverge adrienverge commented Apr 14, 2023

Using setup.py is deprecated and the new recommanded way is to declare a pyproject.toml file (see PEP 517 1).

This commit proposes to use setuptools to achieve that, because setuptools is already used by yamllint, is standard and referenced by the official Python packaging documentation 2, and seems to be the most lightweight solution. An alternative could have been to use Poetry, see the dedicated pull request and discussion 3.

For some period, the setup.py file will be kept (although almost empty), to allow old tools versions to keep working.

Closes #509.


Changes to RPM package
@@ -28,18 +28,19 @@ yamllint does not only check for syntax validity, but for weirdnesses lik>
 repetition and cosmetic problems such as lines length, trailing spaces,
 indentation, etc.
 
+%generate_buildrequires
+%pyproject_buildrequires
+
 %prep
 %autosetup -n %{pypi_name}-%{version} -p1
 # Remove bundled egg-info
 rm -rf %{pypi_name}.egg-info
 
 %build
-%py3_build
+%pyproject_wheel
 
 %install
-# Must do the subpackages' install first because the scripts in /usr/bin are
-# overwritten with every setup.py install.
-%py3_install
+%pyproject_install
 
 %check
 %{__python3} -m unittest discover

Footnotes

  1. https://peps.python.org/pep-0517/

  2. https://packaging.python.org/en/latest/tutorials/installing-packages/

  3. https://github.com/adrienverge/yamllint/pull/557

Using `setup.py` is deprecated and the new recommanded way is to declare
a `pyproject.toml` file (see PEP 517 [^1]).

This commit proposes to use setuptools to achieve that, because
setuptools is already used by yamllint, is standard and referenced by
the official Python packaging documentation [^2], and seems to be the
most lightweight solution. An alternative could have been to use Poetry,
see the dedicated pull request and discussion [^3].

For some period, the `setup.py` file will be kept (although almost
empty), to allow old tools versions to keep working.

Closes #509.

[^1]: https://peps.python.org/pep-0517/
[^2]: https://packaging.python.org/en/latest/tutorials/installing-packages/
[^3]: #557
@adrienverge
Copy link
Owner Author

@DimitriPapadopoulos @staticdev @ssbarnea @barrelful following your remarks from #509 and #557, could you test this pull request (at least python -m build, pip install and basic yamllint usage) and check whether everything works as you expect? Thanks in advance.

@coveralls
Copy link

Coverage Status

Coverage: 99.384% (+0.2%) from 99.196% when pulling bb8300f on build/pyproject.toml into 16eae28 on master.

@coveralls
Copy link

Coverage Status

Coverage: 99.381% (+0.2%) from 99.196% when pulling bb8300f on build/pyproject.toml into 16eae28 on master.

Copy link
Sponsor Contributor

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

You must remove setup.py or you really fail to implement PEP-621 which is what the title claims. Still, if you want to make the switch in several PRs, it is ok.

@adrienverge
Copy link
Owner Author

@DimitriPapadopoulos @staticdev @ssbarnea @barrelful following your remarks from #509 and #557, could you test this pull request (at least python -m build, pip install and basic yamllint usage) and check whether everything works as you expect? Thanks in advance.

No feedback on this after one week, so I'll merge now.

@adrienverge adrienverge merged commit 15eafeb into master Apr 21, 2023
@adrienverge adrienverge deleted the build/pyproject.toml branch April 21, 2023 12:15
@staticdev
Copy link

@adrienverge as commented by @ssbarnea, you need to delete setup.py to get it working properly.

@adrienverge
Copy link
Owner Author

Have a look at the new setup.py of this pull request:

# This is only kept for backward-compatibility with older versions that don't
# support new packaging standards (e.g. PEP 517 or PEP 660):
setup()

If you know another way to build / install the package with setuptools < 61 and without setup.py and you've tested it to get a working .rpm or .deb on "stable" Linux distributions like CentOS 8 (setuptools == 50; poetry not packaged), CentOS 9 (setuptools == 53) or Ubuntu 2020.04 (setuptools == 45; poetry not packaged), please let me know. Distro packaging is a requirement listed in #557 (comment).

Also it's not too late if you want to try and answer #566 (comment) 👍

@staticdev
Copy link

staticdev commented Apr 22, 2023

@adrienverge I don't have a solution using setuptools, maybe why I don't use it. But I know this is not compliant with PEP-517. To be compliant with all packaging peps, I would say only solution I know is hatch (a middle ground between poetry and setuptools-scm). It was just merged on a lib I am also maintainer: sigmavirus24/github3.py#1142

@adrienverge
Copy link
Owner Author

But I know this is not compliant with PEP-517.

I didn't find anything stating this in PEP 517, could you give more details?

My reading of the PEP suggests that it's OK to keep setup.py as a fallback:

If the pyproject.toml file is absent, or the build-backend key is missing, the source tree is not using this specification, and tools should revert to the legacy behaviour of running setup.py ... Projects may still wish to include a setup.py for compatibility with tools that do not use this spec.

@staticdev
Copy link

My mistake then, but I remember I actually had problems having both when I was packaging. You might have unintended behaviour since it can use setup.py or pyproject.toml as source of truth.

@DimitriPapadopoulos
Copy link
Contributor

But setup.py only contains setup() here. Having a vastly different setup.py or setup.cfg would indeed be an issue.

@staticdev
Copy link

Yes, best thing now since it is merged is observe if there is any problem. The alternative solution in this case could be hatch. I am playing with it right now, looks good.

@staticdev
Copy link

staticdev commented Apr 22, 2023

Have a look at the new setup.py of this pull request:

# This is only kept for backward-compatibility with older versions that don't
# support new packaging standards (e.g. PEP 517 or PEP 660):
setup()

If you know another way to build / install the package with setuptools < 61 and without setup.py and you've tested it to get a working .rpm or .deb on "stable" Linux distributions like CentOS 8 (setuptools == 50; poetry not packaged), CentOS 9 (setuptools == 53) or Ubuntu 2020.04 (setuptools == 45; poetry not packaged), please let me know. Distro packaging is a requirement listed in #557 (comment).

Also it's not too late if you want to try and answer #566 (comment) +1

Wait, also this shouldn't be needed by those distros to install the package, just to build. What do you need to build yamllint in them since Github Actions can do it with latest setuptools?

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented Apr 22, 2023

You're right about it @staticdev. A recent version of setuptools is required to build:

yamllint/pyproject.toml

Lines 43 to 45 in 98f2281

[build-system]
build-backend = "setuptools.build_meta"
requires = ["setuptools >= 61"]

The compatibility setup.py is mostly there for other older tools. Removing it won't break builds on older systems like CentOS 8 because setuptools must be updated before building.

@adrienverge
Copy link
Owner Author

The compatibility setup.py is mostly there for other older tools. Removing it won't break builds on older systems like CentOS 8 because setuptools must be updated before building.

Actually it's not that simple; for example for CentOS 8/9 packages (which I maintain), the only setuptools versions available are 50/53 (and obviously you cannot use pip nor anything from the internet to update anything), so we need a setup.py file (while waiting for newer setuptools version to arrive there).

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented Apr 22, 2023

The setup.py file is an empty shell, which just redirects to setuptools which needs to be able to interpret pyproject.toml. A newer setuptools is required to build from pyproject.toml, isn't it?

@adrienverge
Copy link
Owner Author

adrienverge commented Apr 22, 2023

A newer setuptools is required to build from pyproject.toml, isn't it?

Yes 👍

I already spent some hours on this legacy/stable Linux distro packaging. Please share if you have a fully working .spec file to build the .rpm package for CentOS 8 (for example) where only setuptools == 50 is available (you'll probably have to use the old macros %py3_build and %py3_install) without a setup.py/setup.cfg file, tested and working and with all the files installed at the right place (without UNKNOWN-0.0.0 instead of yamllint-1.31.0 for instance).

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented Apr 22, 2023

Indeed, UNKNOWN-0.0.0is typical of an old setuptools.

The following paragraph from the Python Packaging User Guide is about installing from sources, but clearly it has to apply to building as well:

Ensure pip, setuptools, and wheel are up to date

While pip alone is sufficient to install from pre-built binary archives, up to date copies of the setuptools and wheel projects are useful to ensure you can also install from source archives:

Unless you manage to update to a newer setuptools, you might have to add the old setup.cfg to the RPM spec, as a patch, to build on older platforms 😠

@adrienverge
Copy link
Owner Author

Agreed 👍

For info, the latest setuptools documentation about pyproject.toml recommends what I've done in this commit:

Important

If compatibility with legacy builds or versions of tools that don’t support certain packaging standards (e.g. PEP 517 or PEP 660), a simple setup.py script can be added to your project [1] (while keeping the configuration in pyproject.toml):

from setuptools import setup

setup()

adrienverge added a commit to adrienverge/localstripe that referenced this pull request Nov 30, 2023
Using `setup.py` is deprecated and the new recommanded way is to declare
a `pyproject.toml` file (see PEP 517 [^1]).

This commit proposes to use setuptools to achieve that, because
setuptools is already used by localstripe, is standard and referenced by
the official Python packaging documentation [^2], and seems to be the
most lightweight solution.

For some period, the `setup.py` file will be kept (although almost
empty), to allow old tools versions to keep working.

I have done the same migration for yamllint 7 months ago [^3] and it
appeared to work well.

[^1]: https://peps.python.org/pep-0517/
[^2]: https://packaging.python.org/en/latest/tutorials/installing-packages/
[^3]: adrienverge/yamllint#566
adrienverge added a commit to adrienverge/localstripe that referenced this pull request Nov 30, 2023
Using `setup.py` is deprecated and the new recommanded way is to declare
a `pyproject.toml` file (see PEP 517 [^1]).

This commit proposes to use setuptools to achieve that, because
setuptools is already used by localstripe, is standard and referenced by
the official Python packaging documentation [^2], and seems to be the
most lightweight solution.

For some period, the `setup.py` file will be kept (although almost
empty), to allow old tools versions to keep working.

I have done the same migration for yamllint 7 months ago [^3] and it
appeared to work well.

[^1]: https://peps.python.org/pep-0517/
[^2]: https://packaging.python.org/en/latest/tutorials/installing-packages/
[^3]: adrienverge/yamllint#566
adrienverge added a commit to adrienverge/localstripe that referenced this pull request Nov 30, 2023
Using `setup.py` is deprecated and the new recommanded way is to declare
a `pyproject.toml` file (see PEP 517 [^1]).

This commit proposes to use setuptools to achieve that, because
setuptools is already used by localstripe, is standard and referenced by
the official Python packaging documentation [^2], and seems to be the
most lightweight solution.

For some period, the `setup.py` file will be kept (although almost
empty), to allow old tools versions to keep working.

I have done the same migration for yamllint 7 months ago [^3] and it
appeared to work well.

[^1]: https://peps.python.org/pep-0517/
[^2]: https://packaging.python.org/en/latest/tutorials/installing-packages/
[^3]: adrienverge/yamllint#566
adrienverge added a commit to adrienverge/localstripe that referenced this pull request Nov 30, 2023
Using `setup.py` is deprecated and the new recommanded way is to declare
a `pyproject.toml` file (see PEP 517 [^1]).

This commit proposes to use setuptools to achieve that, because
setuptools is already used by localstripe, is standard and referenced by
the official Python packaging documentation [^2], and seems to be the
most lightweight solution.

For some period, the `setup.py` file will be kept (although almost
empty), to allow old tools versions to keep working.

I have done the same migration for yamllint 7 months ago [^3] and it
appeared to work well.

[^1]: https://peps.python.org/pep-0517/
[^2]: https://packaging.python.org/en/latest/tutorials/installing-packages/
[^3]: adrienverge/yamllint#566
adrienverge added a commit to adrienverge/localstripe that referenced this pull request Nov 30, 2023
Using `setup.py` is deprecated and the new recommanded way is to declare
a `pyproject.toml` file (see PEP 517 [^1]).

This commit proposes to use setuptools to achieve that, because
setuptools is already used by localstripe, is standard and referenced by
the official Python packaging documentation [^2], and seems to be the
most lightweight solution.

For some period, the `setup.py` file will be kept (although almost
empty), to allow old tools versions to keep working.

I have done the same migration for yamllint 7 months ago [^3] and it
appeared to work well.

[^1]: https://peps.python.org/pep-0517/
[^2]: https://packaging.python.org/en/latest/tutorials/installing-packages/
[^3]: adrienverge/yamllint#566
adrienverge added a commit to adrienverge/localstripe that referenced this pull request Nov 30, 2023
Using `setup.py` is deprecated and the new recommanded way is to declare
a `pyproject.toml` file (see PEP 517 [^1]).

This commit proposes to use setuptools to achieve that, because
setuptools is already used by localstripe, is standard and referenced by
the official Python packaging documentation [^2], and seems to be the
most lightweight solution.

For some period, the `setup.py` file will be kept (although almost
empty), to allow old tools versions to keep working.

I have done the same migration for yamllint 7 months ago [^3] and it
appeared to work well.

[^1]: https://peps.python.org/pep-0517/
[^2]: https://packaging.python.org/en/latest/tutorials/installing-packages/
[^3]: adrienverge/yamllint#566
adrienverge added a commit to adrienverge/localstripe that referenced this pull request Nov 30, 2023
Using `setup.py` is deprecated and the new recommanded way is to declare
a `pyproject.toml` file (see PEP 517 [^1]).

This commit proposes to use setuptools to achieve that, because
setuptools is already used by localstripe, is standard and referenced by
the official Python packaging documentation [^2], and seems to be the
most lightweight solution.

For some period, the `setup.py` file will be kept (although almost
empty), to allow old tools versions to keep working.

I have done the same migration for yamllint 7 months ago [^3] and it
appeared to work well.

[^1]: https://peps.python.org/pep-0517/
[^2]: https://packaging.python.org/en/latest/tutorials/installing-packages/
[^3]: adrienverge/yamllint#566
adrienverge added a commit to adrienverge/localstripe that referenced this pull request Dec 1, 2023
Using `setup.py` is deprecated and the new recommanded way is to declare
a `pyproject.toml` file (see PEP 517 [^1]).

This commit proposes to use setuptools to achieve that, because
setuptools is already used by localstripe, is standard and referenced by
the official Python packaging documentation [^2], and seems to be the
most lightweight solution.

For some period, the `setup.py` file will be kept (although almost
empty), to allow old tools versions to keep working.

I have done the same migration for yamllint 7 months ago [^3] and it
appeared to work well.

[^1]: https://peps.python.org/pep-0517/
[^2]: https://packaging.python.org/en/latest/tutorials/installing-packages/
[^3]: adrienverge/yamllint#566
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.

setup.pypyproject.toml
5 participants