AppImage / AppImageKit

Package desktop applications as AppImages that run on common Linux-based operating systems, such as RHEL, CentOS, openSUSE, SLED, Ubuntu, Fedora, debian and derivatives. Join #AppImage on irc.libera.chat
http://appimage.org
Other
8.7k stars 557 forks source link

Refuse to run if Categories= is missing in desktop file #873

Open probonopd opened 5 years ago

probonopd commented 5 years ago

appimagetool shoud refuse to run if Categories= is missing in desktop file.

Should help in these cases: https://github.com/AppImage/appimage.github.io/issues/2

azubieta commented 5 years ago

Why to refuse? Why at run-time? It's not enough to verify it when the AppImage is created. I don't see the leak of the Categories entry something that must prevent the execution of an AppImage. Please elaborate this issue more.

TheAssassin commented 5 years ago

@probonopd we shouldn't invent our own validation, we use desktop-file-validate and it worked fine for quite a while.

probonopd commented 5 years ago

Why to refuse? Why at run-time? It's not enough to verify it when the AppImage is created.

Exactly.

We should refuse the creation of an AppImage if Categories= is missing.

we shouldn't invent our own validation, we use desktop-file-validate and it worked fine for quite a while.

As far as I know, desktop-file-validate lets this slip. But the applications then land in "Others" in the menu, which is ugly.

I then have a lot of work rejecting those and getting things sorted in https://github.com/AppImage/appimage.github.io, and am sick and tired of it. See https://github.com/AppImage/appimage.github.io/issues/2

Since @develar is using his own tools, he should check for this too, if I may have a wish :-)

develar commented 5 years ago

Since @develar is using his own tools, he should check for this too, if I may have a wish :-)

According to your suggestions (thanks!), in case of electron-builder this value is never empty — https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/src/targets/LinuxTargetHelper.ts#L102 If we cannot do something smart, warn printed and "Utility" is used.

probonopd commented 5 years ago

Cool, thanks @develar 👍

TheAssassin commented 5 years ago

We shouldn't try to solve issues in Freedesktop standards by adding more and more constraints.

We could show a warning...

probonopd commented 5 years ago

As appimagetool is a developer not a user tool, I am for showing a fatal warning. So that AppImage generation cannot continue unless the Categories key exists. Not having one is bad, and gets the AppImage rejected in https://github.com/AppImage/appimage.github.io anyway, only that it causes me and the developer additional work if not caught early.

TheAssassin commented 5 years ago

I am against restricting developers in that way. We should only show recommendations how to improve users' UX. Such a warning will make people just add Categories=Utilities; and will move the AppImages in there. That's worse than Other, as that will at least make users notify that there's an issue, and perhaps make them request better desktop files upstream.

probonopd commented 5 years ago

The point is, I am currently we are not letting such AppImages pass tests in AppImageHub. Either we let it slip in both places nor none, or else we are just creating additional work for ourselves. And I hate avoidable chores...

TheAssassin commented 5 years ago

There is a big difference between "necessary" and not.

linuxdeploy would be the right place to add such checks for instance. But appimagetool is too late in the toolchain to add constraints.

probonopd commented 5 years ago

One can look at it as "late", but one can also look at it as "low level", meaning most AppImages will run through appimagetool, so checking here means that we catch issues no matter which higher-level tool is used. This doesn't prevent us from putting the same check into the higher-level, "earlier" tools as well.

TheAssassin commented 5 years ago

"low level" is the right word. Low level tools shouldn't apply any non-fatal constraints. And it's a non-fatal one in the sense that it's a valid desktop file.

probonopd commented 5 years ago

Low level tools shouldn't apply any non-fatal constraints.

Where does this theory come from?