Skip to content

Convert error for ../ in license paths into deprecation warning#4896

Closed
abravalheri wants to merge 0 commit intopypa:mainfrom
abravalheri:issue-4892
Closed

Convert error for ../ in license paths into deprecation warning#4896
abravalheri wants to merge 0 commit intopypa:mainfrom
abravalheri:issue-4892

Conversation

@abravalheri
Copy link
Contributor

Summary of changes

I did some experiments, I think it manages to allow the inclusion of the files without breaking the License-File: metadata for other tools (🤞).

/cc @cdce8p @tiran

Closes #4892

Pull Request Checklist

@abravalheri
Copy link
Contributor Author

Docs error /home/runner/work/setuptools/setuptools/NEWS (links).rst:9: WARNING: 'any' reference target not found: distutils.ccompiler._default_compilers [ref.any] seems to be unrelated.

Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks good! I was a bit concerned initially since the "old" code only worked because we just copied every license file into .dist-info. That's not really in the spirit of PEP 639 so I think this can only be a stop-gap measure.

From the issue it became clear that at least in the setuptools world it's more common than one would think to reverence files outside the project directory. Maybe setuptools should provide an option to map this to locations inside the project dir? That would also help with the sdist / wheel issue.

Comment on lines +699 to +704
files_found = set(wf.namelist())
expected_files = {
"test_proj-42.dist-info/licenses/LICENSE.txt",
"test_proj-42.dist-info/licenses/NOTICE.txt",
}
assert expected_files <= files_found
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't know about the <= comparison for sets. Quite a neat trick 👍🏻

@abravalheri
Copy link
Contributor Author

Thank you for the review!

Maybe setuptools should provide an option to map this to locations inside the project dir? That would also help with the sdist / wheel issue.

That does not sound very trivial, so I will defer that to a later discussion 😅.

@abravalheri
Copy link
Contributor Author

Merged manually via git CLI.

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 this pull request may close these issues.

[Request for Reverting Intentional Breaking Change] New license file validation breaks projects with non-standard layout

2 participants