semuconsulting / PyGPSClient

Python Graphical GPS Client Application supporting NMEA, UBX, RTCM3, NTRIP & SPARTN Protocols
BSD 3-Clause "New" or "Revised" License
493 stars 98 forks source link

CI: add workflow for building a windows binary #140

Closed Williangalvani closed 2 months ago

Williangalvani commented 2 months ago

PyGPSClient Pull Request Template

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Testing

Only test I did so far is make sure the software opens on windows.

Checklist:

semuadmin commented 2 months ago

Hi @Williangalvani

Thanks for this contribution, but I'm not seeing a great deal of value in incorporating a PyInstaller-based deployment into the automated GitHub workflow.

The existing platform-agnostic pip install already creates an executable (in the Python scripts/bin folder) which runs fine under Windows 10 & 11, and I've had no reports of any difficulty deploying or running on Windows using this standard method.

Happy to be convinced otherwise (and happy to add your deploy.yml to the \examples folder for anyone who does want to use it).

Williangalvani commented 2 months ago

Hi! I am using PyGPSClient to debug some U-Blox GPSs over UDP (u-center just refuses to connect).

My goal with this PR is to avoid having inexperienced (windows) users having to install python and using the terminal for launching PyGPSClient. I would like to have users just download a file from a release, double-click it, and that is it.

The existing platform-agnostic pip install already creates an executable (in the Python scripts/bin folder) which runs fine under Windows 10 & 11, and I've had no reports of any difficulty deploying or running on Windows using this standard method.

For someone who knows how to deal with python/pip that is fine. but I'm always reluctant in telling users (specially in windows where things never work the way I expect) to open the terminal.

Is there another way we could accomplish this?

semuadmin commented 2 months ago

Hi! I am using PyGPSClient to debug some U-Blox GPSs over UDP (u-center just refuses to connect).

Glad to hear PyGPSClient is proving useful :-).

My goal with this PR is to avoid having inexperienced (windows) users having to install python and using the terminal for launching PyGPSClient. I would like to have users just download a file from a release, double-click it, and that is it.

For someone who knows how to deal with python/pip that is fine. but I'm always reluctant in telling users (specially in windows where things never work the way I expect) to open the terminal.

Appreciate the intent but remain unconvinced this adds significant utility. Assuming Python is installed (I recommend the latest Python.org distribution rather than the Microsoft Store version), all we're asking the Windows user to do is open Terminal (which is a standard Windows app, available via Search or the CMD+R wt shortcut) and copy and paste a single command from the README. They would normally only need to do this once - subsequent updates can be accomplished using PyGPSClient's 'Check for updates' feature (provided the user has appropriate file system privileges). I myself have placed an iconised shortcut to the ../Scripts/pygpsclient.exe executable in my Windows 11 Start menu which works in exactly the same way as any other app icon:

Windows Start Menu

If opening Terminal and/or creating a suitable iconised shortcut to ../Scripts/pygpsclient.exe is genuinely the issue here, I can happily provide (a link to) additional instructions in the README.

Is there another way we could accomplish this?

Aways open to suggestions and appreciate all contributions, but I have a number of reservations about the use of pyinstaller:

  1. The principal intention behind PyGPSClient was to provide a platform-agnostic GNSS utility which works on practically all end-user operating systems and CPU architectures using - as far as is practicable - a single set of deployment artefacts and installation procedures. To my mind, Python, PyPi and pip meet these requirements quite happily.
  2. As far as I am aware (I'll happily stand corrected on this point), pyinstaller is not a cross-compiler and can only create 'single-file' runtime executables for the platform it's run on - in the case of a GitHub Actions windows-latest workflow, that means Windows Server 2022 on x64 architecture. There is, as far as I am aware, no GitHub Actions support for Windows on ARM (e.g. for someone using the latest MS Surface Pros), and if you wanted to provide single-file runtime executables for other operating systems and/or architectures, you'd need a discrete pyinstaller workflow and a discrete executable file for each, which significantly increases the Actions runtime as well as introducing additional potential points of failure and an additional (unpaid) support overhead for me.
  3. Providing pre-compiled binaries potentially increases the risk of malware penetration (this may not be a significant risk in the context of a protected GitHub Actions workflow, but there is always a risk of other sites creating links to malicious versions of these binaries).
  4. It would still be incumbent upon the end user to download the executable from the ../dist folder, save it to a location of their choosing (subject to applicable user file system privileges) and (if required) create an appropriate Start menu shortcut to this location. I would imagine any novice Windows user who struggles to open Terminal may well struggle to perform these steps.
  5. For platforms that already have a Python environment installed, a pyinstaller 'single-file' executable bloats the file system by (in effect) replicating elements of this Python environment. On my Windows machine the resultant ../dist/pygpsclient.exe file was over 20MB in size - the ../Scripts/pygpsclient.exe created by pip is only 106kB.
  6. The load time for the pyinstaller executable was over 5 seconds for every invocation on my Windows machine. The pip executable loads almost instantly.
  7. Unlike the pip installation, the pyinstaller workflow would not create runtime executables for the various pygnssutils CLI utilities that come bundled with PyGPSClient.
  8. I seem to recall older versions of pyinstaller (<=5.0) having multiple platform compatibility issues, though those seem to be largely resolved in the latest versions.

Happy to be challenged on any or all of these points.

Williangalvani commented 2 months ago

These are fair points.

I agree with most of your points, so I won't insist too much, but I do have some interesting notes:

There is, as far as I am aware, no GitHub Actions support for Windows on ARM (e.g. for someone using the latest MS Surface Pros)

Windows 11 on arm can run x86 apps on ARM, it seemed fine on a VM on my M1 Mac. It is not going to run at native performance, but pygpsclient is not a heavy software anyway, right?

It would still be incumbent upon the end user to download the executable from the ../dist folder

while this is not what this pr currently does, this is what I had in mind long-term: https://github.com/Williangalvani/PyGPSClient/releases/tag/test just a one-click download and then run the binary

Upon re-reading the install instructions, I think some of my issues were because I didn't check the "Add to PATH" checkbox.

If opening Terminal and/or creating a suitable iconised shortcut to ../Scripts/pygpsclient.exe is genuinely the issue here, I can happily provide (a link to) additional instructions in the README.

Yes I think that would be great!

Williangalvani commented 2 months ago

Given you do have strong points, I think I'll close this now. Thank you for the feedback and keep up the good work =]

semuadmin commented 2 months ago

Given you do have strong points, I think I'll close this now. Thank you for the feedback and keep up the good work =]

Thank you for the contribution. FYI I've updated the README to include enhanced instructions on creating desktop application launchers for Windows, MacOS and Linux. I believe there are Python packages that will do this more-or-less automatically (e.g. https://pypi.org/project/pyshortcuts/) but I haven't evaluated them myself. I dare say it's also been raised as a feature request for the PyPi pip installer.