daemon: add desktop file location for app to the API #2982

mvo5 · Mar 3, 2017

This implements the API for gnome-software so that it can map desktop files from a snap application.

LP: #1661590

niemeyer · Mar 10, 2017

niemeyer requested changes Mar 10, 2017

View changes

I thought we had a strong convention for how to name desktop files after the app name?

chipaca · Mar 13, 2017

Question: as far as I know we currently assume very little of the mapping between the desktop files a snap ships, and its apps: all .desktop file a snap ships are installed; apps are not even looked at for this. In particular people are free to have multiple desktop files referring to the same app (e.g. for different options).

As a not-too-contrived example,

$ dpkg -L nautilus | grep '\.desktop$' | wc -l
9
$ dpkg -L nautilus | egrep '/s?bin/' | wc -l
3

We're throwing away this flexibility, and it's not clear to me why.

niemeyer · Mar 13, 2017

niemeyer requested changes Mar 13, 2017

View changes

s/files/file/

That said, do we need to expose this at all, given that we use a strong convention to find the file? In other words, it sounds trivial for Gnome Software to build the string internally instead of exposing this via an API that can't possibly be used without access to the underlying filesystem of snapd.

Sorry, last sentence (now removed) was the original sentence which I intended to drop in favor of the actual explanation presented above.

Indeed, we could simply ask gnome-software to implement the lookup. The upside of having it in the API is that if we ever change our mind about the convention there will be less software to change (also that we do not need to implement the convention in each software that uses snapd (snapweb, the kde version of gnome-software etc)). However given that its a really small piece of convention its not a very strong argument :) Happy to remove it from the API again.

This seems fine and welcome, though. May be useful for other reasons.

mvo5 · Mar 15, 2017

@chipaca It looks like this has happend already https://github.com/snapcore/snapcraft/blob/master/docs/metadata.md#desktop-file - I remember we used to have a loose structure precisely because people used desktop files in a very loose way. With the new convention it is still possible to nautilus called in different ways, it would require to add an app to the snap for each of them though. E.g. nautilus.open-background-dialog and then snap_nautilus.open-background-dialog.desktop.

chipaca · Mar 15, 2017

@mvo5 eh, if people who care about desktop files are fine with snapcraft doing that, then I'm fine with us continuing down that road

zyga · Apr 4, 2017

There's a test failure in spread here:

FAIL: api_test.go:352: apiSuite.TestSnapInfoOneIntegration

api_test.go:412:
    c.Check(rsp.Result, check.DeepEquals, expected.Result)
... obtained map[string]interface {} = map[string]interface {}{"confinement":"strict", "description":"description", "icon":"/v2/icons/foo/icon", "summary":"summary", "version":"v1", "tracking-channel":"beta", "devmode":false, "id":"foo-id", "revision":snap.Revision{N:10}, "type":"app", "trymode":false, "resource":"/v2/snaps/foo", "developer":"bar", "status":"active", "jailmode":false, "private":false, "broken":"", "name":"foo", "channel":"stable", "apps":[]daemon.appJSON{daemon.appJSON{Name:"cmd2", Daemon:"", DesktopFile:"", Aliases:[]string(nil)}, daemon.appJSON{Name:"cmd", Daemon:"", DesktopFile:"/tmp/check-1893436019/104/var/lib/snapd/desktop/applications/foo_cmd.desktop", Aliases:[]string(nil)}}, "contact":""}
... expected map[string]interface {} = map[string]interface {}{"tracking-channel":"beta", "description":"description", "developer":"bar", "private":false, "jailmode":false, "trymode":false, "broken":"", "name":"foo", "version":"v1", "channel":"stable", "summary":"summary", "status":"active", "resource":"/v2/snaps/foo", "confinement":"strict", "id":"foo-id", "revision":snap.Revision{N:10}, "icon":"/v2/icons/foo/icon", "type":"app", "devmode":false, "apps":[]daemon.appJSON{daemon.appJSON{Name:"cmd", Daemon:"", DesktopFile:"/tmp/check-1893436019/104/var/lib/snapd/desktop/applications/foo_cmd.desktop", Aliases:[]string(nil)}, daemon.appJSON{Name:"cmd2", Daemon:"", DesktopFile:"", Aliases:[]string(nil)}}, "contact":""}

