feat: add PEP 639 license metadata support - #4070
Conversation
sanchiagarwal0
changed the title
rickeylev
left a comment
rickeylev
left a comment
There was a problem hiding this comment.
Thanks for this!
- PR description needs to be rewritten to be an appropriate commit message.
- Needs tests.
| doc = "An SPDX license expression for the package.", | ||
| default = "", | ||
| ), | ||
| "license_files": attr.label_keyed_string_dict( |
There was a problem hiding this comment.
The license file paths are used, but the files themselves aren't being included in the output
| default = "", | ||
| ), | ||
| "license_files": attr.label_keyed_string_dict( | ||
| doc = "License files to include under the .dist-info/licenses/ directory.", |
There was a problem hiding this comment.
What is the value for this dict? the path under licenses to put the files under? The behavior needs to be documented.
How are files mapped under the licenses directory? Files could come from elsewhere in the repo
This attribute mostly redundant with the extra_distinfo_files attribute. The only differnce is where in dist-info files go (this puts them under licenses)
Given all this, I think we should just remove this field.
| "License-Expression: %s" % ctx.attr.license_expression | ||
| ) | ||
| for _, license_file in sorted(ctx.attr.license_files.items()): | ||
| metadata_contents.append("License-File: %s" % license_file) |
There was a problem hiding this comment.
Is every license file supposed to be listed as a License-File header? I'm guessing a PEP specifies this? Which one?
Because extra_distinfo_files could add license files, I'm thinking the population of License-File should move to the execution phase.
There was a problem hiding this comment.
Yes. This is specified by PEP 639. In the license-files section, it says build tools MUST include all files matched by the configured patterns and MUST list each matched file path under a License-File field in the Core Metadata.
The License-File field itself is also defined in the Core Metadata section of PEP 639 as a multi-use field, where each instance lists one license-related file.
|
@sanchiagarwal0 I get the sense you want to see this PR through on your own? I just ask because normally I would throw my agent at a PR and have it address most things. However, if you'd rather work on it yourself, I can hold off on that |
|
Yes, I'd like to work on it myself. Thanks for offering to have your agent handle it — I appreciate it! I'll look into the |
2 participants
What does this PR do?
Adds support for PEP 639 license metadata in the
py_wheelrule.Changes
license_expressionattribute for SPDX license expressions.license_filesattribute for license files.License-ExpressionandLicense-Filemetadata fields.licenseandlicense_expressionfrom being used together.Testing
git diff --checkpasses.Related issue
N/A