Skip to content

Commit f18fd11

Browse files
olivercalderolivercalder
authored
snap,wrappers: fix common id selection with multiple desktop plugs (#17016)
* snap,wrappers: fix common id selection with multiple desktop plugs When multiple `desktop` plugs are present for a given app, we need to ensure that any `desktop-file-ids` attributes across all such plugs are considered when selecting the desktop file ID for the app. This commit ensures that all `desktop` plugs are collected and sorted by name first, and then for each, if the `desktop-file-ids` plug is present, add its IDs to the list of common IDs for the app. This ensures that if any `desktop-file-ids` attribute is present, the IDs listed always have precedence over the fallback common ID. It also ensures that if multiple plugs are present, the output is always deterministic. Signed-off-by: Oliver Calder <oliver.calder@canonical.com> * tests: add desktop-file and common-id checks to desktop-file-ids test Signed-off-by: Oliver Calder <oliver.calder@canonical.com> * fixup! tests: add desktop-file and common-id checks to desktop-file-ids test Signed-off-by: Oliver Calder <oliver.calder@canonical.com> --------- Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
1 parent e977002 commit f18fd11

14 files changed

Lines changed: 200 additions & 49 deletions

File tree

‎snap/info.go‎

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -968,44 +968,57 @@ func (s *Info) DesktopPrefix() string {
968968
// Note: DesktopPlugFileIDs doesn't check if the desktop plug is connected because
969969
// the desktop-file-ids attribute is controlled by an allow-installation rule.
970970
func (s *Info) DesktopPlugFileIDs() ([]string, error) {
971-
var desktopPlug *PlugInfo
972-
for _, plug := range s.Plugs {
971+
desktopPlugNames := make([]string, 0, len(s.Plugs))
972+
for name, plug := range s.Plugs {
973973
if plug.Interface == "desktop" {
974-
desktopPlug = plug
975-
break
974+
desktopPlugNames = append(desktopPlugNames, name)
976975
}
977976
}
978-
if desktopPlug == nil {
979-
return nil, nil
980-
}
981-
982-
attrVal, exists := desktopPlug.Lookup("desktop-file-ids")
983-
if !exists {
984-
// desktop-file-ids attribute is optional
977+
if len(desktopPlugNames) == 0 {
985978
return nil, nil
986979
}
980+
sort.Strings(desktopPlugNames)
987981

988982
// TODO: The internal errors below should never happen due to validation
989983
// in the desktop interface. It would be a good candidate for telemetry
990984
// error reporting.
991985

992-
// desktop-file-ids must be a list of strings
993-
attrList, ok := attrVal.([]any)
994-
if !ok {
995-
return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`)
996-
}
986+
desktopFileIDs := make([]string, 0)
987+
seenDesktopFileIDs := make(map[string]bool)
988+
for _, plugName := range desktopPlugNames {
989+
desktopPlug := s.Plugs[plugName]
990+
991+
attrVal, exists := desktopPlug.Lookup("desktop-file-ids")
992+
if !exists {
993+
// desktop-file-ids attribute is optional
994+
continue
995+
}
997996

998-
desktopFileIDs := make([]string, 0, len(attrList))
999-
for _, val := range attrList {
1000-
desktopFileID, ok := val.(string)
997+
// desktop-file-ids must be a list of strings
998+
attrList, ok := attrVal.([]any)
1001999
if !ok {
10021000
return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`)
10031001
}
1004-
if !strings.HasSuffix(desktopFileID, ".desktop") {
1005-
logger.Noticef("adding missing .desktop suffix to desktop file ID %s (snap %s)", desktopFileID, s.InstanceName())
1006-
desktopFileID = desktopFileID + ".desktop"
1002+
1003+
for _, val := range attrList {
1004+
desktopFileID, ok := val.(string)
1005+
if !ok {
1006+
return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`)
1007+
}
1008+
if !strings.HasSuffix(desktopFileID, ".desktop") {
1009+
logger.Noticef("adding missing .desktop suffix to desktop file ID %s (snap %s)", desktopFileID, s.InstanceName())
1010+
desktopFileID = desktopFileID + ".desktop"
1011+
}
1012+
if seenDesktopFileIDs[desktopFileID] {
1013+
continue
1014+
}
1015+
seenDesktopFileIDs[desktopFileID] = true
1016+
desktopFileIDs = append(desktopFileIDs, desktopFileID)
10071017
}
1008-
desktopFileIDs = append(desktopFileIDs, desktopFileID)
1018+
}
1019+
1020+
if len(desktopFileIDs) == 0 {
1021+
return nil, nil
10091022
}
10101023
return desktopFileIDs, nil
10111024
}

‎snap/info_test.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2719,6 +2719,63 @@ plugs:
27192719
}
27202720
}
27212721

2722+
func (s *infoSuite) TestDesktopFileIDsMergedFromAllDesktopPlugs(c *C) {
2723+
const desktopAppYaml = `
2724+
name: foo
2725+
version: 1.0
2726+
plugs:
2727+
desktop:
2728+
desktop-extra:
2729+
interface: desktop
2730+
desktop-file-ids: [org.example.Foo, org.example.Bar]
2731+
desktop-file:
2732+
interface: desktop
2733+
desktop-file-ids: [org.example.desktop, org.example.Foo.desktop]
2734+
`
2735+
2736+
info, err := snap.InfoFromSnapYaml([]byte(desktopAppYaml))
2737+
c.Assert(err, IsNil)
2738+
2739+
desktopFileIDs, err := info.DesktopPlugFileIDs()
2740+
c.Assert(err, IsNil)
2741+
c.Assert(desktopFileIDs, DeepEquals, []string{"org.example.Foo.desktop", "org.example.Bar.desktop", "org.example.desktop"})
2742+
}
2743+
2744+
func (s *infoSuite) TestAppDesktopFileUsesDesktopFileIDsAcrossDesktopPlugs(c *C) {
2745+
const desktopAppYaml = `
2746+
name: sample
2747+
version: 1
2748+
apps:
2749+
app:
2750+
command: foo
2751+
common-id: org.example.CommonID.Foo
2752+
plugs:
2753+
desktop:
2754+
desktop-file:
2755+
interface: desktop
2756+
desktop-file-ids:
2757+
- org.example.CommonID.Foo
2758+
`
2759+
2760+
snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{})
2761+
snapInfo, err := snap.ReadInfo("sample", &snap.SideInfo{})
2762+
c.Assert(err, IsNil)
2763+
c.Assert(snapInfo.Plugs["desktop"], NotNil)
2764+
c.Assert(snapInfo.Plugs["desktop-file"], NotNil)
2765+
2766+
c.Assert(os.MkdirAll(dirs.SnapDesktopFilesDir, 0755), IsNil)
2767+
const desktopFileName = "org.example.CommonID.Foo.desktop"
2768+
const mockDesktopFile = `[Desktop Entry]
2769+
X-SnapInstanceName=sample
2770+
Name=foo
2771+
X-SnapAppName=app
2772+
Exec=sample.app
2773+
`
2774+
c.Assert(os.WriteFile(filepath.Join(dirs.SnapDesktopFilesDir, desktopFileName), []byte(mockDesktopFile), 0644), IsNil)
2775+
2776+
c.Check(snapInfo.Apps["app"].DesktopFile(), Matches, `.*/var/lib/snapd/desktop/applications/org.example.CommonID.Foo.desktop`)
2777+
}
2778+
27222779
func (s *infoSuite) TestMangleDesktopFileName(c *C) {
27232780
const desktopAppYaml = `
27242781
name: foo

‎tests/lib/snaps/test-snapd-desktop-file-ids/bin/check-dirs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#!/bin/sh
1+
#!/bin/sh -e
22

33
for dir in "$@"; do
44
ls "$dir"

‎tests/lib/snaps/test-snapd-desktop-file-ids/bin/check-files‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#!/bin/sh
1+
#!/bin/sh -e
22

33
for file in "$@"; do
44
cat "$file"

‎tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/arbitrary.desktop‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[Desktop Entry]
2-
Name=test-snapd-desktop.cmd
3-
Comment=A desktop file for test-snapd-desktop.cmd
4-
Exec=test-snapd-desktop.cmd
2+
Name=test-snapd-desktop-file-ids.cmd
3+
Comment=A desktop file for test-snapd-desktop-file-ids.cmd
4+
Exec=test-snapd-desktop-file-ids.cmd
55
Terminal=true
66
Type=Application
77
X-SnapCommonID=org.example.MyCommonID

‎tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/bar.desktop‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[Desktop Entry]
2-
Name=test-snapd-desktop.cmd
3-
Comment=A desktop file for test-snapd-desktop.cmd
4-
Exec=test-snapd-desktop.cmd
2+
Name=test-snapd-desktop-file-ids.cmd
3+
Comment=A desktop file for test-snapd-desktop-file-ids.cmd
4+
Exec=test-snapd-desktop-file-ids.cmd
55
Terminal=true
66
Type=Application
77
X-SnapCommonID=org.example.NotListed

‎tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/foo.desktop‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[Desktop Entry]
2-
Name=test-snapd-desktop.cmd
3-
Comment=A desktop file for test-snapd-desktop.cmd
4-
Exec=test-snapd-desktop.cmd
2+
Name=test-snapd-desktop-file-ids.cmd
3+
Comment=A desktop file for test-snapd-desktop-file-ids.cmd
4+
Exec=test-snapd-desktop-file-ids.cmd
55
Terminal=true
66
Type=Application

‎tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/org.example.Bar.desktop‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[Desktop Entry]
2+
Name=test-snapd-desktop-file-ids.cmd
3+
Comment=A desktop file for test-snapd-desktop-file-ids.cmd
4+
Exec=test-snapd-desktop-file-ids.cmd
5+
Terminal=true
6+
Type=Application

‎tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/org.example.Fallback.desktop‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[Desktop Entry]
2-
Name=test-snapd-desktop.cmd
3-
Comment=A desktop file for test-snapd-desktop.cmd
4-
Exec=test-snapd-desktop.cmd
2+
Name=test-snapd-desktop-file-ids.cmd
3+
Comment=A desktop file for test-snapd-desktop-file-ids.cmd
4+
Exec=test-snapd-desktop-file-ids.cmd
55
Terminal=true
66
Type=Application

‎tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/org.example.Foo.desktop‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[Desktop Entry]
2-
Name=test-snapd-desktop.cmd
3-
Comment=A desktop file for test-snapd-desktop.cmd
4-
Exec=test-snapd-desktop.cmd
2+
Name=test-snapd-desktop-file-ids.cmd
3+
Comment=A desktop file for test-snapd-desktop-file-ids.cmd
4+
Exec=test-snapd-desktop-file-ids.cmd
55
Terminal=true
66
Type=Application

0 commit comments

Comments
 (0)