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

add license file, setuptools dependency #15

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bollwyvl
Copy link

Hello! Thanks for the work!

Background: I'm working on packaging stopit for conda over on conda-forge:
conda-forge/staged-recipes#4708

If you'd like, I can add you as a maintainer there as well!

Anyhow, as part of that, here's a small PR which:

  • adds a LICENSE file (based on this template)
  • includes the license and the tests in the distribution
  • adds the dependency on setuptools to setup.py

Thanks again!

@@ -47,5 +47,7 @@ def read(*names):
packages=find_packages('src'),
package_dir = {'': 'src'},
test_suite='tests.suite',
install_requires=['setuptools'],

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Author

Choose a reason for hiding this comment

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

Unless i'm mistaken, pkgresources is provided only by setuptools?
https://github.com/glenfant/stopit/blob/master/src/stopit/__init__.py#L10

Copy link
Owner

Choose a reason for hiding this comment

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

That's OK for me but IMHO if tou have pip, you already have setuptools. That's why I didn't add it to setup.py. Unless you nee it for conda packaging.

Choose a reason for hiding this comment

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

Correct @bollwyvl. Wasn't sure if pkg_resources was involved hence the question. Agree that it should be added.

Choose a reason for hiding this comment

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

It shouldn't be added, in some cases setuptools depending on itself caused failures for me while using pip install -U, also this code is already being executed by setuptools (hence from setuptools import setup), if there's no setuptools prerequisite it will fail on import earlier (given dist dist being installed from sdist).
P.S. in some ancient envs pkg_resources has been provided as a separate dist.

Copy link
Contributor

Choose a reason for hiding this comment

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

100% never depend on setuptools

@glenfant
Copy link
Owner

Hi @bollwyvl , and thanks to help improving stopit. Two comments:

  • I'm considering to switch the license to MIT that's more liberal (include stopit in whatever distro you want)

  • I intentionnally didn't include the tests and doc in the distro (the wheel or tar.gz) to keep this package as light as possible. So please don't do this.

@bollwyvl
Copy link
Author

bollwyvl commented Jan 1, 2018

Howdy! Just suggestions, feel free to close this or do with it what you will! If you do keep GPL, definitely add it to your repo, and consider including the license in the distribution, as per the license :P

Yeah, it's possible to not have pip, i guess, and then also possible not to have setuptools. I imagine a virtualenv without pip is possible, too. So kinda pseudo standard lib, and not usually used in this way, from what i've seen.. not sure which PEP would cover that...

@bollwyvl
Copy link
Author

bollwyvl commented Jan 3, 2018 via email

@colin-nolan
Copy link

@glenfant it would be useful if the license could be changed to MIT - GPL is rather restrictive for a small library like this.

@glenfant
Copy link
Owner

glenfant commented Feb 9, 2018

Created the 1.1.2 with license set to MIT, tested with Python 3.5 and Python 3.6.

@bollwyvl
Copy link
Author

Great, we've packaged up the new MIT-licensed one:
https://anaconda.org/conda-forge/stopit/files?version=1.1.2

Would still love to see the LICENSE included in the distribution!

@glenfant
Copy link
Owner

@bollwyvl The MIT license file is in the repo. Is it usual to have it in the source / wheel distro ?

@bollwyvl
Copy link
Author

tl; dr: no, including the license text isn't universal on pypi, or even in many conda packages. Other package management ecosystems, like debian, have it as a hard requirement. In conda-forge, we're trying to get more in line, as the idea is to make free/libre software as usable as possible, both from the technical angle, i.e. "my python works with my R works with my tensorflow" as well as "not going to freak out my boss or legal team" angle. So as we go along, when we package for conda-forge, we include the license directly, as you intend it to be honored, an usually try to make an upstream request to make the process more predictable in the future.

  • IANAL
  • this hasn't been tested, to my knowledge, in court..
  • and not to get too :neckbeard: on it, BUT

a software license is a legal document. Right in the terms of all of them is usually a clause that says, something like, as in MIT:

The above copyright notice and this permission notice shall be included in all 
copies or substantial portions of the Software.

Each download of the package constitutes a copy. So if a package doesn't distribute its license, that copy isn't technically in compliance with the license of the source repo, and one could imagine some lawyers using that in case they were infringing your license (pretty hard, actually with MIT)... but you really don't want to run afoul of the SHOUTING at the bottom, which covers you in case the software is faulty and causes harm.

IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE 
LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY

this is the kind of thing where you end up with "100,000 cases of infringement" or whatever.
So anyhow, MIT is 1kb in the wheel, and will keep both the lawyers and the neckbeards away. Win! If you don't include it in a future release, we'll continue to include it for you in the conda package! Thanks again!

@bollwyvl
Copy link
Author

@@ -1 +1,2 @@
include CHANGES.rst
include LICENSE

Choose a reason for hiding this comment

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

This also needs to be added to setup.cfg as license_file in [metadata] section for the sake of wheels.

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.

6 participants