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

Update org.gnome.Hamster.GUI.metainfo.xml #690

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rabin-io
Copy link

@rabin-io rabin-io commented Nov 17, 2021

Add required missing tags

flatpak run --env=G_DEBUG=fatal-criticals org.freedesktop.appstream-glib validate builddir/*/share/appdata/org.gnome.Hamster.appdata.xml
 in dir /srv/buildbot/worker/build-x86_64-6/build (timeout 1200 secs)
 watching logfiles {}
 argv: b'flatpak run --env=G_DEBUG=fatal-criticals org.freedesktop.appstream-glib validate builddir/*/share/appdata/org.gnome.Hamster.appdata.xml'
 using PTY: False
builddir/files/share/appdata/org.gnome.Hamster.appdata.xml: FAILED:
• tag-missing           : <content_rating> required [use https://odrs.gnome.org/oars]
• file-invalid          : <screenshot> failed to load [https://github.com/projecthamster/hamster/blob/master/screenshot.png]
• file-invalid          : <screenshot> failed to load [https://github.com/projecthamster/hamster/blob/master/data/screenshots/overview2.png]
• tag-missing           : <release> required
Validation of files failed
program finished with exit code 1
elapsedTime=0.237518

@matthijskooijman
Copy link
Member

Looks good to make the XML complete.

AFAIU the content_rating you added just means "no offensive content" (empty XML tag - any noteworthy content would have been added as a subtag), which seems good.

As for adding the "release" tag, I'm not sure that makes sense like this - setting the release to 3.0.2 like you did makes sense for the commit tagged 3.0.2, but not for the master branch in general - if you build a flatpak from master, it should IMHO not advertise itself as a release.

Ideally, I think we would omit the release tag from the XML file in git, and add it (either by editing the file, or better, pass some option to the flatpak build if that is possible) automatically in the github workflow that makes the flatpack build. Then for PR's and regular pushes to master, the release could be set to some string that includes the git hash perhaps, and for tagged releases it could use the version from the tag.

Do you know if such an approach is common for flatpak builds? Also, do you have any suggestions as to the format of the version number? Does flatpak need any particular format? Is it helpful if git builds and published releases are correctly sortable (i.e. by using 3.0.2+git-<hash> or something like that?

Also, I saw you published a flatpak build of hamster on flathub, thanks! Is the XML file in our repository relevant for that (i.e. do you need this change for the flathub build), or does the flathub build use its own copy of the XML file?

@GeraldJansen
Copy link
Contributor

The output of git describe --tags --always --dirty=+ would be a good candidate for a version number. That is used by hamster itself when running uninstalled, for example:

v3.0.2-42-gb2aff82a (uninstalled)

which shows this is 42 commits ahead of the v3.0.2 tag and the current commit is gb2aff82a.

@rabin-io
Copy link
Author

Hi @matthijskooijman

I agree, it doesn't make sense to have it hard coded like this, I will look to see how to change it to cosume a template base file to produce the final file which consumed by the builder.

@matthijskooijman
Copy link
Member

Cool, thanks.

While reading #608, I realized that the python code does currently hardcode the version number as well in two places (which is, I think, also set to 3.0.2 in git master now), so your PR is in line with that, but I'd prefer to replace those version numbers with auto-detected versions based on git as well, so I'd be happy if you could investigate how to do this for the flatpak build as well.

@matthijskooijman
Copy link
Member

I had another look at flatpak versioning, and was a bit confused.

The appstream specs (that define the .metainfo.xml format) seem to document the <releases> tag as an informative element containing a list of releases that exist, with optional changelog info and download links (artifacts). Nothing in that spec seems to suggest any way to select one of those releases as the current release/version.

I could not find any documentation at flatpak about how to define the flatpak application version, literally nothing. However, this post suggests that the version is taken from the <releases> element, using the "latest" (probably topmost?) release. I've tried this and indeed, this seems to happen (but note that the name is now org.gnome.Hamster.GUI.metainfo.xml, while flatpak needs org.gnome.Hamster.metainfo.xml with the contained id also updated to match otherwise it will not read the file at all - also see #725 (comment) for more discussion about these different ids).

So, this <releases> element seems like it could be used to store a full release history with release notes and fixed issues in a machine readable format, which could be interesting but I think it is too cumbersome to maintain (XML is not nice for hand-editing, and we already have a NEWS file which I do not care to duplicate). So, I think we should be using the <releases> element to keep a history of releases.

So we should then probably not store any releases in there, except adding one release entry when generating a flatpak (or maybe more generally, when installing the file) containing the current version. As mentioned before, I'd rather have this version auto-added (I'm slowly composing a plan on how to do versioning, but I'll continue discussion about that in #608).

For this PR, I think it means we should drop the <releases> tag from it, leaving just the content rating. Note that the invalid screenshots mentioned in the original post have been fixed by #469 in the meantime.

This was referenced Nov 20, 2023
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.

3 participants