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

Implement PEP 639: support for Metadata-Version: 2.4; Part I #24

Merged

Conversation

agriyakhetarpal
Copy link
Contributor

Description

This PR implements PEP 0639 for make_wheels.py, which bumps the core metadata version from 2.1 to 2.4.

Here's a short summary of the changes that have been added:

  • the 'License': 'MIT' (now deprecated) field has been changed to the favoured 'License-Expression': 'MIT' field, i.e., the SPDX identifier for the MIT License;
  • License classifiers, while still common, have been deprecated: I removed the MIT license classifier in favour of compliance with the standard (in the above point), even though the deprecation cycle could be very long;
  • The metadata is now not defined as a dictionary but as a list of tuples (the cleanest method I could find to implement without breaking or changing too much of the code), since we need to add multiple instances of the same key for 'License-File':. The multiple uses of this field include all of the licensing-related files I could find in the RECORD file that gets generated and re-checked manually in the source. There is a chance I might have missed a few, so, if you find any, please let me know!
  • Added seven new classifiers to the project that I found are relevant enough to be included here, based on PyPI search related to them. If this is better to do as a follow-up PR, I'm happy to put up another.

I'll add additional information and context in a self-review below where I feel more context is needed, please let me know if I should add a few in-line comments in the code, too, for future references. Thank you!

Copy link
Collaborator

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +74 to +85
f'{dist_info}/METADATA': make_message([
('Metadata-Version', '2.4'),
('Name', name),
('Version', version),
*metadata,
], description),
f'{dist_info}/WHEEL': make_message([
('Wheel-Version', '1.0'),
('Generator', 'ziglang make_wheels.py'),
('Root-Is-Purelib', 'false'),
('Tag', tag),
]),
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 metadata version bump is backwards compatible. Please see https://peps.python.org/pep-0639/#backwards-compatibility for more. I tested recent versions of pip and uv, and they installed a wheel I generated for my M-series device without hiccups. I'll need to see if compliance with PyPI uploads is working, though, that's more important to figure out.

make_wheels.py Show resolved Hide resolved
make_wheels.py Show resolved Hide resolved
('Project-URL', 'Homepage, https://ziglang.org'),
('Project-URL', 'Source Code, https://github.com/ziglang/zig-pypi'),
('Project-URL', 'Bug Tracker, https://github.com/ziglang/zig-pypi/issues'),
('Requires-Python', '~=3.5'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://endoflife.date/python states that security releases for Python 3.7 have ended. Now that 3.13 is just around the corner, maybe we should update to a minimum of 3.8 (either here or as a separate PR). Please let me know.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm aware of the EOL status of Python 3.5 (it was EOL when I wrote this package). I decided that this requirement should be bumped only if the package isn't installable/usable, since there isn't really a cost to keeping it low, and it could help someone using old Python versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, makes sense!

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we do change the launcher in such a way it stops working on earlier Python versions, we should support the last non-EOL version, of course.

zig-pypi/make_wheels.py

Lines 122 to 127 in f1cd7a9

import os, sys
argv = [os.path.join(os.path.dirname(__file__), "{entry_name}"), *sys.argv[1:]]
if os.name == 'posix':
os.execv(argv[0], argv)
else:
import subprocess; sys.exit(subprocess.call(argv))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't access a Python version that old unless I use actions/setup-python in a CI setup – and I remember that your previous mention in our earlier conversations of one to not become a maintenance burden. So we could leave it until someone asks; I doubt anyone will, though, since those Pythons are really old 😄

@whitequark whitequark merged commit f1cd7a9 into ziglang:main Sep 29, 2024
@whitequark
Copy link
Collaborator

Thanks!

@agriyakhetarpal
Copy link
Contributor Author

Running twine --strict on a wheel currently fails:

❯ uvx twine check dist/ziglang-0.13.0-py3-none-macosx_12_0_arm64.whl --strict     
Checking dist/ziglang-0.13.0-py3-none-macosx_12_0_arm64.whl: ERROR    InvalidDistribution: Metadata is missing required fields: Name, Version.                                                                          
         Make sure the distribution includes the files where those fields are specified, and is using a supported Metadata-Version: 1.0, 1.1, 1.2, 2.0,    
         2.1, 2.2, 2.3.

I guess this might be because twine hasn't been updated yet, since the PEP was approved just last month, in August 2024. I'll take a deeper look, and if this would take time, maybe we should bump to just version 2.3 for now? 2.2 and 2.3 did not contain too many changes, and none of those are relevant for us here.

@agriyakhetarpal
Copy link
Contributor Author

Oops, you merged too quickly, maybe! :D I'll open a new PR to fix any changes, though I would request not to upload any missing wheels with this.

@agriyakhetarpal agriyakhetarpal deleted the feature/bump-metadata-version branch September 29, 2024 21:29
@agriyakhetarpal
Copy link
Contributor Author

Upstream issue: pypa/twine#1146

@agriyakhetarpal
Copy link
Contributor Author

A fix should get merged soon: pypa/twine#1123

@whitequark
Copy link
Collaborator

I would request not to upload any missing wheels with this.

Noted.

@agriyakhetarpal
Copy link
Contributor Author

agriyakhetarpal commented Nov 13, 2024

pypi/warehouse#16949 has just been merged, and I've noticed a few issues with my implementation – I intend to fix them before uploading wheels can be ready. In particular, I see:

This is easy to fix, of course.

  • Copying from the PEP: "if the Metadata-Version is 2.4 or greater and one or more License-File fields is specified, the .dist-info directory MUST contain a licenses subdirectory, which MUST contain the files listed in the License-File fields in the METADATA file at their respective paths relative to the licenses directory."

This is something I didn't implement at the time; looks like other backends are doing so in parts, too: pypa/setuptools#4728. What I don't understand about this is that the Zig source in the wheels should remain unchanged, so I'm supposed to move but copy the relevant license files that I added in this PR. But isn't that just going to create duplicate files? I feel that's a limitation of the PEP, because it doesn't cover this case (where multiple licenses are going to be present across subfolders). The other option is to just copy the MIT (Expat) license file for this project (which would be the more usual approach) and add a note about the licensing for the bundled Zig source in the README. Do you have any thoughts?

I'll validate the changes with updated versions of twine and packaging and with whatever APIs are available to perform said validation.

@whitequark
Copy link
Collaborator

But isn't that just going to create duplicate files?

That's fine. The archive isn't winning any size competition anyway, and text compresses well.

@agriyakhetarpal agriyakhetarpal changed the title Implement PEP 639: support for Metadata-Version: 2.4 Implement PEP 639: support for Metadata-Version: 2.4; Part I Nov 14, 2024
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.

2 participants