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

Optional support for new xdg background portal #535

Closed
AsavarTzeth opened this issue Jun 24, 2019 · 25 comments
Closed

Optional support for new xdg background portal #535

AsavarTzeth opened this issue Jun 24, 2019 · 25 comments

Comments

@AsavarTzeth
Copy link
Contributor

AsavarTzeth commented Jun 24, 2019

Thanks to a new portal interface (org.freedesktop.impl.portal.Background) it should now be possible to solve this old issue: flathub/com.github.wwmm.pulseeffects#18

In time this should ideally be used everywhere, whenever possible. It does however add a dependency on xdg-desktop-portal >= 1.4.0, which means distributions providing older versions is the primary reason it can't always be relied upon at this time.

Would it be possible to check for the availability of this DBus API at runtime and if available use that instead of writing pulseeffects-service.desktop directly?

I am no expert but if you need any more information or assistance I will try my best to provide it.

@wwmm
Copy link
Owner

wwmm commented Jun 24, 2019

At least at compile time it is possible to check if it is available as they use pkg-config. I have no idea about runtime. And I also have no idea about how to use it. I think I will need to see a code example.

@wwmm
Copy link
Owner

wwmm commented Jun 24, 2019

These are the places where changes have to be made https://github.com/wwmm/pulseeffects/blob/8e95621d6e11a0376b04e12beed7402e942c5db9/src/general_settings_ui.cpp#L140 and https://github.com/wwmm/pulseeffects/blob/8e95621d6e11a0376b04e12beed7402e942c5db9/src/general_settings_ui.cpp#L124. When we find out how to use this portal it is only a matter of using a preprocessor #ifdef based on the availability of xdg-desktop-portal >= 1.4.0 at compile time. I imagine that the flatpak package is the only one that needs to use this portal. So we could have a build time option that you could use to make meson compile PE with this portal.

@AsavarTzeth
Copy link
Contributor Author

Sorry for confusing things. Just to clarify xdg-desktop-portal is a host dependency, not a bundled depedency. It is a dependency of Flatpak itself as you can see with pacman -Qi xdg-desktop-portal.

I will try to find some example though.

@AsavarTzeth
Copy link
Contributor Author

AsavarTzeth commented Jun 24, 2019

Surely a compile time check would be worthless since we build it once an re-use it it on all distributions, which again might not have xdg-desktop-portal >= 1.4.0.

Edit: Oh and the these portal interfaces are all DBus API:s, so I am pretty sure I caused a major misunderstanding here.

@wwmm
Copy link
Owner

wwmm commented Jun 24, 2019

Maybe we are not even supposed to compile PE with it. Maybe its pkg-config file is meant to be used when flatpak is compiled. I don't know.

Assuming everything can be done through DBUS we could use GLIBMM wrappers for DBUS as it is already done for realtimekit. When we find a code example things will be more clear.

@AsavarTzeth
Copy link
Contributor Author

AsavarTzeth commented Jun 24, 2019

Yes, that is what I failed to convey. You are meant to install this as a regular distribution package, before you can install flatpak apps. It is of course not compiled by us, but as a dependency of Flatpak.

Bundling this would be like bundling core gnome desktop components.

I should have mentioned that these were DBus API:s from the beginning. Back to searching for that example then.

@AsavarTzeth
Copy link
Contributor Author

AsavarTzeth commented Jun 24, 2019

Not sure if it is enough but I found this in the mailing list. Unfortunately the example is not for this particular portal. I thought I would post this just in case.

Yeah, the thing to use here is the background portal (which is kinda
new, so beware it may not be everywhere yet).
"org.freedesktop.impl.portal.Background" is actually the backend (thus
the ".impl"), so what you want to use is
org.freedesktop.portal.Background.

The docs for that are here:
https://github.com/flatpak/xdg-desktop-portal/blob/master/data/org.freedesktop.portal.Background.xml

