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.

Sign up for GitHub

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

Jump to bottom

interfaces/builtin: allow desktop-launch on Core #14193

Closed
wants to merge 1 commit into from
Closed

interfaces/builtin: allow desktop-launch on Core #14193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion interfaces/builtin/desktop_launch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ func (iface *desktopLaunchInterface) Name() string {
func (iface *desktopLaunchInterface) StaticInfo() interfaces.StaticInfo {
return interfaces.StaticInfo{
Summary: desktopLaunchSummary,
ImplicitOnCore: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this would only make sense for core desktop? even though it is not yet supported which is my other concern.

Suggested change
ImplicitOnCore: true,
ImplicitOnCore: release.OnCoreDesktop,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

ImplicitOnClassic: true,
BaseDeclarationPlugs: desktopLaunchBaseDeclarationPlugs,
BaseDeclarationSlots: desktopLaunchBaseDeclarationSlots,
Expand All @@ -112,7 +113,8 @@ func (iface *desktopLaunchInterface) AutoConnect(*snap.PlugInfo, *snap.SlotInfo)
return true
}

// Only implicitOnClassic since userd isn't yet usable on core
// Available on both Core and Classic to allow access to app metadata,
// despite userd being unusable on Core
func init() {
registerIface(&desktopLaunchInterface{})
}
2 changes: 1 addition & 1 deletion interfaces/builtin/desktop_launch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (s *desktopLaunchSuite) TestInterfaces(c *C) {

func (s *desktopLaunchSuite) TestStaticInfo(c *C) {
si := interfaces.StaticInfoOf(s.iface)
c.Assert(si.ImplicitOnCore, Equals, false)
c.Assert(si.ImplicitOnCore, Equals, true)
c.Assert(si.ImplicitOnClassic, Equals, true)
c.Assert(si.Summary, Equals, `allows snaps to identify and launch desktop applications in (or from) other snaps`)
c.Assert(si.BaseDeclarationSlots, testutil.Contains, "desktop-launch")
Expand Down
Loading