PEP 639: Implement License-Expression and License-File #828

ewdurbin · 2024-09-03T16:10:11Z

~~Work in progress, current open questions:~~

~~Is the dependency on license-expression OK? Should vendoring or a new parser be considered?~~
- ~~went the vendoring route per feedback from @abravalheri~~
- migrated to a pure python parser based on pypa/hatchling's implementation
How much validation to apply to License-File, as-is as far as metadata goes the only clear spec is "Path delimiters MUST be the forward slash character (/), and parent directory indicators (..) MUST NOT be used.". It is unclear how to check for other delimiters, as paths may have no delimiters whatsoever
- validations with path lib seem to fit the bill!
~~How to handle deprecations/conflicts. License and License-Expression are mutually exclusive, but it doesn't appear that such a conflict has existed before.~~
- Based on the current behavior of the library, this kind of concern appears to be left to the caller. The metadata module appears to be focused on correctly parsing individual fields.

pyproject.toml

cdce8p · 2024-09-04T11:34:27Z

IIRC one of the concerns raised in the Discourse thread was regarding the total file size if license_expression would be added as a dependency or fully vendored. Just as example, the scancode-licensedb-index.json file is over 800kB.

I believe that's one of the reasons @ofek went a slightly different route with #799.

ewdurbin · 2024-09-04T13:19:08Z

See a828bc7, which is a variant of #799 to build/use a minimal compatible json blob from the upstream for use with the vendored license-expression. Also excluding the data files from the vendored package from the resulting sdist/wheel to save space.

Overall resulting size difference for sdist and wheel

file	24.1	before	after
sdist	148 kB	276 kB	212 kB
whl	54 kB	180 kB	112 kB

uncompressed:

file	24.1	before	after
sdist	2539 kB*	1690 kB	829 kB
whl	176 kB	1281 kB	426 kB

* this appears to be due to the inclusion of tests/.pytest_cache/v/cache/lastfailed and tests/.pytest_cache/v/cache/nodeids files in the sdist, without it it is 608 kB.

ofek

Can you please save the data as Python so deserialization is not required?
I think we shouldn't merge this until we satisfy the parsing needs of backends (i.e. user defined metadata) rather than just consumers of packages like PyPI.
Although I'm not a maintainer, I would be quite weary of packaging beginning to vendor dependencies.

ewdurbin · 2024-09-04T16:34:48Z

Can you please save the data as Python so deserialization is not required?

Looked closer and saw that hatchling's implementation is more complete than I initially understood, so I've migrated this PR to use an adapted version of that, and moved storage of the license data back to using the python based representation

I think we shouldn't merge this until we satisfy the parsing needs of backends (i.e. user defined metadata) rather than just consumers of packages like PyPI.

Which needs are those? Can you delineate them? I'm interested in seeing this merged so PyPI's implementation can progress.

Although I'm not a maintainer, I would be quite weary of packaging beginning to vendor dependencies.

See response to 1. :)

ofek · 2024-09-04T16:49:05Z

I think we shouldn't merge this until we satisfy the parsing needs of backends (i.e. user defined metadata) rather than just consumers of packages like PyPI.

Which needs are those? Can you delineate them? I'm interested in seeing this merged so PyPI's implementation can progress.

Now that you have normalize_license_expression which also raises an exception for invalid stuff, every backend would be satisfied. Thanks!

src/packaging/licenses/__init__.py

ofek

Looks fantastic to my non-maintainer eyes!

befeleme · 2024-09-05T09:55:06Z

Just a tiny nitpick: The commit message contains a typo: "PEP 693" instead of 639 - this will make digging in the git history harder in the future.

hroncok

How much validation to apply to License-File, as-is as far as metadata goes the only clear spec is "Path delimiters MUST be the forward slash character (/), and parent directory indicators (..) MUST NOT be used.". It is unclear how to check for other delimiters, as paths may have no delimiters whatsoever

I think the spirit of the rule is that there are no backlash-delimeters in the path. However, I am not sure if this is enforceable if a filename can contain a backlash.

Also, as the path MUST be relative, we could assert it does not start with /.

I also noticed a license file is never checked for existence. Is that intentional? The PEP states it MUST be present (and the file content MUST be UTF-8 encoded text).

tests/test_metadata.py

although inconsistent (see Requires-External, Provides-Extra, Provides-Dist, Obsoletes-Dist...) pluralize the result for multiple use of `License-File` https://packaging.pypa.io/en/stable/metadata.html#packaging.metadata.RawMetadata > Any core metadata field that can be specified multiple times or can hold multiple values in a single field have a key with a plural name.

Per feedback, integrate a variant of pypa#799 that builds a minimal JSON dataset to feed vendored license-expression 32K src/packaging/_spdx.json vs 848K src/packaging/_vendor/license_expression/data/scancode-licensedb-index.json

