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>

Author: olivercalder

snap/info.go

@@ -968,44 +968,57 @@ func (s *Info) DesktopPrefix() string {
 // Note: DesktopPlugFileIDs doesn't check if the desktop plug is connected because
 // the desktop-file-ids attribute is controlled by an allow-installation rule.
 func (s *Info) DesktopPlugFileIDs() ([]string, error) {
-	var desktopPlug *PlugInfo
-	for _, plug := range s.Plugs {
+	desktopPlugNames := make([]string, 0, len(s.Plugs))
+	for name, plug := range s.Plugs {
 		if plug.Interface == "desktop" {
-			desktopPlug = plug
-			break
+			desktopPlugNames = append(desktopPlugNames, name)
 		}
 	}
-	if desktopPlug == nil {
-		return nil, nil
-	}
-
-	attrVal, exists := desktopPlug.Lookup("desktop-file-ids")
-	if !exists {
-		// desktop-file-ids attribute is optional
+	if len(desktopPlugNames) == 0 {
 		return nil, nil
 	}
+	sort.Strings(desktopPlugNames)
 
 	// TODO: The internal errors below should never happen due to validation
 	// in the desktop interface. It would be a good candidate for telemetry
 	// error reporting.
 
-	// desktop-file-ids must be a list of strings
-	attrList, ok := attrVal.([]any)
-	if !ok {
-		return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`)
-	}
+	desktopFileIDs := make([]string, 0)
+	seenDesktopFileIDs := make(map[string]bool)
+	for _, plugName := range desktopPlugNames {
+		desktopPlug := s.Plugs[plugName]
+
+		attrVal, exists := desktopPlug.Lookup("desktop-file-ids")
+		if !exists {
+			// desktop-file-ids attribute is optional
+			continue
+		}
 
-	desktopFileIDs := make([]string, 0, len(attrList))
-	for _, val := range attrList {
-		desktopFileID, ok := val.(string)
+		// desktop-file-ids must be a list of strings
+		attrList, ok := attrVal.([]any)
 		if !ok {
 			return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`)
 		}
-		if !strings.HasSuffix(desktopFileID, ".desktop") {
-			logger.Noticef("adding missing .desktop suffix to desktop file ID %s (snap %s)", desktopFileID, s.InstanceName())
-			desktopFileID = desktopFileID + ".desktop"
+
+		for _, val := range attrList {
+			desktopFileID, ok := val.(string)
+			if !ok {
+				return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`)
+			}
+			if !strings.HasSuffix(desktopFileID, ".desktop") {
+				logger.Noticef("adding missing .desktop suffix to desktop file ID %s (snap %s)", desktopFileID, s.InstanceName())
+				desktopFileID = desktopFileID + ".desktop"
+			}
+			if seenDesktopFileIDs[desktopFileID] {
+				continue
+			}
+			seenDesktopFileIDs[desktopFileID] = true
+			desktopFileIDs = append(desktopFileIDs, desktopFileID)
 		}
-		desktopFileIDs = append(desktopFileIDs, desktopFileID)
+	}
+
+	if len(desktopFileIDs) == 0 {
+		return nil, nil
 	}
 	return desktopFileIDs, nil
 }

snap/info_test.go

@@ -2719,6 +2719,63 @@ plugs:
 	}
 }
 
+func (s *infoSuite) TestDesktopFileIDsMergedFromAllDesktopPlugs(c *C) {
+	const desktopAppYaml = `
+name: foo
+version: 1.0
+plugs:
+  desktop:
+  desktop-extra:
+    interface: desktop
+    desktop-file-ids: [org.example.Foo, org.example.Bar]
+  desktop-file:
+    interface: desktop
+    desktop-file-ids: [org.example.desktop, org.example.Foo.desktop]
+`
+
+	info, err := snap.InfoFromSnapYaml([]byte(desktopAppYaml))
+	c.Assert(err, IsNil)
+
+	desktopFileIDs, err := info.DesktopPlugFileIDs()
+	c.Assert(err, IsNil)
+	c.Assert(desktopFileIDs, DeepEquals, []string{"org.example.Foo.desktop", "org.example.Bar.desktop", "org.example.desktop"})
+}
+
+func (s *infoSuite) TestAppDesktopFileUsesDesktopFileIDsAcrossDesktopPlugs(c *C) {
+	const desktopAppYaml = `
+name: sample
+version: 1
+apps:
+ app:
+   command: foo
+   common-id: org.example.CommonID.Foo
+plugs:
+  desktop:
+  desktop-file:
+    interface: desktop
+    desktop-file-ids:
+      - org.example.CommonID.Foo
+`
+
+	snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{})
+	snapInfo, err := snap.ReadInfo("sample", &snap.SideInfo{})
+	c.Assert(err, IsNil)
+	c.Assert(snapInfo.Plugs["desktop"], NotNil)
+	c.Assert(snapInfo.Plugs["desktop-file"], NotNil)
+
+	c.Assert(os.MkdirAll(dirs.SnapDesktopFilesDir, 0755), IsNil)
+	const desktopFileName = "org.example.CommonID.Foo.desktop"
+	const mockDesktopFile = `[Desktop Entry]
+X-SnapInstanceName=sample
+Name=foo
+X-SnapAppName=app
+Exec=sample.app
+`
+	c.Assert(os.WriteFile(filepath.Join(dirs.SnapDesktopFilesDir, desktopFileName), []byte(mockDesktopFile), 0644), IsNil)
+
+	c.Check(snapInfo.Apps["app"].DesktopFile(), Matches, `.*/var/lib/snapd/desktop/applications/org.example.CommonID.Foo.desktop`)
+}
+
 func (s *infoSuite) TestMangleDesktopFileName(c *C) {
 	const desktopAppYaml = `
 name: foo

wrappers/desktop_test.go

@@ -188,6 +188,35 @@ func (s *desktopSuite) TestEnsurePackageDesktopFilesWithDesktopFileName(c *C) {
 	s.testEnsurePackageDesktopFilesWithDesktopInterface(c, desktopFileID)
 }
 
+func (s *desktopSuite) TestEnsurePackageDesktopFilesWithMultipleDesktopPlugs(c *C) {
+	const desktopAppYaml = `
+name: foo
+version: 1.0
+apps:
+  foobar:
+plugs:
+  desktop:
+  desktop-file:
+    interface: desktop
+    desktop-file-ids: [org.example.Foo]
+`
+
+	info := snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{Revision: snap.R(11)})
+	c.Assert(info.Plugs["desktop"], NotNil)
+	c.Assert(info.Plugs["desktop-file"], NotNil)
+
+	baseDir := info.MountDir()
+	c.Assert(os.MkdirAll(filepath.Join(baseDir, "meta", "gui"), 0755), IsNil)
+	c.Assert(os.WriteFile(filepath.Join(baseDir, "meta", "gui", "org.example.Foo.desktop"), mockDesktopFile, 0644), IsNil)
+	c.Assert(os.WriteFile(filepath.Join(baseDir, "meta", "gui", "foobar.desktop"), mockDesktopFile, 0644), IsNil)
+
+	err := wrappers.EnsureSnapDesktopFiles([]*snap.Info{info})
+	c.Assert(err, IsNil)
+
+	c.Check(filepath.Join(dirs.SnapDesktopFilesDir, "org.example.Foo.desktop"), testutil.FilePresent)
+	c.Check(filepath.Join(dirs.SnapDesktopFilesDir, "foo_foobar.desktop"), testutil.FilePresent)
+}
+
 func (s *desktopSuite) TestEnsurePackageDesktopFilesWithBadDesktopFileIDs(c *C) {
 	const desktopAppYamlTemplate = `
 name: foo