The portal apis are a bit complex since they are long-running and
abortable. But what it boils down to is calling the
RequestBackground() method with autostart=true and
commandline=/app/bin/data-collect-app (and possibly
dbus-activatable=true if the collector is a dbus service, which may be
a good way to talk to it). This will ask the user if it is ok for the
app to run in the background, and if so record this for the future (to
allow the app to run in the background)) and generate an autostart
file with the right contents to have your app started on login.
Once this is done you will get a signal on the request object that
this succeeded (or was denied).

Some portal are automatic behind other apis, but this one you need to
call manually. Here is some portal calling code you can use as a
template:
https://gitlab.gnome.org/GNOME/glib/blob/master/gio/gopenuriportal.c#L244

There has been discussions of creating a wrapper library for these
APIs, but there is none atm.

Substitute data-collect-app with pulseeffects

@AsavarTzeth
Copy link
Contributor Author

AsavarTzeth commented Feb 9, 2021

I'm hoping we can resurrect this old issue and perhaps finally close flathub/com.github.wwmm.pulseeffects#18.

Since this was first posted things have developed. We now have libportal, which should greatly simplify the use of the background portal.

When we find out how to use this portal it is only a matter of using a preprocessor #ifdef

We can still do this, now with libportal. For comparison that was done with eog, following issue 137. Even though that issue seems based on fundamental misunderstandings, I can think of reasons for keeping it optional.

That said, I do think it should be enabled by default, when the dependency is found. Portals do work outside of flatpaks and sometimes bring the advantage of desktop integrations and a more uniform user experience. If that leads to issues, you can always change it later on.

Unfortunately I still can't find any app that uses the background portal specifically, nor any code example. That made things even harder when I tried to implement this myself. I ultimately failed, due to my lacking programming skills, but it does seem like a simple job if you know what you're doing.

Other apps that uses this library (other portals though) can be found in the "Required by" section at packages.archlinux.org. Documentation ships with the library in the form of gtk-doc generated html.

I will be available for testing and anything else you might need me for.

@wwmm
Copy link
Owner

wwmm commented Feb 12, 2021

I have been a little busy but I will try to remember to take a look at this in the next days.

@wwmm
Copy link
Owner

wwmm commented Feb 15, 2021

I tried to follow what is done in this link https://github.com/flatpak/xdg-desktop-portal/blob/669e91c2774850944ac8f11881de4bd69831c182/tests/background.c#L67 but it does not even compile:

/usr/include/libportal/portal-helpers.h:57:19: error: expected unqualified-id before ‘export’
   57 |   XdpParentExport export;

I don't know what I am missing...

@wwmm
Copy link
Owner

wwmm commented Feb 15, 2021

Just including the header is enough to have a failed compilation. Even with dependency('libportal') in the meson file...

@AsavarTzeth
Copy link
Contributor Author

Then it would seem you came as far as I did. I too never figured out that error and I just assumed it was due to my own inexperience. At least, now I don't feel as stupid :|

I will see if I can get some answers and get back to you.

@vchernin
Copy link
Contributor

Ok, I'd like to try to get this working because it is still useful for EasyEffects in Flatpak.

One detail which is somewhat new: apparently for the most part libportal/xdg desktop portals shouldn't be used for non-sandboxed apps flatpak/libportal#33. So libportal should probably be an optional dependency for now.

I am really not sure how exactly an implementation should work, I am not very good at programming nor am I very familar with how portals work. But I will try.

Do you remember what you tried to do here? #535 (comment). Maybe I can try to redo that now.

@wwmm
Copy link
Owner

wwmm commented Aug 25, 2021

One detail which is somewhat new: apparently for the most part libportal/xdg desktop portals shouldn't be used for non-sandboxed apps flatpak/libportal#33. So libportal should probably be an optional dependency for now.

Unless they decide to support non sandboxed apps it will have to be an optional dependency forever.

Do you remember what you tried to do here? #535 (comment). Maybe I can try to redo that now.

I couldn't even try to do something. Just including the library header was enough to get a compilation error.

@vchernin
Copy link
Contributor

vchernin commented Aug 25, 2021

Unless they decide to support non sandboxed apps it will have to be an optional dependency forever.

Yeah, I do think eventually that will change, but who knows...

I couldn't even try to do something. Just including the library header was enough to get a compilation error.

