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

Linux improvements & development builds #216

Merged
merged 13 commits into from
Jun 11, 2023
Merged

Conversation

guihkx
Copy link
Contributor

@guihkx guihkx commented Jun 10, 2023

This adds many Linux improvements and fixes (more details in each commit's description).

This also adds support for development builds for Linux through GitHub Actions.

Closes #113

DevilXD and others added 8 commits June 4, 2023 16:08
On Linux, pystray's icon.stop() currently doesn't work well with the
AppIndicator backend.

So, instead of destroying the icon when the user restores from tray
(only to later re-create it when the user minimizes to tray again), we
have the option to control the icon's visibility instead! Luckily, that
works on Windows too!

Note that for this change to work properly on Linux, we need to switch
to the master version of pystray, because we need this specific fix:

moses-palmer/pystray@5943ed4

After pystray releases a version newer than 0.19.4, we can switch back.

Co-authored-by: DevilXD <[email protected]>
This fixes an issue on Linux (more specifically, on GNOME Shell 44),
where the window would be identified as "Tk".
Makes the app exit correctly on Linux when the main window is closed.
Because Linux distributions don't have an 'universal' path for the
system certificate store, bundling libssl built on distro A might not
work on distro B, because distros tend to change path to the
certificate store at build time.

This means that creating a PyInstaller package on Ubuntu will include
the libssl built there, and if you try to run this package on a
different distro (e.g. Arch Linux), any attempt to create an SSL
connection will fail, because Arch Linux has a different path for the
certificate store than Ubuntu.

This issue is usually worked around by bundling certificates with the
app (usually using certifi), and then pointing the SSL_CERT_FILE
environment to it.

However, truststore seems to be a better alternative, since it tries to
use the certificate store from the system instead.

The list of benefits are listed on the project's page on GitHub:

https://github.com/sethmlarson/truststore
Because PyInstaller modifies the LD_LIBRARY_PATH environment variable
to make things work correctly on Linux, when we try to launch any
subprocess (such as the web browser), that modified variable gets
picked up as well, causing things to go south.

To work around this Linux-only problem, we have to:

1. Save the current value of LD_LIBRARY_PATH
2. Move the value of LD_LIBRARY_PATH_ORIG to LD_LIBRARY_PATH
3. Launch subprocess
4. Restore the saved LD_LIBRARY_PATH again

Reference:

https://pyinstaller.org/en/stable/runtime-information.html#ld-library-path-libpath-considerations

Co-authored-by: DevilXD <[email protected]>
@guihkx guihkx linked an issue Jun 10, 2023 that may be closed by this pull request
@DevilXD
Copy link
Owner

DevilXD commented Jun 10, 2023

Very well. I've added some small stylistic and wording modifications from my side, which you can find on my linux branch. Please make them a part of this PR as well. The Windows build notes aren't really fitting too much in this PR, but I wanted to avoid any potential merge conflicts later on, and the linux branch is going to disappear anyway, so outside of this PR, there will be no trace of which branch the commit came from.

@DevilXD DevilXD added the Enhancement New feature or request label Jun 10, 2023
@guihkx
Copy link
Contributor Author

guihkx commented Jun 10, 2023

Thanks! Is it okay if I squash two of your commits (this and this) into mines and add you as a co-author of them instead of pushing them directly into the branch I'm working on? I personally think that's more organized. The commit related to Windows I will include directly.

guihkx and others added 3 commits June 10, 2023 19:22
This greatly reduces the final size of the UPX-compressed binary, from
78.7 MiB to 54.9 MiB (a 23.8 MiB difference)!

Co-authored-by: DevilXD <[email protected]>
This fixes the following error when you launch the app with an emoji
font installed (e.g. Noto Emoji):

X Error of failed request:  BadLength (poly request too large or internal Xlib length error)
  Major opcode of failed request:  139 (RENDER)
  Minor opcode of failed request:  20 (RenderAddGlyphs)
  Serial number of failed request:  277
  Current serial number in output stream:  300

This has been fixed[1] in libXft 2.3.5[2], but unfortunately Ubuntu
20.04 (which we currently use in the Linux CI workflow), still has
version 2.3.3, so we have to build it ourselves.

[1] https://gitlab.freedesktop.org/xorg/lib/libxft/-/merge_requests/12
[2] https://lists.freedesktop.org/archives/xorg-announce/2022-September/003209.html
@DevilXD
Copy link
Owner

DevilXD commented Jun 11, 2023

Honestly, I don't really care, as long as it keeps the expected structure - a single commit message explains a single addition. You'll probably have to edit some commit message(s) to get it right, but sure, you can squash them.

I pushed those 3 commits right before going to sleep, and didn't catch some missing backticks here and there, so there's a 4th commit now. You can squash that commit too as well.

@guihkx
Copy link
Contributor Author

guihkx commented Jun 11, 2023

Alright, done. Let me know if the commits look good enough.

Copy link
Owner

@DevilXD DevilXD left a comment

Choose a reason for hiding this comment

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

Very well. Thank you so much for your contribution 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linux binary Linux Support?
2 participants