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

libappstream regression: Flathub generates invalid remote icon entry #4222

Open
sophie-h opened this issue Jun 8, 2023 · 51 comments
Open

libappstream regression: Flathub generates invalid remote icon entry #4222

sophie-h opened this issue Jun 8, 2023 · 51 comments
Labels
appstream Issues related to appstream/appstream-compose, docs

Comments

@sophie-h
Copy link

sophie-h commented Jun 8, 2023

Bold claim in the issue title, but generating apps.gnome.org is broken since Jun 6, 2023 6:16am GMT+0200. The reason is an unexpected value in type="remote" for icons.

remote icons loaded from a remote URL. Currently, only HTTP/HTTPS urls are supported. This icon type should have width and height properties.

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

So rust appstream refuses the app/drey/EarTag/... value in the example below:

    <icon height="48" type="cached" width="48">app.drey.EarTag.png</icon>
    <icon height="64" type="cached" width="64">app.drey.EarTag.png</icon>
    <icon height="128" type="remote" width="128">app/drey/EarTag/48e5a929d7aad9d76df6deeb49cbf86f/icons/128x128/app.drey.EarTag.png</icon>
    <icon height="128" type="cached" width="128">app.drey.EarTag.png</icon>
    <icon type="stock">app.drey.EarTag</icon>
@razzeee
Copy link
Member

razzeee commented Jun 8, 2023

