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

Ensure the generated flathub:: values is compliant #101

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

aleixpol
Copy link
Contributor

@aleixpol aleixpol commented Feb 4, 2024

According to the spec, is not a thing. is the correct one.

https://freedesktop.org/software/appstream/docs/chap-Metadata.html#tag-custom

Fixes flathub/flathub#4917

@aleixpol
Copy link
Contributor Author

aleixpol commented Feb 5, 2024

@barthalion ping

@ximion
Copy link
Contributor

ximion commented Feb 6, 2024

Indeed, metadata was a "proprietary" GNOME extension in ancient times, has never been supported anywhere but in GNOME and isn't even supported there anymore.

Looks like we accidentally resurrected something from the past here.
So +1 from me for this patch :-)

@razzeee
Copy link
Member

razzeee commented Feb 6, 2024

Emitting both and patching/backporting this in gnome software asap is the only way out. Fortunately it seems quiet simple.

Just should have a followup cleanup issue for some when in the future.

@ximion
Copy link
Contributor

ximion commented Feb 6, 2024

Emitting both and patching/backporting this in gnome software asap is the only way out. Fortunately it seems quiet simple.

That isn't even needed to be fixed in GNOME Software! I just noticed it upgrades "metadata" to "custom" in all versions!
So we could likely even do without that fallback! See https://gitlab.gnome.org/GNOME/gnome-software/-/blob/main/plugins/flatpak/gs-flatpak.c?ref_type=heads#L766

Honestly I think I should do some refactoring in GS so this is less confusing. I think we may actually be able to completely ditch some of that deprecated stuff to make the code more readable.

@barthalion
Copy link
Member

Hmm, yeah, I guess I must have implemented it first with <custom>, then realized it's not what appstream-glib does. @razzeee we're going to have to switch backend and frontend to use the new field too at some point.

@barthalion barthalion merged commit f84f9af into flathub-infra:main Feb 6, 2024
1 of 2 checks passed
@ximion
Copy link
Contributor

ximion commented Feb 6, 2024

The good thing is that you don't need to keep the duplicated value, as GNOME Software and appstream-glib will read custom just fine in pretty much any version :-)
So once the website is fixed, this can be greatly simplified.

@razzeee
Copy link
Member

razzeee commented Feb 6, 2024

@razzeee razzeee mentioned this pull request Feb 6, 2024
@ximion
Copy link
Contributor

ximion commented Feb 7, 2024

Website is likely also fine https://github.com/flathub-infra/website/blob/main/backend/app/utils.py#L216

Hmm... A bit OT, but did you know that you could get appstream2dict relatively efficiently via AppStream directly?

import gi
import yaml

gi.require_version('AppStream', '1.0')
from gi.repository import AppStream

def appstream2dict(xml_fname) -> dict[str, dict]:
    mdata = AppStream.Metadata()
    mdata.set_locale('ALL')
    mdata.set_format_style(AppStream.FormatStyle.CATALOG)
    mdata.set_parse_flags(AppStream.ParseFlags.IGNORE_MEDIABASEURL)

    with lzma.open(xml_fname, 'r') as f:
        xml_catalog_data = str(f.read(), 'utf-8')

    mdata.clear_components()
    mdata.parse_data(xml_catalog_data, AppStream.FormatKind.YAML)

    return yaml.safe_load(mdata.components_to_catalog(AppStream.FormatKind.YAML))

You might not want that for various reasons, but it would be dramatically less code ^^

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.

Unreachable verified information
4 participants