Flatpak requests us to list the services that will be exposed to the outside,
better have it the name of the application without a random number in the end.
In these systems the pid doesn't add much anyway, because two instances of
the application will always end up with the same pid anyway, from the point
of view of the application.
Details
- Reviewers
jgrulich aacid - Group Reviewers
Frameworks - Commits
- R271:8b79a5374afc: Don't include the pid in the dbus path when on flatpak
Applied the patch to flatpak-kde-runtime, fixes the issue for katomic and blinken.
Diff Detail
- Repository
- R271 KDBusAddons
- Branch
- arcpatch-D5775
- Lint
No Linters Available - Unit
No Unit Test Coverage
Looks good, it really doesn't make sense to use pid when running app in flatpak, because you cannot guarantee what pid it will get and without that you cannot allow dbus access in app's flatpak manifest.
This means we get the same path if we start the process twice, does it still work? Don't you get some kind of collision at the dbus level?
Yes, with this patch you can only open 1 of the applications (despite multiple). This is the error I get:
katomic(3)/(default) unknown: "Couldn't register name 'org.kde.katomic' with DBUS - another process owns it already!"
While this is a regression, we'll still need to come up with an alternative, it will be an alternative ad-hoc to flatpak (and potentially anything else wrapping dbus).
I don't think this is a fix.
I can't really i good faith recommend people to use Okular from a flatpak if that means they can't start two okular instances.
You have this problem even without this patch. When you now run two instances of Okular in flatpak then both will probably have same pid. Point of this patch is to make the dbus name predictable so you can allow access to it in flatpak manifest.
Albert has a point. Maybe it's something to bring to flatpak upstream, for example it could make sense to expose the actual pid as an env variable? Or a different but unique identifier?
Problem is that the name, even when the pid would be unique, won't be predictable. You still need to change it to something like org.kde.appName.pid so you can use org.kde.appName.* in flatpak manifest. Problem is that you need to allow to own those dbus services and because of that the first instance won't let the other instance to register itself.
Right, so the PID is not what we want maybe. I have the impression that the usage of PID here is a bit random though, the important thing is to be able to expose all okular instances to dbus.
Alternate suggestion:
register app-name + QDbus::connection()->baseServiceName.
it'll be guaranteed unique.
(though does anything even use the DBus service name in multi mode? It doesn't seem to be very useful. kquitapp5 doesn't support it.)
d_ed: I use the DBus name in multi mode to talk to running processes ;-)
(for introspection, debugging, automation, etc.)
I think this still doesn't solve the general problem we have with sandboxed applications as you cannot still allow this dbus service in app manifest. You can use "org.kde.AppName.*" , but then the dbus name would need to be in form of "org.kde.AppName.appPid" or "org.kde.AppName.baseServiceName", mean that you have to replace dash with a dot. When you use the dash the wildcard won't apply. On the other hand you also cannot use o.k.AppName.appPid or so because it has to start with a letter and not with a number or any special character which is case of baseService() returning e.g. :1:30.
+1
good idea, it should solve all the problems, we should use similar approach in KNotifications for SNI support.
Actually this won't work for KNotifications and SNI, still problem with --own-name=o.k.StatusNotifierItem.appName.* which won't let you register another SNI as the previous one owns all the service names. This however should work for this as org.kde.appName.* is allowed by default.
Are you sure that we can't change the status notifier name to something else e.g. org.kde.appname.StatusNotifierItem.<stuff>?
You are totally right, I didn't realize that we can change it to have name which is already allowed by flatpak.
Can somebody point out a problem with this approach? Or approve instead?
If nobody does I'll just push the patch.