They are relative now, there is a media base url (I'm actually not sure, it might get stripped right now) on one of the root elements

We handle this hardcoded on the website for now flathub-infra/website@f740030#diff-9985c8101c5c6fdd34d0bed35f279f01d429eee5f739521c86e114335c3e1fcbR64

@sophie-h
Copy link
Author

sophie-h commented Jun 8, 2023

I think it's missing in here. (The version number also seems sus.)

<components version="0.8" origin="flatpak">

@razzeee
Copy link
Member

razzeee commented Jun 8, 2023

Bart is aware, but there's a state holiday and he has a vacation day tomorrow

@mcrha
Copy link

mcrha commented Jun 12, 2023

gnome-software bug https://gitlab.gnome.org/GNOME/gnome-software/-/issues/2196

The paths look odd too, and they care not valid with added https://dl.flathub.org/ prefix to them (using wget).

The same applies also to <screenshots>.

@razzeee
Copy link
Member

razzeee commented Jun 12, 2023

The base path should be https://dl.flathub.org/media and not just https://dl.flathub.org

@mcrha
Copy link

mcrha commented Jun 12, 2023

I see. That explains why it did not work for me. It was only a guess from my side.

@bilelmoussaoui bilelmoussaoui added the appstream Issues related to appstream/appstream-compose, docs label Jun 18, 2023
@ximion
Copy link

ximion commented Sep 13, 2023

If flatpak-builder or whatever runs appstreamcli compose passes the --no-partial-urls flag, all URLs will be complete and the media-baseurl will not be used, for increase backwards compatibility. That could help here too!
(this was a feature that was previously not exposed on the command line, but is available with AppStream 0.16.3).

@bbhtt
Copy link
Contributor

bbhtt commented Feb 17, 2024

This should be solved by flathub/org.flatpak.Builder@3bacdbb

Everything should be relative to the mirror URL https://dl.flathub.org/media/

@sophie-h can you please retrigger a build, so that the catalogue data gets composed with libappstream now and check?

@sophie-h
Copy link
Author

Everything should be relative to the mirror URL https://dl.flathub.org/media/

Hm, why should they be relative if the linked patch adds --no-partial-urls?

@bbhtt
Copy link
Contributor

bbhtt commented Feb 17, 2024

I meant to say they will be complete URLs starting with https://dl.flathub.org/media/

the current app/drey/EarTag/48e5a929d7aad9d76df6deeb49cbf86f/icons/128x128/app.drey.EarTag.png will come after that eg https://dl.flathub.org/media/net/blumia/pineapple-pictures/5e52b2373bf419ddedd2458c1911429f/icons/128x128/net.blumia.pineapple-pictures.png

@sophie-h
Copy link
Author

Yes, seems to work. The only app with a relative URL is a leftover from the previous transition to libappstream

WARN apps_gnome_org::generate: URL parser error: relative URL without a base
 | <component type="desktop">
 |   <id>org.lyx.LyX</id>
 |   <translation type="gettext">LyX</translation>
 |   <name>LyX</name>
 |   <summary>The Document Processor</summary>
 |   <developer_name>LyX</developer_name>
 |  …

@Altonss
Copy link

Altonss commented Mar 25, 2024

I'm still encountering this issue with some flathub apps.

@bbhtt
Copy link
Contributor

bbhtt commented Mar 25, 2024

Where?

@Altonss
Copy link

Altonss commented Mar 25, 2024

Where?

In the "list" view, in Gnome Software (45.3), on apps like Kdenlive, Discord or Extension manager (on the detailed app page, the icon shows fine).
image

@bbhtt
Copy link
Contributor

bbhtt commented Mar 25, 2024

In the "list" view, in Gnome Software (45.3), on apps like Kdenlive, Discord or Extension manager (on the detailed app page, the icon shows fine).

Can you open a separate issue for this? This is unrelated to the issue here.

It seems libappstream is giving only 128px now, 64 px icons are gone from the server. flatpak can't download it anymore in the appstream cache.

ls app-info/media/com/discordapp/Discord.desktop/2c3067076b8497e454766b79db8589e6/icons/        
 128x128

The list view depends on a 64px icon. @ximion, any idea what to do here?

@ximion
Copy link

ximion commented Mar 25, 2024

The list view depends on a 64px icon. @ximion, any idea what to do here?

Do you apply any odd configuration to appstreamcli? What is its full command-line?

@bbhtt
Copy link
Contributor

bbhtt commented Mar 25, 2024

The list view depends on a 64px icon. @ximion, any idea what to do here?

Do you apply any odd configuration to appstreamcli? What is its full command-line?

https://github.com/flatpak/flatpak-builder/blob/8c036e00630e35423c03388aacc06cd00dda74ea/src/builder-manifest.c#L3052-L3086

appstreamcli compose --no-partial-urls --prefix=/ --origin=com.discordapp.Discord --media-baseurl=https://dl.flathub.org/media/ --media-dir=/home/bbhtt/Downloads/com.discordapp.Discord/.flatpak-builder/rofiles/rofiles-7yoATF/files/share/app-info/media --result-root=/home/bbhtt/Downloads/com.discordapp.Discord/.flatpak-builder/rofiles/rofiles-7yoATF/files --data-dir=/home/bbhtt/Downloads/com.discordapp.Discord/.flatpak-builder/rofiles/rofiles-7yoATF/files/share/app-info/xmls --icons-dir=/home/bbhtt/Downloads/com.discordapp.Discord/.flatpak-builder/rofiles/rofiles-7yoATF/files/share/app-info/icons/flatpak '--components=com.discordapp.Discord,com.discordapp.Discord.desktop' /home/bbhtt/Downloads/com.discordapp.Discord/.flatpak-builder/rofiles/rofiles-7yoATF/files

@ximion
Copy link

ximion commented Mar 25, 2024

Okay, that command-line looks good. A 64x64px icon should always be generated unconditionally, if it isn't there, compose should fail. So I guess the next thing would be to examine the output of that command (not sure how I could test that, but maybe I could have a look later, maybe tomorrow).

@bbhtt
Copy link
Contributor

bbhtt commented Mar 25, 2024

Okay, that command-line looks good. A 64x64px icon should always be generated unconditionally, if it isn't there, compose should fail. So I guess the next thing would be to examine the output of that command (not sure how I could test that, but maybe I could have a look later, maybe tomorrow).

You can manually run the compose step. This should be roughly equivalent.

diff --git a/com.discordapp.Discord.json b/com.discordapp.Discord.json
index 6c9dd79..bbada2d 100644
--- a/com.discordapp.Discord.json
+++ b/com.discordapp.Discord.json
@@ -7,6 +7,7 @@
     "sdk": "org.freedesktop.Sdk",
     "command": "com.discordapp.Discord",
     "separate-locales": false,
+    "appstream-compose": false,
     "tags": [
         "proprietary"
     ],
@@ -62,9 +63,12 @@
                 "install -d /app/share/applications",
                 "sed -e 's/Icon=discord/Icon=com.discordapp.Discord/' -e 's|Exec=/usr/share/discord/Discord|Exec=com.discordapp.Discord|' /app/discord/discord.desktop > /app/share/applications/${FLATPAK_ID}.desktop",
                 "install -Dm644 /app/discord/discord.png /app/share/icons/hicolor/256x256/apps/${FLATPAK_ID}.png",
-                "install -Dm644 com.discordapp.Discord.appdata.xml /app/share/appdata/com.discordapp.Discord.appdata.xml",
+                "install -Dm644 com.discordapp.Discord.appdata.xml /app/share/metainfo/com.discordapp.Discord.appdata.xml",
                 "patch-desktop-filename ${FLATPAK_DEST}/discord/resources/app.asar"
             ],