Do you remember what file needed the header? I'd probably need to include libportal in the right meson.build file too right? I can try to build it locally and see what happens.

@wwmm
Copy link
Owner

wwmm commented Aug 25, 2021

Do you remember what file needed the header?

Probably this one https://github.com/wwmm/easyeffects/blob/master/include/general_settings_ui.hpp as this is the class handling the autostart switch.

@vchernin
Copy link
Contributor

vchernin commented Aug 25, 2021

Ok, I seem to be able to get it to build with the background portal header. #1115

I'll try to actually get it to do something useful later.

@GeorgesStavracas
Copy link

Hi, libportal maintainer. Portals are supposed to work both inside and outside any kind of sandbox. libportal is still maturing up, but it is very unlikely that the background portal API will change. You are encouraged to include libportal in your Flatpak manifest, pointing to whatever commit or tag works. If anything changes, we will bump the libportal version.

@vchernin
Copy link
Contributor

vchernin commented Aug 25, 2021

Hi, libportal maintainer. Portals are supposed to work both inside and outside any kind of sandbox. libportal is still maturing up, but it is very unlikely that the background portal API will change. You are encouraged to include libportal in your Flatpak manifest, pointing to whatever commit or tag works. If anything changes, we will bump the libportal version.

Thanks Georges!

I do recall someone said on Matrix the background portal specifically might not work in host? But there's a tool to check so I guess I'll try that later. (edit: for reference, tool is: https://github.com/bilelmoussaoui/ashpd/tree/master/ashpd-demo)

Ultimately it will be up to @wwmm to decide if EasyEffects should depend on libportal and use the background portal on non-sandboxed packages. I of course would ideally like it to as I like portals :)

It will probably help that EasyEffects only has usable packages on distros like Arch or other rolling ones, so I think recent libportal exists there.

@wwmm
Copy link
Owner

wwmm commented Aug 25, 2021

Hi, libportal maintainer. Portals are supposed to work both inside and outside any kind of sandbox.

Interesting. @GeorgesStavracas is there an example somewhere showing how to use libportal to autostart an app in the user login? We are considering using libportal because flatpak apps can not write to ~/.config/autostart/.

Ultimately it will be up to @wwmm to decide if EasyEffects should depend on libportal and use the background portal on non-sandboxed packages. I of course would ideally like it to as I like portals :)

Considering how often our flatpak is used I do not mind adding libportal as dependency. I am more concerned about how hard it is to use it for what we need.

@GeorgesStavracas
Copy link

Interesting. @GeorgesStavracas is there an example somewhere showing how to use libportal to autostart an app in the user log in? We are considering using libportal because flatpak apps can not write to ~/.config/autostart/.

Yup, there's a plugin for GNOME To Do that does just that. You can check it here.

The key part of it is that GTK4 apps must include <libportal/portal-gtk4.h>, and GTK3 → <libportal/portal-gtk3.h>, and Qt5 → <libportal/portal-qt5.h>

@vchernin
Copy link
Contributor

vchernin commented Aug 25, 2021

Yup, there's a plugin for GNOME To Do that does just that. You can check it here.

The key part of it is that GTK4 apps must include <libportal/portal-gtk4.h>, and GTK3 → <libportal/portal-gtk3.h>, and Qt5 → <libportal/portal-qt5.h>

I see in To Do there is:

#include <libportal/portal.h>
#include <libportal/portal-gtk4.h>

Is there any point to have this, which compiled here: #1115?

#include "libportal/background.h"

@GeorgesStavracas
Copy link

No big point, just me being overly explicit with the includes 🙂

@vchernin
Copy link
Contributor

vchernin commented Sep 17, 2021

So for now the Flatpak will be patched to use libportal in the next Flathub release. This is mostly due to flatpak/xdg-desktop-portal#579. Because of that portal issue the background portal doesn't work properly on the host, so it means two codepaths would need to be maintained which is sub optimal.

See #1141 for more discussion.

@wwmm
Copy link
Owner

wwmm commented Oct 29, 2022

The flatpak build has been using libportal for a while. We can close this issue.

@wwmm wwmm closed this as completed Oct 29, 2022
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 a pull request may close this issue.

4 participants