fix(doctor): validate QT_QPA_PLATFORMTHEME instead of trusting its presence (#3240)

* fix(doctor): validate QT_QPA_PLATFORMTHEME instead of trusting its presence dms doctor reported any non-empty QT_QPA_PLATFORMTHEME as healthy, so a wrong value passed with "All checks passed!" while every Qt app silently ran without a platform theme: palettes stayed unstyled and QIcon lookups returned nothing, which is how Quickshell ends up drawing its missing- icon placeholder for every icon an SNI app such as blueman publishes in tray menus. The real-world trap is qt6ct-kde — the AUR package name — set as the theme value; the settings UI toast already documents that only the qt6ct theme requires the qt6ct-kde package, but the doctor, the tool meant to catch exactly this, blessed the broken value. - checkQtPlatformThemePlugin validates both QT_QPA_PLATFORMTHEME and QT_QPA_PLATFORMTHEME_QT6 (the latter was not reported at all): qt6ct-kde gets a targeted warning naming the fix, and any other value has its plugin probed the way the qtengine check does — qmake/qtpaths are asked for QT_INSTALL_PLUGINS and the conventional file names are looked up, warning with the exact expected path on a miss - a miss never warns when it cannot be sure: no Qt build tools and the version-dependent plasma-integration file name both degrade to info, mirroring the qtengine check's no-false-alarm policy - the plugin-root interrogation moves into qtPluginRoots, shared with the qtengine check On an affected system: before: ○ QT_QPA_PLATFORMTHEME · qt6ct-kde ✓ All checks passed! after: ○ QT_QPA_PLATFORMTHEME · qt6ct-kde ● qt6ct-kde Package name, not a theme * fix(doctor): probe qtengine plugin with a bare file name qtenginePluginFile("", major) returns a path with a directory component, and qtPluginPath joins platformthemes/ again, so the qtengine check stat'd <root>/platformthemes/platformthemes/libqt6engine-plugin.so and warned "Not found" on every correctly installed system. Pass the bare file name instead, and pin the resolved path with a regression test that fails on the double join. Found in review by claude[bot]. * Update core/cmd/dms/commands_doctor.go Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
2026-10-17 00:34:01 -04:00 · 2026-08-31 21:04:56 -04:00
parent 99c5f96575
commit 16c098d09b
2 changed files with 272 additions and 20 deletions
@@ -322,6 +322,11 @@ func checkSystemInfo() []checkResult {
 func checkEnvironmentVars() []checkResult {
 	var results []checkResult
 	results = append(results, checkEnvVar("QT_QPA_PLATFORMTHEME")...)
+	results = append(results, checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME")...)
+	results = append(results, checkEnvVar("QT_QPA_PLATFORMTHEME_QT6")...)
+	if os.Getenv("QT_QPA_PLATFORMTHEME_QT6") != os.Getenv("QT_QPA_PLATFORMTHEME") {
+		results = append(results, checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME_QT6")...)
+	}
 	results = append(results, checkEnvVar("QS_ICON_THEME")...)
 	results = append(results, checkXDGMenuPrefix()...)
 	if matugen.QtengineActive() {
@@ -383,19 +388,7 @@ var qtQueryBinaries = []struct{ bin, flag string }{
 func checkQtenginePlugin() []checkResult {
 	url := doctorDocsURL + "#environment-variables"

-	pluginRoots := make(map[string]string)
-	for _, q := range qtQueryBinaries {
-		if _, err := exec.LookPath(q.bin); err != nil {
-			continue
-		}
-		major, _, _ := strings.Cut(qtQuery(q.bin, q.flag, "QT_VERSION"), ".")
-		if major == "" || pluginRoots[major] != "" {
-			continue
-		}
-		if root := qtQuery(q.bin, q.flag, "QT_INSTALL_PLUGINS"); root != "" {
-			pluginRoots[major] = root
-		}
-	}
+	pluginRoots := qtPluginRoots()

 	// qmake/qtpaths ship in dev packages most users lack — never warn on a Qt
 	// that could not be interrogated, and never claim it is fine either.
@@ -436,21 +429,133 @@ func checkQtenginePlugin() []checkResult {
 	return results
 }

-// qtenginePluginPath returns the first of roots that holds the qtengine platform
-// theme plugin for the given Qt major, or "" if none does.
-func qtenginePluginPath(roots []string, major string) string {
+// checkQtPlatformThemePlugin validates one of the QT_QPA_PLATFORMTHEME
+// variables beyond mere presence: the value must name a platform theme whose
+// plugin Qt can actually load. A wrong value fails silently at run time — Qt
+// apps fall back to a built-in theme, icon lookups return nothing, and
+// Quickshell tray menus draw a placeholder for every icon an app publishes.
+func checkQtPlatformThemePlugin(varName string) []checkResult {
+	value := os.Getenv(varName)
+	// Unset is reported by checkEnvVar; qtengine has its own plugin check.
+	if value == "" || value == "qtengine" {
+		return nil
+	}
+
+	url := doctorDocsURL + "#environment-variables"
+
+	// qt6ct-kde is the AUR package name, not the theme value: with it set,
+	// Qt finds no platform theme at all and every Qt app falls back silently.
+	if value == "qt6ct-kde" {
+		return []checkResult{{
+			catEnvironment, value, statusWarn, "Package name, not a theme",
+			fmt.Sprintf("qt6ct-kde is the AUR package name; the theme value Qt expects is qt6ct. Qt apps get no platform theme, so palettes stay unstyled and icon lookups fail. Set %s=qt6ct, then restart the shell.", varName),
+			url,
+		}}
+	}
+
+	// DMS themes Qt6 through these variables; qt5ct is the Qt5 counterpart.
+	major := "6"
+	if value == "qt5ct" {
+		major = "5"
+	}
+	files := qtPlatformThemePluginFiles(value)
+	name := fmt.Sprintf("%s plugin (Qt%s)", value, major)
+
+	root := qtPluginRoots()[major]
+	// qmake/qtpaths ship in dev packages most users lack — never warn on a Qt
+	// that could not be interrogated, and never claim it is fine either.
+	if root == "" {
+		return []checkResult{{
+			catEnvironment, name, statusInfo, "Cannot verify (no qmake/qtpaths)",
+			"Install a Qt development package to let dms doctor check the plugin against the path Qt actually searches.",
+			url,
+		}}
+	}
+
+	// Qt also searches QT_PLUGIN_PATH, so a plugin found there is loadable.
+	plugin := qtPluginPath(append([]string{root}, filepath.SplitList(os.Getenv("QT_PLUGIN_PATH"))...), files...)
+	if plugin == "" {
+		// plasma-integration ships the kde theme under a file name that has
+		// changed across versions; a miss says nothing reliable about the
+		// install, so degrade to info instead of warning at a working setup.
+		if value == "kde" {
+			return []checkResult{{
+				catEnvironment, name, statusInfo, "Cannot verify (plasma-integration)",
+				"The kde theme comes from plasma-integration, whose plugin file name varies across versions. Verify visually that Qt apps follow your theme.",
+				url,
+			}}
+		}
+		expected := filepath.Join(root, "platformthemes", files[0])
+		return []checkResult{{
+			catEnvironment, name, statusWarn, fmt.Sprintf("Not found in Qt%s plugin path", major),
+			fmt.Sprintf("Expected %s — Qt apps silently fall back to a built-in theme, breaking icon lookups and palettes. Install the package providing it, or correct %s.", expected, varName),
+			url,
+		}}
+	}
+	return []checkResult{{catEnvironment, name, statusOK, "Installed", plugin, url}}
+}
+
+// qtPlatformThemePluginFiles lists the plugin file names that can provide a
+// QT_QPA_PLATFORMTHEME value, most conventional first. Qt resolves the value
+// through plugin metadata keys rather than file names, so these are the names
+// each integration is known to ship; unmapped values fall back to Qt's
+// libq<value> naming convention.
+func qtPlatformThemePluginFiles(value string) []string {
+	if files, ok := map[string][]string{
+		"qt6ct": {"libqt6ct.so"},
+		"qt5ct": {"libqt5ct.so"},
+		"gtk3":  {"libqgtk3.so"},
+		"kde":   {"libqkdeplatformtheme.so"},
+	}[value]; ok {
+		return files
+	}
+	return []string{"libq" + value + ".so", "lib" + value + ".so"}
+}
+
+// qtPluginRoots asks every available Qt build tool where its Qt looks for
+// plugins, keyed by Qt major. A binary's name does not reliably say which Qt
+// it belongs to — plain "qmake" is Qt5 on Arch and Qt6 elsewhere — so each one
+// is asked for QT_VERSION and the major is taken from the answer.
+func qtPluginRoots() map[string]string {
+	pluginRoots := make(map[string]string)
+	for _, q := range qtQueryBinaries {
+		if _, err := exec.LookPath(q.bin); err != nil {
+			continue
+		}
+		major, _, _ := strings.Cut(qtQuery(q.bin, q.flag, "QT_VERSION"), ".")
+		if major == "" || pluginRoots[major] != "" {
+			continue
+		}
+		if root := qtQuery(q.bin, q.flag, "QT_INSTALL_PLUGINS"); root != "" {
+			pluginRoots[major] = root
+		}
+	}
+	return pluginRoots
+}
+
+// qtPluginPath returns the first of roots that holds any of files under
+// platformthemes/, or "" if none does.
+func qtPluginPath(roots []string, files ...string) string {
 	for _, root := range roots {
 		if root == "" {
 			continue
 		}
-		path := qtenginePluginFile(root, major)
-		if _, err := os.Stat(path); err == nil {
-			return path
+		for _, file := range files {
+			path := filepath.Join(root, "platformthemes", file)
+			if _, err := os.Stat(path); err == nil {
+				return path
+			}
 		}
 	}
 	return ""
 }

+// qtenginePluginPath returns the first of roots that holds the qtengine platform
+// theme plugin for the given Qt major, or "" if none does.
+func qtenginePluginPath(roots []string, major string) string {
+	return qtPluginPath(roots, fmt.Sprintf("libqt%sengine-plugin.so", major))
+}
+
 func qtenginePluginFile(root, major string) string {
 	return filepath.Join(root, "platformthemes", fmt.Sprintf("libqt%sengine-plugin.so", major))
 }
@@ -1,6 +1,11 @@
 package main

-import "testing"
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)

 func TestQuickshellVersionFailureDetails(t *testing.T) {
 	tests := []struct {
@@ -88,3 +93,145 @@ QT_VERSION:5.15.19`
 		})
 	}
 }
+
+// stubQtQueryTool writes a fake Qt build tool answering QT_VERSION and
+// QT_INSTALL_PLUGINS from the given values, and points PATH at its directory
+// alone so the real toolchain cannot interfere.
+func stubQtQueryTool(t *testing.T, bin, version, plugins string) string {
+	t.Helper()
+	dir := t.TempDir()
+	script := "#!/bin/sh\ncase \"$2\" in\n  QT_VERSION) echo " + version +
+		" ;;\n  QT_INSTALL_PLUGINS) echo " + plugins + " ;;\nesac\n"
+	if err := os.WriteFile(filepath.Join(dir, bin), []byte(script), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("PATH", dir)
+	return dir
+}
+
+func writePlugin(t *testing.T, pluginsDir, file string) {
+	t.Helper()
+	if err := os.MkdirAll(filepath.Join(pluginsDir, "platformthemes"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(pluginsDir, "platformthemes", file), nil, 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// qtenginePluginFile("", major) is a path with a directory component, and
+// qtPluginPath joins platformthemes/ again — a regression here silently warns
+// at every correctly installed system, so pin the full path down.
+func TestQtenginePluginPath(t *testing.T) {
+	root := t.TempDir()
+	if got := qtenginePluginPath([]string{root}, "6"); got != "" {
+		t.Fatalf("missing plugin should yield an empty path, got %q", got)
+	}
+	want := filepath.Join(root, "platformthemes", "libqt6engine-plugin.so")
+	if err := os.MkdirAll(filepath.Join(root, "platformthemes"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(want, nil, 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if got := qtenginePluginPath([]string{root}, "6"); got != want {
+		t.Fatalf("qtenginePluginPath = %q, want %q", got, want)
+	}
+}
+
+func TestCheckQtPlatformThemePlugin(t *testing.T) {
+	t.Run("qt6ct-kde is flagged as a package name, not a theme", func(t *testing.T) {
+		t.Setenv("QT_QPA_PLATFORMTHEME", "qt6ct-kde")
+		t.Setenv("PATH", t.TempDir()) // no Qt tools reachable
+		results := checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME")
+		if len(results) != 1 {
+			t.Fatalf("got %d results, want 1: %+v", len(results), results)
+		}
+		if results[0].status != statusWarn {
+			t.Fatalf("status = %q, want warn", results[0].status)
+		}
+		if !strings.Contains(results[0].details, "package name") || !strings.Contains(results[0].details, "qt6ct") {
+			t.Fatalf("details should name the package/value mixup, got %q", results[0].details)
+		}
+	})
+
+	t.Run("installed plugin reports OK", func(t *testing.T) {
+		t.Setenv("QT_QPA_PLATFORMTHEME", "qt6ct")
+		plugins := t.TempDir()
+		writePlugin(t, plugins, "libqt6ct.so")
+		stubQtQueryTool(t, "qmake6", "6.11.2", plugins)
+		results := checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME")
+		if len(results) != 1 || results[0].status != statusOK {
+			t.Fatalf("got %+v, want one OK result", results)
+		}
+		if !strings.Contains(results[0].details, "libqt6ct.so") {
+			t.Fatalf("details should point at the plugin, got %q", results[0].details)
+		}
+	})
+
+	t.Run("missing plugin warns with the expected path", func(t *testing.T) {
+		t.Setenv("QT_QPA_PLATFORMTHEME", "qt6ct")
+		plugins := t.TempDir()
+		stubQtQueryTool(t, "qtpaths6", "6.11.2", plugins)
+		results := checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME")
+		if len(results) != 1 || results[0].status != statusWarn {
+			t.Fatalf("got %+v, want one warn result", results)
+		}
+		want := filepath.Join(plugins, "platformthemes", "libqt6ct.so")
+		if !strings.Contains(results[0].details, want) {
+			t.Fatalf("details should contain expected path %s, got %q", want, results[0].details)
+		}
+	})
+
+	t.Run("no Qt build tools degrades to info", func(t *testing.T) {
+		t.Setenv("QT_QPA_PLATFORMTHEME", "qt6ct")
+		t.Setenv("PATH", t.TempDir())
+		results := checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME")
+		if len(results) != 1 || results[0].status != statusInfo {
+			t.Fatalf("got %+v, want one info result", results)
+		}
+	})
+
+	t.Run("unset and qtengine are left to their own checks", func(t *testing.T) {
+		t.Setenv("PATH", t.TempDir())
+		t.Setenv("QT_QPA_PLATFORMTHEME", "")
+		if results := checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME"); results != nil {
+			t.Fatalf("unset should yield no results, got %+v", results)
+		}
+		t.Setenv("QT_QPA_PLATFORMTHEME", "qtengine")
+		if results := checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME"); results != nil {
+			t.Fatalf("qtengine should yield no results, got %+v", results)
+		}
+	})
+
+	t.Run("qt6 variable is not satisfied by a Qt5 tool alone", func(t *testing.T) {
+		t.Setenv("QT_QPA_PLATFORMTHEME_QT6", "qt6ct")
+		plugins := t.TempDir()
+		stubQtQueryTool(t, "qmake-qt5", "5.15.19", plugins)
+		results := checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME_QT6")
+		if len(results) != 1 || results[0].status != statusInfo {
+			t.Fatalf("got %+v, want one info result", results)
+		}
+	})
+
+	t.Run("kde plugin miss degrades to info", func(t *testing.T) {
+		t.Setenv("QT_QPA_PLATFORMTHEME", "kde")
+		plugins := t.TempDir()
+		stubQtQueryTool(t, "qmake6", "6.11.2", plugins)
+		results := checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME")
+		if len(results) != 1 || results[0].status != statusInfo {
+			t.Fatalf("got %+v, want one info result", results)
+		}
+	})
+
+	t.Run("gtk3 resolves through the libq convention", func(t *testing.T) {
+		t.Setenv("QT_QPA_PLATFORMTHEME", "gtk3")
+		plugins := t.TempDir()
+		writePlugin(t, plugins, "libqgtk3.so")
+		stubQtQueryTool(t, "qmake6", "6.11.2", plugins)
+		results := checkQtPlatformThemePlugin("QT_QPA_PLATFORMTHEME")
+		if len(results) != 1 || results[0].status != statusOK {
+			t.Fatalf("got %+v, want one OK result", results)
+		}
+	})
+}