+            "post-install": [
+                "appstreamcli compose --verbose --no-partial-urls --prefix=/ --origin=${FLATPAK_ID} --media-baseurl=https://dl.flathub.org/media/ --media-dir=${FLATPAK_DEST}/share/app-info/media --result-root=${FLATPAK_DEST} --data-dir=${FLATPAK_DEST}/share/app-info/xmls --icons-dir=${FLATPAK_DEST}/share/app-info/icons/flatpak --components=${FLATPAK_ID}.desktop ${FLATPAK_DEST}"
+            ],
             "sources": [
                 {
                     "type": "archive",

It seems to be generated but not put into app-info/media/com/discordapp/Discord.desktop/<hash>/icons/ and doesn't have a remote entry like 128px. Is 64px supposed to be cached only?

@ximion
Copy link

ximion commented Mar 25, 2024

It seems to be generated but not put into app-info/media/com/discordapp/Discord.desktop/<hash>/icons/ like 128px. Is 64px supposed to be cached only?

That is expected, I think - only stuff that is to be served from a remote location ends up in media.
The 64x64px icon is supposed to be cache-only, the AppStream spec mandates that this size must always be available, so serving it as remote icon makes no sense if we will always have it on disk.

@bbhtt
Copy link
Contributor

bbhtt commented Mar 25, 2024

So one issues is, appstream is generating the cached icon entry as <icon type="cached" width="64" height="64">com.discordapp.Discord.desktop.png</icon> with a .desktop suffix but flatpak is committing them with the $FLATPAK_ID instead com.discordapp.Discord in the appstream ref

https://dl.flathub.org/repo/appstream/x86_64/icons/64x64/com.discordapp.Discord.png is there but https://dl.flathub.org/repo/appstream/x86_64/icons/64x64/com.discordapp.Discord.desktop.png is not.

This seems to affect applications with appstream id ending in .desktop but FLATPAK_ID not ending in .desktop eg. Loupe, Keepassxc, Discord etc.

I'm not sure why flatpak's appstream cache is missing com.discordapp.Discord.png even though it is on the server.

@bbhtt
Copy link
Contributor

bbhtt commented Mar 25, 2024

So seems like it is hitting https://github.com/flatpak/flatpak/blob/f9cbfe1fd67b608b0df1f3f1c333e2b972fda225/common/flatpak-utils.c#L5311-L5312 then https://github.com/flatpak/flatpak/blob/f9cbfe1fd67b608b0df1f3f1c333e2b972fda225/common/flatpak-utils.c#L5209-L5210 because that's what I locally get when generating the appstream ref:

F: No icon at size 64x64 for com.discordapp.Discord
F: No icon at size 128x128 for com.discordapp.Discord

I guess Flathub has it in some kind of cache but flatpak update downloads the latest commit of the appstream ref which doesn't have it.

I guess appstream-glib ignored the ID and generated icons with $FLATPAK_ID and everything used to work.

@bbhtt
Copy link
Contributor

bbhtt commented Mar 25, 2024

This seems hard to solve, maybe applications can change the id tag in metainfo file and add the replaces stuff and everything should be back to normal?

There is an apparent disconnection between $FLATPAK_ID and <id> which breaks things.

@ximion
Copy link

ximion commented Mar 25, 2024

Yeah, compose will always use the current component-ID for the icon name, verbatim without changing it. We had some issues in the past when compose tried to be a bit too clever with it, so just using the name was the result :-)

Why doesn't the builder tool enforce that $FLATPAK_ID == appstream-component-ID?

@bbhtt
Copy link
Contributor

bbhtt commented Mar 25, 2024

Why doesn't the builder tool enforce that $FLATPAK_ID == appstream-component-ID?

Seems for compat flatpak/flatpak-builder@7292b2d

bbhtt added a commit to flathub/com.discordapp.Discord that referenced this issue Mar 26, 2024
Appstreamcli creates icons following the component id instead of
$FLATPAK_ID but Flatpak ignores it because it doesn't match $FLATPAK_ID
when copying the icons to appstream ref. So the ref is missing 64px
and 128px icons entirely for the app.

This makes GNOME software show a blank icon in list view because it
looks up icons from flatpak's appstream cache.

Make both match and link older IDs with replaces and provides.

See flathub/flathub#4222 (comment)
@bbhtt
Copy link
Contributor

bbhtt commented Mar 26, 2024

This is the list of affected apps:

Apps
app.lith.Lith
ar.com.softwareperonista.Rockarrolla
art.taunoerik.tauno-monitor
br.app.pw3270.terminal
ca._0ldsk00l.Nestopia
ca.desrt.dconf-editor
cc.arduino.arduinoide
com.albiononline.AlbionOnline
com.anydesk.Anydesk
com.bixense.PasswordCalculator
com.bladecoder.adventure-editor
com.chatterino.chatterino
com.diffingo.fwbackups
com.discordapp.Discord
com.dosbox.DOSBox
com.doycho.euterpe.gtk
com.dropbox.Client
com.ekonomikas.merkato
com.elsevier.MendeleyDesktop
com.endlessm.photos
com.frac_tion.teleport
com.github.amikha1lov.RecApp
com.github.appadeia.Taigo
com.github.bajoja.indicator-kdeconnect
com.github.birros.WebArchives
com.github.bjaraujo.Bombermaaan
com.github.dariasteam.cowsrevenge
com.github.donadigo.appeditor
com.github.elth0r0.iqpuzzle
com.github.fabiocolacio.marker
com.github.hluk.copyq
com.github.huluti.Coulr
com.github.inercia.k3x
com.github.JannikHv.Gydl
com.github.kmwallio.thiefmd
com.github.liferooter.textpieces
com.github.maoschanz.DynamicWallpaperEditor
com.github.miguelmota.Cointop
com.github.mikacousin.olc
com.github.paolostivanin.OTPClient
com.github.quaternion
com.github.theironrobin.siglo
com.github.wwmm.easyeffects
com.github.wwmm.pulseeffects
com.github.Xenoveritas.abuse
com.gitlab.bitseater.meteo
com.gitlab.coringao.JAG
com.google.AndroidStudio
com.googleplaymusicdesktopplayer.GPMDP
com.hack_computer.Clubhouse
com.hack_computer.OperatingSystemApp
com.hack_computer.Sidetrack
com.jagex.RuneScape
com.kavilgroup.gelectrical
com.kavilgroup.gestimator
com.leinardi.gkraken
com.leinardi.gst
com.leinardi.gwe
com.leinardi.gx52
com.lettier.movie-monad
com.mattjakeman.ExtensionManager
com.ozmartians.VidCutter
com.realm667.Wolfenstein_Blade_of_Agony
com.rosegardenmusic.rosegarden
com.skype.Client
com.sourcepole.kadas
com.teeworlds.Teeworlds
com.toggl.TogglDesktop
com.ultimaker.cura
com.uploadedlobster.peek
com.viber.Viber
com.vixalien.decibels
com.wings3d.WINGS
com.zquestclassic.ZQuest
de.gunibert.Hackgregator
de.philippun1.Snoop
dev.jamiethalacker.window_painter
de.wwwtech.ColorMate
de.zwarf.picplanner
edu.stanford.Almond
eu.ad5001.LogarithmPlotter
id.sideka.App
im.gitter.Gitter
info.bibletime.BibleTime
info.olasagasti.revelation
io.atom.Atom
io.botfather.Botfather
io.exodus.Exodus
io.github.aerocyber.sitemarker
io.github.amit9838.mousam
io.github.betaflight.BetaflightConfigurator
io.github.dida_code.OpstaKultura
io.github.diegoivanme.flowtime
io.github.diegoivan.pdf_metadata_editor
io.github.diegopvlk.Dosage
io.github.dyegoaurelio.simple-wireplumber-gui
io.github.FailurePoint.RandomNumberFive
io.github.fizzyizzy05.binary
io.github.flattool.Warehouse
io.github.hakandundar34coding.mini-system-monitor
io.github.hakandundar34coding.system-monitoring-center
io.github.ImEditor
io.github.leonardschardijn.Chirurgien
io.github.limads.Queries
io.github.lxndr.gswatcher
io.github.martinrotter.textosaurus
io.github.mki1967.mki3dgame
io.github.mmstick.FontFinder
io.github.nate_xyz.Chromatic
io.github.nate_xyz.Conjure
io.github.nate_xyz.Paleta
io.github.nate_xyz.Resonance
io.github.NhekoReborn.Nheko
io.github.nokse22.asciidraw
io.github.nokse22.inspector
io.github.nokse22.minitext
io.github.nokse22.teleprompter
io.github.nokse22.trivia-quiz
io.github.nokse22.ultimate-tic-tac-toe
io.github.orontee.Argos
io.github.phastmike.tags
io.github.qwersyk.Newelle
io.github.Rirusha.Cassette
io.github.RodZill4.Material-Maker
io.github.thiefmd.themegenerator
io.github.unrud.RecentFilter
io.gitlab.celleron56.libretile
io.gitlab.gregorni.Calligraphy
io.gitlab.gregorni.Letterpress
io.gitlab.gwendalj.package-transporter
io.gitlab.Turtlico
io.liri.Calculator
io.mgba.mGBA
io.otsaloma.gaupol
io.otsaloma.nfoview
it.mijorus.collector
it.mijorus.smile
it.mijorus.whisper
me.hergert.Schemes
me.iepure.devtoolbox
me.sanchezrodriguez.passes
net.bartkessels.getit
net.danigm.loop
net.lugsole.bible_gui
net.mediaarea.AVIMetaEdit
net.mediaarea.DVAnalyzer
net.mediaarea.MediaConch
net.mediaarea.MOVMetaEdit
net.mediaarea.QCTools
net.meijn.onvifviewer
net.oz9aec.Gpredict
net.redeclipse.RedEclipse
net.runelite.RuneLite
net.sf.fuse_emulator
net.sourceforge.Chessx
net.sourceforge.DuneLegacy
net.sourceforge.electrip.Electrip
net.sourceforge.Fillets
net.sourceforge.projectM
net.sourceforge.Teo
net.veloren.veloren
org.adishatz.syncpasswd
org.apache.netbeans
org.chrisoft.qmidiplayer
org.davmail.DavMail
org.develz.Crawl
org.eclipse.iot.fourdiac.Ide
org.emilien.Password
org.emilien.SpaceLaunch
org.fedoraproject.MediaWriter
org.freefilesync.FreeFileSync
org.fritzing.Fritzing
org.gabmus.unifydmin
org.gahshomar.Gahshomar
org.geogebra.GeoGebra
org.gnode.NixView
org.gnome.Boxes
org.gnome.Builder
org.gnome.Calculator
org.gnome.Calendar
org.gnome.Characters
org.gnome.Cheese
org.gnome.ColorViewer
org.gnome.Connections
org.gnome.Contacts
org.gnome.Devhelp
org.gnome.dspy
org.gnome.eog
org.gnome.Epiphany
org.gnome.Evolution
org.gnome.frogr
org.gnome.gbrainy
org.gnome.Geary
org.gnome.gedit
org.gnome.gitlab.bazylevnik0.Convolution
org.gnome.gitlab.Cowsay
org.gnome.gitlab.johannesjh.favagtk
org.gnome.gThumb
org.gnome.HexGL
org.gnome.Loupe
org.gnome.meld
org.gnome.moserial
org.gnome.Music
org.gnome.NetworkDisplays
org.gnome.Nibbles
org.gnome.Notes
org.gnome.OfficeRunner
org.gnome.Polari
org.gnome.PowerStats
org.gnome.Recipes
org.gnome.Rhythmbox3
org.gnome.Shotwell
org.gnome.SoundJuicer
org.gnome.SoundRecorder
org.gnome.SwellFoop
org.gnome.TextEditor
org.gnome.Totem
org.godotengine.Godot
org.kde.akregator
org.kde.ark
org.kde.artikulate
org.kde.blinken
org.kde.bomber
org.kde.bovo
org.kde.cantor
org.kde.choqok
org.kde.digikam
org.kde.dolphin
org.kde.elisa
org.kde.falkon
org.kde.filelight
org.kde.gcompris
org.kde.granatier
org.kde.gwenview
org.kde.index
org.kde.isoimagewriter
org.kde.juk
org.kde.kaffeine
org.kde.kalzium
org.kde.kamoso
org.kde.kanagram
org.kde.kapman
org.kde.katomic
org.kde.kbibtex
org.kde.kblackbox
org.kde.kblocks
org.kde.kbounce
org.kde.kbreakout
org.kde.kbruch
org.kde.kcachegrind
org.kde.kcalc
org.kde.kcolorchooser
org.kde.kdenlive
org.kde.kdevelop
org.kde.kdiamond
org.kde.kdiff3
org.kde.kfind
org.kde.kfourinline
org.kde.kgeography
org.kde.kgoldrunner
org.kde.kgraphviewer
org.kde.khangman
org.kde.kid3
org.kde.kig
org.kde.kigo
org.kde.kimagemapeditor
org.kde.kiriki
org.kde.kiten
org.kde.kjumpingcube
org.kde.kleopatra
org.kde.klettres
org.kde.klickety
org.kde.kmahjongg
org.kde.kmines
org.kde.kmplot
org.kde.kmymoney
org.kde.knavalbattle
org.kde.knetwalk
org.kde.knights
org.kde.koko
org.kde.kolf
org.kde.kollision
org.kde.kolourpaint
org.kde.kommit
org.kde.kompare
org.kde.kongress
org.kde.konquest
org.kde.konsole
org.kde.kontact
org.kde.konversation
org.kde.krdc
org.kde.krename
org.kde.kreversi
org.kde.kronometer
org.kde.kruler
org.kde.ksirk
org.kde.ksnakeduel
org.kde.ksquares
org.kde.kstars
org.kde.ksudoku
org.kde.kteatime
org.kde.ktimetracker
org.kde.ktorrent
org.kde.ktouch
org.kde.ktuberling
org.kde.kturtle
org.kde.kubrick
org.kde.kwordquiz
org.kde.kwrite
org.kde.kxstitch
org.kde.labplot2
org.kde.lokalize
org.kde.lskat
org.kde.marble
org.kde.massif-visualizer
org.kde.minuet
org.kde.okteta
org.kde.okular
org.kde.palapeli
org.kde.parley
org.kde.peruse
org.kde.picmi
org.kde.pix
org.kde.rocs
org.kde.ruqola
org.kde.skanpage
org.kde.SymbolEditor
org.kde.tellico
org.kde.umbrello
org.kde.vvave
org.kde.yakuake
org.keepassxc.KeePassXC
org.KekikAkademi.BTKSorgu
org.kekikakademi.eFatura
org.kekikakademi.ntvHaber
org.libreoffice.LibreOffice
org.libsdl.Maelstrom
org.mapeditor.Tiled
org.neverball.Neverball
org.nickvision.cavalier
org.octave.Octave
org.openclonk.OpenClonk
org.openfodder.OpenFodder
org.openmw.OpenMW
org.openttd.OpenTTD
org.parlatype.Parlatype
org.processing.processingide
org.pyzo.pyzo
org.qgis.qgis
org.quassel_irc.QuasselClient
org.quetoo.Quetoo
org.raceintospace.Raceintospace
org.remmina.Remmina
org.sauerbraten.Sauerbraten
org.shadered.SHADERed
org.solarus_games.solarus.Launcher
org.sparkleshare.SparkleShare
org.tabos.maxcontrol
org.tabos.saldo
org.thonny.Thonny
org.vim.Vim
org.worldpossible.ScriptLauncher
org.xonotic.Xonotic
page.codeberg.Imaginer.Imaginer
pm.mirko.Atoms
re.sonny.Retro
se.manyver.Manyverse
us.zoom.Zoom
work.openpaper.Paperwork
ws.openarena.OpenArena
xyz.slothlife.Jogger

@mcrha
Copy link

mcrha commented Mar 26, 2024

This is the list of affected apps:

which is 370, +/- 1.

@barthalion barthalion reopened this Mar 26, 2024
@ximion
Copy link

ximion commented Mar 26, 2024

which is 370, +/- 1.

Brr... That's an amount that would definitely warrant some kind of workaround, fixing that for every app would be a significant effort :-/

All of this really should be a different bug though, the issue here is actually something completely different that has been fixed already.

@hfiguiere
Copy link
Contributor

you should know that NOW flatpak-builder does fix the app-id on rename-appdata-file or rename-desktop-file. It only did it for the latter if it didn't match the desktop-file.

@ximion
Copy link

ximion commented Mar 26, 2024

Renaming the component-ID is pretty evil - but if we could do it only once to get rid of this discrepancy, I'd be all for it!
It just feels like that with this many affected apps, unless there is a way to automate it and ensure nothing breaks, we can't really do this and need some automated workaround...

@hfiguiere
Copy link
Contributor

As I mentionned in a different channel, there were 2022 blog post reviewed by people in the know that still used the .desktop in app-id. And the fact that the templates used that, and that flathub explicitely allowed it didn't help.

The reason I implemented the rename in flatpak-builder is because otherwise it's very painfull to fix it as there is not tooling to "edit" the appstream file. (appstream-glib has a "modify" that would work for this, but it's being phased out).

