search

probonopd / go-appimage

Go implementation of AppImage tools
MIT License
818 stars 71 forks source link

AppStream validator gives wrong results and can't be disabled #272

Open pbs3141 opened 9 months ago

pbs3141 commented 9 months ago

Running appstreamcli validate on this file

org.inkscape.Inkscape.appdata.xml

says that it is valid:

✔ Validation was successful: pedantic: 1

But when packaging it using appimagetool-817-x86_64.AppImage, it complains about errors:

Trying to validate AppStream information with the appstreamcli tool
ERROR appstreamcli: exit status 3
ERROR: AppStream metainfo file file contains errors. Please fix them. Please see https://www.freedesktop.org/software/appstream/docs/chap-Quickstart.html#sect-Quickstart-DesktopApps
org.inkscape.Inkscape.appdata.xml
  I: org.inkscape.Inkscape:8: unknown-tag developer
  W: org.inkscape.Inkscape:247: url-invalid-type vcs-browser
  W: org.inkscape.Inkscape:248: url-invalid-type contribute

In particular, it says <developer/> is an unknown tag, then provides a link to a page explicitly documenting it.

Is the bundled appstreamcli out of date, or is it genuinely supposed to be checking something different?

If not, then there should at least be a way to disable this validation check, since it's currently hard-coded to true.

pbs3141 commented 9 months ago

It seems the system version of appstreamcli is v1.0.1 while the bundled version is v0.12.9, which is indeed too old to support these features (by 5 years).

probonopd commented 8 months ago

That the AppStream spec keeps changing has indeed been a constant source of annoyance. Now that it has reached 1.0 state we should probably mandate version 1.0.x of appstreamcli tool, and bundle that one inside the AppImage.

ivan-hc commented 8 months ago

I've built more than 60 AppImages in my repositories and now I switched all my workflows to this new fantastic version of appimagetool, so thanks @probonopd !

I had the AppStream error only in five (5) of them, but I've solved by removing the content of /usr/share/metainfo

probonopd commented 8 months ago

Well, entirely removing the AppStream metainfo is what I'd like to avoid, but I have to admit that it's been hard to support due to its ever-changing nature. I hope that we can eventually switch to 1.0 and then leave it alone. According to its developer @ximion

what happened with 1.0 was the removal of deprecated features. Removal of deprecated stuff will (very likely) never happen again.

So ideally there would be a test tool that could verify that an AppStream file is 1.0 compatible, while just ignoring any tags that might be introduced after 1.0. So that we could settle with 1.0 and get no warnings or errors once future AppStream versions allow additional tags.

Thinking about it, maybe it'd be easiest to write such a validator myself in Go.

ximion commented 8 months ago

AppStream is backwards-compatible, but has never been (and will not be) forward-compatible. Here, the version of appstreamcli is simply too old to validate metadata that was written for a newer spec version.

ivan-hc commented 8 months ago

@probonopd this is a module i use in my package managers "AM" and AppMan, I've named this option "nolibfuse":

https://github.com/ivan-hc/AM/blob/main/modules/nolibfuse.am

it is using your new version of appimagetool, more details in the two recent releases:

and this is a video that shows how it works

https://private-user-images.githubusercontent.com/88724353/316023408-8b45d2c2-d2da-4a07-8b43-0cd77ffcb7cc.mp4?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTEzODgxMzcsIm5iZiI6MTcxMTM4NzgzNywicGF0aCI6Ii84ODcyNDM1My8zMTYwMjM0MDgtOGI0NWQyYzItZDJkYS00YTA3LThiNDMtMGNkNzdmZmNiN2NjLm1wND9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDAzMjUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwMzI1VDE3MzAzN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTZjODRjYTU3NjAzNDFkOGI3NTFjM2I5YWUxZGVmMjZlMTQ4ZWQ2Y2ZhNzBjYjA1YTRhMDAzOGFhNWRhNWU2N2QmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.8Hs7EhnuyT55teHUe5yC2_wLhy25Dhfi9W9iO42Sfvg

in brief, it converts the installed Type2 AppImages to Type3.

The thing that amazed me is that Appimages updatable using zsync can be updated without loosing their new Type3 status, also the big ones can be smaller (about 20-30 MB less).

But as I've said, there are some cases where to do this conversion is necessary to remove the content of /usr/share/metainfo (where is available), but in my 60+ Appimages only 5 had to use this workaround, and all of them where built from an old Debian repository on a third parti repo for .deb packages (for example Debian Multimedia). This kind of apps are patched from the mantainer of these .deb packages, so they are not the "original ones" from the upstream... on the contrary, my "Archimages" are all originally built from the upstream and packaged for Arch Linux, so no dirty workaround like this is needed to create the AppImages.

Don't worry, my goal here is to promote the use of the new Type3 standard, to prevent these patches I did.

probonopd commented 8 months ago

@ivan-hc, Type 3, where did you get that from? There is no Type 3 yet.

probonopd commented 8 months ago

AppStream is backwards-compatible, but has never been (and will not be) forward-compatible. Here, the version of appstreamcli is simply too old to validate metadata that was written for a newer spec version.

@ximion, thanks for your explanation.

In the AppImage project, we aim to provide a stable format that can be trusted to also work in the future. Let's say, 10 years down the road I would like to run an AppImage from today, just like I can run an AppImage from 2014 today.

