Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic desktop file checks #245

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Conversation

bbhtt
Copy link
Contributor

@bbhtt bbhtt commented Dec 31, 2023

Fixes #244

WIP I need to check if it breaks anything and add a few more checks.

@bbhtt bbhtt marked this pull request as draft December 31, 2023 10:59
@bbhtt bbhtt force-pushed the desktop-file branch 7 times, most recently from 8907565 to 9824b70 Compare December 31, 2023 18:37
@bbhtt
Copy link
Contributor Author

bbhtt commented Dec 31, 2023

The following recipe will reproduce the brokenness in #244.

flatpak-builder (at least the version used in org.flatpak.Builder) happily returns success on the build. The libappstream port/flatpak-builder 1.4.0 made things better by returning a bunch of errors, although that doesn't protect against direct uploads where the final ref is only present, being broken.

manifest
{
    "app-id":"org.flathub.test",
    "runtime":"org.freedesktop.Platform",
    "runtime-version":"23.08",
    "sdk":"org.freedesktop.Sdk",
    "command":"echo",
    "finish-args":[
        "--device=dri"
    ],
    "modules":[
        {
            "name":"desktop",
            "buildsystem": "simple",
            "sources":[
                {
                    "type":"file",
                    "path":"org.flathub.test.desktop"
                },
                {
                    "type":"file",
                    "path":"foo.png"
                }
            ],
            "build-commands": [
                "mkdir -p /app/share/icons/hicolor/128x128/apps",
                "mkdir -p /app/bin",
                "touch /app/bin/echo",
                "mv foo.png /app/share/icons/hicolor/128x128/apps/",
                "install -Dm644 $FLATPAK_ID.desktop /app/share/applications/$FLATPAK_ID.desktop"
            ]
        }
    ]
}
[Desktop Entry]
////=Bar
Exec=
Icon=foo
***=true
Terminal=false
Type=Application
Categories=Network;News;
Version=1.1
MimeType=application/rss+xml;application/atom+xml;application/rdf+xml;x-scheme-handler/feed
  • foo.png is an actual 128px png icon
flatpak-builder output
flatpak run org.flatpak.Builder build org.flathub.test.json --disable-download --force-clean --repo=repo
Emptying app dir 'build'
Starting build of org.flathub.test
Cache miss, checking out last cache hit
========================================================================
Building module desktop in /home/bbhtt/Downloads/tests/org.flathub.test/.flatpak-builder/build/desktop-7
========================================================================
Running: mkdir -p /app/share/icons/hicolor/128x128/apps
Running: mkdir -p /app/bin
Running: touch /app/bin/echo
Running: mv foo.png /app/share/icons/hicolor/128x128/apps/
Running: install -Dm644 $FLATPAK_ID.desktop /app/share/applications/$FLATPAK_ID.desktop
Committing stage build-desktop to cache
Cleaning up
Committing stage cleanup to cache
Finishing app
Exporting share/applications/org.flathub.test.desktop
Not exporting share/icons/hicolor/128x128/apps/foo.png, non-allowed export filename
Please review the exported files and the metadata
Committing stage finish to cache
Exporting org.flathub.test to repo
WARNING: Failed to validate desktop file /home/bbhtt/Downloads/tests/org.flathub.test/build/export/share/applications/org.flathub.test.desktop: /home/bbhtt/Downloads/tests/org.flathub.test/build/export/share/applications/org.flathub.test.desktop: error: file contains key "////" in group "Desktop Entry", but key names must contain only the characters A-Za-z0-9- (they may have a "[LOCALE]" postfix)
/home/bbhtt/Downloads/tests/org.flathub.test/build/export/share/applications/org.flathub.test.desktop: error: file contains key "***" in group "Desktop Entry", but key names must contain only the characters A-Za-z0-9- (they may have a "[LOCALE]" postfix)
/home/bbhtt/Downloads/tests/org.flathub.test/build/export/share/applications/org.flathub.test.desktop: error: required key "Name" in group "Desktop Entry" is not present