If you scroll all the way right and look at it long enough it seems that apps are different. I didn't investigate further.

zyga · Apr 4, 2017

We also have another prune error:

----------------------------------------------------------------------
FAIL: overlord_test.go:360: overlordSuite.TestEnsureLoopPrune

overlord_test.go:384:
    c.Assert(st.Change(chg1.ID()), Equals, chg1)
... obtained *state.Change = (*state.Change)(nil)
... expected *state.Change = &state.Change{state:(*state.State)(0xc8200b7f10), id:"1", kind:"abort", summary:"...", status:0, clean:false, data:state.customData{}, taskIDs:[]string{"1"}, lanes:0, ready:(chan struct {})(0xc820300600), spawnTime:time.Time{sec:63626897358, nsec:782465000, loc:(*time.Location)(0xd07640)}, readyTime:time.Time{sec:63626897358, nsec:884343000, loc:(*time.Location)(0xd07640)}}

zyga · Apr 4, 2017

Sorry, didn't mean to close it.

zyga · Apr 4, 2017

17:00 < Chipaca> zyga— so, apps is different
17:00 < Chipaca> zyga— missing desktopfile stuff
17:01 < Chipaca> ?
17:01 < Chipaca> ah!
17:01 < Chipaca> no
17:01 < Chipaca> zyga— something is wrong in that test or something
17:01 < Chipaca> there is no order to apps and the test is checking an ordered struct
17:01 < zyga> Chipaca: aah
17:02 < zyga> Chipaca: thanks, I'll look at fixing that
...
17:21 < zyga> Chipaca: about map[string]daemon.appJSON, should the key be just the app name?
17:22 < Chipaca> zyga— yah
17:22 < zyga> Chipaca: the problem is that this is client-visible protocol, right?
17:22 < zyga> Chipaca: alternatively I can sort by that
17:22 < zyga> Chipaca: so that it shows up in good order
17:23 < Chipaca> yeah
17:23 < zyga> Chipaca: but I also recall the need to have a "launch" button that may imply order cannot be alphabetic
17:23 < Chipaca> sorting works
17:23 < zyga> Chipaca: and needs to be something special :/
17:23 < Chipaca> the launch is per desktop
17:23 < zyga> Chipaca: yes but in gnome software you have one buttn
17:23 < zyga> button
17:23 < Chipaca> zyga— imagine :shrug:, but where each stroke is a ¯\_(ツ)_/¯
...
17:28 < zyga> Chipaca: something like this http://paste.ubuntu.com/24313896/
17:28 < zyga> Chipaca: if you +1 I will push that into mvo's PR

chipaca · Apr 4, 2017

chipaca reviewed Apr 4, 2017

View changes

it's probably easier and clearer to leave this bit as it was, and then add a sort.Slice call at the end

17:41 < zyga> Chipaca: odd, I don't have sort.Slice
17:41 < zyga> Chipaca: is that a 1.7 thing?
17:41 < Chipaca> ah
17:41 < Chipaca> if it is, then ignore me
17:41 < Chipaca> zyga— quite possibly
17:41 < pedronis> living in the future (well past) ?
17:41 < zyga> Chipaca: I wish go docs had a @since thing
17:41 < Chipaca> yeah
17:41 < zyga> :/
17:41 < Chipaca> me too

niemeyer · Apr 5, 2017

niemeyer approved these changes Apr 5, 2017

View changes

Thanks, and LGTM assuming this is what they need on the desktop end.

mvo5 · Apr 5, 2017

@robert-ancell Could you please comment if this is what you need for gnome-software ? If so, it can go in shortly.

robert-ancell · Apr 6, 2017

Tested this branch and it works well, thanks!

mvo5 added this to the 2.24 milestone Apr 4, 2017

