skip-appstream-check is ignored

[ilya-fedin]

I'm getting appstream-failed-validation despite having skip-appstream-check being set.

PS: what's the point of having appstream check two times?

[bbhtt]

The linter expects the flathub.json to be present in builddir/files/flathub.json or repo/files/flathub.json but nothing at the moment copies it there.

https://github.com/flathub-infra/flatpak-builder-lint/blob/655d3fbeb53f11dcd54472851aec70472fc19451/flatpak_builder_lint/builddir.py#L112

https://github.com/flathub-infra/flatpak-builder-lint/blob/655d3fbeb53f11dcd54472851aec70472fc19451/flatpak_builder_lint/ostree.py#L80

If you add something temporarily in the manifest to copy it to /app/flathub.json it should work. https://github.com/flathub/org.freedesktop.appstream.cli/pull/5

Note. This might clash with another app if your app is used as a base: for another app. Please make sure the second app cleans up "your" flathub.json during build otherwise it'll get caught in the linter.

example

```yaml - name: flathub-json buildsystem: simple build-commands: - cp flathub.json ${FLATPAK_DEST}/flathub.json sources: - type: file path: flathub.json ``` ```json { "name": "flathub-json", "buildsystem": "simple", "build-commands": [ "cp flathub.json ${FLATPAK_DEST}/flathub.json" ], "sources": [ { "type": "file", "path": "flathub.json" } ] } ```

point of having appstream check two times?

One of them runs on the appdata file and the other one runs on that and the resultant build directory as well. There are also extra checks and warnings for things appstream-glib doesn't error on eg. missing developer_name, missing icon in appstream catalogue etc.

[bbhtt]

The update here is that appstream and icon check is no longer run in the flathub builders as a separate step. All checks are now in the linter, which checks the manifest and the repo once build is done.

baseapps, extensions and console applications will be allowed to have "skip-appstream-check" in flathub.json and/or skip the other appstream/icon checks in linter. Some parts of this is already done but there might be a few cases remaining. Baseapps can already skip through every check.

The flathub.json will be installed like above for the builddir and repo checks for every console-application on Flathub some time later.

Regular applications cannot skip through some of this without exceptions or having flathub.json installed like above.

Please make sure to see the note I mentioned above.

[ilya-fedin]

You mean having the exception for having the key in flathub.json is not enough? My case is regular application

[bbhtt]

You mean having the exception for having the key in flathub.json is not enough? My case is regular application

Right now, no change is necessary on your part.

[bbhtt]

https://github.com/flathub-infra/flatpak-builder-lint/pull/249 removed flathub.json handling for appstream checks.

It isn't deployed to flathub yet so the checks won't take effect right now.

I assume you're talking about Telegram, with that PR it is failing:

{
    "errors": [
        "flathub-json-skip-appstream-check",
        "appstream-failed-validation",
        "appid-ends-with-lowercase-desktop"
    ],
    "warnings": [
        "appstream-screenshot-missing-caption"
    ],
    "appstream": [
        "style-invalid         : Too many <p> tags for a good description [12/10]"
    ]
}

You have to

a) remove installing flathub.json to /app b) apply for an exception for appid-ends-with-lowercase-desktop (this can be granted automatically, based on the fact that telegram predated the linter) https://docs.flathub.org/docs/for-app-authors/linter#exceptions c) Fix the <p> tag issue in appdata.

[bbhtt]

Actually (c) should not be reached at all imo and should be relaxed to 15/20 (validate-relax default).

There is already a patch in org.flatpak.Builder to relax it to 15 https://github.com/flathub/org.flatpak.Builder/blob/master/patches/appstream-glib-0005-Change-min.-paragraph-length-to-5-and-max.-number-of.patch but looks like it affects releases only.

I can add a patch to relax it for descriptions too.

[ilya-fedin]

b) apply for an exception for appid-ends-with-lowercase-desktop (this can be granted automatically, based on the fact that telegram predated the linter) https://docs.flathub.org/docs/for-app-authors/linter#exceptions

But it is already existing: https://github.com/flathub-infra/flatpak-builder-lint/blob/master/flatpak_builder_lint/staticfiles/exceptions.json#L781

Actually (c) should not be reached at all imo and should be relaxed to 15/20 (validate-relax default).

As far as I remember, the main problem with the linter is that it doesn't allow links in changelogs while Telegram changelogs frequently have links.

[bbhtt]

But it is already existing

Oh sorry looks like forgot to use --exceptions locally

As far as I remember, the main problem with the linter is that it doesn't allow links in changelogs while Telegram changelogs frequently have links.

Yea, appstream-util doesn't want links in releases. Flathub patches it to allow in descriptions but not in releases.

Then you have to apply for the appstream-failed-validation exception or modify the workflow to use links within <url type="details">URL</url> inside release tags.

