-
-
Notifications
You must be signed in to change notification settings - Fork 158
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 binary #113
Comments
Hello o/ Thank you for taking interest in this topic 🙂
Interesting, didn't know that. I'm assuming it requires a Linux environment to build though? I'm not using Linux myself so I think that'd be quite hard to handle, but I could try figuring something out.
First time I'm hearing about this, I have Python 3.10 installed at my work laptop from which I was able to build a local non-release executable for testing, and it worked just fine there. Maybe it doesn't work only for Linux?
That's weird. Tkinter is a library that's normally installed together with Python, per this picture from the Python installation guide I made for Windows: https://github.com/DevilXD/TwitchDropsMiner/wiki/Python-Installation If you don't check it, you won't get tkinter and will need to install it separately as you said. A quick google search suggests that in Linux case, all you need to do after installing
I can see that you've reenabled console, probably to see any loading errors that might occur. Smart move.
Pyinstaller warnings about missing packages should be taken with a huge grain of salt. During the analisys process, all imports are collected to determine which libraries have to be included in the final binary for it to run without errors. This includes conditional imports, that try to import a library, but are just fine to fail the import if said library is not present. This is the main reason I've started using venvs since around In general, unless you're getting runtime errors about missing libraries or files, there's no need to bother with even looking at PyInstaller warnings at all.
A quick search suggests that all you need is an additional hidden import to be added to the |
yes you do have to build in on linux
Referring to Note that Python 3.10.0 contains a bug making it unsupportable by Pyinstaller (but looks like my mistake could be only 3.10.0 not 3.10.x.
I followed your installation guide
That's correct, I do have python3-tk installed, but it doesn't work...
Alright, makes sense because I wasn't able to fix it :D
Yes I've seen many threats about that as well, it didn't work for me though. Edit: It is indeed only 3.10.0 I just build a working binary with 3.10.9 |
That's fine then, if it really needs to be there, so be it. What's the resulting binary size?
That could be because of the
You've installed it from the normal binary installer instead of the packages system? In that case, SO suggests to rerun the installer, use "Modify" and ensure Tk/Tcl is ticked as an installation component. I have no idea how Linux handles normal GUI installers vs packages from their system in this regard, it could be the same or completely independent installations. If they're independent, installing If I had to guess, from the two paragraphs above, I have a VPS with ubuntu installed on it, and I can try setting up the build there, to see what trouble I could encounter myself. It's a no-GUI, console-only, so called "headless" instance, so I won't be able to actually go very far with running it, but it should at least build properly. Will do and let you know how it went. |
This is correct,
Well I removed all tk tk-dev tcl tcl-dev dependencies from my system and only used python3-tk for the last install. Same problem without hidden import. |
Update: and a works for me so far. |
And with hidden import, it does work? The import is fine if necessary, it's more about Tk/Tcl being included in the build properly, using the one that installs with Python, not some 3rd party package.
Actually, it's there. You've put two tabs instead of 8 spaces like in the original file, and the line was thus yeeted into the right so far I haven't noticed it. |
Installing Your build scripts, revised by me. Added Build itself (had to use Dropbox cos of the file size): https://www.dropbox.com/s/ubjldhesss7s50p/Twitch%20Drops%20Miner%20%28by%20DevilXD%29?dl=1 As expected, this is the error I've got:
No errors related to Tk/Tcl, but it also didn't really launch, so... I can't test it myself, but you can. |
Ah that's what caused the warning: You're correct about the error level check did it in hurry and wanted to fix it soon^tm :D Running your binary I get:
using the build script it does run though |
I have been experimenting with a Linux build on my personal fork here: master...guihkx:TwitchDropsMiner:linux-wip And builds are available here (look for build jobs on the https://github.com/guihkx/TwitchDropsMiner/actions Keep in mind that this branch is not stable; I rebase it constantly to try stuff out. Additionally, I have added some changes to make debugging the build on Linux a bit easier. For instance, the app is not just a single executable yet. Now, about the quality of the build itself: In theory, it should work just fine (with a lot of caveats 😄), on any Debian-based distro that has glibc 2.31 (or newer). However, I'm currently facing an issue that prevents the app from working on Arch Linux (which is what I daily-drive), and also on Fedora 38 (which I tested on a virtual machine). This error pops up just a few seconds after the app launches: The error looks looks like the one reported in #80, which doesn't seem to have a proper solution yet, I think. But after running the binary with
And shortly after that, the runtime error pops up. Now, this might be a red herring or I might be talking out of my ass, but I think this error happens because the On Ubuntu, root certificates seem to be stored in
However, on Arch and Fedora they're stored in
And what also makes me think this is the cause of the issue, it's because if you package the app on Arch Linux and then try to run it also on Arch Linux, things works just fine (and this is probably the same for Fedora, though I haven't tried it). So, maybe something goes really bad behind the scenes because the app fails to find the root certificates? I'm not really sure, but I was able to get rid of that error just by configuring the
🤷 |
@guihkx I just tried your binary but starting it resulted in those errors on a kali linux (vm) and glibc 2.36 (ui didn't even show up):
|
Based on what I found on Google, you could try one of these two things: |
@guihkx as you can see a post above your first one I had the same problem while using the binary of Devil
After compiling the latest build (I didn't check your build) the executable is working even with the current build.spec file, which is an improvement from the last time I compiled it |
Yeah, unfortunately it's also been my experience that packaging on the machine in which you intend to run the app works much better... |
So it turns out that the Thankfully though, version 2.3.5 of libXft has fixed that ([1], [2]), but my build remained broken because Ubuntu 20.04 (which I'm using in the CI workflow) still has version 2.3.3 in its official repositories. I worked around this simply by building a recent version of libXft and then telling PyInstaller to bundle the new version instead of the old system version. Another fix I recently added is related to that weird Linux users, feel free to give the latest build a shot here: https://github.com/guihkx/TwitchDropsMiner/actions/runs/5037436242 The most important features of the app should now work as expected on virtually any Linux distro! 🤞 There are some other important issues that also need investigation, however (they are not related to packaging):
Some other minor Linux-only issues I discovered:
|
The non-packaging related issues might be related to them having platform checks in place.
This one does two different things on Windows and Linux. On Windows, it hooks up to the app window's event queue and catches the event that is emitted when the system does it's shutdown sequence. On Linux, it uses the built-in protocol listener to catch the event of the user closing the window, then proceeds with the shutdown. Lines 1873 to 1895 in 5b8b6c7
Ref SO post: https://stackoverflow.com/questions/73497704/how-to-handle-a-wm-endsession-in-tkinter
Yes, that's listed in one of the Pystray issues: moses-palmer/pystray#29 (comment)
There is absolutely nothing in Pystray issues about that system. I could create a new issue and ask about it, but the last activity in the repo was a year ago, and the project seems kinda abandoned a bit. I can still try, but I think I'd need to install KDE Plasma on a VM first. You could also create an issue yourself, since you already have a way to test in some way, but still, I'm not sure where would that take us, really. Does more mainstream Linux systems work fine? Ubuntu, Fedora, Debian? I'm not really a Linux person, but I always thought those three are the most common. If it works there, I'd say it's good enough already.
The code that handles autostart has an intentional check that makes it do nothing on non-Windows systems: Lines 1566 to 1581 in 5b8b6c7
I just never learned how exactly one can do autostart on Linux. I know a way with systemd, but as far as I'm aware, that creates a service and thus needs a whole service definition file etc. Sounds way too complicated for a simple "run this when system starts". I just never sat down and figured out how to do it, really, so making it at least not crash the entire application (trying to import
More Pystray problems. No idea, I couldn't find any issues on the Pystray repo about this either. They don't work at all, even on the mainstream systems?
Link opening functionality is done via Lines 271 to 289 in 5b8b6c7
|
Indeed. Plus, I haven't really investigated them properly yet, so for now these are here only for documentation purposes. :P
Thanks for the pointer! After a quick Google search, this one was actually very easy to fix: I just had to replace the
I think there was a misunderstanding here, hahah. The comment I made about GNOME needing a system tray extension was just an observation, really. xD Unfortunately, it is known that the GNOME desktop hasn't had the concept of a system tray for a very long time now... In fact, popular distros like Ubuntu include that system tray extension by default, because for many it's still a crucial feature, but GNOME developers apparently don't think so. 🤷♂️ So no worries, a GUI rewrite is not really needed because that's definitely not on you. :P
Don't sweat about it. Like I said, I didn't really investigate any of these problems thoroughly just yet. But I'll let you know if I find anything.
Yeah, that's not what we usually do for desktop (GUI) applications on Linux. Instead, most apps ship with a desktop file, which you can then simply copy to the I'm not a Python dev by any means, but I'm pretty sure I could implement this very easily. I just can't guarantee that the quality of the code will be stellar... 😅
I haven't really tested this particular feature on different distros because it takes some time for these desktop notifications to be triggered. I'm sure I could create a simple test case for this, but to be honest it's not really at the top of my priorities list at the moment. Ironically though, if you run the Windows build of the app on Linux using Wine (which is something I was actually doing before working on problems from the Linux build), tray notifications work just fine: Not only that, but running the Windows app on Linux using Wine has none of the issues I mentioned in my previous post (except for the autostart one, but that's by design)... 😆
Hmm, that's good to know and it will definitely be useful when I debug this. My suspicion that this is a packaging-related issue is because opening these links actually works when I run the app without being packaged by PyInstaller. I could be wrong, though. Whenever I click on a link, this message pops up (not sure if it's just a red herring):
Anyway, I'll try to look into those thoroughly eventually. |
Cool! Just a caution note from me though, even though
Oh, it's fine! You can try writing some code for it if you'd want to. If it's just a matter of dropping a text file into that path though, it'll be something like: # this import needs to be added at the top
from textwrap import dedent
# rest of the code
# this adds double quotes around the path to the executable
# if that wouldn't be needed, just use: self_path = str(SELF_PATH.resolve())
self_path = f'"{SELF_PATH.resolve()!s}"'
if tray:
self_path += " --tray"
# contents of the desktop file - modify this up to what you think is needed
# 'dedent' is used to remove the 4 spaces from in front of each of the lines,
# so the code still looks nice while the file has them omitted
contents = dedent(f"""
[Twitch Drops Miner]
Exec={self_path}
Name=Twitch Drops Miner
"""
# this is what actually opens the file (in create-or-overwrite mode), writes the contents, then closes it
with open("~/.config/autostart/TDM.desktop", "w", encoding="utf8") as file:
file.write(contents)
# use this to delete the file when the option is unticked
try:
os.unlink("~/.config/autostart/TDM.desktop")
except FileNotFoundError:
pass
Funny. I've heard about Wine a long time ago, when someone tried to run Windows-only games on Linux. Sounds cool. "Try using Wine" sounds like a useful note to add for anyone who'd like to try using this on Linux in the future for sure.
After some searching around, it seems to be an open Python bug. Refs: Not sure what to do here. There's no going around this one, unfortunately. The only way to open URLs without the standard lib would be using a 3rd party lib, which I'd like to avoid. What do you think about the discussion under that issue? I'm not sure how scary those "zombie processes" are, is that something to worry about, or not really? |
Good to know. Thanks!
Thanks for that. I haven't tried implementing it yet, but your snippet will definitely help!
Most definitely! I haven't had a single issue by running TDM on Linux using Wine. The RAM usage is a bit higher than running the native Linux version, but other than that, I'd say it's just as good as running on a real Windows machine.
Hmm, I don't know, but I believe the issues you linked don't seem related to the actual problem. Without being packaged by PyInstaller, I found this PyInstaller issue: And even though the original reporter is using Windows, if you scroll at the end of the thread, you'll see this comment, which based on my Linux experience, seems very plausible to be the actual root cause of the problem, which is caused by PyInstaller modifying The person even posted a workaround that looks very promising, but I haven't actually tried it yet. Regarding the tray icon issue, it was never about the format of the icon (ICO or PNG), but instead simply because If I replace that by just My attempt at implementing the tray icon in a separate thread as suggested by the creator of |
That looks easy to implement. The suggested code seems good enough to remedy the issue as well. The usage could be simplified into a simpler context manager though: import os
from contextlib import contextmanager
@contextmanager
def restore_lib_path():
lib_key = "LD_LIBRARY_PATH"
if (curr_env := os.environ.get(lib_key)) is not None:
if (orig_env := os.environ.get(f"{lib_key}_ORIG")) is None:
os.environ.pop(lib_key)
else:
os.environ[lib_key] = orig_env
yield
if curr_env is not None:
os.environ[lib_key] = curr_env
def open_link(url: str):
with restore_lib_path():
webbrowser.open_new_tab(url) All of the above assumes this
Ah, well, I used But, since it doesn't work on Linux, it seems I'll need to do it "the hard way" anyway 😅 To run something in a separate thread, the default loop executor could be used: https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_in_executor There's a chance it wouldn't work either, in which case I can prepare a branch that will have those implemented, together with the autostart bit implemented too, if you'd like. I'm currently struggling with being slightly overworked and can't really find the time for this project for now, but I'll try making this branch for you, if it'd allow you to continue testing the Linux build. |
That would be incredible, thank you!
Don't even worry about it, I completely understand! And take your time too, the last thing I want is to take the fun out of working on personal a project. If ultimately you don't find the time to do it, I'll keep working on it myself. ^^ As a matter of fact, I am (slowly) trying to implement those changes, but without proper Python expertise they will probably end up looking like crap. XD Here are the changes I added so far (feel free to change any of them): master...guihkx:TwitchDropsMiner:linux (btw, I tested that |
No worries, when I was starting out with Python, my code was less than ideal too x) All I can say, is that you're more than welcome to try and make things work for now, and we can handle "make code look nice" later.
Great! That's one less issue then :) |
I've implemented all 3 changes we've talked about here. The one replacing the environment variable has received a little rewrite, because MyPy was arguing that PyInstaller may not set I did exactly zero testing of these, beyond doing a static typing check via MyPy. If you can, please check those. Diff of changes: guihkx/TwitchDropsMiner@linux...DevilXD:TwitchDropsMiner:linux |
Awesome, thanks! I have some feedback. The autostart option is still borked, likely because the
On Linux, the However, the actual, standardized way to do create this
Still regarding the I'd also choose a more descriptive name for the file itself, perhaps Unfortunately, commit d886b7d does not work correctly. :/ While the tray icon does get added after I click on 'Minimize to Tray', immediately the main window gets completely unresponsive and you can't quit the app normally: frozen.mp4 |
Just to be clear, I didn't expect things to work on the first try x)
That's a classic mistake of me. Please try this: db7dc23 Regarding the autostart feature, I've switched the logic to using the Changes: a9605d2 |
Sorry for the delay... :S
Hahah yeah of course. No worries. :)
That's so much better now! I found a minor issue, but as far as I'm concerned, it could be left as-is because it don't seem to break the app in any way (I think): The issue is that, once created, the tray icon is permanent through the lifetime of the app: permanent-tray-icon.mp4This causes a rather funny behavior as well: If the app is not currently minimized to tray, the tray icon is not interactive (i.e. left/right clicks yield nothing). But as soon as you click on 'Minimize to Tray' from the app, the tray icon becomes interactive again, and immediately dispatches any previous left/right clicks. And after that, at least on KDE Plasma, the tray icon gets invisible 😭: lol.mp4But like I said, I don't think these are major issues. And I think that if you're able to destroy the icon after the app is restored from tray, these two issues will likely go away. Still regarding the tray icon, I just learned that pystray supports many system tray implementations on Linux. The current best one seems to be AppIndicator. However, this implementation requires quite a lot of native libraries, which ends up increasing the final size of the app quite considerably: From ~25 MB to ~80 MB (after being UPX-compressed). If the AppIndicator library is not found on the system, pystray tries to use the
That didn't work for me, unfortunately. The standard mentions that the Additionally, the Exec="/home/user/Downloads/Twitch Drops Miner/Twitch Drops Miner (by DevilXD)" --tray I think that wrapping the path of the binary in quotes isn't really supported...? At least it doesn't work on KDE Plasma. So you can remove those. However, now you'd also have to escape a bunch of special characters, which is really annoying. A much easier way to do this would be: Exec=sh -c '"/home/user/Downloads/Twitch Drops Miner/Twitch Drops Miner (by DevilXD)" --tray' However, this approach could be problematic if the path has quotes or apostrophes, but I don't think those are common in paths anyway, so whatever. In any case, if you want to try to develop a "perfect solution", here's the specification for the Exec key so you can have nightmares. FYI: I've cleanup some commits from my linux branch (I did include your changes too), so you might want to hard reset your branch against mine, just so you can get the most recent changes I added. Thanks! |
The thing is, that's kinda already what happens. When the program's main window is restored, the icon's thread is stopped, and the icon itself gets its last reference removed, letting Python dealocate it at the earliest applicable time: Lines 1081 to 1084 in 4664a9d
There's not much more I can do/call here, besides the
This is selected automatically by Pystray, which handles the tray icon. Not entirely sure if I can influence the choice it makes. It will pick the best possible implementation it can use, and that's it. Maybe the final doc should recommend installing "AppIndicator" as it seems to be more reliable? Regarding autostart issues, adding in |
Oh :( Oh well, then you can leave it as is. No biggie.
After a lot of trouble learning how PyInstaller works, I was able to include all the required dependencies for the AppIndicator backend (the changes are in this commit), so the user won't have to install these libraries manually. The app is a bit larger because of that, but IMO we're making a good trade-off here.
It's a similar situation on Linux, actually. But from what I could understand from the specification, we'd have to escape white space characters ( Now, personally I tried this solution and it didn't work, so I still think the |
Alright :) I've updated my |
Thanks! And sorry for the mess of duplicates commit caused; I tend to rebase my branch quite often to keep it as clean as possible, and if you do a Speaking of which, I've included your recent cleanups and rebased again, so keep that in mind... 😅
Nice, it works great now! I've been trying to find a better approach for the tray icon problem, and I think I came up with a decent solution. Instead of Note that this particular change requires we switch to the My implementation itself you can find here, so feel free to make it better if you want, and I can incorporate it later, as usual. :P Remaining issue I could find:
Overall, I'd say that's some awesome progress! Thanks a lot for the help! |
My git skills aren't the greatest, I usually google most of the commands. The That being said, if I won't forget, I'll try rebasing next time. I don't ever recall doing so, so I imagine I'll mess something up anyway. My VSCode does most git things for me already, so hopefully it won't be too bad. Hopefully >.>
Sounds okay. To avoid complicating the logic, the icon could work like this on Windows as well. I never explored using visibility to hide the icon, simply because the current implementation already worked, so there was no incentive for me to do so. EDIT: Just force-replaced my local linux branch with yours, and changed some things around to use icon.visible to hide and show the icon on all systems. Did a quick test and it works smooth on Windows. Does this remove the AppIndicator requirement, or is it still needed, just more reliable?
I did a quick research on this, and this library looks like it could do: https://pypi.org/project/desktop-notifier/ Windows notifications are supported, but we'd only really be interested in using it for Linux. Async interface will integrate well with the existing application. If whatever the library's page reads sounds enough for you, I can try adding to the |
Indeed. And in the end it's just a matter of preference, I guess. :p
Interestingly enough, I remember trying the previous backend (GtkStatusIcon) yesterday and I was having some issues. But I just briefly tested now, and the issue seem to be gone. But I'd like to test a little bit more. If GtkStatusIcon proves to be solid, I'll remove the AppIndicator changes. :) One minor issue that I immediately noticed with GtkStatusIcon, is that the icon looks blurry (we probably can't do anything about it either): And with AppIndicator the icon looks fine:
That's great. However, I tested your changes on Linux, and for some odd reason, when I minimize to tray for the first time, the icon ends up hiding as well. I narrowed the issue down to this line: Line 1090 in 977f997
And I fixed it with this diff: diff --git a/gui.py b/gui.py
index 2092a93..5781e67 100644
--- a/gui.py
+++ b/gui.py
@@ -1087,7 +1087,8 @@ class TrayIcon:
if self.icon is None:
self._start()
assert self.icon is not None
- self.icon.visible = True
+ else:
+ self.icon.visible = True
self._manager._root.withdraw()
def restore(self): It's probably a bug in pystray. But can you check if the change above doesn't break things on Windows? If it doesn't, then I think it's safe to use it.
That looks like a solid project to me.
I could give it a shot implementing it, yes. But honestly it might take days, especially since I'm not too familiar with the code base (or with Python, for that matter lol). So, if you have the time and will to do it, be my guest. 😅 |
RE: blurry icon - it's really up to decide. Someone needs to decide if having a slightly blurry icon is worth cutting down the app size in half. Since I'm not a Linux user, you'll have to decide on it :p RE: icon.visible - Back when I started using Pystray, some studying of the code showed me that RE: desktop-notifier mockup for testing - I'll try to find some time for a test program. If everything works, I'll try implementing something in the actual application. I've been busy lately, so it may take a while, but I'll do it eventually. |
The size increase is caused by libraries related to Gtk (which are needed by PyGObject). However, even though GtkStatusIcon doesn't need PyGObject, I think it still needs most of the same Gtk libraries to work, so in the end I don't think it will make much difference... I say let's keep the AppIndicator one.
Awesome! :D
Thank you! No need to rush. |
@DevilXD So... It turns out desktop notifications are already supported on Linux by pystray. My bad! 🤡 When the documentation stated "notifications aren't supported on macOS and Xorg", I mistakenly assumed they meant Linux as a whole. But that literally means only the Xorg backend of pystray doesn't support notifications. The others do... 🤦♂️ In fact, I confirmed that just now: So I'd say the Linux build is pretty much ready for the masses! Assuming you want these changes merged, how do you want to proceed now? Should I open pull requests for each of my commits? Or all changes in a single PR? Some other way you prefer...? By the way, in my latest change I was able to reduce the size of the Linux build by 23.8 MiB! It went from 78.7 MiB down to 54.9 MiB. Still not as small as the Windows build, but much better than before. :P |
Bruh x) Alright then, that's good news. Happy about the size reduction :) I'd prefer a single PR. As far as commits go, it's either merge as-is, or squash first. I'll decide between the two, when it's time to do so, but probably just merge it in, unless there's many commits, then squash instead. There's one more thing remaining. The Linux build has to have added a short description/explanation in the This addition should be a part of the PR. You're much more familiar with the subject. Would you be able to write something up? A simple bullet point list would do. |
In #35 linux support got added and released with v15 and it's working like a charm.
As of the last comment by Devil I tried my best to make an executable with some instructions but failed.
What I found out:
I had some big trouble settings up tkinter, and it looks like I still have (maybe).
Important
You have to install tkinter besides of pip. (sudo apt-get install tcl-dev && tk-dev), check for weird paths mine wasn't stored in
/usr/lib/tcltk/
but instead in/usr/share/tcltk
Many fails later I finally managed to run pyinstaller and build an executable (yey) but now I'm stuck again and have no clue what's wrong and wth I'm doing wrong...
After using this build.spec file I was able to build an executable with this output:
pyinstaller.txt
After investigating, I found those WARNINGS:
So I searched for those files and all of them are present...
Now to the actual error when starting the compiled binary:
I'm unsure if everything is clear I made a TON of "fixes" without even knowing anymore, so some information might be missing. If this helps, great if not well...
I hope someone catches up on this. Cheers
The text was updated successfully, but these errors were encountered: