interfaces/builtin: allow desktop-launch on Core #14193
Conversation
Despite userd being unusable on Core, access to app metadata is still desirable.
There was a problem hiding this comment.
In general, this seems fine. What kind of metadata are you looking to access?
If it's .desktop files, I wish desktop-launch granted access to /var/lib/snapd/desktop/applications/*.desktop instead, as this is where all the nice cleaned .desktop files are with additional snapd-inserted metadata. The ones in /snap/*/*/meta/gui/*.desktop have e.g. relative paths to icon files. But I guess this isn't really a problem for this PR...
olivercalder
requested review from
alfonsosanchezbeato,
ZeyadYasser and
sergio-costas
|
@olivercalder it does that: And yeah, we're after the cleaned-up desktop files, and the icons pointed from there. Ref.: |
pedronis
added
the
Needs Samuele review
There was a problem hiding this comment.
In my opinion, this makes sense because Core Desktop requires it. But I think that @jhenstridge should also review it.
|
@ZeyadYasser the interface allows access to a DBus endpoint, the fact that there's nothing to serve that endpoint is just because there isn't a user session on Core. Same as if you had a Classic system with no session running. So I don't think one precludes the other. In fact, you could run a user session on Core and it would then work just fine. And Core Desktop is another approach solving the same. I suppose the comment should just go away. |
|
Ping? Any thoughts on whether there's anything missing here? |
There was a problem hiding this comment.
I am sure I might be missing some context, but I am more inclined for a separate interface that just allows access to snaps metadata (e.g. desktop-metadata) if that's is all what is needed instead of growing desktop-launch to other use cases to avoid the confusion of giving a false sense of currently supporting desktop launch on core when it is not supported yet.
@pedronis @zyga would love to know you input and suggestions
| @@ -90,6 +90,7 @@ func (iface *desktopLaunchInterface) Name() string { | |||
| func (iface *desktopLaunchInterface) StaticInfo() interfaces.StaticInfo { | |||
| return interfaces.StaticInfo{ | |||
| Summary: desktopLaunchSummary, | |||
| ImplicitOnCore: true, | |||
There was a problem hiding this comment.
I believe this would only make sense for core desktop? even though it is not yet supported which is my other concern.
| ImplicitOnCore: true, | |
| ImplicitOnCore: release.OnCoreDesktop, |
There was a problem hiding this comment.
The thing is, most SnapD interfaces only allow access to resources, whether they exist on a system or not. As documented above, you can have a user session, with userd running, on Core. You just don't, by default. Same if you install Ubuntu Server.
IMO the reason to split interfaces out is only if you want to allow a subset of what the existing one provides - not whether they're used or not. So yes, if you wanted a snap to only have access to app metadata, but not allowing it to launch them, then you'll need separate interfaces. Otherwise we'll grow the number of interfaces quicker than necessary.
|
if the reason to use this interface is to get access to the desktop app metadata, we should really introduce a separate interface that is only about being able to read that data |
Despite userd being unusable on Core, access to app metadata is still desirable.