This is done. As you mentioned, snapcraft needs fixing so you get a correct desktop file (ping @sergiusens; or as @jamesh mentioned, you can ship your own and it will pass automated review). As a temporary measure you can request manual review for revisions you’re interested in and I can approve it.
1 Like
I’ve tried your suggestion and somehow sed can’t seem to find the .desktop file:
sed: can’t read /__w/1/s/.build/linux/snap/x64/code-insiders-x64/parts/gnome/install/usr/share/applications/code-insiders.desktop: No such file or directory
Did I get that right that I should still call snapcraftctl build in the override-build step, before that workaround code?
Thanks for your help.
I will propose a PR.
1 Like
We just released a new VS Code stable version. @popey can you approve https://dashboard.snapcraft.io/snaps/code/revisions/14/? Thanks.
I was suggesting adding this to the part that actually installed the desktop file. From the look of it, it’s the code part that installs the file, so that’s the part you’d want to add override-build to.
You are correct that a snapcraftctl build call is necessary to call the normal build routine first.
Interestingly, I started researching this issue today: https://github.com/microsoft/vscode/issues/72042
The same root cause is behind it: snapcraft incorrectly patches the line to be Exec=code-insiders, which makes the app launcher use the binary located in the PATH which breaks the aforementioned issue. Thanks for that pointer.
Thanks for the upcoming fix @sergiusens.
Hi @sergiusens, any update on that PR? Thanks for the help.
Meanwhile, @popey can we get https://dashboard.snapcraft.io/snaps/code-insiders/revisions/246/ approved? Thanks.
Done.
1 Like
@joaomoreno (cc @popey): please note that the updated review-tools are in production and the failure is legitimate. Your snaps need to be updated to no longer use an absolute path to the file /usr/share/pixmaps/com.visualstudio.code.png or /usr/share/pixmaps/com.visualstudio.code.insiders.png. We can continue to manually approve these for a little while, but if you fix your snaps, they’ll pass automated review again.
@sergiusens - you mentioned proposing a PR, but I couldn’t find it. Can you post the PR link here when you do?
@jdstrand I was under the impression the problem here is the patching of the .desktop files by snapcraft which is faulty. Shouldn’t the tool be the one correctly adding ${SNAP} to those directives, which would be the fix eventually submitted in PR by @sergiusens?
Yes, which is why I asked @sergiusens for more information. However, you are free to update your use of the tool and either patch up the desktop file or ship a compliant one in the meantime as discussed elsewhere in the topic. That would enable you to unblock your uploads until snapcraft is fixed.
1 Like
Oh missed this reply, my bad. Giving this a try. Thanks!
I created a PR which gives you all the control you want
If you don’t mind providing some simple instructions on what your build process looks like to get to the “snap” I do not mind iterating myself.
Thanks to @sergiusens we’re back to normality, thanks!
@popey I just have to ask you for one more manual approval, for our candidate stable release on which I haven’t placed the fix yet: https://dashboard.snapcraft.io/snaps/code/revisions/15/ Thanks.
1 Like
This is failing differently.
invalid icon path '/usr/share/pixmaps/com.visualstudio.code.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). If using snapcraft, consider using 'desktop: <file>' since the Icon paths in the desktop will be rewritten to use ${SNAP}/<file>. lint-snap-v2_desktop_file_icon (code.desktop, /usr/share/pixmaps/com.visualstudio.code.png)
I’d recommend ensuring the desktop file has lines such as:
Icon=${SNAP}/usr/share/pixmaps/com.visualstudio.code.png
Which should resolve this.
This is the same as the previous one I mentioned in New Store automated review rule is incorrect. Since @joaomoreno understands this is invalid but the snapcraft fix is on the way, I approved it.
2 Likes
did you test it properly? Why was it applied at all? What for exactly do you need this test?
I don’t quite understand this comment but will say that, yes, the rule was tested and yes it is finding a legitimate problem in the snap. It also found a legitimate problem in what snapcraft is doing and now snapcraft is being fixed. Finally, this is being fixed in vscode while the snapcraft fix is pending.
The comment asks if you tested the fix properly, as what has been applied can not have been properly tested. On that point, sure, I would love to know how you tested it before applying it globaly. Is it wrong to ask which steps you/staff undertakes for testing code before it is globaly applied to ensure quality of the service for published software over snapcraft.io?
From your answer, you seem to have less understanding for the question where I have even less understanding when you say that you tested it. It only tells me that “test” performed for snapcraft stable are not testing at all already published snaps which results in breaking working packages.
So, is it to much asked that only stable code, properly tested wanders into something that is globally applied?
I also understand that software is never perfect and never will be, but to take it as excuse is here wrong, because if you would have taken most popular snaps under which is vscode too, you would immediately face the problem, fact that you did not face is telling that it was not properly tested.
Not that I cant fix it, I fixed it already, but again, if fix on snapcraft is coming, you created overhead for quite many people, solution for them is to wait for weeks/months/… for a fix, now those who applied their own fixes, they will have again some overhead to revert those changes, even it it is one command to apply git patch, it still requires somebodies time. All that makes snapcraft.io less reliable service and again.
In previous post I only asked you if you properly tested the fix, above I hopefully explained what the question meant which you did not understand, in previous post I assumed that you understand what I mean and wanted to save you those few minutes of reading.