Skip to content

Conversation

@cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Jun 28, 2025

Followup to #113 (comment)

This PR moves the static project metadata from setup.py to pyproject.toml. It also removes setuptools as a required dependency, it's only necessary as part of the build system. Furthermore, I've removed the license classifier in preparation for PEP 639. This is supported by setuptools >=77.0, however it's only available for Python 3.9+. So this has to wait until 3.8 is dropped.

Metadata diff

 ...
-Author: Mikhail Korobov
-Author-email: [email protected]
+Author-email: Mikhail Korobov <[email protected]>
-Home-page: https://github.com/pytries/marisa-trie
+Project-URL: Source, https://github.com/pytries/marisa-trie
 License: MIT
 ...
-Classifier: License :: OSI Approved :: MIT License
 ...
-Requires-Dist: setuptools
 ...



def test_check_pypi_rendering():
subprocess.check_call(["python3", "setup.py", "sdist"])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this line as well to fix the test. The sdist build is essentially tested with each CI test run (pip install .[test]) as well as with the dedicated "Build & Publish" job for changes to setup.py and pyproject.toml.

@BoboTiG
Copy link
Member

BoboTiG commented Jun 28, 2025

Maybe can you fix this warning at the same time?

           ********************************************************************************
          Please use a simple string containing a SPDX expression for `project.license`. You can also use `project.license-files`. (Both options available on setuptools>=77.0.0).
  
          By 2026-Feb-18, you need to update your project and remove deprecated calls
          or your builds will no longer be supported.
  
          See https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license for details.
          ********************************************************************************

@cdce8p
Copy link
Contributor Author

cdce8p commented Jun 28, 2025

you fix this warning at the same time?

This is only possible with setuptools >=77.0 which would require dropping support for Python 3.8.

@BoboTiG
Copy link
Member

BoboTiG commented Jun 28, 2025

OK for me to drop 3.8, it's EOL since 2024-10-07.

[project]
name = "marisa-trie"
version = "1.2.1"
license = "MIT AND (BSD-2-Clause OR LGPL-2.1-or-later)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There has been some discussion in the Python discuss forum whether project.license should correspond to "all" included licenses or without bundled files. The consensus so far seems to be that it should refer to everything. I.e. the project AND the license of the bundled files.

- name: Build wheels
run: python -m cibuildwheel
env:
CIBW_SKIP: "pp*" # skip PyPy releases
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the latest cibuildwheel version, PyPy is skipped by default.

run: python -m cibuildwheel
env:
CIBW_SKIP: "pp*" # skip PyPy releases
CIBW_ARCHS_MACOS: "x86_64 universal2 arm64"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this to just auto x86_64 which is equal to x86_64 arm64. Since both versions are build anyway there is no need for separate universal2 wheels.

Copy link
Member

Choose a reason for hiding this comment

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

We will loose universal2 wheels then, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. You can find the full list in the Publish job. https://github.com/pytries/marisa-trie/actions/runs/15952291034/job/44994668006?pr=120#step:3:6

For comparison this was from the PR to add free-threading: https://github.com/pytries/marisa-trie/actions/runs/15560917303/job/43818763321#step:3:6

It seems with the removal of the QEMU step we'll also loose the i686 wheels. IMO that's probably fine but if you want, I'll add them back. Only x86 is still build by "auto" even on a 64-bit system. Note though that this will likely change at some point as well. These are the defaults from cibuildwheels: https://cibuildwheel.pypa.io/en/stable/options/#archs

Copy link
Member

@BoboTiG BoboTiG Jun 29, 2025

Choose a reason for hiding this comment

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

Hm I guess this is OK as-is.

If one wants universal2, it can be easily crafted using delocate.

And about i686, no need to bother I would say.

@BoboTiG BoboTiG merged commit d15dca4 into pytries:master Jun 29, 2025
24 checks passed
@BoboTiG
Copy link
Member

BoboTiG commented Jun 29, 2025

Thanks for the work @cdce8p 🥂

@cdce8p cdce8p deleted the move-metadata branch June 29, 2025 09:45
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