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

instructions to compile proxybroker to shared executable for Major platforms (Windows, Linux, and MacOS) #95

Merged
merged 4 commits into from
Aug 30, 2022

Conversation

ziloka
Copy link

@ziloka ziloka commented Aug 23, 2022

Instructions for #31.

The steps for macos should be the same.
For windows it should be very similar the end of the $(which proxybroker) should be replaced with the location of the proxybroker script

@ziloka
Copy link
Author

ziloka commented Aug 23, 2022

the line in proxybroker/utils.py was stolen from this stackoverflow comment

@bluet
Copy link
Owner

bluet commented Aug 23, 2022

@ziloka Is this for Linux/macOS or Windows also? AFAIK commands on Windows should be different, no?

It's OK if this is for Linux/macOS only, just change the PR title to clarify it. :-)

@ziloka
Copy link
Author

ziloka commented Aug 23, 2022

It works on windows too, I just don't have the develop environment on windows to test it because I usually work on Linux distributions.

@ziloka
Copy link
Author

ziloka commented Aug 23, 2022

AFAIK commands on Windows should be different, no?

Yes they are different, which is why I stated that $(which proxybroker) needs to be replaced with the location of the python script

@ziloka
Copy link
Author

ziloka commented Aug 23, 2022

I think python 3.9 build is broken for windows when compiling. Here is the error and the fix for it

@ziloka
Copy link
Author

ziloka commented Aug 23, 2022

Looking from this github comment it seems weird that the best solution is to not use python 3.9 on windows, or add visual studio c++ compiler or the workaround is downgrade to python 3.8. I'm not sure about python 3.10.

This seems solution to Collaborate that you don't need c++ compiler on 3.8 or 3.10 but you need it for 3.9

@ziloka
Copy link
Author

ziloka commented Aug 23, 2022

Okay just to note here

Because the program makes a temporary folder with a unique name, you can run multiple copies of the app; they won’t interfere > with each other. However, running multiple copies is expensive in disk space because nothing is shared.

The _MEIxxxxxx folder is not removed if the program crashes or is killed (kill -9 on Unix, killed by the Task Manager on
Windows, “Force Quit” on macOS). Thus if your app crashes frequently, your users will lose disk space to multiple _MEIxxxxxx > temporary folders.

