-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
Add flatpak support #185
Add flatpak support #185
Conversation
Great idea, I hope this will work out. There might be issues with sandboxing, let's see. |
Nevermind, I don't think it's necessarily possible to disable the sandbox for an app. |
What we could do (as far as I understand):
I would prefer the first option. It's more work, but doesn't completely disable the sandbox and makes it visible to the users what files we access. |
I rebased my branch on the 3.3 beta and have 80% working. I'll need to test and make sure the systemd timers are working correctly, and Firefox is still not working quite right. |
Hey @chase9, are you still working on this? I think it's fine to disable the Firefox plugin in the Flatpak, I can add some logic for that. |
I fell off after my last update but I would be happy to pick it up again! If you are ok with disabling Firefox in the flatpak for the time being, that should make things easy. FWIW I have been running the flatpak on my main machine for several months now and things appear to be stable. |
A couple of notes:
|
I guess there is a freedesktop interface via DBUS that we can use for notifications. For the systemd unit to work, I think we just need to change the command to use |
I personally think it's worth keeping support for installing the application outside of Flatpak so that it continues to stay accessible. I think the solution is easier than I thought since we can run custom commands when building the Flatpak. I'm running a sed command now to replace the executable path for the .desktop and the .service when we build the Flatpak. I've been trying to get the .desktop file working this morning but I'm having a lot of trouble with this: I think we might need to add a way to launch the application with just something like |
I think what's going on here is that we need to setup this project in such a way where it normally installs itself into the users PATH. Flatpak handles sandboxing, but we still need an entrypoint available to launch. |
Note: This now relies on my other PR since it builds off of that. I'm SUPER happy to report that I have the Flatpak build working! You should be able to test by cloning my branch and running The basics are working but it will need some cleanup to make sure that all plugins are seeing profiles. I also noticed that the systemd units are not cleaned up if you remove the Flatpak, but I don't know if we can work around this since there doesn't appear to be an export for .service and .timer files. |
Thanks for the review, I'll go through it tomorrow. FYI in case it's a blocker, I'm unable to get this to run on my M1 Mac. I'm consistantly running into an error with a library: ❯ flatpak run --branch=master --arch=aarch64 --command=runner.sh sh.oskar.yin-yang
Traceback (most recent call last):
File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
exec(code, run_globals)
File "/app/lib/python3.10/site-packages/yin_yang/__main__.py", line 9, in <module>
from PySide6 import QtWidgets
ImportError: libdouble-conversion.so.3: cannot open shared object file: No such file or directory
|
I just tried to build and run it on my machine, and it works! I used |
"--share=network", | ||
"--socket=x11", | ||
"--socket=wayland", | ||
"--socket=system-bus", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to change in the future, if we want to publish to Flathub:
Applications should not grant --socket=system-bus or --socket=session-bus, unless they are development tools. The --log-session-bus and --log-system-bus flags can be used with flatpak run in order to track down usage.
https://docs.flathub.org/docs/for-app-authors/requirements/#dbus-access
Its fine for now though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know much about dbus but it appears the above flags give us unlimited ownership, which is the problem. Finding the dbus paths we need to talk to and adding the --talk flag instead will hopfully work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thats the better approach. I added DBusPlugins in order to find these more easily, but they can't be used everywhere.
Another issue is that we don't know the interfaces before runtime, f.e. Okular appends -pid
to its interfaces. So we kinda need access to everything
We may want to write a wiki page with workarounds for using the flatpak. For example, installing Kvantum themes will install them into /usr, which will never be usable with flatpak. When using the flatpak it's recommended to install Kvantum themes into |
I think we need this only to see available themes. If we fallback to a simple text input when running from flatpak, we still can allow switching to any theme. |
I'm a bit confused with the flatpak documentation:
So is |
It looks like it's blacklisted from being included in our sandbox paths, but is mounted under /var/run/host/usr. I added some logic to the Kvantum plugin to look at that path and I was able to use the full set of themes installed on my PC. |
That's great! Now, we should introduce a simple helper function for those system paths ( |
I just rebased the add-flatpak branch on my latest beta branch |
297e1ff
to
7945ad4
Compare
After many force pushes, the github action for automatically building both a .whl and .flatpak are working! You can download them from the artifacts section of the last build. I think the final steps are to refactor the action (it takes a bit too long for my tastes), and to make sure any errant issues in the Flatpak version are fixed (I was having trouble getting automatic location working on my system). |
Awesome! Thank you so much for your work, this will help a lot. Automatic location is a general problem, but a fix is on the way: #285 |
Of course!Great news about location getting a fix soon. If that’s the case, do you think I should pause additional commits until we can merge this and then work on additional issues/additions in future PRs? I worry about this PR getting too unwieldy.
Chase
Please excuse any grammatical errors as this was sent from my phone.On Apr 19, 2024, at 3:45 PM, Lorenz
Hoffmann ***@***.***> wrote:
Awesome! Thank you so much for your work, this will help a lot.
Automatic location is a general problem, but a fix is on the way: #285
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Yes, I think that is a good idea. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I took a first look at it. Seems like most changes are just changes in quotation marks, this makes it really confusing - so please revert these.
Hi @l0drex! Are there any other changes you would like to see with this PR? I would love to get this merged, if possible. |
Sorry, I was kind of busy the last days. Tried it out today. I noticed the Konsole Plugin throws an error (File not found, maybe So I am going to take a look at #290, hoping it might solve the Konsole issue. I'll then merge this into a new branch until the theming is sorted out. |
This PR aims to add flatpak support to make it easier to distribute and install Yin-Yang.
This PR is currently in WIP status and is not ready to be merged.