What I mean is: I am looking for a tool that checks that the (essential) tags of (let's say) AppStream MetaInfo 1.0 are there and are valid.

Quickly wrote a small validator at https://github.com/probonopd/appstreamlint/; I think I wouldn't need much more than that?

ivan-hc commented 8 months ago

@ivan-hc, Type 3, where did you get that from? There is no Type 3 yet.

I talked to a mutual friend of ours, while I was building an unofficial AppImage for his application (Bottles), and he asked me if my AppImages were Type2 or Type3... then he explained to me about the situation regarding the security holes in libfuse2...and this was a couple of months ago. But I only discovered this tool of yours now, and from here I deduced that it was really the Type3 AppImages, given their efficiency.

So you're telling me I'm wrong? Like the GNOME users who, by dint of calling GNOME4 "GNOME 40" gave it this name? lol :laughing:

However, regarding my workaround, I am very aware that it is a "dirty trick". In fact my goal is to maintain Appstream datas in the AppImage, convincing developers to move their works to what "only I am calling" (at this point) Type3... or Type2.99, whatever you prefer. lol :rofl:

probonopd commented 8 months ago

Currently we are simply calling it the "static runtime". But maybe there will be one day a type 3 (or version 3 of the spec) that will require the runtime to be static (= not require libfuse).

ivan-hc commented 8 months ago

@probonopd As far as I'm concerned, this work of yours is excellent as it is. And this is in spite of those who still throw mud at AppImages with the history of obsolete FUSE libraries. If I were you, I would see it as a huge payback. Excellent!

ivan-hc commented 8 months ago

I remember that the previous appimagetool had a -n option to bypass AppStream Validate check. Is there no way to implement it into this version?

ximion commented 8 months ago

From my experience, allowing a "just ignore warnings/validation" flag is a recipe for disaster, unless it can be limited to builds or actions that are guaranteed to never leave the development environment and slip into production.

ivan-hc commented 8 months ago

From my experience, allowing a "just ignore warnings/validation" flag is a recipe for disaster, unless it can be limited to builds or actions that are guaranteed to never leave the development environment and slip into production.

it is this

unless it can be limited to builds or actions that are guaranteed to never leave the development environment and slip into production

for personal use

pbs3141 commented 7 months ago

Even if the AppStream validator were updated, I would vote to add the -n option. Here's why:

  W: org.inkscape.Inkscape:243: url-not-reachable
       https://inkscape.org/learn - Unexpected status code: 502
  W: org.inkscape.Inkscape:244: url-not-reachable
       https://inkscape.org/support-us/donate - Unexpected status code: 502

The validator checks whether the urls are valid by making network requests.

So when inkscape.org goes down, which happens time to time when it gets DDOSed (for some reason), or is otherwise under maintenance, CI will break. This is a prime candidate for using the -n option.

This is before we even get to the issue of offline building, or whether go-appimage should silently be making network requests in a way that cannot be disabled.

ximion commented 7 months ago

This is before we even get to the issue of offline building, or whether go-appimage should silently be making network requests in a way that cannot be disabled.

Passing --no-net to appstreamcli unconditionally probably makes sense, or only allow network access when the network is confirmed working and URL validation is desired.

probonopd commented 7 months ago

Since which version is --no-net supported?

ximion commented 7 months ago

Since which version is --no-net supported?

Pretty much forever, since validate existed. There's no version without it.

TheLastRar commented 3 months ago

Discovered this yesterday

The version of appstreamcli you bundle will segfault on a metainfo having a <screenshot> entry containing only an <image> entry (that is, missing the <caption> entry).

This issue wasn't obvious, as newer versions of appstreamcli didn't have any issue with this, not even emitting a warning.

JulianGro commented 1 month ago

This seems like a pretty bad issue because: The oldest system we should support is Ubuntu 20.04, as per the rules for AppImageHub. Ubuntu 20.04 doesn't come with appstreamcli 1.0, in fact Ubuntu 22.04 also doesn't; Ubuntu 24.04 is the first one to come with it. The package that supplies appstreamcli is also a dependency of Ubuntu's package manager, so uninstalling it also uninstalls the package manager.

What I will do for our software (Overte) is to build AppImageKit or go-appimage from source, with the check disabled.

pbs3141 commented 1 month ago

This issue has been open for 8 months, with the true fix (updating the validator) indefinitely stalled (PR). The commenter above has even forked the project just to disable the validator, while Inkscape is using a grotesque hack to get around it. Surely an -n disable switch is reasonable at this point? Or an option to override the validator with the system one?

Samueru-sama commented 1 month ago

This issue has been open for 8 months, with the true fix (updating the validator) indefinitely stalled (PR). The commenter above has even forked the project just to disable the validator, while Inkscape is using a grotesque hack to get around it. Surely an -n disable switch is reasonable at this point? Or an option to override the validator with the system one?

Hey I took a look at the Inkscape appimage script

If the grotesque hack is using appimagekit for the second step, a better solution would be this appimagetool instead since it uses zstd comp and the static runtime by default, aka you would get the same appimage as if go-appimage had made the second step. And it has the -n option which disables appstream validation.

probonopd commented 1 month ago

The real fix is to get a static version of the AppStream validator to build:

ximion commented 1 month ago

The real fix is to get a static version of the AppStream validator to build:

* [AppStream 1.0 static-tools#47](https://github.com/probonopd/static-tools/pull/47)

It's entirely unclear to me why that would fail with 1.0 but not with previous versions, as nothing has changed in AppStream with regards to how it is compiled or its dependencies. If you can use dynamic libraries with linker path changes / rpath, that would solve the issue too though. Or use the executable from the host.

probonopd commented 3 weeks ago

The executable from the host is not under our control and may be any random version.

Statically building fails since there is now a libxml dependency that was not there back then when the static build succeeded. Or maybe I am just doing it wrong.

ximion commented 3 weeks ago

Statically building fails since there is now a libxml dependency that was not there back then when the static build succeeded. Or maybe I am just doing it wrong.

AppStream has always depended on libxml2, since its very first version from 2012.