ximion / appstream

Tools and libraries to work with AppStream metadata
http://www.freedesktop.org/wiki/Distributions/AppStream/
GNU Lesser General Public License v2.1
213 stars 115 forks source link

Compose errors should be clear and concise, and provide resources for troubleshooting #579

Closed Lyude closed 9 months ago

Lyude commented 9 months ago

Hi! I'm entirely new to flatpak, and unfortunately the way I've found my way to filing this issue is pretty simple: I maintain neovim-gtk and we recently received a pull request from a contributor to add flatpak support. Of course, I was happy to oblige and finally got a chance to sit down and try building things today so I could try submitting this to flathub. Unfortunately, I was a little surprised by how obtuse and confusing the build process has turned out to be - and certainly not for a lack of trying, either.

So: https://github.com/Lyude/neovim-gtk/pull/104 is the pull request in question, and trying to build it resulted in this:

➜  neovim-gtk git:(main) ✗ flatpak-builder flatbuild com.github.Lyude.neovim-gtk.yaml --force-clean --install --user        
Downloading sources
Starting build of com.github.Lyude.neovim-gtk
Cache hit for neovim-gtk, skipping build
Cache miss, checking out last cache hit
Cleaning up
Running appstreamcli compose
Only accepting components: com.github.Lyude.neovim-gtk, com.github.Lyude.neovim-gtk.desktop
Processing directory: /home/lyudess/Projects/neovim-gtk/neovim-gtk/.flatpak-builder/rofiles/rofiles-AYp8c3/files
Composing metadata...
Run failed, some data was ignored.
Errors were raised during this compose run:
general
  E: filters-but-no-output

com.github.Lyude.neovim-gtk
  E: metainfo-license-invalid
Refer to the generated issue report data for details on the individual problems.
Error: ERROR: appstreamcli compose failed: Child process exited with code 1

This, unfortunately is really not usable information to someone who has just started working with flatpak. For one: people being introduced to flatpak are introduced to flatpak, not appstreamcli or any of the tools used internally. So, naturally people like myself are going to start searching the internet for things like "flatpak metainfo-license-invalid", "metainfo-license-invalid", etc. along with going through flatpak documentation. If you try looking for these terms right now, at least on DDG you get no useful results or information. To start - it seems a bit silly that there are no hyperlinks in the output from this tool to direct people to resources for troubleshooting appstreamcli or to give people any kind of troubleshooting steps. There's also this rather unhelpful bit

"Refer to the generated issue report data for details on the individual problems."

This is very vague. Which report? Where can I find it? Even passing -v doesn't tell me that. Perhaps starting appstreamcli manually would, I don't know, but I just got here - that's knowledge a new person isn't going to have with this as their first introduction to flatpak. If such things give more information, they're steps that should be mentioned right in the error output to ensure that flatpak adoption is as easy as possible for new developers.

ximion commented 9 months ago

This is in a way a Flatpak issue - if you passed --explain to the compose command or had access to the report file and viewed that, you would get a detailed explanation of the issue. It's running in terse mode by default in Flatpak.

I am working on a documentation change at the moment which will make these tags available for search online as well, that should help a bit.

hfiguiere commented 9 months ago

No. requiring to pass --explain to get a smidge of error explanation is just bad user experience.

bbhtt commented 9 months ago

if you passed --explain to the compose command

Hi, neither appstreamcli compose 0.16.1 or 1.0.1 has an --explain argument, is this something coming in the future? Thanks

flatpak run org.freedesktop.appstream.cli compose --explain
Failed to parse arguments: Unknown option --explain

I am working on a documentation change at the moment which will make these tags available for search online as well, that should help a bit.

Having the error codes documented from as-validator-issue-tag.h and as-hints-tag.c` would be great. Thanks!

ximion commented 9 months ago

@bbhtt Sorry, compose does not have --explain, that is for validate only. The compose reports can be incredibly verbose and are designed to be viewed either as HTML (specify an output location with --hints-dir=DIR) directly, or returned as YAML document to be read into other tools for display.

If you really, really want to view more details on the command-line (which should not be too bad if just one component is analyzed, as opposed to hundreds), you may also pass --print-report=full to compose. This will make an attempt to print the whole report to the command-line (with some missing bits like URLs that can't be properly represented on the console as opposed to HTML).

If people using Flatpak are really supposed to fish through logs to find the cause of an error, having the full report in the logs at all times makes sense (so, set that flag unconditionally). Otherwise feeding the machine-readable result into a different too and having it represented nicely (e.g. by a GitHub bot or a dashboard like https://appstream.debian.org/trixie/main/) makes a lot more sense.

For regular validation, all tags are listed at https://www.freedesktop.org/software/appstream/docs/chap-Validation.html - adding compose validation there is a bit trickier, because issues from compose contain a lot more detailed context. So you would want to look at the report in any case.

razzeee commented 8 months ago

I am working on a documentation change at the moment which will make these tags available for search online as well, that should help a bit.

can we somehow help to get this to be real sooner?

ximion commented 8 months ago

See https://www.freedesktop.org/software/appstream/docs/chap-Validation.html#sect-ValidatorIssues for regular validator issues.

The compose issues make a bit less sense to add there though, as they embed information. So that list would contain messages like "Unable to store icon <code>{{fname}}</code>: {{msg}}" which aren't helpful at all.

The data is exported as HTML and also as machine-readable data. It would be nice if Flathub could just provide that data at some location. For example, on Debian we have pages like this one: https://appstream.debian.org/sid/main/issues/libreoffice-impress.html

Here is an example of the contents of the hints directory if you passed --hints-dir=DIR to compose: example.hints.tar.gz

The easiest and convenient way would just be to upload that HTML file somewhere and have the GitHub "this build has failed" bot provide a link to it. Another simple way would be to just pass --print-report=full, but then you force people to read a lot of the log file, which is less nice.

The most ideal way would be to parse the generated machine.readable output and display it in some kind of developer dashboard, or simply have the Bot write that information onto the issue, so people don't need to grep log files.

SoundDesignerToBe commented 8 months ago

I have that issue too, but only if I use flatpak-builder >= 1.4, versions below work fine. Has somebody found out where to find that "generated issue report data"?

barthalion commented 8 months ago

I think that, ultimately, compose should be more permissive than the regular validation. Minor issues such as the lack of categories should not be considered a blocker; anything that is good enough to be parsable per the spec should be let through.

ximion commented 8 months ago

@barthalion

I think that, ultimately, compose should be more permissive than the regular validation. Minor issues such as the lack of categories should not be considered a blocker; anything that is good enough to be parsable per the spec should be let through.

That is exactly how it operates, validator errors are even demoted to compose warnings. However, no categories in desktop apps is a violation of the spec, so rejecting them is the right thing to do (software centers will also not display uncategorized desktop apps).

@SoundDesignerToBe

Has somebody found out where to find that "generated issue report data"?

The data ends up in --hints-dir=DIR, wherever that is pointing. But Flathub doesn't upload that data anywhere, so I don't think you can access it right now...

barthalion commented 8 months ago

That is exactly how it operates, validator errors are even demoted to compose warnings. However, no categories in desktop apps is a violation of the spec, so rejecting them is the right thing to do (software centers will also not display uncategorized desktop apps).

Flathub is a software center, and we display uncategorized apps, so I guess there's that?

barthalion commented 8 months ago

Actually, my bad, I think when there are no categories, it still outputs a useful message. It's bad when the XML is completely invalid, e.g. https://buildbot.flathub.org/#/builders/37/builds/12601 which misses the closing </component> tag.

~/d/f/com.albiononline.AlbionOnline sajw12/master [1] appstreamcli compose --no-partial-urls --prefix=/ --origin=com.albiononline.AlbionOnline --media-baseurl=https://dl.flathub.org/media --media-dir=builddir/files/share/app-info/media --result-root=builddir/files --data-dir=builddir/files/share/app-info/xmls --icons-dir=builddir/files/share/app-info/icons/flatpak --components=com.albiononline.AlbionOnline,com.albiononline.AlbionOnline.desktop --print-report=full builddir/files
Only accepting components: com.albiononline.AlbionOnline, com.albiononline.AlbionOnline.desktop
Processing directory: builddir/files
Composing metadata...
Run failed, some data was ignored.
Errors were raised during this compose run:
general
  E: filters-but-no-output
     Component filters were set, but no output was generated at all. Likely none of the filtered
     components were found, try to relax the filters and ensure the input data is valid.
Refer to the generated issue report data for details on the individual problems.

Even with --print-report=full, it's nowhere close to the relatively clarify of the validate output:

~/d/f/com.albiononline.AlbionOnline sajw12/master ❱ appstreamcli validate com.albiononline.AlbionOnline.appdata.xml 
E: ~:~: xml-markup-invalid
     Could not parse XML data: Entity: line 56: parser error : Premature end of data in tag
     component line 2

     ^

✘ Validation failed: errors: 1

If I add the missing tag, the output is exactly the same, while validation explicitly says the file is just bonkers:

(...)
Run failed, some data was ignored.
Errors were raised during this compose run:
general
  E: filters-but-no-output
Refer to the generated issue report data for details on the individual problems.
Error: ERROR: appstreamcli compose failed: Child process exited with code 1
FB: Unmounting read-only fs: fusermount -uz /home/bpiotrowski/.cache/flatpak-builder/rofiles/rofiles-StpkJA
~/d/f/com.albiononline.AlbionOnline sajw12/master• 2.2s [1] git diff
diff --git a/com.albiononline.AlbionOnline.appdata.xml b/com.albiononline.AlbionOnline.appdata.xml
index 5d50a23..20de3cf 100644
--- a/com.albiononline.AlbionOnline.appdata.xml
+++ b/com.albiononline.AlbionOnline.appdata.xml
@@ -53,3 +53,4 @@
     <content_attribute id="money-gambling">none</content_attribute>
   </content_rating>
 </application>
+</component>
~/d/f/com.albiononline.AlbionOnline sajw12/master• ❱ appstreamcli validate com.albiononline.AlbionOnline.appdata.xml

(appstreamcli:112577): GLib-CRITICAL **: 16:01:50.156: g_str_has_suffix: assertion 'str != NULL' failed
E: ~:~: component-id-missing
E: ~:~: component-name-missing
E: ~:~: component-summary-missing
I: ~:3: unknown-tag application
E: ~:~: metadata-license-missing
E: ~:~: app-description-required
E: ~:~: desktop-app-launchable-missing
I: ~:~: content-rating-missing

✘ Validation failed: errors: 6, infos: 2, pedantic: 1

So no matter what switches are passed, there's no way to make the compose show what the problem is.

barthalion commented 8 months ago

For what it's worth, appstream-glib's compose was re-using the same function that was used by the validate-relax command, so the output was somewhat actionable. I understand it's not unreasonable to ask developers to validate their files, but it's not the reality we live in.

ximion commented 8 months ago

Actually, my bad, I think when there are no categories, it still outputs a useful message. It's bad when the XML is completely invalid, e.g. https://buildbot.flathub.org/#/builders/37/builds/12601 which misses the closing </component> tag.

That's a bug, and a fairly important one... An AppStream release is scheduled today, but this issue might just delay it a bit if I can't resolve it in time. The compose code should display that the XML is broken, not something else.

ximion commented 8 months ago

Wasn't as bad of an issue as I thought, just an unfortunate combination of flags that hasn't been used much yet.