It is possible to control the location of the _MEIxxxxxx folder by using the [--runtime-tmpdir](https://pyinstaller.org/en/stable
/usage.html#cmdoption-runtime-tmpdir) command line option. The specified path is stored in the executable, and the bootloader > will create the _MEIxxxxxx folder inside of the specified folder. Please see Defining the Extraction Location for details.
Running the program and it crashing it suddenly will cause users to lose disk space due to pyinstaller having to extract said files

References:
Stackoverflow How to prevent pyInstaller temporary files from being deleted?
Github How to prevent pyInstaller temporary files from being deleted?

Source: How the One-File Program Works

It seems like the one-folder executable would be more desirable in most cases compared to the one file executable.

@ziloka
Copy link
Author

ziloka commented Aug 24, 2022

Just curious, should we make automated builds to build executables for this project.

  1. Just to know if the project can still be built with the changes made
  2. People can just download them from github actions instead of compiling it themselves. For convenience sake?

@bluet
Copy link
Owner

bluet commented Aug 24, 2022

I've been in pure Linux env for over 15 years and don't have a Windows environment too. Actually, I can't say I know how to use it to develop or test python programs 🤣

Before we can confirm if it really works on Windows through testing all the functions, we can't say "It works". People might believe our words and waste lots of time following the instructions we provide, which only get errors lol

If making it work on Windows is difficult for now, we can reduce the scope of this PR to support just Linux/macOSX first (and review/merge it first). For the Windows support, you can create another PR and spend time digging deeper. 😃

@bluet
Copy link
Owner

bluet commented Aug 24, 2022

@ziloka auto-builds would be awesome. I just don't have the time (and knowledge, and passion) to figure out how to make windows builds. 🥲

If you know how, PRs are welcome! 😃

@ziloka
Copy link
Author

ziloka commented Aug 24, 2022

alright, i was thinking of like building them for the three major platforms for three different versions of python (3.8, 3.9, 3.10) because thats what we support.

@ziloka
Copy link
Author

ziloka commented Aug 24, 2022

Okay so I dont think mac builds work either? https://github.com/ziloka/proxybroker2/runs/8000511714?check_suite_focus=true
RIght now it seems like Windows and Mac builds dont want to build which is kinda annoying.

@ziloka
Copy link
Author

ziloka commented Aug 24, 2022

From my research I honestly just think we should limit this PR to Linux only. Until I have some type of solution for windows and macOS.

@ziloka ziloka changed the title instructions to compile proxybroker to shared executable instructions to compile proxybroker to shared executable for linux Aug 24, 2022
@ziloka ziloka force-pushed the update-readme branch 4 times, most recently from 7c89f18 to e8c00f2 Compare August 24, 2022 18:24
@ziloka
Copy link
Author

ziloka commented Aug 25, 2022

Okay I have a solution for the other platforms and I think its just using a different tool for those platforms.

@ziloka
Copy link
Author

ziloka commented Aug 25, 2022

Nevermind I used the wrong flag (for whatever reason), and Linux and macOS builds are fine
https://github.com/ziloka/proxybroker2/runs/8007781077?check_suite_focus=true

FYI, python 3.10 builds will never work because they deprecated the loop parameter, so you would have to sync #88 to master first before syncing this pull request.

@ziloka ziloka changed the title instructions to compile proxybroker to shared executable for linux instructions to compile proxybroker to shared executable for Unix Aug 25, 2022
@ziloka
Copy link
Author

ziloka commented Aug 25, 2022

Okay just to let you know, since I did a lot of research today, the reason why I can't make github actions build for windows is because of the project structure. It would honestly be better if there was a single python file that we wrote in side this project. Instead of generating that file and calling pyinstaller on it.

But I'm afraid to make a ton of reorganizing changes because I'm sure the project was structured like this for a reason.

@bluet
Copy link
Owner

bluet commented Aug 25, 2022

Okay just to let you know, since I did a lot of research today, the reason why I can't make github actions build for windows is because of the project structure.
It would honestly be better if there was a single python file that we wrote in side this project.

Do you mean to combine EVERYTHING into ONE file?

Instead of generating that file and calling pyinstaller on it.

I don't quite understand what you mean "generating that file".

But I'm afraid to make a ton of reorganizing changes because I'm sure the project was structured like this for a reason.

Split different sets of codes into different files/modules will make it cleaner and easier to maintain, and they'll also be reusable.

@bluet
Copy link
Owner

bluet commented Aug 25, 2022

@ziloka BTW, #88 has been merged into master 3 days ago. Maybe you'd like to sync the changes back to your branch?

@ziloka
Copy link
Author

ziloka commented Aug 25, 2022

Oh im sorry i mean #94

@ziloka
Copy link
Author

ziloka commented Aug 25, 2022

Okay just to let you know, since I did a lot of research today, the reason why I can't make github actions build for windows is because of the project structure.
It would honestly be better if there was a single python file that we wrote in side this project.

Do you mean to combine EVERYTHING into ONE file?

Instead of generating that file and calling pyinstaller on it.

I don't quite understand what you mean "generating that file".

But I'm afraid to make a ton of reorganizing changes because I'm sure the project was structured like this for a reason.

Split different sets of codes into different files/modules will make it cleaner and easier to maintain, and they'll also be reusable.

  1. I mean to explicitly have a command line .py file, instead of having setuptools generate that file. However im not sure if thats possible for a python application to be a library and a command line application while having a command line entrypoint that was written by programmers instead of having setup tools generate that file for us. This is related to the second point I stated. The executable entry point is based off the executable python script that was generated by setup tools. That's why I was able to make Linux and MacOS builds.

  2. What i meant by generating that file is that. Essentially that when you run pip install . On linux and MacOS setuptools will generate a python script and it'll be in the path so when you run proxybroker you can call that executable python script. However on windows its different. console_scripts (for further notice look at setup.py) are wrapped in a console executable. It is the same thing with gui_scripts. ASCII executable on Linux and MacOS. But it is wrapped in a gui executable. (read the notes)

  3. I dont reallly know why I said this. Disregard it for now.

@bluet
Copy link
Owner

bluet commented Aug 25, 2022

I think we can postpone the windows build at the moment and open another PR for it after we have other issues fixed 🤣

BTW, if the PR is in Work In Progress status, you can add the "WIP: " prefix in PR title, and remove it when you think it's ready for review/merge later. 😃

@ziloka
Copy link
Author

ziloka commented Aug 25, 2022

I think its ready

@ziloka
Copy link
Author

ziloka commented Aug 25, 2022

Alright, next time I will draft the PR instead of just like letting it be like this

@ziloka
Copy link
Author

ziloka commented Aug 27, 2022

Going to make a PR for convert py to exe for windows because it will cause a merge conflict with #94.

EDIT: I think I should just do it in on this branch, because after this I'm gonna make the pr for ci/cd github actions py2exe.

@ziloka ziloka changed the title instructions to compile proxybroker to shared executable for Unix instructions to compile proxybroker to shared executable for Major platforms (Windows, Linux, and MacOS) Aug 27, 2022
@ziloka ziloka force-pushed the update-readme branch 4 times, most recently from 70f8db8 to 34bf9b7 Compare August 27, 2022 02:09
@ziloka
Copy link
Author

ziloka commented Aug 27, 2022

https://github.com/ziloka/proxybroker2/runs/8047370449?check_suite_focus=true

Windows, Linux and MacOS builds are now fully supported 🥳

I just need to modify the script so when it upload the artifacts the MacOS and Linux builds don't conflict with each other (because the file name is both proxybroker, and on windows its proxybroker.exe). I guess github actions upload artifact overwrite them or something. I need to dig deeper.

@bluet
Copy link
Owner

bluet commented Aug 27, 2022

@ziloka cool.

BTW, something still need to be done in this PR, could you add WIP prefix in title?

@bluet
Copy link
Owner

bluet commented Aug 27, 2022

And could you check the content and format are correct?

I don't think people need to install binutils etc on Windows too lol

Also there's a missing ``` at the end of docker part in README. I did have that in my previous code suggestions but maybe due to a bug in GitHub it's missing now.

bluet
bluet previously approved these changes Aug 27, 2022
@bluet bluet self-requested a review August 27, 2022 08:03
@bluet bluet dismissed their stale review August 27, 2022 08:05

Something else needs be done

@ziloka ziloka marked this pull request as draft August 27, 2022 11:22
@ziloka ziloka changed the title instructions to compile proxybroker to shared executable for Major platforms (Windows, Linux, and MacOS) WIP instructions to compile proxybroker to shared executable for Major platforms (Windows, Linux, and MacOS) Aug 27, 2022
@ziloka ziloka force-pushed the update-readme branch 2 times, most recently from 67edf10 to 18220ab Compare August 27, 2022 11:48
@sonarcloud
Copy link

sonarcloud bot commented Aug 27, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ziloka
Copy link
Author

ziloka commented Aug 27, 2022

Okay I think I fixed it now

@ziloka ziloka marked this pull request as ready for review August 27, 2022 12:06
@ziloka ziloka changed the title WIP instructions to compile proxybroker to shared executable for Major platforms (Windows, Linux, and MacOS) nstructions to compile proxybroker to shared executable for Major platforms (Windows, Linux, and MacOS) Aug 27, 2022
@ziloka ziloka changed the title nstructions to compile proxybroker to shared executable for Major platforms (Windows, Linux, and MacOS) instructions to compile proxybroker to shared executable for Major platforms (Windows, Linux, and MacOS) Aug 27, 2022
@bluet bluet merged commit 5c27520 into bluet:master Aug 30, 2022
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