← Back to issue list

fix(desktop-file): verify theme icons before rewriting icon

View original Github issue

Metadata

Project
snapcraft
Number
#6374
Type
pull request
State
open
Author
soumyaDghosh
Labels
—
Created
Updated
Closed
—

Current evaluation

A two-line fix defers rewriting theme-resolved icons until existence validation, avoiding false missing-icon warnings. It awaits review with two unresolved comments and failing CLA/Ubuntu 26.04 checks.

Suggested action: needs review

Reason: The +2/-2 change directly addresses #6364, and lint plus fast and unit tests pass. Maintainer review remains necessary because two comments are unresolved, no regression test is included, and CLA plus Ubuntu 26.04 integration fail; the unavailable comment and failure details leave some uncertainty. Current main also contains #6385's later refactor of the same logic, so the fix should be reconciled against it rather than merged blindly.

Evaluated by: mr-cal-bot

Impact: 42 Quick Win: 12 Complexity: 18 Confidence: 89 Actionability: 35

Issue body

Theme-resolved icon path was written to the desktop file without checking the file exists, and the subsequent existence check ran against the bare icon name stored in `icon` variable, emitting a spurious "not found" warning. Assign the resolved path to the local var first so the same check covers theme icons. Fixes #6364 IMO, this complete function should be refactored, including early return when icon is None, converting these 2 lines into a single one liner and remove any prefixed `$SNAP` as well ```diff @@ -89,11 +89,8 @@ class DesktopFile: if icon_path is not None: icon = icon_path - # Strip any leading slash. - icon = icon[1:] if icon.startswith("/") else icon - - # Strip any leading ${SNAP}. - icon = icon[8:] if icon.startswith("${SNAP}") else icon + # Strip any leading slash, ${SNAP} and $SNAP variable + icon.removeprefix("/").removeprefix("${SNAP}").removeprefix("$SNAP") # If icon is just a name (no path separator), try to resolve it from the hicolor icon theme. if "/" not in icon: ``` --- - [x] I've followed the [contribution guidelines](https://github.com/canonical/snapcraft/blob/main/CONTRIBUTING.md). - [x] I've signed the [CLA](http://www.ubuntu.com/legal/contributors/). - [x] I've successfully run `make lint && make test`. - [x] I've added or updated any relevant documentation. - [x] In documents I changed, I [added a meta description](https://canonical-starflow.readthedocs-hosted.com/how-to/add-a-page-meta-description/) if one was missing. - [x] I've updated the relevant release notes.

Evaluation history

Date Model Scores Action Summary
stealth/space-bunny-alpha
Impact: 42
Quick Win: 12
Complexity: 18
Confidence: 89
Actionability: 35
needs review A two-line fix defers rewriting theme-resolved icons until existence validation, avoiding false missing-icon warnings. It awaits review with two unresolved comments and failing CLA/Ubuntu 26.04 checks.
stealth/space-bunny-alpha
Impact: 38
Quick Win: 21
Complexity: 8
Confidence: 89
Actionability: 60
needs review Checks the theme-resolved icon path before writing it, eliminating false "not found" warnings. Final review is pending; CLA and Ubuntu 26.04 integration checks fail.
qwen3.6-35b-a3b-mtp-q6
Impact: 30
Quick Win: 26
Staleness: 75
Complexity: 15
Confidence: 70
close not mergeable PR to fix desktop file icon theme warning; diff targets outdated code and doesn't address the described bug
qwen/qwen3.6-35b-a3b
Staleness: 5
Complexity: 10
Confidence: 90
needs review Fixes a bug where theme-resolved icon paths were written to desktop files without existence checks, causing spurious warnings. Currently awaiting maintainer review with CI mostly passing.
qwen3.6-35b-a3b-mtp-q6
Staleness: 5
Complexity: 5
Confidence: 85
needs review Fixes a bug where theme-resolved icon paths were written to desktop files without existence checks, causing spurious warnings. Currently pending maintainer review with CI passing except for a one-time CLA check.

Update history

Date Change
updated
created

Related work

  • Likely Fixed By: canonical/snapcraft#6364 (confidence 99%)

    The PR explicitly says it fixes #6364, which reports the same spurious missing-icon warning for resolved theme icons.

  • Related To: canonical/snapcraft#6385 (confidence 84%)

    That merged refactor changed the same DesktopFile icon branch, so its behavior must be checked to avoid duplicating or conflicting with it.

Related issues

No related issues found above the similarity threshold.