WARNING: Icon not matching app id in /home/bbhtt/Downloads/tests/org.flathub.test/build/export/share/applications/org.flathub.test.desktop: foo
WARNING: Icon referenced in desktop file but not exported: foo
Commit: 46af1abfa65d6e5b9c6b075963e0d94fa0531f802cc94bfda7acec1540d5ddda
Metadata Total: 25
Metadata Written: 10
Content Total: 6
Content Written: 3
Content Bytes Written: 1224 (1.2 kB)
Pruning cache
linter output
→ flatpak-builder-lint builddir build 
{
    "errors": [
        "appstream-missing-icon-file",
        "appstream-metainfo-missing",
        "desktop-file-icon-key-wrong-value",
        "appstream-missing-appinfo-file",
        "desktop-file-failed-validation"
    ],
    "warnings": [
        "desktop-file-exec-key-empty"
    ],
    "desktopfile": [
        "error: file contains key \"***\" in group \"Desktop Entry\", but key names must contain only the characters A-Za-z0-9- (they may have a \"[LOCALE]\" postfix)",
        "error: file contains key \"////\" in group \"Desktop Entry\", but key names must contain only the characters A-Za-z0-9- (they may have a \"[LOCALE]\" postfix)",
        "error: required key \"Name\" in group \"Desktop Entry\" is not present"
    ]
}
x flatpak-builder-lint repo repo 
{
    "errors": [
        "desktop-file-icon-key-wrong-value",
        "appstream-screenshots-not-mirrored-in-ostree",
        "desktop-file-failed-validation"
    ],
    "warnings": [
        "desktop-file-exec-key-empty"
    ],
    "desktopfile": [
        "error: required key \"Name\" in group \"Desktop Entry\" is not present",
        "error: file contains key \"***\" in group \"Desktop Entry\", but key names must contain only the characters A-Za-z0-9- (they may have a \"[LOCALE]\" postfix)",
        "error: file contains key \"////\" in group \"Desktop Entry\", but key names must contain only the characters A-Za-z0-9- (they may have a \"[LOCALE]\" postfix)"
    ]
}

All checks are skipped for console applications, baseapps and extensions/runtimes.

Only the desktop file matching the app-id and the main desktop group ie. [Desktop Entry] is checked.

GUI applications must have a desktop file matching appid, an exec key, value in icon key must match app-id. hidden or nodisplay key with true value is not allowed for GUI applications.

@bbhtt bbhtt marked this pull request as ready for review December 31, 2023 18:59
@bbhtt bbhtt mentioned this pull request Dec 31, 2023
@hadess
Copy link
Contributor

hadess commented Dec 31, 2023

Looks good, thanks for working on this.

@bbhtt
Copy link
Contributor Author

bbhtt commented Jan 1, 2024

Some applications using extra-data write their desktop files and icon directly to export/share/applications through apply_extra at runtime https://github.com/search?q=org%3Aflathub+%22export%2Fshare%2Fapplications%2F%22&type=code

These will now get caught as desktop-file-not-installed because without a desktop file inside builddir or repo, they can ignore checks in spite of being GUI apps.

Right now org.flatpak.Builder supports this case without any error, but with the libappstream port they fail because appdata file was placed in /app/share/{appdata, metainfo} but neither desktop file nor icon was available during compose run.

Run failed, some data was ignored.
Errors were raised during this compose run:
general
  E: filters-but-no-output

com.viber.Viber.desktop
  E: no-stock-icon

@barthalion
Copy link
Member

I think it's fair – just 10 apps (one is EOL), and not shipping desktop and icon files as part of the app has been historically causing issues here and there.

@barthalion barthalion merged commit 77528da into flathub-infra:master Jan 3, 2024
1 check passed
@bbhtt bbhtt deleted the desktop-file branch January 3, 2024 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check for icon sizes
3 participants