@ximion
Copy link

ximion commented Mar 26, 2024

I'd be fine with adding a feature to edit MetaInfo files to appstreamcli, but if it can be avoided not for component-IDs... Changing them is an antifeature, as so much stuff depends on them. I could see a feature to change the ID and place the previous ID in a <replaces/> block though, that's a supported pattern. And I'd rather do that than injecting component-ID heuristics into the compose process, if it helps.

But is there anything that speaks against the solution outlined at the very bottom of #4222 (comment)? Does Flatpak maybe require the icon name to match the FLATPAK_ID? (that would require us to search for a new solution - I do have some more ideas then though ^^).

@TingPing
Copy link
Member

Does Flatpak maybe require the icon name to match the FLATPAK_ID?

Icons can match $FLATPAK_ID or $FLATPAK_ID.*. So $FLATPAK_ID.desktop.png is totally valid.

@ximion
Copy link

ximion commented Mar 26, 2024

Icons can match $FLATPAK_ID or $FLATPAK_ID.*. So $FLATPAK_ID.desktop.png is totally valid.

In that case, and if I understand the problem correctly, this may actually be pretty trivial to resolve :-)

@bbhtt
Copy link
Contributor

bbhtt commented Mar 27, 2024

What is stopping from dropping this line https://github.com/flatpak/flatpak/blob/f1088e3013adc5010012e16d060178b0f1d48226/common/flatpak-utils.c#L5311-L5312 to fix the issue? I don't think it regresses anything and will fix the issue here.

