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

Support pyproject.toml-style configuration (PEP 621) - Round 2 #2970

Merged
merged 63 commits into from Mar 24, 2022

Conversation

abravalheri
Copy link
Contributor

@abravalheri abravalheri commented Dec 25, 2021

Summary of changes

Based on the experience acquired in #2924, I created this PR as a simplification (please check the discussion there for details).
There are many individual steps, but the following image illustrates how they are organised in 4 main groups of stacked changes:

a) abravalheri/setuptools@1a1397a...4a98608: Update build_meta to use dist objects instead of simply running setup.py
b) abravalheri/setuptools@4a98608...7fe7a41: Separate setup.cfg parsing and extract common post-processing functions

(new vendorised packages)

c) abravalheri/setuptools@fab3b79...5666556: Add means to apply configuration from pyproject.toml files
d) abravalheri/setuptools@7fe7a41...d34794d: Integrate pyproject.toml configuration into existing classes

Finally there is a completely non-required group of changes that only adds some type hints to improve (my) level of confidence in the implementation of the setuptools.config sub-package:
e) abravalheri/setuptools@d34794d...0aff4ea: Add some type hints to the setuptools.config subpackage

image

Unfortunately GitHub does not provide tools to facilitate the review of stacked changes/PRs, but I am open to suggestions on how to facilitate the review. I hope that this organisation in groups makes sense and help in the process.

I also have another branch with a proof-of-concept where I remove setuptools' own setup.cfg and add all the existing configurations in the pyproject.toml.

Closes #1688, #2671 (mostly... see discussion bellow).

This PR adds 2 vendorised dependencies: tomli and validate-pyproject.

Pull Request Checklist

@abravalheri
Copy link
Contributor Author

abravalheri commented Dec 25, 2021

Differences from #2924

In #1688 the proposed approach (which have some level of consensus) is to deprecate configuration via setup.cfg files but keep supporting them by automatically converting setup.cfg into pyproject.toml and then proceed with the parsing.

In PR #2924, I try to not only completely follow this approach but also added a series of safe-guards to catch regression problems and ensure backward compatibility. This made the PR very complex and not review-friendly...

My proposal now is to avoid performing this automatic conversion, because it reduces complexity. The internals of setuptools are not prepared to deal with metadata and configs in the way PEP 621 specifies, so the automatic conversion seems a bit pointless, as if we go to the from the INI-state-space to TOML-state-space and then back to something very similar to the INI-state-space.

I think the best would be first re-working the internals of setuptools to use a more modern version of the Core Metadata spec, and then proceed with the automatic conversion.

Another difference is that in #2924 I try to use the latest version of the Core Metadata spec as "lingua franca" between pyproject.toml and the existing way of configuring setuptools. So it should also be possible to scavenge the code in #2924 to help updating support for Core Metadata (but that is left as a future work).

Please note I am providing an external package (abravalheri/ini2toml) anyway to assist in the migration. This package can still be used in the future.

@abravalheri
Copy link
Contributor Author

abravalheri commented Dec 25, 2021

Setuptools-specific tool table in pyproject.toml

As many have previously pointed out PEP 621 does not specify many of the configuration fields required by setuptools to work properly, which seems to indicate that inevitably the users would have to use a non-standardised [tool.setuptools] sub-table in pyproject.toml (and that would "undermine" the efforts for migrating the configuration format).

I would argue that, this is not necessarily the case. As per the implementation in this PR, any configuration coming from pyproject.toml will consider include_package_data=True when the field is absent, as opposed to the behaviour in setup.cfg (which is preserved for backward compatibility).

If we sum to this the changes proposed in #2894 for automatically auto-discovering flat and src-layouts, I believe that most of the packages will be able to use pyproject.toml without a [tool.setuptools] sub-table. (Users will still be able to use it for further customisations or to declare dynamic values to be backfilled by setuptools).

The equivalence between setup.cfg and pyproject.toml and the intuition to convert between them is explained in the ini2toml documentation.

@abravalheri
Copy link
Contributor Author

abravalheri commented Dec 25, 2021

Proof-of-concept converting setuptools' own setup.cfg file to pyproject.toml and blockers

As I previously mentioned, in this branch a version of setuptools that does not have a setup.cfg (but instead uses pyproject.toml for its own configuration) can be found.

I decided to not include it in this PR because of the following blockers:

  1. I believe that the scripts in the tools/ folder that automate releases currently can only handle setup.cfg (e.g. when bumping the version).
  2. Using a top-level table for pytest in pyproject.toml offends PEP 518 jaraco/pytest-enabler#4 (which will cause errors when setuptools runs the validations based on validate-pyproject

