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

0.9.0 final release #199

Closed
frenzymadness opened this issue Oct 10, 2024 · 16 comments · Fixed by #210
Closed

0.9.0 final release #199

frenzymadness opened this issue Oct 10, 2024 · 16 comments · Fixed by #210

Comments

@frenzymadness
Copy link

Hello. What are the plans for the final release of 0.9.0? Using beta releases means some additional work for me so I'm asking whether it's worth it and we'll have only beta releases in the near future or we can expect a final release soon. Thank you.

@henryiii
Copy link
Collaborator

I'm waiting for @dnicolodi to have a chance to try with meson-python, though people with --pre are getting it already, so that's probably pretty safe. I'd like to get scikit-build/scikit-build-core#917 ready to verify it works. pdm-project/pdm-backend#263 is in. I'd also like to check #196 before release. At the worst, I think that's end of next week. At best, maybe end of this week.

@frenzymadness
Copy link
Author

Thank you for the quick reply. I'm working on pdm-backend but without the bundled dependencies. I'll wait then.

@dnicolodi
Copy link
Contributor

It is my understanding that for supporting PEP 639 correctly the license expression needs to be validated and normalized. This is left to the build backends to do via the new packagng.licenses.canonicalize_license_expression() function being introduced in pypa/packaging#828 Until that is merged and released, build backends that use pyproject-metadata cannot claim that they support PEP 639. Am I missing something?

@henryiii
Copy link
Collaborator

That's SHOULD, not MUST. Though it does make me wonder if we should drop the immutability of non-dynamic fields. Setting this to a normalized license expression is a valid thing to do. Currently it can be done with dataclasses.replace only. A benefit of keeping it that way, though, is we can rerun validation.

Another option would be to also implement canonicalize_license_expression ourselves.

@henryiii
Copy link
Collaborator

I rather like shipping with validation in the first version, so we don't create backward incompat later.

@dnicolodi
Copy link
Contributor

I think doing validation and normalization in the build backend is the right thing. I suppose an invalid license expression would be rejected at upload time otherwise. Wouldn't it? AFAICT, both validation and normalization require keeping a list of valid licenses and I don't think that there is value in duplicating that in packaging and pyproject-metadata (as much as I don't like how the validation is implemented in packaging).

I'm agnostic whether the validation should be implemented in pyproject-metadata or in the backends. However, it the latter case, being able to update the field in place would be handy. I didn't notice that this is not possible now. How does the immutability play along with dynamic fields?

@henryiii
Copy link
Collaborator

I don't want to maintain a list of licenses here, and I don't want to force a really recent (not yet released!) version of packaging as a required dependency, so I think we should leave it up to the backends, but have a clear example of how to do it with packaging, or maybe a way to opt in if packaging is new enough. We can do this by default for #140.

Dynamic fields are mutable. Only ones not listed in the Dynamic table are immutable.

@henryiii
Copy link
Collaborator

What I'd probably do in scikit-build-core would be to warn if the validation isn't available, asking users to update packaging. We could do that here instead - validate if packaging is new enough, otherwise warn that it can't be validated.

@dnicolodi
Copy link
Contributor

I'll be happy with that solution or with implementing the validation in the back-end. Although in the latter case, having the possibility to mutate the license field in-place would be handy.

@dnicolodi
Copy link
Contributor

I've prepared mesonbuild/meson-python#681 and as far as I can tell everything looks good. @henryiii I would appreciate if you could give a cursory look at that PR to see if I did miss anything in the implementation of PEP 639.

@henryiii
Copy link
Collaborator

I didn't notice that this is not possible now. How does the immutability play along with dynamic fields?

I was trying to keep a similar (but incorrectly implemented) protection in 0.8 and earlier. But I'm thinking this isn't helpful. Some sort of API to easily ask "is this field in the dynamic list" would probably be better.

@dnicolodi
Copy link
Contributor

Some sort of API to easily ask "is this field in the dynamic list" would probably be better.

Isn't that there already? It is enough to compare a field name with the content of the dynamic attribute. Instance attributes and fields are not spelled exactly the same (but substitution of dashes for underscores is the only transformation that comes to mind) but I don't think this is an issue. At least this is how we check this in meson-python.

On the other hand, bypassing the check is trivial thus if it is required only to update license, I would keep the immutability in place.

@henryiii
Copy link
Collaborator

henryiii commented Oct 15, 2024

Isn't that there already?

Yes, doesn't really need a new API. Very tempted to drop it - simpler, and I've also had to work around it here:

https://github.com/scikit-build/scikit-build-core/blob/389443027bcee539a4d0449ab7f514d8d8e26de3/src/scikit_build_core/build/metadata.py#L79-L82

@henryiii
Copy link
Collaborator

All PRs are passing, so I'll release soon unless anyone has last-minute objections!

@frenzymadness
Copy link
Author

Thank you!

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.

3 participants