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

Be more explicit with requires so pip install works out of the box. Also... #31

Closed
wants to merge 1 commit into from

Conversation

chrislea
Copy link

Trivial changes that will make pip install gevent so it will work out of the box. Also makes it easier to make debian packages for this.

…lso makes it easier to make debian packages.
@rgalanakis
Copy link
Owner

Unfortunately this isn't trivial. The idea of goless was to support multiple backends (stackless, pypy-stackless, gevent, etc). I do not want to bake in gevent as a requirement. It crossed my mind many times because it makes goless not pip-installable reliably, but I don't want to tie it to gevent (at least I hope the error message when you have no backend is descriptive).

I would be open to first testing for stackless (import stackless), and then only having the requires if stackless isn't present.

@chrislea
Copy link
Author

Fair enough.

Is it okay with you, being the author, if I publish Ubuntu packages of this library on Launchpad that simply require gevent? I think it's certainly going to be the most common use case for people who just want to type

sudo apt-get install python-goless

and have something working.

@@ -0,0 +1 @@
gevent>=1.0
Copy link
Owner

Choose a reason for hiding this comment

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

What's the point of the requirements file?

Copy link
Author

Choose a reason for hiding this comment

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

It's what pip uses to figure out dependencies.

https://pip.readthedocs.org/en/1.1/requirements.html

Copy link
Owner

Choose a reason for hiding this comment

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

I meant more, why bother with the duplication between setup.py and requirements.txt. AFAIK you can just not have the requirements file at all if everything is in setup.py.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, that I honestly don't know. I just always see them duplicated in other libraries. I suspect it's for different installer things. Meaning, I think easy_install may use what's in setup.py.

@rgalanakis
Copy link
Owner

Absolutely, but let's try the conditional install_requires first :) If you don't want to do it I can take a stab.

@chrislea
Copy link
Author

Oh, I don't have to modify the setup.py to do this. I can just require gevent as part of the debian packaging independently of the library.

@rgalanakis
Copy link
Owner

Alright, sounds good. I'm going to see if the conditional requirements works, and if so upload a 0.7.1. You can wait for that, or just explicitly list the dependency, up to you.

@chrislea
Copy link
Author

I'll wait for you. :)

Thanks for writing this though. Goroutines are great, but I have enough investment in python that "just port it all to Go" isn't a very good option. Looking forward to playing around with this!

@rgalanakis
Copy link
Owner

Ok, this is committed and seems to be working.
a4b902c
Post an issue if you run into any problems!

@rgalanakis
Copy link
Owner

This caused an issue with Python3, since it needs gevent installed from github until a new version is released. Figuring it out now but may need to roll back.
See #32

@chrislea
Copy link
Author

No worries. I've made Ubuntu packages, but only for Python2 due to the aforementioned gevent incompatibility. I'll make packages for both when a new gevent is actually released.

rgalanakis added a commit that referenced this pull request Jul 25, 2014
setup.py will install gevent if stackless is not imported *and* we're using Python2.
It was too difficult to install gevent from source as part of setup.py,
so we just warn instead.
Adjusted tox to reflect the less need for explicit dependencies.
Address #32, which is fallout from #31
@rgalanakis
Copy link
Owner

Ok. I did what I could here, and just warn in the case of python3, as in the commit description.

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.

2 participants