Not exactly sure what's the history behind not allowing links in releases, but I can probably add a patch for it too if there are no major issues.

[ilya-fedin]

Then you have to apply for the appstream-failed-validation exception

Does skip-appstream-check in flathub,json still has any effect? If yes, why the exception should be done two times?

Not exactly sure what's the history behind not allowing links in releases, but I can probably add a patch for it too if there are no major issues.

Would be really nice.

[bbhtt]

Does skip-appstream-check in flathub,json still has any effect? If yes, why the exception should be done two times?

Nope, all appstream checks are skipped either based on app type or exceptions. It's getting caught (ie. flathub-json-skip-appstream-check) for repo and build check because you have it installed to /app. You can remove it from /app and remove the key from flathub.json in the repo and it'll be fine.

[ilya-fedin]

Should I wait for the patch for links in release?

[hfiguiere]

Then you have to apply for the appstream-failed-validation exception

no. you patch it. releases have the url tag for that.

[hfiguiere]

and then you submit the changes upstream.

[ilya-fedin]

no. you patch it. releases have the url tag for that.

I'm afraid I can't do that...

[bbhtt]

Looks like the spec says no plaintext url in release descriptions https://www.freedesktop.org/software/appstream/docs/sect-Metadata-Releases.html#tag-release-description and appstreamcli also errors on this. (The wording should be stronger on that, I'll open an issue)

I don't think it'd be sane to patch that out part. I'd also suggest modifying the tooling to use <url> or <issues>

[ilya-fedin]

Just to be clear, I don't understand the scripts that generate the appdata from changelog.txt in tdesktop and if it couldn't be fixed without changing tdesktop's scripts and exception couldn't be given then I will just give up on the package. tdesktop lacks contributors for anything but the functionality demanded by the business (flatpak is not one of them) so I'm doubt anyone will fix tdesktop.

[hfiguiere]

no. you patch it. releases have the url tag for that.

I'm afraid I can't do that...

You can modify any file.

[bbhtt]

I don't have an issue with the exception for "appstream-failed-validation" in general, if it is justified, considering it is effectively the replacement for "skip-appstream-check".

I looked at changelog.txt, there are 5 very old instances of lines having links like "open https://username.t.me" which fits neither url nor issues and nothing I found could be fitted into those two tags. Seems it is rare that an entry has a link.

My suggestion would be to strip any http(s) parts from changelog lines/replace them with a placeholder text either in the script or while building through patch/sed.

At worst, you only need to do it once. And the patch wouldn't be too complicated because the appdata has only release tag for the latest version.

[hfiguiere]

I don't have an issue with the exception for "appstream-failed-validation" in general,

There are only a few were it should be allowed. Malformed release entries isn't one of them.

[ilya-fedin]

My suggestion would be to strip any http(s) parts from changelog lines/replace them with a placeholder text either in the script or while building through patch/sed.

Wouldn't that look weird to the users?

[bbhtt]

You could just remove the https part and preserve the rest, no software store renders it as clickable links if it is in release description afaik. If any does, at worst, this requires a manual copy and paste of the URL.

[ilya-fedin]

But why should https:// be stripped if the stores can interpret the text the way they want anyway?

[ilya-fedin]

The

- A new format was supported for username links, in addition to "t.me/username." You can now open Telegram accounts, groups or channels using links like "username.t.me" or "https://username.t.me."

would become

- A new format was supported for username links, in addition to "t.me/username." You can now open Telegram accounts, groups or channels using links like "username.t.me" or "username.t.me."

which is still weird

[hfiguiere]

contains a brief description of what is new in the release.

(emphasis mine)

So:

A new format was supported for username links, in addition to "t.me/username."

[bbhtt]

But why should https:// be stripped if the stores can interpret the text the way they want anyway?

Because that's how appstream checks for hyperlinks. I think GNOME software follows the spec and doesn't render them as clickable. I don't know what every store does, but they should probably do the same.

which is still weird

It's a very minor inconvenience for something autogenerated, imo.

[ilya-fedin]

So:

No one will do special changelogs for flatpak in tdesktop. The text is made by some marketing team that likely doesn't even know about existence of flatpak or linux.

Because that's how appstream checks for hyperlinks.

Maybe it should stop doing the check if it doesn't have any sense?

[bbhtt]

Maybe it should stop doing the check if it doesn't have any sense?

That you have to ask in https://github.com/ximion/appstream/issues I don't know the reasons, I'm following the spec and behaviour.

[bbhtt]

The linter changes were deployed yesterday and two telegram builds happened since then that were successful.

Unless there are any specific issues with the linter, I think this can be closed as "skip-appstream-check" is now gone wrt. appstream validation.

Whether the exception should be accepted is better debated in a PR under specific circumstances, imo.