I noticed that overnight a couple more automated review rules were added (from 59 to 62). One in particular seems wrong, which is now preventing us from releasing our VS Code Insiders snap:
malformed desktop file: ‘Icon=’ specified multiple times lint-snap-v2_desktop_file (icon, code-insiders.desktop)
The specified file does have two Icon= directives, but each is in its own section, which should be fine given the spec.
I’m inlining the contents of code-insiders.desktop:
Can you please go to the particular revision’s page on https://dashboard.snapcraft.io and “request manual review”? (from your list of snaps go to “code-insiders”, then on the left-hand side click on the revision with the red exclamation mark, and the controls are at the bottom).
We do nightlies so this workaround will only go so far. Any ETA for the fix to be deployed to the Store? This will let us plan better our upcoming release and whether we need to update the .desktop file to pass the faulty lint rule.
The review-tools update needed to allow multiple Icon= lines in a .desktop file is now in production; upcoming code-insiders uploads should go through automatically. Please poke here if that’s not the case!
Interestingly, it looks like Snapcraft isn’t properly sanitising the desktop file it puts in your snap. Here’s what the desktop file included in your snap looks like:
Note that the Icon line for the new-empty-window action is not ${SNAP}/ relative, and that the Exec line also points outside of the snap. The version installed by snapd ends up looking like:
… and when PR #6767 is merged, that problematic Icon line will be removed too. Some of this could be considered a Snapcraft bug (since it is usually the one adding the ${SNAP}/ prefix to the Icon line), but the handling of the Exec line might be your responsibility.
- lint-snap-v2:desktop_file_icon:code-insiders.desktop:/usr/share/pixmaps/com.visualstudio.code.insiders.png
invalid icon path '/usr/share/pixmaps/com.visualstudio.code.insiders.png'. Should either specify the basename of the file (with or without file extension), snap.<snap name>.<snap command>[.(png|svg)] or ${SNAP}/path/to/icon.(png|svg)
The tools are showing a real problem in the snap which should be addressed (though that may be a snapcraft bug as @jamesh mentioned).
If the asset is put into snap/{gui,hooks} it is copied over to meta/{gui,hooks} with little to no intervention (as little as chmod for hooks) as those are handcrafted files.
When using the desktop key, snapcraft however only looks at the “Desktop Entry” section, we can revisit the freedesktop spec and see it is safe to iterate over all the sections looking for Icon.
I reviewed the spec when making the change to fix the ‘multiple Icon=’ issue and found that Actions, like the one VS Code Insiders is using, can also have Icon= entries.
I can’t see revision 241, since the only published revision is 239 (at least for amd64). That revision looks like what I pasted above:
$ snap download code-insiders
Fetching snap "code-insiders"
Fetching assertions for "code-insiders"
Install the snap with:
snap ack code-insiders_237.assert
snap install code-insiders_237.snap
$ unsquashfs code-insiders_237.snap
Parallel unsquashfs: Using 6 processors
3229 inodes (7332 blocks) to write
[=============================================================\] 7332/7332 100%
created 3048 files
created 1009 directories
created 181 symlinks
created 0 devices
created 0 fifos
$ cat squashfs-root/meta/gui/code-insiders.desktop
[Desktop Entry]
Name=Visual Studio Code - Insiders
Comment=Code Editing. Redefined.
GenericName=Text Editor
Exec=code-insiders %U
Icon=${SNAP}/usr/share/pixmaps/com.visualstudio.code.insiders.png
Type=Application
StartupNotify=false
StartupWMClass=Code - Insiders
Categories=Utility;TextEditor;Development;IDE;
MimeType=text/plain;inode/directory;
Actions=new-empty-window;
Keywords=vscode;
[Desktop Action new-empty-window]
Name=New Empty Window
Exec=/usr/share/code-insiders/code-insiders --new-window %F
Icon=/usr/share/pixmaps/com.visualstudio.code.insiders.png
There is a second $SNAP/usr/share/applications/code-insiders.desktop that matches your content, but that’s not the one that snapd will preprocess and install.
The fact that Snapcraft has replaced the Exec line with a simple code-insiders %U also sounds a bit like a bug:
we’ve lost the --unity-launch argument
the application may now be invoked with URLs instead of file paths (since the %F was changed to %U).
This is the code that seems to be responsible for that rewrite:
If you don’t want to wait for Snapcraft to be updated, you could try creating the desktop file yourself. The changes you’d need are roughly the following:
remove the desktop: key from your app in the snapcraft.yaml
have one of your parts stage a file meta/gui/code-insiders.desktop. The primary requirements are:
the desktop file name should match the command.
the Exec line(s) should start with the command you want to run (in this case code-insiders). Not a path within your snap, since this is referencing the wrapper snapd creates for your app.
the Icon line(s) should use a ${SNAP}/ prefix to reference a file inside your snap.
Looking at the upstream desktop file you’re starting with, adding something like the following to the the of the override-build stanza of your part might do the trick:
mkdir -p $SNAPCRAFT_PART_INSTALL/meta/gui
sed 's|^Icon=|Icon=${SNAP}|;s|^Exec=/usr/share/code-insiders/code-insiders|Exec=code-insiders|' \
$SNAPCRAFT_PART_INSTALL/usr/share/applications/code-insiders.desktop \
> $SNAPCRAFT_PART_INSTALL/meta/gui/code-insiders.desktop
I think we’ll wait for the update. Did I get that right that we’d need the store update (which fixes the validation rules) as well as the snapcraft tool update (which fixes the patching)? Can we get an ETA on both? Thanks!
Since we’re about to release a new stable version, I’d require a manual validation approval from you again sometime later this week. I’ll just ping in here once that comes. Thanks!