zyga closed this Apr 4, 2017

zyga reopened this Apr 4, 2017

mvo5 requested a review from robert-ancell Apr 5, 2017

+        +		return nil
+        +	}
+        +	appName := filepath.Base(app.WrapperPath())
+        +	re, err := regexp.Compile(fmt.Sprintf(`(?m)^Exec=%s .*`, appName))

-        -	Aliases      []string `json:"aliases,omitempty"`
+        +	Name        string   `json:"name"`
+        +	Daemon      string   `json:"daemon"`
+        +	DesktopFile string   `json:"desktop-files,omitempty"`

-        -	}
-        -	return res
+        +func (app *AppInfo) DesktopFile() string {
+        +	return filepath.Join(dirs.SnapDesktopFilesDir, fmt.Sprintf("%s_%s.desktop", app.Snap.Name(), app.Name))

          	apps := make([]appJSON, 0, len(localSnap.Apps))
-        -	for _, app := range localSnap.Apps {
+        +	for _, appName := range appNames {
+        +		app := localSnap.Apps[appName]

snapcore/snapd

daemon: add desktop file location for app to the API #2982

mvo5 commented Mar 3, 2017

niemeyer requested changes Mar 10, 2017

View changes

mvo5 added some commits Mar 13, 2017

chipaca commented Mar 13, 2017

niemeyer requested changes Mar 13, 2017

View changes

mvo5 commented Mar 15, 2017

chipaca commented Mar 15, 2017

mvo5 added this to the 2.24 milestone Apr 4, 2017

zyga commented Apr 4, 2017 •

Edited 1 time

zyga
Apr 4, 2017

zyga commented Apr 4, 2017

zyga closed this Apr 4, 2017

zyga reopened this Apr 4, 2017

zyga commented Apr 4, 2017

zyga commented Apr 4, 2017

chipaca reviewed Apr 4, 2017

View changes

niemeyer approved these changes Apr 5, 2017

View changes

mvo5 requested a review from robert-ancell Apr 5, 2017

mvo5 commented Apr 5, 2017

robert-ancell commented Apr 6, 2017

mvo5 merged commit `bdbe7fd` into snapcore:master Apr 6, 2017
6 checks passed

6 checks passed

snapcore/snapd

Join GitHub today

daemon: add desktop file location for app to the API #2982

Conversation

mvo5 commented Mar 3, 2017

Some checks were not successful

niemeyer requested changes Mar 10, 2017 View changes

mvo5 added some commits Mar 13, 2017

Some checks were not successful

chipaca commented Mar 13, 2017

niemeyer requested changes Mar 13, 2017 View changes

Some checks were not successful

mvo5 commented Mar 15, 2017

chipaca commented Mar 15, 2017

Some checks were not successful

mvo5 added this to the 2.24 milestone Apr 4, 2017

Some checks were not successful

zyga commented Apr 4, 2017 • edited Edited 1 time zyga Apr 4, 2017

zyga commented Apr 4, 2017

zyga closed this Apr 4, 2017

zyga reopened this Apr 4, 2017

zyga commented Apr 4, 2017

All checks have passed

zyga commented Apr 4, 2017

chipaca reviewed Apr 4, 2017 View changes

niemeyer approved these changes Apr 5, 2017 View changes

mvo5 requested a review from robert-ancell Apr 5, 2017

mvo5 commented Apr 5, 2017

robert-ancell commented Apr 6, 2017

Hide details View details mvo5 merged commit bdbe7fd into snapcore:master Apr 6, 2017 6 checks passed

6 checks passed

niemeyer requested changes Mar 10, 2017

View changes

niemeyer requested changes Mar 13, 2017

View changes

zyga commented Apr 4, 2017 •

Edited 1 time

zyga
Apr 4, 2017

chipaca reviewed Apr 4, 2017

View changes

niemeyer approved these changes Apr 5, 2017

View changes

mvo5 merged commit `bdbe7fd` into snapcore:master Apr 6, 2017
6 checks passed