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

[FR] Implement PEP 639 #4629

Open
1 task done
befeleme opened this issue Sep 3, 2024 · 10 comments
Open
1 task done

[FR] Implement PEP 639 #4629

befeleme opened this issue Sep 3, 2024 · 10 comments

Comments

@befeleme
Copy link
Contributor

befeleme commented Sep 3, 2024

What's the problem this feature will solve?

The chaotic land of expressing license metadata in Python packaging will become more unified.

Describe the solution you'd like

See: https://peps.python.org/pep-0639
This will update the Core Metadata version to 2.4, so the previous features, updating them to 2.2 and 2.3, should land in first.

Alternative Solutions

No response

Additional context

A previous draft of the PEP has been already implemented in setuptools.
There's a feature request for another bit of the draft: #3596
The full implementation, including the Core Metadata bump, will require also the support on the side of PyPI first: pypi/warehouse#16620

Code of Conduct

  • I agree to follow the PSF Code of Conduct
@befeleme befeleme added enhancement Needs Triage Issues that need to be evaluated for severity and status. labels Sep 3, 2024
@abravalheri
Copy link
Contributor

abravalheri commented Sep 3, 2024

Thank you very much @befeleme.

Also possibly pending on pypa/packaging#799 + packaging exposing a validation API (unless we skip the validation for now, which was deemed acceptable as per the discussion in the Python Discourse1).

Any member of the community interested in giving it a go is very welcome!

Footnotes

  1. The interpretation that the wording SHOULD implies that backends can skip the validation it if they have good reason to do so.

@abravalheri abravalheri added help wanted and removed Needs Triage Issues that need to be evaluated for severity and status. labels Sep 3, 2024
@abravalheri
Copy link
Contributor

abravalheri commented Sep 5, 2024

