Skip to content
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

Setuptools will install licenses if included in setup.cfg #1536

Merged
merged 8 commits into from
Jan 27, 2019

Conversation

dtaneli
Copy link
Contributor

@dtaneli dtaneli commented Oct 27, 2018

python setup.py sdist now includes the license file if license_file
is included in setup.cfg unless it is explicitly excluded in MANIFEST.in.

Co-Authored-By: Poyzan Nur Taneli 31743851+ptaneli@users.noreply.github.com

Summary of changes

Closes #357

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details

Addressing pypa#357

`python setup.py sdist` now includes the license file if `license_file`
is included in `setup.cfg` unless it is explicitly excluded in `MANIFEST.in`.

Co-Authored-By: Poyzan Nur Taneli <31743851+ptaneli@users.noreply.github.com>
@pganssle
Copy link
Member

@dtaneli Can you add some tests for this?

Co-Authored-By: Poyzan Nur Taneli <31743851+ptaneli@users.noreply.github.com>
@dtaneli
Copy link
Contributor Author

dtaneli commented Oct 27, 2018

Hi @pganssle, I added some tests but I am not sure if it is better to merge these four test cases into a single test or not. Any preferences?

Also, does this change warrant a changelog update?

@pganssle
Copy link
Member

@dtaneli It is generally preferable to have separate tests, or parametrized tests, but I think in the spirit of DRY (don't repeat yourself), it might be better to refactor these a single parametrized test.

@pganssle
Copy link
Member

And yes, this is a behavior change, it definitely needs a changelog.

@dtaneli
Copy link
Contributor Author

dtaneli commented Oct 28, 2018

Made the tests parametrized, added a changelog entry and updated thesetup.cfg documentation.

@pganssle, during the London sprint, we discussed re-using license = file:<path> instead of license_file = <path> since the usage of these attributes kind of overlap. Or maybe we would like to support both?

@pganssle
Copy link
Member

@dtaneli The license field is something completely different, it's used for the name of the license. You'd put something like Apache 2.0 in that field, not the text of the license, or the license file.

@dtaneli
Copy link
Contributor Author

dtaneli commented Oct 28, 2018

Ok, I just wanted to make sure because we found that you could actually pass a file according to https://setuptools.readthedocs.io/en/latest/setuptools.html#metadata


Is there anything else that is missing for the scope of this issue?

setuptools/command/sdist.py Outdated Show resolved Hide resolved
setuptools/command/sdist.py Outdated Show resolved Hide resolved
setuptools/command/sdist.py Outdated Show resolved Hide resolved
setuptools/tests/test_egg_info.py Outdated Show resolved Hide resolved
setuptools/tests/test_egg_info.py Show resolved Hide resolved
@jaraco jaraco merged commit 97e8ad4 into pypa:master Jan 27, 2019
@jakirkham
Copy link
Contributor

Thanks for doing this @dtaneli and @jaraco for reviewing!

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.

5 participants