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

Use of the url tag #296

Closed
fedelibre opened this issue Dec 30, 2020 · 7 comments
Closed

Use of the url tag #296

fedelibre opened this issue Dec 30, 2020 · 7 comments

Comments

@fedelibre
Copy link

Can you please help me to debug the errors in this metainfo file?

org.frescobaldi.Frescobaldi.appdata.xml.txt

The file is generated with appstreamcli news-to-metainfo, see here.

I'm currently building a new flatpak and I see these errors in the generated file:

$ appstreamcli validate ./_stable/files/share/appdata/org.frescobaldi.Frescobaldi.appdata.xml
E: org.frescobaldi.Frescobaldi:203: description-enum-item-invalid ul
W: org.frescobaldi.Frescobaldi:160: description-has-plaintext-url ul
I: org.frescobaldi.Frescobaldi:153: url-not-secure http://frescobaldi.org/
E: org.frescobaldi.Frescobaldi:162: description-para-markup-invalid url
I: org.frescobaldi.Frescobaldi:3: cid-contains-uppercase-letter org.frescobaldi.Frescobaldi
E: org.frescobaldi.Frescobaldi:176: description-enum-item-invalid ul

I don't understand the errors about ul items.
And why is the url tag invalid? I read the spec and I thought I could use it. What I'm missing?

Thanks in advance

@ximion
Copy link
Owner

ximion commented Dec 30, 2020

You can pass --explain to appstreamcli to get some more info on issues.
The problem with your file is that you have multiple levels of ul nesting, while only one level is allowed. So one ul or ol tag is only permitted to contain li tags, not other ul tags for deeper nesting. Remove those, and this part should validate.

The url tag is not permitted within description-type blocks, its parent is the release tag, and software centers will always show it as "Details" or "More info about this release" link below the original description text. Move the url one level up, and this issue should also be gone.
All the other issues appear to be infos/pedantic hints, so those will not prevent validation.

@fedelibre
Copy link
Author

The url tag is not permitted within description-type blocks, its parent is the release tag

Ok, it woud be nice to have it documented here. I'll keep this issue open for this, if you agree.

Thanks for the hint about --explain: it slows down the validation but it's very helpful in case of errors.

I thought that the lines created by news-to-metainfo would have been syntactically correct or at least an error or warning would have been printed. Wrong assumption: I'll make sure to validate the final metainfo file.

@ximion
Copy link
Owner

ximion commented Dec 31, 2020

Ok, it woud be nice to have it documented here. I'll keep this issue open for this, if you agree.

I think the specification is pretty much clear here already - all the tags listed are parents of the component tag (meaning even the url one you linked is only valid for components, not for release which permits url with different type values (or no type for the generic "more info" link)), and https://www.freedesktop.org/software/appstream/docs/chap-Metadata.html#tag-description is very explicit in what is allowed for descriptions right now (and this one is referenced by the release tag description).
Adding info to each tag in the specification where it isn't allowed could become incredibly verbose and hard to read quickly.

Thanks for the hint about --explain: it slows down the validation but it's very helpful in case of errors.

This should only slow down the validation by the amount it takes for the lines to print to stdout... Can you measure the performance difference?

I thought that the lines created by news-to-metainfo would have been syntactically correct or at least an error or warning would have been printed.

The generated lines should always be correct, anything else would be a bug. But the problematic lines are not in the NEWS file, as far as I can see, but were already in the metainfo file - see https://github.com/frescobaldi/frescobaldi/blob/master/linux/org.frescobaldi.Frescobaldi.metainfo.xml.in#L110
news-to-metainfo will not replace already existing lines. If those were generated by appstreamcli in the past, then I couldn't find their source in the NEWS file or anywhere else.

@fedelibre
Copy link
Author

Well, a lot of reading is required to understand which tags are allowed and where. If the url tag was presented only within the release tag, it would have been clear that it's allowed only there. A quick reader - like me - would read just the url tag explanation and think it could be used everywhere. But I understand it's a complicate matter...

I've tested again the performance with --explain switch and this time I cannot see substantial differences. So please ignore it.

Finally, the problems were definitely in the NEWS file. I fixed them and made a patch by manually editing the NEWS file, then run appstreamcli news-to-metainfo.

@ximion
Copy link
Owner

ximion commented Jan 5, 2021

Actually, news-to-metainfo should never produce bad output, even if the input is terribly wrong.
I just can't seem to reproduce your output, even if I feed the command a NEWS file with multiple levels of bullet points... Is there any trick to this?

@fedelibre
Copy link
Author

You are right. I fixed the multiple levels of bullet points, but Frescobaldi author somehow reintroduced them in a later commit, see history here. I'll try to rearrange the Makefiles to avoid these errors.

However, I think that any URL is passed over to metainfo by news-to-metainfo. Did you try? Do you consider it a bug?

@vchernin
Copy link

vchernin commented Jan 8, 2022

I'm able to produce bad metainfo using news-to-metainfo. Here's a demo with bad input, using a correct metainfo file from this repo.

NEWS:

Version 2.2
~~~~~~~~~~~~~~
Released: 2022-01-08

Features:
- Updated https://github.com/ximion/appstream

Bugfixes:
- Fixed bug https://github.com/ximion/appstream/issues/296

Run:

wget https://raw.githubusercontent.com/ximion/appstream/master/data/org.freedesktop.appstream.cli.metainfo.xml
appstreamcli news-to-metainfo NEWS org.freedesktop.appstream.cli.metainfo.xml
appstreamcli validate --pedantic --explain org.freedesktop.appstream.cli.metainfo.xml

Outputs:

W: org.freedesktop.appstream.cli:38: description-has-plaintext-url ul
   The description contains a web URL in plain text. This is not allowed, please use the <url/> tag
   instead to share links.

W: org.freedesktop.appstream.cli:34: description-has-plaintext-url ul
   The description contains a web URL in plain text. This is not allowed, please use the <url/> tag
   instead to share links.

Validation failed: warnings: 2

appstream-0.14.6-1.fc35.x86_64

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

No branches or pull requests

3 participants