@befeleme one thing that I was also commenting some time ago in the discourse (see https://discuss.python.org/t/enforcing-consistent-metadata-for-packages/50008/101 and https://discuss.python.org/t/enforcing-consistent-metadata-for-packages/50008/102) is that I don't believe setuptools is in a condition to adopt version 2.2 of the core metadata, specifically Dynamic.

As discussed in that thread, I don't think we can right now ensure 100% that Dynamic will work as specified in PEP 643. And I also believe that changing setuptools such that we become able to implement that is not trivial, likely causing lots of disruption for existing packages.

So that is why we cannot "bump" into 2.2, and therefore we cannot bump into 2.4 either.

If that is OK, we can adopt PEP 639 without bumping the metadata to 2.4. Or bump the metadata to 2.4 and use Dynamic only as "best-effort" indicator, without providing the guarantees that the PEP 643 wants us to enforce.

/cc @pfmoore @jaraco


One example in which setuptools would have trouble to provide guarantees is when a package has pyproject.toml without [project] and uses a plugin that overwrites metadata. Another situation is if an user adds custom command classes that write PKG-INFO or METADATA (like egg-info, sdist, bdist_wheel, editable_wheel) - this way they have complete control over the contents of the file.

In general setuptools can provide a good indicator if a field is filled dynamically or not, but I don't think we can "guarantee" it in all scenarios1.

Footnotes

  1. Even if the users adopt PEP 621, plugins can still take over and add a custom egg_info command that modifies PKG-INFO/METADATA or redefine a egg_info.writers entrypoint.

@pfmoore
Copy link
Member

pfmoore commented Sep 5, 2024

In the worst case, why can you not simply set every field to be dynamic, except for Name, Version and Metadata-Version? The only complexity for those fields1 is with versions generated from VCS metadata (setuptools-scm) and other backends seem to have managed to handle that.

To be clear, Dynamic is intended as the escape hatch here - it's perfectly acceptable to mark a field as dynamic if you're not sure whether it can vary. It's suboptimal to say that a field is dynamic when it's actually static, of course, but it's not wrong.

Footnotes

  1. Barring code that deliberately violates the rules, for example by generating a random project name on each invocation. I'd be perfectly fine if you simply stated in the documentation that plugins are not allowed to do that. It's not the job of setuptools to prevent buggy plugins from existing...

@pfmoore
Copy link
Member

pfmoore commented Sep 5, 2024

I'm assuming your concern here is with the statement in PEP 643 that says

Backends MUST NOT mark a field as Dynamic if they can determine that it was generated from data that will not change at build time.

That was never intended to make it hard for backends to implement the spec. I was always assuming that setuptools in particular would have a hard time "determining that metadata was generated from data that will not change", and so would default to dynamic.

Maybe the PEP should have omitted that statement. It doesn't add much in practice - it effectively says "if you can do this, you must", which is pretty meaningless given that you wouldn't claim conformance to the spec if you didn't intend to implement it... Luckily it never made it into the formal specification.

@abravalheri
Copy link
Contributor

abravalheri commented Sep 5, 2024

Ok, in that case, it may make sense. Because we can record the calculated values and have the Dynamic marked, we can adopt the approach of marking things as Dynamic "just in case". It is not going to be optimal, but at least we can move on with the other PEPs.

Thank you very much, @pfmoore.

@abravalheri
Copy link
Contributor

abravalheri commented Sep 5, 2024

Just to document what seems to be a viable approach for PEP 643:

  1. Document that plugins are not allowed to dynamically generate inconsistent values for name and version metadata (they may generate these, but the values need to be exactly the same across all builds from both source tree or sdist)
  2. Document that plugins are not allowed to modify values provided statically as metadata in pyproject.toml
  3. When metadata is provided via pyproject.toml, setuptools will attempt to use Dynamic only when an equivalent dynamic is present in pyproject.toml.
  4. When metadata is provided via setup.py, setuptools will add Dynamic for the metadata fields to ensure other tools do not accidentally trust them as static (just in case).

@pfmoore
Copy link
Member

pfmoore commented Sep 5, 2024

Sounds good to me. Are plugins permitted to mess with data read from pyproject.toml? If so, you might have to weaken (3) to say "only if no plugins are present". I'm assuming you at least know if the user has any plugins installed...

Having said that, not respecting the value from pyproject.toml when the field is not marked as dynamic, is a clear violation of PEP 621 right now, and that has nothing to do with sdist metadata or metadata 2.2. I assume you'd deem that to be one of

  • A plugin bug
  • User error, if code in setup.py caused this
  • A setuptols bug

So on reflection, (3) could probably be strengthened to say "When metadata is provided by pyproject.toml, setuptools will mark a field as dynamic only if it is marked as dynamic in pyproject.toml, following PEP 621".

@abravalheri
Copy link
Contributor

abravalheri commented Sep 5, 2024

Are plugins permitted to mess with data read from pyproject.toml?

"Permitted", I don't think so.

But both plugins (via entry-points) and the user (via the cmdclass config) can replace the default "commands" for egg_info, dist_info, sdist, bdist_wheel and editable_wheel, so they effectively change PKG-INFO and METADATA. Plugins can also replace the standard rendering of core metadata via the egg_info.writers entrypoint.

This is an example of a plugin trying to change dependencies: #3982. In this particular case setuptools was able to block the attempt because the developer used the finalize_distribution_options hook. But in the case of custom commands or egg_info.writers, I don't think it would be possible to prevent.

@pfmoore
Copy link
Member

pfmoore commented Sep 5, 2024

Cool. I think your response to the plugin author in the case you linked to was exactly right (and to be fair, the author seemed happy with that response). Ideally, as in this case, setuptools can stop the plugin doing things like that, but I think it's fine if you don't always manage to, and simply document that plugins and user code are required to conform to all relevant standards when overriding setuptools functionality.

I imagine that if a plugin manages to violate standards, finding the reason for the resulting incorrect behaviour would be a bit of a nightmare, but that's a different problem ☹️

@abravalheri
Copy link
Contributor

#4698 is an attempt to implement PEP 643. I am trying a slightly unorthodox approach by marking the static values coming from setup.cfg/pyproject.toml instead of the dynamic ones, in an attempt to identify when plugins/customisations overwrite them. I say the approach is unorthodox because I introduce sub-classes of built-in data types. That is a bit awkward, but it makes sure the change is backwards compatible with the existing codebase in both setuptools and distutils.

If anyone is interested in having a look on the approach, please let me know your ideas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants