iframe-proxy

cdce8p · 2021-04-17T19:04:32Z

Summary of changes

This PR adds the License-File field to the package metadata. Although not yet part of the official metadata spec, including this field will enable third party tools easier access to all included license files through importlib.metadata. This is especially useful if a file doesn't have an easily recognizable name or is in an unusual location.

setuptools already parses the license_file and license_files options and cross-references them with all project files to see which ones should be included. With this change, the result of that would be made available for others to use as well.

Closes #2631

--
For reference: PEP 639, currently still in a draft, plans to add the License-File field to the metadata spec, link

--
Example use case

from importlib import metadata
meta = metadata.metadata('my-project')
project_files = metadata.files('my-project')

license_file_paths = [
    path for path in project_files
    if str(path) in meta.get_all('License-File')
]

Open questions

1. What should the value of License-Path be for files not in main project directory?
From a technical standpoint it would probably be best to use the actual relative path and mirror that in the .dist-info directory. However, at the moment wheel flattens the folder structure and just puts all license files into .dist-info, AFAIK. This could lead to name conflicts.

2. Should license files be copied into the .egg-info directory if the legacy install is used?

3. How should license_files and MANIFEST.in continue to work together?
At the moment, the MANIFEST.in is parse at the end and thus overwrites all previous file includes. However, license_files_computed will only look at the license_file and license_files options (unless we parse the manifest there too). That means that the License-File field might contain files that are later excluded by the manifest.
-> I would recommend to change the behavior so that the license_file and license_files options take precedent over the manifest. A license file is most likely excluded by accident anyway. Additionally, specifying license_files = (without any values) won't add any default patterns and thus not match any license files.

Pull Request Checklist

Changes have tests
News fragment added in changelog.d/.
(See documentation for details)

jaraco · 2021-05-19T21:27:38Z

+                if path not in files and os.path.isfile(path):
+                    files.add(path)
+
+        self.metadata.license_files_computed = sorted(files)


Can we retain order of license files rather than enforcing order? For the manifest, that's less important, but for metadata, it may prove useful to honor the user's supplied order.

The user only provides patterns which are used for glob. As such, I don't think retaining any order would make much sense.

Alright. I'll probably re-write it to avoid sorting or at least avoid sorting user-supplied values.

jaraco

I like where this is going. Just a couple of concerns.

cdce8p · 2021-05-20T00:51:41Z

I've updated the PR. Additionally, I also added a commit to change when license files are added to the source list. As explained in the open questions section (3. How should license_files and MANIFEST.in continue to work together?), to make sure a license file which is listed in License-File is included, the license_files option needs to be able to overwrite the MANIFEST.in

jaraco · 2021-05-21T21:12:07Z

        'project_urls': dict,
        'provides_extras': ordered_set.OrderedSet,
-        'license_files': ordered_set.OrderedSet,
+        'license_file': None,


I had kind-of hoped this would go away, especially since the value is deprecated. I tried removing it and got an error, but I think it can be eliminated by being more defensive at the one point where it's queried.

I'm happy to remove it if you want. Personally, I would also be open to removing the option license_file entirely and return an error. Should be easy enough for the user to fix then.

jaraco · 2021-05-21T21:12:42Z

        'provides_extras': ordered_set.OrderedSet,
-        'license_files': ordered_set.OrderedSet,
+        'license_file': None,
+        'license_files': None,


Here it probably makes sense to just declare the default here rather than declare a null value and then set a default if the value is a null value.

That was my initial idea as well. However, with that we can't determined if the user didn't specify the option (thus the defaults would be loaded) or left it empty (in which case no license files would be included). Both would equal an empty list.

Oh, that's very interesting. And the test cases capture this aspect. Consider me impressed.

jaraco · 2021-05-21T21:13:10Z

I added a couple more comments, but I'll take the first stab at adjusting the behavior.

cdce8p · 2021-05-22T11:15:03Z

@jaraco Pushed new change to address some of your points. First, I modified the logic to match license files so that the user sorting is kept as best as possible. Second, I removed the default license_file metadata attribute. Let me know if you what me to change anything else.

cdce8p · 2021-05-22T11:48:15Z

Small update: I needed to revert the removal of the license_file attribute. Without it the deprecation warning wouldn't be printed (even with -Wd).

jaraco · 2021-05-22T23:05:30Z

I spent some time trying to simplify the logic by having the defaults for "license_files" be defined in the defaults for license files (rather than having a null value as the default and defining defaults in the finalizer), but when doing so, I encountered two problems:

The config parser won't override a default with an explicit value if bool(default) == True.
The logic for the default/fallback values is contingent both on license_file and license_files not being specified, so until license_file can be removed, the two settings are inherently entangled.

At some point soon, I'd like to remove support for license_file and remove that check for overriding a non-False default. For now this should do.

I've added one additional commit that removes the sorting entirely and uses a generator to build the list. I see I don't have push access to the PR, so I'll try sending a PR to your branch. If that doesn't work, I'll probably just merge it all together manually.

* needed for 'License-File' metadata, as this is written before MANIFEST is read

…seen to dedupe.

cdce8p · 2021-05-22T23:56:24Z

I've added one additional commit that removes the sorting entirely and uses a generator to build the list. I see I don't have push access to the PR, so I'll try sending a PR to your branch. If that doesn't work, I'll probably just merge it all together manually.

Strange. I've allowed edits / commits by maintainers 🤔 Anyway, I merged your changes and, while I was add it, rebased the PR to resolve the merge conflict.

jaraco · 2021-05-23T13:01:10Z

Strange. I've allowed edits / commits by maintainers 🤔 Anyway, I merged your changes and, while I was add it, rebased the PR to resolve the merge conflict.

Seems the mistake is on my side, an artifact of having switched from hub to gh and gh not handling pr checkouts as elegantly (cli/cli#2189).

hroncok · 2021-07-23T12:51:08Z

pypa#2645

cdce8p force-pushed the license-file branch from a09ff3c to 20407bf Compare April 17, 2021 19:23

cdce8p force-pushed the license-file branch from 20407bf to 8d2c7be Compare May 9, 2021 18:32

jaraco reviewed May 19, 2021

View reviewed changes

Comment thread setuptools/command/sdist.py

jaraco reviewed May 19, 2021

View reviewed changes

Comment thread setuptools/dist.py Outdated

jaraco reviewed May 19, 2021

View reviewed changes

jaraco approved these changes May 19, 2021

View reviewed changes

jaraco reviewed May 21, 2021

View reviewed changes

cdce8p force-pushed the license-file branch 2 times, most recently from 91f8e31 to af7b7b5 Compare May 22, 2021 10:48

jaraco mentioned this pull request May 22, 2021

Replace for/if/add/extend with generator on patterns. cdce8p/setuptools#1

Merged

cdce8p and others added 8 commits May 23, 2021 01:42

Add License-File field to package metadata

3544de7

Remove license_files_computed field

b16725a

Overwrite exlude from MANIFEST with license_files option

482e8e7

* needed for 'License-File' metadata, as this is written before MANIFEST is read

Keep user sorting for license files

3d1cce5

Fix after rebase

e8d7f88

Remove license_file

540a912

Revert removal of license_file

c837956

Replace for/if/add/extend with generator on patterns. Use unique_ever…

4429ffb

…seen to dedupe.

cdce8p force-pushed the license-file branch from 41cc2eb to 4429ffb Compare May 22, 2021 23:42

Fix flaky test

acf4807

jaraco merged commit 75e6496 into pypa:main May 23, 2021

cdce8p deleted the license-file branch May 23, 2021 00:11

cdce8p mentioned this pull request May 23, 2021

(Frozen release plans) Progress to 4.0 raimon49/pip-licenses#89

Closed

13 tasks

hroncok mentioned this pull request Jul 23, 2021

[BUG] Unusually named license_files not included in dist-info #2739

Closed

1 task

CAM-Gerlach mentioned this pull request Nov 29, 2021

PEP 639: Make requested additions and changes, add additional helpful info, reformat, update and copyedit python/peps#2164

Merged

hroncok pushed a commit to hroncok/setuptools that referenced this pull request Jan 12, 2022

Add License-File field to package metadata

e1aa394

pypa#2645

This was referenced Mar 10, 2022

allow to include license text file in the wheel metadata PyO3/maturin#829

Closed

Package license files in .dist-info PyO3/maturin#837

Merged

abravalheri mentioned this pull request Aug 30, 2023

[BUG] pyproject.toml: Unicode character in a license file prevents dependencies from being installed #4033

Open

1 task

hauntsaninja mentioned this pull request Dec 17, 2024

Stop writing License-File #4771

Closed

cdce8p mentioned this pull request Jan 9, 2025

[BUG] tool.setuptools.license-files results in invalid metadata #4759

Closed

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Uh oh!

Conversation

cdce8p commented Apr 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Open questions

Pull Request Checklist

Uh oh!

Uh oh!

Uh oh!

jaraco May 19, 2021

Choose a reason for hiding this comment

Uh oh!

cdce8p May 20, 2021

Choose a reason for hiding this comment

Uh oh!

jaraco May 21, 2021

Choose a reason for hiding this comment

Uh oh!

jaraco left a comment

Choose a reason for hiding this comment

Uh oh!

cdce8p commented May 20, 2021

Uh oh!

jaraco May 21, 2021

Choose a reason for hiding this comment

Uh oh!

cdce8p May 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaraco May 21, 2021

Choose a reason for hiding this comment

Uh oh!

cdce8p May 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaraco May 22, 2021

Choose a reason for hiding this comment

Uh oh!

jaraco commented May 21, 2021

Uh oh!

cdce8p commented May 22, 2021

Uh oh!

cdce8p commented May 22, 2021

Uh oh!

jaraco commented May 22, 2021

Uh oh!

cdce8p commented May 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaraco commented May 23, 2021

Uh oh!

hroncok commented Jul 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cdce8p commented Apr 17, 2021 •

edited

Loading

cdce8p May 21, 2021 •

edited

Loading

cdce8p May 21, 2021 •

edited

Loading

cdce8p commented May 22, 2021 •

edited

Loading