@abravalheri
Copy link
Contributor Author

Future Works

The following tasks/themes can be further explored in future PRs (either by me or other member of the community):

  • Include documentation about how to use pyproject.toml to configure the distribution
  • Update the version of Core Metadata produced by setuptools.

(These were excluded from this PR to avoid increasing even more its size).

@abravalheri

This comment has been minimized.

@abravalheri abravalheri marked this pull request as ready for review December 25, 2021 20:33


def find_packages(
*, namespaces=False, root_dir: Optional[_Path] = None, **kwargs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should namespaces default to True here? (It makes sense since implicit namespaces are standardised with a PEP...)

We can implement it in a backwards compatible way by tweaking setuptools.config.setupcfg, this way configuration coming from setup.cfg files or setup.py will still default to namespaces=False, but new configuration coming from pyproject.toml will default to namespaces=True...

Copy link
Member

Choose a reason for hiding this comment

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

I have been wanting to transition to supporting namespace packages by default, so +1 to the proposal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was suggested by @webknjaz that we address this transition in a follow up comment.

value = _expand_dynamic(dynamic_cfg, field, package_dir, root_dir, silent)
project_cfg[field] = value

if "version" in dynamic and "version" in dynamic_cfg:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This value can be given by extensions such as setuptools-scm, so we don't have to fail if no configuration for version is given in [tool.setuptools.dynamic].

@@ -0,0 +1,330 @@
"""Utility functions to expand configuration directives or special values
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was originally extracted from the existing implementation of setuptools.config and after that some improvements were added (e.g. allowing explicitly specifying a project root directory)

@marvinpfoertner
Copy link

@abravalheri First of all, thank you for taking care of this, this is a feature I've been wanting for a while now :) If you don't mind me asking: what is the status of this PR? I plan on integrating this into my projects as soon as possible.

@abravalheri
Copy link
Contributor Author

abravalheri commented Jan 24, 2022

Hi @marvinpfoertner thanks for the kind words. Since this is a complex change in setuptools, I am waiting for when other maintainers might have some time to review it.

If you want to help, you are more than welcome to leave your review also 😄


I will rebase the code later this week with the latest version of main to get rid of the conflict files.

@marvinpfoertner
Copy link

Hi @marvinpfoertner thanks for the kind words. Since this is a complex change in setuptools, I am waiting for when other maintainers might have some time to review it.

If you want to help, you are more than welcome to leave your review also smile

I will rebase the code later this week with the latest version of main to get rid of the conflict files.

Thanks for the quick reply! :) I'm not sure if I can contribute anything with my review, since I have absolutely no idea about the internals of setuptools 😀

Comment on lines 10 to 11
Please note that the legacy backend is not guaranteed to work with
``pyproject.toml`` configuration.
Copy link
Member

@pradyunsg pradyunsg Jan 24, 2022

Choose a reason for hiding this comment

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

Can we instead make it a clean unambigous failure? One of the frustating properties of working with distutils/setuptools is that there are a lot of ways to get subtle failures / accidental behaviours, and it'd be nice to eliminate a potential class of issues here by simply making it an explicit failure.

We can aways relax things later, if we so choose, so it'd be nice to start with strict and clear boundaries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much for the review @pradyunsg.

I have nothing against raising a proper exception here. Just please note that right now there are no intrinsic problems with using pyproject.toml configurations with the legacy backend, I just wanted to make explicit that this is not part of the supported interface.

That aside, do you have any recommendation on how to implement this? Right now the configuration files are being read inside setuptools.dist:Distribution.parse_config_files, which is unaware about which backend was chosen.

For the "non-legacy" backend, in this PR I managed to avoid the "blind execution" of the setup.py script, so we can have more control about how/when all these functions are called. However for the "legacy" backend I purposefully wanted to keep the execution as backwards compatible as possible, so all parsing related functions are invoked via distutils.core, which is not easily changeable...

Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

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

Overall, this looks good to me. I haven't had a chance to review it in depth, and I don't know when I will, but I'd like to get it out so users can experiment with it, but also to identify any regressions in the existing behaviors.

How difficult would it be to present this feature as opt-in, so the new behavior isn't available by default but is only available with a feature flag, like SETUPTOOLS_ENABLE_FEATURES="PEP 621". That way, users can experiment with it but would recognize that the feature isn't universally available.

@KOLANICH
Copy link
Contributor

KOLANICH commented Jan 30, 2022

I'm against of putting it under an env variable. If it is not ready, it shouldn't land into main. If it is ready, it should be just available for everyone who has installed the needed version of setuptools. Installing any version having this feature, be it installing a version from this branch, or installing one from main or installing any release supporting this feature, is by itself the opt-in.

@blink1073
Copy link
Contributor

blink1073 commented Jan 30, 2022

If it were behind an env variable, would that not affect a consumer of an sdist unless they also knew to set that variable?

@jaraco
Copy link
Member

jaraco commented Jan 31, 2022

If it were behind an env variable, would that not affect a consumer of an sdist unless they also knew to set that variable?

Yes that's right. It would only be available for local testing/experimentation. I'd like to validate the use-case against some packages before users come to depend on it.

I'm against of putting it under an env variable. If it is not ready, it shouldn't land into main. If it is ready, it should be just available for everyone who has installed the needed version of setuptools. Installing any version having this feature, be it installing a version from this branch, or installing one from main or installing any release supporting this feature, is by itself the opt-in.

If we go with that approach, users could upload packages that depend on the feature. Users who happen to use the feature before it's stable might find those packages no longer install if a later version removes the feature or makes a breaking change to it. I just want to emphasize the message that this feature is experimental and projects shouldn't depend on it until it's proven stable.

@KOLANICH
Copy link
Contributor

KOLANICH commented Jan 31, 2022

If we go with that approach, users could upload packages that depend on the feature.

The same way they can upload packages depending on libs that are non-public.

Users who happen to use the feature before it's stable might find those packages no longer install if a later version removes the feature or makes a breaking change to it.

  1. it is OK, the features not yet landed into a release are explicitly unstable.
  2. Even if they had landed, it would be OK to break compatibility if it is really needed.

I just want to emphasize the message that this feature is experimental and projects shouldn't depend on it until it's proven stable.

We can emit a warning to make everyone aware the feature is unstable.

I personally am going to convert all my projects to PEP 621 as soon as the feature appears in main (if editable install is working). In an unlikely scenario of anything changing I will just write a script fixing all my projects. I have already done things like that.

@agronholm
Copy link
Contributor

Ok, I tried this again with the latest changes, and the build passed. I got two warnings though:

warning: check: missing required meta-data: url
warning: check: missing meta-data: either (author and author_email) or (maintainer and maintainer_email) should be supplied

I have a bunch of project URLs defined – is "url" really required?
Also, I have this: authors = [{name = "Alex Grönholm", email = "alex.gronholm@nextday.fi"}], so what is it complaining about?

@moshez
Copy link

moshez commented Mar 6, 2022

@agronholm -- I got the same warnings.

Note that the package is created correctly: https://github.com/moshez/pyproject-only-example/tree/a79f89201915a4c2ac1cf2c14f370cc29449884d resulted in https://pypi.org/project/orbipatch/2022.3.6.1/ (note that the Homepage and author links are correct). The warnings are spurious.

@agronholm
Copy link
Contributor

Yup, I was just looking at the generated PKG-INFO when you posted that. Indeed it seems the metadata is generated correctly.

@abravalheri
Copy link
Contributor Author

abravalheri commented Mar 7, 2022

Hi @moshez and @agronholm , thank you very much for testing it out!

The warning with the author is related to this: pypa/distutils#116

I still need to have a look on the URL one 😅

@moshez
Copy link

moshez commented Mar 7, 2022

@abravalheri -- thank you for the explanation, link, and context. In order to avoid multiple people reporting the same issues, do you think it would make sense to edit your top-level post in https://discuss.python.org/t/help-testing-experimental-features-in-setuptools/13821 with "Known issues:"?

@abravalheri
Copy link
Contributor Author

Hi @agronholm, I am investigating the url warning.

Could you tell me what are the URLs you have defined? I think this warning also comes directly from distutils if you don't have a "homepage". (It can be "Home-page", "homepage", "home-page" etc...)

@abravalheri
Copy link
Contributor Author

@abravalheri
Copy link
Contributor Author

To those following this PR:

Thank you very much for all the feedback!
As some might have noticed, I have created a series of follow-up PRs that include some initial docs, better adherence to PEP 621 in terms of the handling of the license field, auto-discovery, etc... These PRs were merged to experimental/support-pyproject.

My plan is to merge experimental/support-pyproject into main soon (which should automatically close this PR), but preserve the experimental status for the new features, so we can still react to user feedback and examine more thoroughly the design of the [tools.setuptools] table.

@abravalheri abravalheri merged commit 7ff3dc1 into pypa:main Mar 24, 2022
willbarton added a commit to cfpb/wagtail-treemodeladmin that referenced this pull request Dec 20, 2022
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

This change also enables support for Wagtail 3.x by removing the `<3` version pin. This will result in the last release of TreeModelAdmin to support Wagtail < 4.

Wagtail 4.x is not yet supported.

The version is bumped to 1.5.0 for anticipated release to permit installing with Wagtail 3.x.
willbarton added a commit to cfpb/wagtail-treemodeladmin that referenced this pull request Dec 20, 2022
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).

This change also enables support for Wagtail 3.x by removing the `<3` version pin. This will result in the last release of TreeModelAdmin to support Wagtail < 4.

Wagtail 4.x is not yet supported.

The version is bumped to 1.5.0 for anticipated release to permit installing with Wagtail 3.x.

I’ve also brought in some quality-of-life improvements for coverage checking from the Django-Flags tox/GHA setup.
willbarton added a commit to cfpb/wagtail-treemodeladmin that referenced this pull request Dec 20, 2022
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).

This change also enables support for Wagtail 3.x by removing the `<3` version pin. This will result in the last release of TreeModelAdmin to support Wagtail < 4.

Wagtail 4.x is not yet supported.

The version is bumped to 1.5.0 for anticipated release to permit installing with Wagtail 3.x.

I’ve also brought in some quality-of-life improvements for coverage checking from the Django-Flags tox/GHA setup.
willbarton added a commit to cfpb/wagtail-treemodeladmin that referenced this pull request Dec 20, 2022
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).

This change also enables support for Wagtail 3.x by removing the `<3` version pin. This will result in the last release of TreeModelAdmin to support Wagtail < 4.

Wagtail 4.x is not yet supported.

The version is bumped to 1.5.0 for anticipated release to permit installing with Wagtail 3.x.

I’ve also brought in some quality-of-life improvements for coverage checking from the Django-Flags tox/GHA setup.
willbarton added a commit to cfpb/wagtail-treemodeladmin that referenced this pull request Dec 20, 2022
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).

This change also enables support for Wagtail 3.x by removing the `<3` version pin. This will result in the last release of TreeModelAdmin to support Wagtail < 4.

Wagtail 4.x is not yet supported.

The version is bumped to 1.5.0 for anticipated release to permit installing with Wagtail 3.x.

I’ve also brought in some quality-of-life improvements for coverage checking from the Django-Flags tox/GHA setup.
willbarton added a commit to cfpb/wagtail-treemodeladmin that referenced this pull request Dec 20, 2022
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).

This change also enables support for Wagtail 3.x by removing the `<3` version pin. This will result in the last release of TreeModelAdmin to support Wagtail < 4.

Wagtail 4.x is not yet supported.

The version is bumped to 1.5.0 for anticipated release to permit installing with Wagtail 3.x.

I’ve also brought in some quality-of-life improvements for coverage checking from the Django-Flags tox/GHA setup.
willbarton added a commit to cfpb/wagtail-flags that referenced this pull request Jan 11, 2023
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).
willbarton added a commit to cfpb/wagtail-flags that referenced this pull request Jan 11, 2023
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).
willbarton added a commit to cfpb/wagtail-flags that referenced this pull request Jan 11, 2023
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).
willbarton added a commit to cfpb/wagtail-flags that referenced this pull request Jan 12, 2023
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).

Co-authored-by: Andy Chosak <andy.chosak@cfpb.gov>
willbarton added a commit to cfpb/wagtail-inventory that referenced this pull request Feb 27, 2023
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).

This change also continues support for Wagtail 3.
willbarton added a commit to cfpb/wagtail-inventory that referenced this pull request Feb 27, 2023
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).

This change also continues support for Wagtail 3.
willbarton added a commit to cfpb/wagtail-inventory that referenced this pull request Mar 1, 2023
With [PEP 621 support added to setuptools](pypa/setuptools#2970) last year, and with pip supporting editable installs as of 21.3, we can move to `pyproject.toml` and deprecate `setup.py`. The file remains for legacy pip support.

Because [flake8 does not want to support pyproject.tml yet](PyCQA/flake8#234), unlike all the other tools we use, I’m removing it in favor of [ruff](https://github.com/charliermarsh/ruff).

This change also continues support for Wagtail 3.

Co-authored-by: Andy Chosak <andy.chosak@cfpb.gov>
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.

Discussion: support for pyproject.toml configuration