"milksnake" should not be listed in install_requires

[anthrotype]

In the current README.md, the milksnake package is listed in the example setup.py's install_requires, besides the setup_requires.

I understand it being a build requirement, but why does milksnake also need to be listed as an installation requirement, ie. required at runtime by the extension module that used milksnake in its building process?

Note that I haven't used this package yet -- i'm just passing by and considering to use in my projects and too lazy to try and see what happens.

Thanks anyway!

[anthrotype]

I finally found the time to try milksnake. I can confirm the example module does not require milksnake as an installation requirement.
The universal wheel can be installed on both python 2 and 3, and continues to work after I uninstall milksnake (which gets automatically installed because of the "install_requires" in setup.py).

I suggest to remove that from the documentation and the example, as others may get confused.

[mitsuhiko]

Might be worth a change. Originally it was there because we had runtime utilities in milksnake. I wonder if I want to bring them back or not.

[anthrotype]

actually, a big selling point for me is that milksnake is only a build-time dependency rather than run-time...

[mitsuhiko]

In that case I would ay we decide on that and leave the runtime dependency for an optional package.

[mitsuhiko]

However that means you still need to manually set CFFI as an install dependency. That might be a bit annoying to do.

[mitsuhiko]

Actually not going to change this for now because most likely this is going to break everything since the CFFI dep would go away at install time for some setups.

[anthrotype]

CFFI is always required at runtime for any cffi extension module, because of the _cffi_backend, but that's different from milksnake

[mitsuhiko]

But if the install_requires of milksnake is gone so is the install_requires of cffi. Which means that packages like symbolic which currently do not mention an direct dependency on cffi would break if I change that.

[anthrotype]

I think that's their problem. the cffi docs explicitly say that one should add cffi to the install_requires

[mitsuhiko]

But the milksnake docs do not say that. So that is a milksnake problem not a CFFI one.

[anthrotype]

http://cffi.readthedocs.io/en/latest/cdef.html?highlight=install_requires

And that's why I opened the issue :)

[mitsuhiko]

As I mentioned that has nothing to do with CFFI but that CFFI is currently a transitive dependency from the install requires of milksnake. So I cannot remove it now and there would have to be a discussion if that is the right thing to do. Since milksnake is effectively never imported at runtime anyways there does not seem to be that much to gain from having it removed whereas if we do remove it, there is a high chance someone ends up with a cffi mismatch.

[anthrotype]

I disagree, but you're the author so of course you do what you think it's right.

I think that adding something to install_requires signals the fact that something is imported at run-time, which is not the case with milksnake, as the latter is simply a setuptools extension, and setuptools is just a build tool, not something that you import from unless you're pip or similar.

CFFI docs are clear that the cffi extension modules require CFFI as an installation requirement. You just need to clarify that in the README and example setup.py, and make clear that milksnake internally uses cffi to wrap the rust or whatever shared library.

Anyway, your call. Thanks anyway.

[mitsuhiko]

I disagree, but you're the author so of course you do what you think it's right.

It's not about being right or wrong. It's about what is the case so far which is that the documented use case is to use the transitive dependency of milksnake to CFFI. I can't just change that now since it was against what the intended use case was. If I change this now this would literally break sentry without also updating our dependencies there. So this cannot happen right now.

[mitsuhiko]

Closing this. This will stay as an installation dependency.