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

Add flatpak support #185

Merged
merged 31 commits into from
Jun 19, 2024
Merged

Add flatpak support #185

merged 31 commits into from
Jun 19, 2024

Conversation

chase9
Copy link
Contributor

@chase9 chase9 commented Apr 16, 2023

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.

@l0drex
Copy link
Collaborator

l0drex commented Apr 17, 2023

Great idea, I hope this will work out. There might be issues with sandboxing, let's see.

@chase9
Copy link
Contributor Author

chase9 commented Apr 17, 2023

@l0drex If you're ok with it, I think the scope of this should be to enable easier packaging and not necessarily sand boxing. That will make it easier to implement and will still be a net-benefit to the project since it's easier to distribute!

Nevermind, I don't think it's necessarily possible to disable the sandbox for an app.

@l0drex
Copy link
Collaborator

l0drex commented May 13, 2023

What we could do (as far as I understand):

  • explicitly add every path we want to access with correct read / write permissions
  • add access to everything, and add the flatpak prefix to every path (/var/run/host)

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.

@l0drex l0drex linked an issue Jun 9, 2023 that may be closed by this pull request
@chase9
Copy link
Contributor Author

chase9 commented Jun 24, 2023

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.

@l0drex
Copy link
Collaborator

l0drex commented Nov 27, 2023

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.

@chase9
Copy link
Contributor Author

chase9 commented Dec 4, 2023

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.

@chase9
Copy link
Contributor Author

chase9 commented Dec 4, 2023

A couple of notes:

  • Notifications aren't working but I don't think it's due to the Flatpak. My system doesn't have notify-send on it. Should we try to find a better way to send notifications? For now I put in an error catch so it doesn't crash the app
  • The systemd unit is working but the application doesn't launch when systemd calls it. We'll need to investigate this.

@l0drex
Copy link
Collaborator

l0drex commented Dec 4, 2023

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 flatpak-run. Now, this would break the app outside flatpak. So either we keep the flatpak patches in a different branch or repo, or we just require flatpak and make the native version unsupported. The latter would be my preference (as a developer), since it's less to think about. And flatpak should work on every system. But I can also see that some users wouldn't want to use it...

@chase9
Copy link
Contributor Author

chase9 commented Dec 5, 2023

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: /usr/bin/python3: No module named yin_yang

I think we might need to add a way to launch the application with just something like yin-yang since flatpak is having trouble resolving modules.

@chase9
Copy link
Contributor Author

chase9 commented Dec 13, 2023

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.

@chase9
Copy link
Contributor Author

chase9 commented Dec 19, 2023

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 flatpak-builder --install --user build sh.oskar.yin-yang.json --force-clean

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.

yin_yang/NotificationHandler.py Outdated Show resolved Hide resolved
yin_yang/config.py Outdated Show resolved Hide resolved
yin_yang/helpers.py Outdated Show resolved Hide resolved
yin_yang/helpers.py Outdated Show resolved Hide resolved
@chase9
Copy link
Contributor Author

chase9 commented Dec 20, 2023

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

@l0drex
Copy link
Collaborator

l0drex commented Dec 20, 2023

I just tried to build and run it on my machine, and it works! I used flatpak run sh.oskar.yin-yang though, with your command I get an error that the app is not installed

"--share=network",
"--socket=x11",
"--socket=wayland",
"--socket=system-bus",
Copy link
Collaborator

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

Copy link
Contributor Author

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.

Copy link
Collaborator

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

@chase9
Copy link
Contributor Author

chase9 commented Jan 20, 2024

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 ~/.config/Kvantum/ instead.

@l0drex
Copy link
Collaborator

l0drex commented Jan 20, 2024

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 ~/.config/Kvantum/ instead.

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.

@l0drex
Copy link
Collaborator

l0drex commented Jan 21, 2024

I'm a bit confused with the flatpak documentation:

As mentioned above the host option does not actually provide complete access to the host filesystem. The main rules are:

  • These directories are blacklisted: /lib, /lib32, /lib64, /bin, /sbin, /usr, /boot, /root, /tmp, /etc, /app, /run, /proc, /sys, /dev, /var
  • These directories are mounted under /var/run/host: /etc, /usr

https://docs.flatpak.org/en/latest/sandbox-permissions.html#filesystem-access

So is /usr blacklisted, or is it mounted under /var/run/host/usr? My guess is the latter.

@chase9
Copy link
Contributor Author

chase9 commented Jan 21, 2024

I'm a bit confused with the flatpak documentation:

As mentioned above the host option does not actually provide complete access to the host filesystem. The main rules are:

  • These directories are blacklisted: /lib, /lib32, /lib64, /bin, /sbin, /usr, /boot, /root, /tmp, /etc, /app, /run, /proc, /sys, /dev, /var
  • These directories are mounted under /var/run/host: /etc, /usr

https://docs.flatpak.org/en/latest/sandbox-permissions.html#filesystem-access

So is /usr blacklisted, or is it mounted under /var/run/host/usr? My guess is the latter.

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.

@l0drex
Copy link
Collaborator

l0drex commented Jan 21, 2024

That's great! Now, we should introduce a simple helper function for those system paths (/usr and /etc)

@chase9
Copy link
Contributor Author

chase9 commented Apr 8, 2024

I just rebased the add-flatpak branch on my latest beta branch

@chase9 chase9 force-pushed the add-flatpak branch 10 times, most recently from 297e1ff to 7945ad4 Compare April 19, 2024 20:00
@chase9
Copy link
Contributor Author

chase9 commented Apr 19, 2024

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).

@l0drex
Copy link
Collaborator

l0drex commented Apr 19, 2024

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

@chase9
Copy link
Contributor Author

chase9 commented Apr 19, 2024 via email

@l0drex
Copy link
Collaborator

l0drex commented Apr 19, 2024

Yes, I think that is a good idea.

Copy link
Collaborator

@l0drex l0drex left a 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.

pyproject.toml Outdated Show resolved Hide resolved
runner.sh Outdated Show resolved Hide resolved
yin_yang/daemon_handler.py Outdated Show resolved Hide resolved
yin_yang/plugins/colors.py Outdated Show resolved Hide resolved
yin_yang/NotificationHandler.py Show resolved Hide resolved
@chase9
Copy link
Contributor Author

chase9 commented May 6, 2024

Hi @l0drex! Are there any other changes you would like to see with this PR? I would love to get this merged, if possible.

@l0drex
Copy link
Collaborator

l0drex commented May 9, 2024

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 qdbus is not accessible?).
The app doesn't use the native Qt theme because the qt package from pip is used instead of a native one. Not really sure how to solve that.
The dark mode of yin Yangs GUI doesn't change even though the rest of the system does, which is kinda funny. I guess this is related to the issue above?

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.
Later we could try to use CMake like Lutris does, hoping that solves theming issues and might improve the packaging process. I'd like to have a CI to update the flatpak SDK and runtime (in the future).

@l0drex l0drex merged commit 55877f1 into oskarsh:beta Jun 19, 2024
3 checks passed
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.

Support Flatpak.
2 participants