This was introduced for an even rarer appstream ID ie. two .desktop-s com.telegram.desktop.desktop flatpak/flatpak@7dd92d8 Telegram has since then changed the appstream ID at some point and now it is only com.telegram.desktop.

In case of two .desktops-s appstreamcli produces the icons as com.telegram.desktop.desktop.png which would again hit the same issue here as flatpak will fail to copy the icon, but it is already so very rare. I have yet to see a two .desktop component ID in the appstream data.

I think we can add a check in linter to make sure no such double desktop IDs go through on Flathub (or go a step even further and start requiring appstream-cid == $FLATPAK_ID after creating exceptions for all of the above to block new submissions getting in with the same mess - this is off topic though). This would save everyone from adding anymore workarounds.

@ximion
Copy link

ximion commented Mar 27, 2024

What is stopping from dropping this line https://github.com/flatpak/flatpak/blob/f1088e3013adc5010012e16d060178b0f1d48226/common/flatpak-utils.c#L5311-L5312 to fix the issue? I don't think it regresses anything and will fix the issue here.

I think this would work, and would be a fairly easy solution. I just checked the 2792 components in Debian's repository, none has a .desktop.desktop suffix. I think even if such a case would appear, it would be much easier to fix that one case than to adapt everything else instead.

I think we can add a check in linter to make sure no such double desktop IDs go through on Flathub (or go a step even further and start requiring appstream-cid == $FLATPAK_ID after creating exceptions for all of the above to block new submissions getting in with the same mess - this is off topic though). This would save everyone from adding anymore workarounds.

