-
Notifications
You must be signed in to change notification settings - Fork 2.4k
add support for PEP 639 License Clarity #10413
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
Conversation
Reviewer's GuideAdd PEP 639 License Clarity support by simplifying license specification, introducing a Class diagram for license specification changes in pyproject.tomlclassDiagram
class ProjectMetadata {
+license: str
+license_files: List[str]
}
ProjectMetadata : -license (table syntax) [deprecated]
ProjectMetadata : +license (SPDX string)
ProjectMetadata : +license_files (glob patterns)
ProjectMetadata : +license_files = default patterns if not specified
ProjectMetadata : +license_files = [] disables inclusion
Class diagram for updated build process including license filesclassDiagram
class Builder {
+build_sdist()
+build_wheel()
+include_license_files(patterns: List[str])
}
Builder --> ProjectMetadata
Builder : +include_license_files uses license_files from ProjectMetadata
Builder : +include_license_files uses default patterns if not specified
Builder : +include_license_files skips if patterns is empty
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Deploy preview for website ready! ✅ Preview Built with commit 94098f6. |
687a4aa to
103aa7c
Compare
f6216e5 to
be62db9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `tests/console/commands/test_check.py:176` </location>
<code_context>
Error: Invalid source "not-exists" referenced in dependencies.
Error: Invalid source "not-exists2" referenced in dependencies.
Error: poetry.lock was not found.
+Warning: [project.license] is not a valid SPDX identifier.\
+ This is deprecated and will raise an error in the future.
Warning: A wildcard Python dependency is ambiguous.\
Consider specifying a more explicit one.
</code_context>
<issue_to_address>
Consider adding tests for valid SPDX license identifiers and edge cases.
Adding tests for valid identifiers and edge cases like mixed-case, extra whitespace, or multiple licenses will help ensure comprehensive coverage.
</issue_to_address>
### Comment 2
<location> `tests/masonry/builders/test_editable_builder.py:195` </location>
<code_context>
"""
- assert metadata == dist_info.joinpath("METADATA").read_text(encoding="utf-8")
+ if project == "simple_project":
+ metadata = metadata.replace("License:", "License-Expression:").replace(
+ "Classifier: License :: OSI Approved :: MIT License\n", ""
+ )
</code_context>
<issue_to_address>
Test could be improved by explicitly checking license-files inclusion in built distributions.
Add assertions to verify that license files are present and correct in the built distribution, as specified by the license-files field.
Suggested implementation:
```python
assert dist_info.joinpath("METADATA").read_text(encoding="utf-8") == metadata
# Check license-files inclusion in built distribution
import re
# Extract license-files from metadata
license_files = []
license_files_match = re.search(r"license-files = \[(.*?)\]", metadata, re.DOTALL)
if license_files_match:
files_str = license_files_match.group(1)
# Split and clean up file names
license_files = [f.strip().strip('"').strip("'") for f in files_str.split(",") if f.strip()]
# Assert each license file is present and correct in dist_info
for license_file in license_files:
dist_license_path = dist_info.parent.joinpath(license_file)
source_license_path = project_dir.joinpath(license_file)
assert dist_license_path.exists(), f"License file {license_file} missing from built distribution"
assert dist_license_path.read_text(encoding="utf-8") == source_license_path.read_text(encoding="utf-8"), f"License file {license_file} content mismatch"
with open(dist_info.joinpath("RECORD"), encoding="utf-8", newline="") as f:
reader = csv.reader(f)
sys.exit(baz.boom.bim())
"""
```
- Ensure that `project_dir` is defined and points to the source project directory containing the license files.
- If the license-files field is not in the metadata as a TOML array, adjust the regex to match the actual format.
- If the test setup uses a different way to specify license files, adapt the extraction logic accordingly.
</issue_to_address>
### Comment 3
<location> `docs/pyproject.md:121` </location>
<code_context>
+]
```
+Per default, Poetry will include the following files:
+- `LICENSE*`
+- `LICENCE*`
</code_context>
<issue_to_address>
Replace 'Per default' with 'By default' for correct English usage.
'Per default' is a direct translation from German and is not standard English. Please use 'By default' instead.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
Per default, Poetry will include the following files:
=======
By default, Poetry will include the following files:
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Pull Request Check List
Requires: python-poetry/poetry-core#870
Resolves: #9670
Summary by Sourcery
Implement PEP 639 support by switching to SPDX license expressions, introducing a
license-filessetting, deprecating table-based definitions, bumping packaging to 24.2, and updating documentation and tests to reflect these changesNew Features:
licensefield in pyproject.tomllicense-filesfield to specify custom license file patternsEnhancements:
packagingdependency requirement to >=24.2 to enable PEP 639 supportLicense-Expressionheader instead ofLicenseDocumentation:
license-filesfield, and deprecated syntaxTests:
License-Expressionmetadata header