- back away from vendoring - adapt hatchling's license expression parser to accept well-formed LicenseRef- - back to python storage of license data

ewdurbin · 2024-09-05T10:30:04Z

Just a tiny nitpick: The commit message contains a typo: "PEP 693" instead of 639 - this will make digging in the git history harder in the future.

🤦🏼 resolved with a rebase/force-push

sethmlarson

Only had a few nits, otherwise this LGTM. Thanks for all the work on this!

src/packaging/metadata.py

tests/test_metadata.py

brettcannon · 2024-10-18T19:06:52Z

Thanks for your patience with this, @ewdurbin !

@pradyunsg did you want to review this or are you okay to merge? Once we do I will add the new label for issues.

ewdurbin · 2024-10-22T21:02:14Z

Given that this has approval from at least one maintainer... I put the effort into a draft PR for implementation in warehouse is now up at pypi/warehouse#16949

pradyunsg · 2024-10-23T23:39:47Z

@pradyunsg did you want to review this or are you okay to merge?

Okay to merge.

brettcannon · 2024-10-24T17:01:06Z

Merged! Thanks, everyone!

Off to create a new label for our latest module!

henryiii · 2024-10-24T17:50:49Z

Of to go rebasing my PRs on this now! ("Now" meaning after teaching today, probably)

ewdurbin · 2024-10-24T17:52:30Z

Thanks everyone! Eagerly awaiting a release so that PyPI's implementation can be deployed :)

henryiii · 2024-10-24T17:58:26Z

I'd like to get a PR for extra validation in before a release, as it's easier to add validation before a release than afterwords. I should know if it's needed and have it added by tomorrow.

henryiii · 2024-10-24T18:26:59Z

The validation errors I was looking for are included, so good to go from me!

Question, though: would adding a warning if a License :: classifier is present and metadata 2.4+ is encountered be a good idea? Likewise, if 2.4 is set and a classic license is used. I don't see another similar warning in packaging.metadata, though.

Basically, this: https://github.com/pypa/pyproject-metadata/blob/cb7450073acecefc714cc5de82816799841777b9/pyproject_metadata/__init__.py#L505-L517

Warning can always be added after a release, unlike errors, so not a blocker at all.

ewdurbin · 2024-10-24T18:30:17Z

From the PEP:

If the License-Expression field is present, build tools MAY raise an error if one or more license classifiers is included in a Classifier field, and MUST NOT add such classifiers themselves.

and

If only the License field is present, tools MAY issue a warning informing users it is deprecated and recommending License-Expression instead.

So both of those warnings seem to be inline with the PEP.

henryiii · 2024-10-24T18:34:14Z

Yes, that’s where I got the warnings, but they are MAY, I assume we get to pick.

brettcannon · 2024-10-25T21:47:15Z

would adding a warning if a License :: classifier is present and metadata 2.4+ is encountered be a good idea?

It's a hard call for in 'packaging' itself. If we put it in it will definitely come up more, but for a "MAY" it feels like we might be going beyond just being a library.

ofek · 2024-10-25T21:59:36Z

Please do not emit a warning in this foundational library.

henryiii · 2024-10-25T22:24:10Z

Okay, sounds good. Users can add the MAY themselves, then.

ewdurbin mentioned this pull request Sep 3, 2024

Implement PEP 639 pypi/warehouse#16620

Open

abravalheri reviewed Sep 3, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

ewdurbin force-pushed the pep_639 branch 3 times, most recently from 4bbae23 to e3dec5c Compare September 3, 2024 23:27

This was referenced Sep 4, 2024

[FR] Implement PEP 639 pdm-project/pdm-backend#256

Closed

Implement PEP 639 python-poetry/poetry#9670

Open

ewdurbin force-pushed the pep_639 branch from 7eae541 to fd1d83b Compare September 4, 2024 13:15

ofek reviewed Sep 4, 2024

View reviewed changes

ewdurbin force-pushed the pep_639 branch from d72a43f to b8e03f2 Compare September 4, 2024 16:36

ewdurbin force-pushed the pep_639 branch from b8e03f2 to c1530bd Compare September 4, 2024 17:01

ofek reviewed Sep 4, 2024

View reviewed changes

src/packaging/licenses/__init__.py Outdated Show resolved Hide resolved

ewdurbin mentioned this pull request Sep 4, 2024

Implement PEP 639 #829

Open

ofek approved these changes Sep 4, 2024

View reviewed changes

hroncok reviewed Sep 5, 2024

View reviewed changes

tests/test_metadata.py Outdated Show resolved Hide resolved

ewdurbin added 7 commits September 5, 2024 06:29

PEP 639: Implement License-Expression and License-File

de8b239

wire in vendoring library to manage vendored dependencies

44beda7

vendor license_expression and boolean.py

2b1a9f7

migrate to parser based on hatchling's