That also sounds like a really good idea :-) In combination, this should avoid the same issue in future, while also not introducing too much (or any!) breakage.

bbhtt added a commit to flathub/org.flatpak.Builder that referenced this issue Mar 27, 2024
bbhtt added a commit to flathub/org.flatpak.Builder that referenced this issue Mar 27, 2024
bbhtt added a commit to bbhtt/flatpak that referenced this issue Mar 27, 2024
This will pass the exact appstream component ID to copy_icon

This was introduced in flatpak@7dd92d8
to handle appstream component IDs that ended in two `.desktop` suffixes.
Recent analysis of appstream data shows that on Flathub no such
appstream cid exists anymore and Telegram has component ID
`com.telegram.desktop` now.

With the switch to libappstream, appstreamcli-compose produces icons in
`share/app-info/flatpak` named by the appstream component ID instead of
the `$FLATPAK_ID` used by appstream-glib. This causes applications whose
`$FLATPAK_ID` does not end with `.desktop` but their appstream-component
ID ends in `.desktop` ie. `$FLATPAK_ID != appstream-cid` to loose icons
from the appstream ostree ref as `copy_icon` was being fed the id
without `.desktop` but icons were created by appstreamcli
with `.desktop` in them.

This will avoid adding anymore ID heuristics/workarounds on either side,
per the discussion in flathub/flathub#4222

An application with `$FLATPAK_ID` `com.telegram.desktop` and appstream ID
`com.telegram.desktop.desktop` will be broken with this change but such
dual `.desktop` IDs are non existent and should be fixed individually
or be blocked on an app store level.
bbhtt added a commit to bbhtt/flatpak-builder-lint that referenced this issue Mar 27, 2024
For historical reasons flatpak-builder passes ``$FLATPAK_ID.desktop`
as a valid component ID to appstreamcli. This mismatch creates issues
like the one described in flatpak/flatpak#5752
and flathub/flathub#4222

So require that both match and avoid new submissions leaking in with a
mismatch.
bbhtt added a commit to bbhtt/flatpak-builder-lint that referenced this issue Mar 27, 2024
For historical reasons flatpak-builder passes ``$FLATPAK_ID.desktop`
as a valid component ID to appstreamcli. This mismatch creates issues
like the one described in flatpak/flatpak#5752
and flathub/flathub#4222

So require that both match and avoid new submissions leaking in with a
mismatch.
@bbhtt
Copy link
Contributor

bbhtt commented Mar 27, 2024

PRs opened, the flatpak PR has the repro steps. I'll wait for others to chime in.

I won't deploy anything to Flathub, until bart returns from vacation.

@ximion
Copy link

ximion commented Mar 27, 2024

