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

Fix generation of app files #96

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cassidyjames
Copy link
Contributor

@cassidyjames cassidyjames commented Aug 28, 2024

It looks like removing the legacy files themselves was missed in #94, and they aren't being removed by CI. This should fix generation of app files (and thus app listing pages); when it runs in CI next, we should see a large amount of outdated files in _apps/ removed. This includes apps marked as EOL, as they are no longer present in the appstream downloaded from flatpak.elementary.io.

I did not add/commit the changes to the _apps/ files themselves since that will be handled by the hourly CI run, but I can if it makes it easier to review/understand the changes.

  • generate-flatpak: Apparently HTTPS works, now, so use it

    I was getting weird behavior locally when trying to use the HTTP URL as before, so out of curiosity I updated it to HTTPS—and it worked! Maybe something changed with the infrastructure configuration. 🤷🏻

  • Workflow: delete existing apps before generating new ones

    Apparently old app files were sticking around because we weren't deleting them before regenerating them. This updates the workflow to clear the folder out so it's always starting fresh. Updates to existing files should still come through the same way since CI checks a git diff after all operations are complete.

  • generate-flatpak: Remove outdated comments

    I forgot to remove the comment about HTTPS not working, and then realized since the script has been pared down to only deal with Flatpak, we probably don't need the giant FLATPAK comment do denote that section.

  • README: mention deleting existing files

    Hopefully makes it more clear what's being done in the CI (and prevents similar issues in the future).

  • generate-flatpak: allow desktop-application, strip translations

    This re-adds newer apps that were generated within the last month, stripping them of the (unused) translated strings so we don't end up accidentally using a translation in place of the default string.

  • Fix icons by using app ID

    I noticed we were putting the icons into the markdown files, but for Flatpaks, they're predictably at a well-known URL based on the app ID. So, skip all that and use the path directly. I also updated the fallback icon since the old rawgit link was dead.

@cassidyjames cassidyjames marked this pull request as draft August 28, 2024 20:00
@cassidyjames
Copy link
Contributor Author

cassidyjames commented Aug 28, 2024

Hm, I'm seeing a removal of legacy apps (ones built for old versions of elementary OS/as debs), but EOL are still showing up here after running locally. Investigating.

Ah, it just took some time for the appstream to update. I'm seeing the EOL apps start to fall off.

@cassidyjames cassidyjames marked this pull request as ready for review August 28, 2024 20:03
@cassidyjames
Copy link
Contributor Author

You can see the result of running rm _apps/* and then ruby generate-flatpak.rb in this branch: cassidyjames/appcenter-web@regenerate-apps...regenerate-apps-result

@danirabbit danirabbit requested a review from a team August 28, 2024 20:24
@cassidyjames
Copy link
Contributor Author

Ugh, part of the weirdness here is that generate-flatpak.rb is omitting metainfo from components that were generated more recently and have <component type="desktop-application"> instead of type="desktop". I can solve that, but previous tricks to get the non-translated/English tags don't work, because the order changed (they aren't guaranteed to be the first child any more).

It appears the above is fundamentally why getting new info for apps broke about a month ago: if there was an upgrade to how the MetaInfo is being generated in preparation of OS 8, the Ruby script here broke due to outdated assumptions.

@cassidyjames
Copy link
Contributor Author

cassidyjames commented Aug 28, 2024

Okay, as of the latest commit, this should be good to review. I stripped out the translations for now since we weren't using them, anyway and allowed components of type desktop-application. The results are finally looking correct. 🎉

cassidyjames/appcenter-web@regenerate-apps...regenerate-apps-result

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.

2 participants