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

project.version field is required #2416

Closed
heinrich5991 opened this issue Dec 28, 2024 · 10 comments · Fixed by #2417
Closed

project.version field is required #2416

heinrich5991 opened this issue Dec 28, 2024 · 10 comments · Fixed by #2417

Comments

@heinrich5991
Copy link

💥 maturin failed
  Caused by: `project.version` field is required in pyproject.toml unless it is present in the `project.dynamic` list
Error: command ['maturin', 'pep517', 'write-sdist', '--sdist-directory', '/home/runner/work/libtw2/libtw2/uniffi/huffman/dist'] returned non-zero exit status 1

I had a pyproject.toml file without

[project]
version = "1.0.0"

and it failed with the above error in my CI. What are the backward compatibility guarantees of maturin, is something like this expected?

@andy-maier
Copy link

andy-maier commented Dec 29, 2024

We also have that error in some of our projects when installing 'pywinpty', since maturin 1.8.0 was released. There is already andfoy/pywinpty#486 open for that.

The issues for maturin are IMO, in addition to the question asked by heinrich5991:

  • The change log for 1.8.0 does not mention this incompatibility: https://github.com/PyO3/maturin/blob/main/Changelog.md#180 - Please mention incompatibilities explicitly in future change logs.

  • The user guide does not mention the possibility of specifying a dynamic version, and how the version is determined in that case. I think the user guide should be updated to add that.

@messense
Copy link
Member

What are the backward compatibility guarantees of maturin, is something like this expected?

This one is expected because it was considered as bugfix as the previous behavior was not spec compliant, quoting my comment in #2417 (comment)

According the spec: https://packaging.python.org/en/latest/specifications/pyproject-toml/#dynamic

If the metadata does not list a key in dynamic, then a build back-end CANNOT fill in the requisite metadata on behalf of the user (i.e. dynamic is the only way to allow a tool to fill in metadata and the user must opt into the filling in).

the current behavior is spec compliant, change it to warn only may seem to resolve the error, it can resurface when user try to install a package from sdist (because wheel isn't avaiable for some reason), for eample when using uv: #2390

  • The change log for 1.8.0 does not mention this incompatibility

The changelog has a link to #2391, I agree it should be improved, PR welcome.

  • The user guide does not mention the possibility of specifying a dynamic version

We are following PEP 621, IMHO it's not reasonable for us to document all of the possible options of PEP 621 in the user guide. As for this specific option, I think the solution is clearly indicated in the error message.

  • how the version is determined in that case. I think the user guide should be updated to add that.

I agree, we should definitely document how maturin determintes metadata that are missing from pyproject.toml.

@messense
Copy link
Member

Aside from the documentation improvements, should we undo the behavior change and defer it to a 2.0 release? My current feeling is that it's fine to keep and move on, because

  1. maturin init generated projects has the required dynamic = ["version"] in pyproject.toml since v1.5.0
  2. this only affects building from source distributions, most users should use binary wheel which continues to work
  3. projects that are impacted by this change should fix this spec-incompliant issue in their pyproject.toml, otherwise their sdist will not work when installing with a spec-compliant installer like uv

But I'm happy to listen to other opinions and find an alternative way to proceed.

@ijl
Copy link
Contributor

ijl commented Dec 30, 2024

Configuration that built on maturin v1.0.0 should build until v2 if semantic versioning is being followed.

Artifacts published since May 2023 depending on build system maturin>=1,<2 and not specifying a version or that version is dynamic in pyproject.toml are just permanently unbuildable after v1.8.0.

I see 13 repositories have issues linked against the change right now with usage down lots due to holidays so I don't think this is one or two projects doing something they never should have expected to work.

@messense
Copy link
Member

Artifacts published since May 2023 depending on build system maturin>=1,<2 and not specifying a version or that version is dynamic in pyproject.toml are just permanently unbuildable after v1.8.0.

Note that they are also unbuildable when building using frontends like uv even with maturin < 1.8.0: astral-sh/uv#9989

@mhils
Copy link
Contributor

mhils commented Dec 30, 2024

dynamic = ["version"] was added in 2074b11 (and then 2d8708e). This means that most projects that started with maturin init before September 2023 are likely to be affected. I'm greatly in favor of making this a loud and annoying warning (and maybe making maturin develop fail), but otherwise it feels like an unnecessary regression to me. Especially considering that there was no advance warning.

As @ijl mentioned, this breaks semver. As a user, if I follow the recommended practices with Maturin 1.0, I expect that my sdist continues to build. Maturin 2.0 can (and maybe should) break compatibility here.

ghuls added a commit to ghuls/bigtools that referenced this issue Dec 30, 2024
Building pybigtools with maturing 1.8.0 failed with:
  Caused by: `project.version` field is required in pyproject.toml unless it is present in the `project.dynamic` list

`project.dynamic` is already supported since maturin 1.4:
  PyO3/maturin#2416
@messense
Copy link
Member

maybe making maturin develop fail

Sounds good, if you are intertested in implementing it in #2417 we can get it merged and cut a new patch release soon. Thanks.

@messense
Copy link
Member

For anyone follows this thread, you are still encouraged to add dynamic = ["version"] to the pyproject.toml even when we downgrade the error to a warning because while maturin won't fail (after a new release), other tools like uv can still fail to build your sdist.

@mhils
Copy link
Contributor

mhils commented Dec 30, 2024

maybe making maturin develop fail

Sounds good, if you are intertested in implementing it in #2417 we can get it merged and cut a new patch release soon. Thanks.

Sounds good, happy to implement that change. 👍

My availability in the next 48 hours is limited, so my proposal would be we ship a patch release with #2417 as-is now and I open a new PR in the coming days, which will then make it into the next release?

@messense
Copy link
Member

I’m also having limited time today, will try to get to it later tonight.

messense added a commit that referenced this issue Dec 30, 2024
…toml` (#2418)

This PR implements the changes discussed in
#2416 (comment) on
top of #2417. Got this done faster than expected this morning, but won't
have any time to incorporate feedback for the next ~ two days. Feel free
to pull this in and butcher it any way you like. :)

Thanks again for your fantastic work on maturin! 🍰

---------

Co-authored-by: messense <[email protected]>
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 a pull request may close this issue.

5 participants