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

How do include LICENSE files? #114

Closed
wants to merge 1 commit into from

Conversation

doctaweeks
Copy link

This partially reverts commit edab033.

Using data_files causes LICENSE to be installed in prefix which could clobber another file. Using only MANIFEST will include it in the tarball without this risk.

This partially reverts commit edab033.
@codecov-io
Copy link

codecov-io commented Aug 2, 2017

Codecov Report

Merging #114 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #114   +/-   ##
=======================================
  Coverage   95.77%   95.77%           
=======================================
  Files           9        9           
  Lines         686      686           
=======================================
  Hits          657      657           
  Misses         29       29

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffcea49...f6c1cc0. Read the comment docs.

@dol-sen
Copy link

dol-sen commented Aug 2, 2017

Yeah, I just packaged up crossbar yesterday for Gentoo. I had to sed that data_files= line out of setup.py in order to properly install the code. It caused file collisions which halted the install process. We have packaging tools to take care of those files correctly. So they only need be included in the tarball, not attempt to install them into the main file system.

@oberstet
Copy link
Contributor

so how do you suggest to distribute the LICENSE file?

@dol-sen
Copy link

dol-sen commented Aug 24, 2017

Usually just a MANIFEST.in entry suffices. That way it is included in the tarball produced by setup.py sdist.
From there the linux distro packager handles it in whichever way is usual for that distro and pacakge management system.

echo "include LICENSE" > MANIFEST.in

Plus any other miscellaneous files such as README, CHANGELOG,... that you wish to distribute with your tarball.

@doctaweeks
Copy link
Author

@dol-sen is correct that a MANIFEST.in entry is what you want. Since that already exists, this patch simple removes the data_files entry.

@oberstet
Copy link
Contributor

right. so the LICENSE won't get into wheels or bdists. how do I make it that the LICENSE is also bundled in these distribution formats?

@dol-sen
Copy link

dol-sen commented Aug 24, 2017

A quick google search found me at this explanation which seems to fit best.

quote:

Neither package_data or data_files should be part of any "regular" project. They are ridiculously hard to get right. data_files behaves inconsistently across tools. I consider those anti-patterns in packaging [1]. Having a correct MANIFEST.in and include_package_data=True is most always the right choice.

/quote
the include_package_data=True is to be added to the setup()
you can find it here: pypa/sampleproject#30

@oberstet
Copy link
Contributor

oberstet commented Aug 24, 2017

yeah, sucks. as the primary distribution channel of this project is pypi, not linux distros, we adjust to the quirks this comes with. until someone comes up with a better solution that also makes LICENSE part of wheels and bdists.

@oberstet oberstet closed this Aug 24, 2017
@oberstet
Copy link
Contributor

looking through our different projects, this is indeed handled differently:

my main issue, as said, how do we preserve the inclusion of LICENSE (as similar) files also when publishing wheels and bdists inside those? how do we distribute the LICENSE text together with the software to the user when using the former package formats.

I am willing to adjust all projects to "the right way" .. given there is one.

@meejah what's your take?

@oberstet oberstet reopened this Aug 24, 2017
@oberstet oberstet changed the title Do not include LICENSE in data_files How do include LICENSE files? Aug 24, 2017
@meejah
Copy link
Contributor

meejah commented Aug 24, 2017

So from what I can see, include_package_data=True only works if the "data" is "inside a package". So, e.g., top-level stuff like LICENSE just never gets included in wheels or bdists unless we moved it down a level into txaio/ directory (thus making it "part of a package") and include it in the MANIFEST.in

It would also be nice to include the documentation -- as, e.g., the MANIFEST.in here specifies. But, I can't see any way to do that except for package_data= or data_files=. I see I have a fixme comment in txtorcon about this exact issue for exactly the same reasons: I'd like to include README, docs etc in bdists .. but there's no way to do that except via data_files= that I can see.

The only mention of licensing information in a bdist or wheel comes in the metadata which just says the kind of license (e.g. "MIT License").

p.s. .whl files are actually just zip files, so unzip -v ....blah.whl will show you what's in it

@meejah
Copy link
Contributor

meejah commented Aug 24, 2017

Okay so it turns out there's a workaround/undocumented feature specifically for license files. This can be included in setup.cfg:

[metadata]
license_file = LICENSE

...and then the text of the license will be in the dist-info (and, it seems, the actual package as well). There's no way to do this for "other" files (e.g. README, docs etc) that I can find .. those can only go into source dists.

@meejah
Copy link
Contributor

meejah commented Aug 24, 2017

...in any case, it seems that setup.cfg trick should work for this very specific use-case of LICENSE text

@oberstet
Copy link
Contributor

oberstet commented Aug 24, 2017

@meejah the setup.cfg trick via license_file sounds perfect! I guess that achieves both goals: ship the LICENSE file, and don't pollute the install target tree. awesome. =)

I can live with not shipping the README or other things. The LICENSE file is important - even not so much for practical reasons (users will get where to check the license for this stuff), but for legal reasons.

note: now that this final solution is found, we really should roll it out over all projects ..

@oberstet
Copy link
Contributor

merged via #115

@oberstet oberstet closed this Aug 24, 2017
@meejah
Copy link
Contributor

meejah commented Aug 24, 2017

Thanks to @agronholm for pointing me to this in #python :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants