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

Eggify websockify #31

Merged
merged 1 commit into from
Feb 18, 2012
Merged

Eggify websockify #31

merged 1 commit into from
Feb 18, 2012

Conversation

desaintmartin
Copy link
Contributor

Websockify is now an egg. (clean pull request version)
The egg only includes python code and not js files. It may be changed (tell me, and I'll try in another pull request).

It will allow easy deployment ($ easy_install websockify) and can be uploaded to pypi.python.org.

kanaka added a commit that referenced this pull request Feb 18, 2012
@kanaka kanaka merged commit 31716a7 into novnc:master Feb 18, 2012
@kanaka
Copy link
Member

kanaka commented Feb 18, 2012

Looks good. I pulled the change. It would be good if most of the files in include could be included too.

include/websock.js is designed to be an easy interface to websockify. include/web-socket-js includes the Flash based fallback/shim for browsers without native WebSocket support. Those are both pretty important components of the full websockify suite. I'm not sure what the logical location is for those to be installed, but the README and documentation do assume that those pieces are part of websockify.

If you can get around to figuring out the logical solution for that it would be great. Thanks!

@kanaka
Copy link
Member

kanaka commented Feb 18, 2012

After merging, I decided to test it. I'm not sure that everything is being picked up correctly (it's been a while since I've done python dist builds though so I might be doing something wrong).

$ python setup.py install --root root

$ find root
root/
root/usr
root/usr/local
root/usr/local/bin
root/usr/local/bin/websockify
root/usr/local/lib
root/usr/local/lib/python2.6
root/usr/local/lib/python2.6/dist-packages
root/usr/local/lib/python2.6/dist-packages/websockify-0.1_dev.egg-info
root/usr/local/lib/python2.6/dist-packages/websockify-0.1_dev.egg-info/not-zip-safe
root/usr/local/lib/python2.6/dist-packages/websockify-0.1_dev.egg-info/SOURCES.txt
root/usr/local/lib/python2.6/dist-packages/websockify-0.1_dev.egg-info/requires.txt
root/usr/local/lib/python2.6/dist-packages/websockify-0.1_dev.egg-info/PKG-INFO
root/usr/local/lib/python2.6/dist-packages/websockify-0.1_dev.egg-info/entry_points.txt
root/usr/local/lib/python2.6/dist-packages/websockify-0.1_dev.egg-info/dependency_links.txt
root/usr/local/lib/python2.6/dist-packages/websockify-0.1_dev.egg-info/top_level.txt

The /usr/local/bin/websockify file is actually just a stub. So I don't think any of the content made it into the install directory. python setup.py bdist_egg doesn't seem to result in any different in the egg file either.

@desaintmartin
Copy link
Contributor Author

It worked in my special case but indeed does not work in generic case.

I changed setup.py in https://github.com/SlapOS/websockify to work when doing "python setup.py ...", feel free to try it before I do another crappy pull request.

In fact, setting a full package, including .js files, would need addition of another "dummy" file : __init__py. setuptools requires it.
We can dismiss creation of this file, but in this case AFAIK we can't include files in the package.

A solution to clean all those new files would be to create an src directory containing init.py, websockify.py and websocket.py (and other source files), but maybe it does not fit your needs.

About numpy, do you want websockify to require it always or just use it if already present, but not explicitly requiring it?

@desaintmartin
Copy link
Contributor Author

Ping. Have you tried it to install as an egg?
I cleaned up our fork (that we are using in production for quite long time with buildout, working like a charm) on SlapOS account, maybe I should do a pull request to have real working egg.
But maybe you don't like the idea of adding init.py file.

Then, depending on your strategy, you may upload it into pypi.python.org to have one-line installation over different OSes.

@timkurvers
Copy link
Contributor

I would love to see support for pip. pip install git+git://github.com/kanaka/websockify.git succeeds, however does not seem to install any of the required sources. I'm assuming the current configuration is not using distutils?

@desaintmartin
Copy link
Contributor Author

This is because setup.py has some problems. I will create a pull request to fix it.
On my computer it works with python setup.py install, and it works as well inside of buildout (which uses setuptools).
I'll try this afternoon a bit more with pip then push the request.

@desaintmartin
Copy link
Contributor Author

Finally : I can make it work (see pip install git+git://github.com/SlapOS/websockify.git) but it adds the init.py file.

Maybe it is a good idea to follow #39 to clean up the directory structure and have :
/
/setup.py
/websockify/
/websockify/init.py
/websockify/websockify.py
/websockify/websocket.py

But, Kanaka, you decide! If you like it this way (current slapos websockify fork with my 3 commits), I can do a pull request.:)

@kanaka
Copy link
Member

kanaka commented Jul 6, 2012

@desaintmartin, yep, now is probably the time to refactor into a directory, however, I would like to do a more minimal refactor in the short term rather than everything suggested in issue 39:

  • Rather than the websockify binary in a bin directory I would like that at the top level. The executable can be a very simple import of the websockify module and then a call of websockify_init (i.e. leave the argument processing in websockify/websockify.py for now).
  • Leave the html/css/js in 'include/' for now. It can move to www later, but that requires some minor code changes to websockify (and to noVNC).

If you would like to make those changes, test them and issue a pull request I'll merge them in. Thx.

@timkurvers
Copy link
Contributor

@kanaka, wouldn't the binary conflict with the package name-wise?

@kanaka
Copy link
Member

kanaka commented Sep 17, 2012

I just pushed a change to push the code down into a websockify subdirectory. The setup.py scripts seems to work much better now. Instead of a websockify script at the top level which would conflict with the directory (@timkurvers) I just have run (symlinked to websockify.py).

@timkurvers
Copy link
Contributor

Fabulous! Thanks :)

@desaintmartin
Copy link
Contributor Author

Thanks, I'll try it soon!

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.

3 participants