Thanks a lot! :-) Having just one ID for an app will help us greatly in future, I believe.

bbhtt added a commit to bbhtt/flatpak-builder-lint that referenced this issue Mar 28, 2024
For historical reasons flatpak-builder passes ``$FLATPAK_ID.desktop`
as a valid component ID to appstreamcli. This mismatch creates issues
like the one described in flatpak/flatpak#5752
and flathub/flathub#4222

So require that both match and avoid new submissions leaking in with a
mismatch.
barthalion pushed a commit to flathub/org.flatpak.Builder that referenced this issue Mar 28, 2024
bbhtt added a commit to bbhtt/flatpak-builder-lint that referenced this issue Mar 31, 2024
For historical reasons flatpak-builder passes ``$FLATPAK_ID.desktop`
as a valid component ID to appstreamcli. This mismatch creates issues
like the one described in flatpak/flatpak#5752
and flathub/flathub#4222

So require that both match and avoid new submissions leaking in with a
mismatch.
bbhtt added a commit to bbhtt/flatpak-builder-lint that referenced this issue Mar 31, 2024
For historical reasons flatpak-builder passes ``$FLATPAK_ID.desktop`
as a valid component ID to appstreamcli. This mismatch creates issues
like the one described in flatpak/flatpak#5752
and flathub/flathub#4222

So require that both match and avoid new submissions leaking in with a
mismatch.
bbhtt added a commit to bbhtt/flatpak-builder-lint that referenced this issue Apr 5, 2024
For historical reasons flatpak-builder passes ``$FLATPAK_ID.desktop`
as a valid component ID to appstreamcli. This mismatch creates issues
like the one described in flatpak/flatpak#5752
and flathub/flathub#4222

So require that both match and avoid new submissions leaking in with a
mismatch.
bbhtt added a commit to flathub-infra/flatpak-builder-lint that referenced this issue Apr 5, 2024
For historical reasons flatpak-builder passes ``$FLATPAK_ID.desktop`
as a valid component ID to appstreamcli. This mismatch creates issues
like the one described in flatpak/flatpak#5752
and flathub/flathub#4222

So require that both match and avoid new submissions leaking in with a
mismatch.
@bbhtt
Copy link
Contributor

bbhtt commented Apr 16, 2024

Flathub should no longer accept id tags with mismatches, changes have been deployed with exceptions for all existing apps and apps marked as ready in the submission queue.

What remains is getting one of the Flatpak patches merged, getting it backported to Flatpak 1.14.x, getting a new release and getting a PPA build - then it can be deployed to the Flathub servers running Ubuntu.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appstream Issues related to appstream/appstream-compose, docs
Projects
None yet
Development

No branches or pull requests