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

Pyinstaller support #384

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

Pyinstaller support #384

wants to merge 6 commits into from

Conversation

cowo78
Copy link

@cowo78 cowo78 commented Mar 7, 2019

Added a PyInstaller .spec file to build a frozen distribution.
Also set "C" locale to limit required encodings modules.

@CendioOssman
Copy link
Member

Thank you for your contribution. This configuration file looks a bit large though, and difficult to maintain. Do we really need to have a large list of exclusion?

And we don't want to mess up the locale like that as we would like to respect the system settings.

@cowo78
Copy link
Author

cowo78 commented Apr 3, 2019

Regarding size/maintenability: this is pretty much standard pyinstaller config, so I basically don't expect any changes to be needed (exept i.e. if websockify entry point changes). The exclusion list is of course optional: on python3.7/64bit leaving it out changes output size from 11MB to 14MB.

Regarding the locale: since no currencies are involved it basically means datetime. AFAIK logging in websockify has a custom formatter that only displays message so I don't see any changes happening.

However these are not standpoints to me so I'll stick to your decision.

@CendioOssman
Copy link
Member

Going from 11 MB to 14 MB doesn't seem like a big deal. Can we remove the entire exception list in that case?

@cowo78
Copy link
Author

cowo78 commented Apr 4, 2019

Depends on how much flash you have on your device, but surely that's not a deal on common PC-based hardware.
I can comment the exclusion list, revert the setlocale and add some comments in the .spec on how to reduce output size.
Sounds good?

@CendioOssman
Copy link
Member

Sounds good. 👍

cowo78 pushed a commit to cowo78/websockify that referenced this pull request Apr 4, 2019
proceed to re-enable them (see PR novnc#384 discussion)
@cowo78
Copy link
Author

cowo78 commented Apr 4, 2019

Please have a look.
It may be interesting to distribute the self-contained binaries (think mainly on windows, though I don't even know if websockify works on the platform) along with other formats in a release cycle.

@CendioOssman
Copy link
Member

Looks good, thanks. Could you rebase everything in to a single commit?

It may be interesting to distribute the self-contained binaries (think mainly on windows, though I don't even know if websockify works on the platform) along with other formats in a release cycle.

Might be interesting, yes. I don't have much experience building self-contained python executables though.

Giuseppe Corbelli and others added 6 commits April 15, 2019 15:01
Added a websockify.spec file to be used with pyinstaller to build a frozen distribution

Explicit excludes have been disabled and comments added on how to
proceed to re-enable them (see PR novnc#384 discussion)
We should only start the server if we are the main module, and not
imported some other way. This is important for multiprocessing to
work correctly on Windows.
ForkingMixIn isn't available on Windows. This is the simple server
without features, so use ThreadingMixIn to keep things consistent.
It works well enough now with the recent fixes and a modern Python.
cowo78 pushed a commit to cowo78/websockify that referenced this pull request May 15, 2019
Added a websockify.spec file to be used with pyinstaller to build a frozen distribution

Explicit excludes have been disabled and comments added on how to
proceed to re-enable them (see PR novnc#384 discussion)
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.

None yet

3 participants