cec33f1

- back away from vendoring - adapt hatchling's license expression parser to accept well-formed LicenseRef- - back to python storage of license data

string -> re

cfb3af1

ewdurbin force-pushed the pep_639 branch from 2e7cfb8 to cfb3af1 Compare September 5, 2024 10:29

ewdurbin changed the title ~~PEP 693: Implement License-Expression and License-File~~ PEP 639: Implement License-Expression and License-File Sep 5, 2024

ewdurbin and others added 2 commits October 3, 2024 19:15

pass globals/locals to eval

42d6452

Merge branch 'main' into pep_639

461d183

sethmlarson approved these changes Oct 8, 2024

View reviewed changes

src/packaging/metadata.py Outdated Show resolved Hide resolved

tests/test_metadata.py Show resolved Hide resolved

ewdurbin added 3 commits October 8, 2024 15:51

fix: unncessary string concatenation from reformatting

9546938

licenses: add some testcases for whitespace normalization

da3f04b

lint

23df2ac

dnicolodi mentioned this pull request Oct 11, 2024

0.9.0 final release pypa/pyproject-metadata#199

Closed

This was referenced Oct 11, 2024

Rework audit-licenses check [ci] home-assistant/core#123119

Draft

Use SPDX license identifier prettytable/prettytable#315

Merged

brettcannon approved these changes Oct 18, 2024

View reviewed changes

ewdurbin mentioned this pull request Oct 22, 2024

Implement PEP 639, Metadata 2.4 pypi/warehouse#16949

Draft

henryiii mentioned this pull request Oct 23, 2024

Merge this library into packaging? pypa/pyproject-metadata#103

Open

henryiii mentioned this pull request Oct 24, 2024

feat: pyproject-metadata #847

Draft

6 tasks

Merge branch 'main' into pep_639

042a335

brettcannon merged commit 029f415 into pypa:main Oct 24, 2024
33 checks passed

ewdurbin deleted the pep_639 branch October 24, 2024 17:57

henryiii mentioned this pull request Oct 24, 2024

chore: update pre-commit pins and fix issues #836

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PEP 639: Implement License-Expression and License-File #828

PEP 639: Implement License-Expression and License-File #828

ewdurbin commented Sep 3, 2024 •

edited

Loading

cdce8p commented Sep 4, 2024

ewdurbin commented Sep 4, 2024 •

edited

Loading

ofek left a comment

ewdurbin commented Sep 4, 2024

ofek commented Sep 4, 2024 •

edited

Loading

ofek left a comment

befeleme commented Sep 5, 2024 •

edited

Loading

hroncok left a comment •

edited

Loading

ewdurbin commented Sep 5, 2024

sethmlarson left a comment

brettcannon commented Oct 18, 2024

ewdurbin commented Oct 22, 2024

pradyunsg commented Oct 23, 2024

brettcannon commented Oct 24, 2024

henryiii commented Oct 24, 2024

ewdurbin commented Oct 24, 2024

henryiii commented Oct 24, 2024

henryiii commented Oct 24, 2024

ewdurbin commented Oct 24, 2024

henryiii commented Oct 24, 2024

brettcannon commented Oct 25, 2024

ofek commented Oct 25, 2024

henryiii commented Oct 25, 2024

PEP 639: Implement License-Expression and License-File #828

PEP 639: Implement License-Expression and License-File #828

Conversation

ewdurbin commented Sep 3, 2024 • edited Loading

cdce8p commented Sep 4, 2024

ewdurbin commented Sep 4, 2024 • edited Loading

ofek left a comment

Choose a reason for hiding this comment

ewdurbin commented Sep 4, 2024

ofek commented Sep 4, 2024 • edited Loading

ofek left a comment

Choose a reason for hiding this comment

befeleme commented Sep 5, 2024 • edited Loading

hroncok left a comment • edited Loading

Choose a reason for hiding this comment

ewdurbin commented Sep 5, 2024

sethmlarson left a comment

Choose a reason for hiding this comment

brettcannon commented Oct 18, 2024

ewdurbin commented Oct 22, 2024

pradyunsg commented Oct 23, 2024

brettcannon commented Oct 24, 2024

henryiii commented Oct 24, 2024

ewdurbin commented Oct 24, 2024

henryiii commented Oct 24, 2024

henryiii commented Oct 24, 2024

ewdurbin commented Oct 24, 2024

henryiii commented Oct 24, 2024

brettcannon commented Oct 25, 2024

ofek commented Oct 25, 2024

henryiii commented Oct 25, 2024

ewdurbin commented Sep 3, 2024 •

edited

Loading

ewdurbin commented Sep 4, 2024 •

edited

Loading

ofek commented Sep 4, 2024 •

edited

Loading

befeleme commented Sep 5, 2024 •

edited

Loading

hroncok left a comment •

edited

Loading