From 829f0a503c969c9a8bd1f7927253e1b3b422cb31 Mon Sep 17 00:00:00 2001 From: Simon Lambin Date: Wed, 9 Sep 2026 23:07:45 +0200 Subject: [PATCH] feat: fix the davinco .desktop launcher --- davinci/CLAUDE.md | 73 +++++++++++++++++++++++++++++++++++++++++------ davinci/README.md | 37 ++++++++++++++++-------- 2 files changed, 90 insertions(+), 20 deletions(-) diff --git a/davinci/CLAUDE.md b/davinci/CLAUDE.md index b1a5b8e..cfa5e28 100644 --- a/davinci/CLAUDE.md +++ b/davinci/CLAUDE.md @@ -80,13 +80,18 @@ art alone. container level, which `--nvidia`'s driver-file bind-mount alone doesn't set up). Not resolved here; flagged as an open question for whoever tests this first. -- **App menu entry uses Resolve's own shipped `.desktop`/icon files** - (`/opt/resolve/share`, `/opt/resolve/graphics`), unlike Houdini where one - had to be hand-written from scratch. This is a real difference in what - each app ships, not an inconsistency to "fix" — davincibox's - `add-davinci-launcher` script does the same `sed`-based adaptation, - simplified in the README here since we don't need its toolbox/distrobox - branching (this repo is distrobox-only) or its `remove` mode. +- **App menu entry: hand-written `.desktop`, same approach as Houdini — + reversed from an earlier version of this doc.** Originally adapted + Resolve's own shipped `.desktop` (`/opt/resolve/share/DaVinciResolve.desktop`) + via `sed`, matching davincibox's `add-davinci-launcher`. Abandoned after a + real-hardware test (2026-09-09, see "App menu entry bug hunt" below) hit + two real bugs in that approach in a row (a `distrobox-export` double-wrap, + then a Resolve crash on the shipped file's `%u` field code) — at which + point Simon called it: not worth carrying davincibox's `sed` adaptation + and its edge cases just to reuse an icon reference, when a 10-line + hand-written `.desktop` (Houdini's pattern) sidesteps all of it. The + shipped `.desktop`'s only real advantage — the `MimeType=` line for + file-manager "open project" association — isn't worth the fragility. - **Not ported from davincibox:** `switcheroo-control` multi-GPU handling (`list-gpus`/`switcherooctl launch` wrapping in `run-davinci`) and runtime GPU auto-detection in the launcher (`lshw`-based, only needed because @@ -203,6 +208,58 @@ real GPU) actually working end to end, not just the image build. playback, OpenCL compute correctness, NVIDIA path (this host is AMD), Studio+dongle, app-menu-entry end-to-end. +## App menu entry bug hunt (2026-09-09) + +The README originally had Simon adapt Resolve's shipped `DaVinciResolve.desktop` +via `sed` (davincibox's approach — see the "Why these choices" entry above, +now reversed). Testing that end to end on real hardware surfaced two real +bugs, back to back, before it was abandoned in favor of a hand-written +`.desktop` (Houdini's pattern): + +1. **`distrobox-export` double-wrap.** The README's `sed` prepended + `Exec=distrobox enter -n davinci -- /usr/local/bin/davinci-run ` onto the + shipped `Exec=` line *before* running `distrobox-export --app` — but + `distrobox-export --app` *also* wraps whatever `Exec=` it's given with + `distrobox-enter -n --` at export time. Result: the exported + `.desktop` ran `distrobox-enter -n davinci -- distrobox enter -n davinci + -- ...` — the outer enter (host, has podman) worked, but the *inner* + `distrobox enter` then tried to run **inside the box itself**, where + podman/distrobox aren't installed → "podman not installed" error. Fix + at the time: drop the manual `distrobox enter -n davinci --` prefix from + the `sed` and let `distrobox-export` do that wrapping alone. +2. **Resolve crashes on any stray argument.** Fixing bug 1 wasn't enough — + the shipped `.desktop`'s `Exec=RESOLVE_INSTALL_LOCATION/bin/resolve %u` + still carried a `%u` field code through to the final `Exec=`. Any + launcher that doesn't strip unused field codes (confirmed on Simon's + Hyprland/caelestia setup — not every launcher implements the + freedesktop desktop-entry spec's field-code stripping) passes the + literal string `"%u"` as an argument. Resolve does not ignore unknown + arguments gracefully: it tries to load whatever it's given as a config + file, fails, and hits `resolve: .../AppConfig.cpp:272: void + AppConfig::LoadAllSiteInfo(): Assertion \`m_SiteEnabledIdx > 0' failed` + (SIGABRT). An earlier, different bad argument (the literal path to + Resolve's own binary, from a still-broken version of bug-1's fix) + produced a *different* crash, SIGSEGV, confirmed via `coredumpctl` + + installed `gdb` — same underlying lesson: Resolve is not defensive + about its argv at all, so the `.desktop`'s `Exec=` must never pass it + anything unexpected. +- **Testing-methodology false alarm folded into this, same session:** + the "Failed to create application support directories" error from the + "Real GUI launch test" section above turned out to be a *third*, + unrelated artifact (the `head`-pipe SIGPIPE issue) — three different + causes produced superficially similar "Resolve won't start" symptoms in + the same debugging session. Lesson for next time: don't assume a repeat + of a previously-diagnosed failure mode; check the actual current error + output (`coredumpctl list`, the log file, or in this case the process's + own crash dump) before re-explaining an old theory. +- **Decision:** rather than keep patching the shipped-`.desktop` adaptation + for whatever the next edge case turns out to be, switched to a + hand-written `.desktop` with no field codes at all (see README) — same + pattern Houdini already uses, and it sidesteps this entire class of bug + by construction. Confirmed working end to end after the switch: icon → + `distrobox-export`'s wrapper → `davinci-run` → Project Manager window, + no crash. + ## What's still unverified after this build test | Area | Confidence | Note | @@ -214,7 +271,7 @@ real GPU) actually working end to end, not just the image build. | AppImage magic-bytes workaround (fat-tire's Arch/Manjaro-specific issue) | Not ported | Not hit with the 21.1 `.run` tested here; add back from fat-tire/resolve's Dockerfile if a future version's `--appimage-extract` fails on it. | | `amd` variant (ROCm + Intel packages install) | High | Build-confirmed at Docker-build time, and now real-hardware-confirmed: `davinci-run` launched successfully on an AMD Radeon 680M host with `rocm-opencl`/`intel-compute-runtime` installed, 2026-09-09. OpenCL kernel *compute correctness* (actual color-science rendering) still not specifically checked — only that Resolve starts and its windows render normally. | | `nvidia` variant / `--nvidia` vs CDI | Low | Still a real open question — see "NVIDIA gotcha" in README. Image builds fine either way; only the *runtime* GPU passthrough method is unverified. The 2026-09-09 real-hardware test above was AMD, not NVIDIA. | -| App menu entry steps | Medium | Directly adapted from a working upstream script, simplified; confirmed the `.desktop` files exist at the documented paths (`/opt/resolve/share/*.desktop`), not run end-to-end. | +| App menu entry steps | High | Real-hardware-confirmed 2026-09-09 after switching to a hand-written `.desktop` (Houdini's pattern) — see "App menu entry bug hunt" above. The originally-documented shipped-`.desktop`-adaptation approach is abandoned; do not resurrect it without re-reading that section. | | Actual GUI launch (`distrobox create` + `davinci-run`) | High | Real-hardware-confirmed 2026-09-09 on AMD/Hyprland: reached Project Manager → Create New Project → Transcode window, all mapped/focused normally. See "Real GUI launch test" above. | | Audio | Low (confirmed broken as predicted) | `ResolveDebug.txt` shows the exact `libasound_module_pcm_pipewire.so` failure the README's "Known gotchas" section already predicted. Fix documented there, not yet applied/tested. | | Rendering correctness / playback / licensing / project work | Untested | The 2026-09-09 test only confirmed the app starts and its dialogs render — no project was opened, no clip played back, no license flow exercised. | diff --git a/davinci/README.md b/davinci/README.md index 8b0806f..163f759 100644 --- a/davinci/README.md +++ b/davinci/README.md @@ -110,23 +110,36 @@ reports something like "Unsupported GPU processing mode" on launch. ## App menu entry (icon, no terminal) -Resolve ships its own `.desktop` files and icons under -`/opt/resolve/share` and `/opt/resolve/graphics` — no need to write one by -hand: +Same approach as Houdini's: write a minimal `.desktop` by hand and export +it, rather than adapting Resolve's own shipped `.desktop` files. Resolve's +shipped `DaVinciResolve.desktop` uses the `%u` field code for file-manager +"open with" support, and Resolve crashes (`AppConfig::LoadAllSiteInfo` +assertion) if it's ever launched with a stray/unexpanded argument — not +every app launcher strips unused field codes before running `Exec=`. A +hand-written `.desktop` with no field codes at all sidesteps this +entirely. See CLAUDE.md for how this was found. ```bash distrobox enter davinci -mkdir -p ~/.local/share/applications -cp /opt/resolve/share/*.desktop ~/.local/share/applications -rm ~/.local/share/applications/DaVinciResolveInstaller.desktop -sed -i 's/RESOLVE_INSTALL_LOCATION/\/opt\/resolve/' \ - ~/.local/share/applications/{blackmagicraw*,DaVinci*}.desktop -sed -i "s,Exec=,Exec=distrobox enter -n davinci -- /usr/local/bin/davinci-run ," \ - ~/.local/share/applications/{blackmagicraw*,DaVinci*}.desktop -distrobox-export --app "$HOME/.local/share/applications/DaVinciResolve.desktop" +cat > /tmp/davinci.desktop << 'EOF' +[Desktop Entry] +Type=Application +Name=DaVinci Resolve +Comment=Editing, visual effects, color correction and audio post production +Exec=davinci-run +Icon=/opt/resolve/graphics/DV_Resolve.png +Terminal=false +Categories=AudioVideo;AudioVideoEditing; +StartupWMClass=resolve +EOF +distrobox-export --app /tmp/davinci.desktop ``` -(Adapted from davincibox's `add-davinci-launcher` script — see CLAUDE.md.) +This copies the icon to `~/.local/share/icons/` on the host and writes +`~/.local/share/applications/davinci-davinci.desktop`. Distrobox also +auto-creates a generic `davinci.desktop` ("Terminal entering davinci") when +the box is created — set `NoDisplay=true` in that file, or delete it, if you +don't want both entries in the app menu. ## Known gotchas (from upstream, not yet hit here ourselves)