ximion / appstream-generator

A fast AppStream metadata generator
GNU Lesser General Public License v3.0
43 stars 29 forks source link

Icon tarballs only generated for first architecture #99

Closed pabloyoyoista closed 2 years ago

pabloyoyoista commented 2 years ago

Icon tarballs generated by appstream-generator do not contain an architecture-specific suffix, which suggests that they include icons for all the architectures. However, when trying to get the generator working for a distro, I realized that the log lines:

INFO: Creating icon tarball.
INFO: Icon tarball(s) built.

only appear once per suite while exporting data for the first architecture that gets process.

Looking at the code, cached icon tarballs are only generated for the first architecture that calls exportData because both in processSuiteSection and in publishMetadataForSuiteSection, the boolean that controls the icon tarball generator is swapped after the first pass. If my understanding of the code in exportData is correct, that will generate an icon tarball using only the icons in packages present in the said architecture. If other architectures have additional packages with icons, those will get missing. Is there a specific reason for this behavior? Am I missing something and what I described is not what the code is doing (first time looking at D, so I apologize if this is the case)?

ximion commented 2 years ago

The icon tarballs are built once per suite and contain icons for all architectures (as the data is arch-independent). So the code is correct, this is only supposed to run once per suite. If you don't have icons, that must have other reasons. What distribution are you building for? Are the icons extracted correctly on the client?

pabloyoyoista commented 2 years ago

The icon tarballs are built once per suite and contain icons for all architectures (as the data is arch-independent). So the code is correct, this is only supposed to run once per suite.

That makes total sense. However, then I guess there might be a bug, where the archive is generated at the beginning of the sequence instead of at the end (or maybe some other bug I'm not caching).

I am trying to improve the overall status of the appstream data in alpine. I have done this simplified experiment with which I am able to reproduce what I understand is a bug.

After this process, the icons tar archive is created, but empty. However, if I totally wipe all the repositories, swap the architectures section so it looks like: "architectures": ["aarch64", "x86_64"] and rerun the generator, then the cups_cups.png file is included in the icons archive.

Let me know if you would like me to share the logs for the two different runs. Also, I have not attempted this with any other backend different from "alpinelinux". Let me know if I should try with a different backend, although it might take a while until I figure out how to build repositories for other distros. I have only tested with v0.8.5, but could also retry with v0.8.6 once we get it updated in edge.

Thank you for your time and sorry maybe I should have started publishing this in the first place...

ximion commented 2 years ago

I'll have a look at this when I am no longer travelling (maybe next week) - does anything change if you use the --force option? Maybe a suite is skipped because it wasn't updated, and the other one was skipped because it had no components, and that's how you ended up with no icons. You can also try to drop the custom icon policy you have (the whole Icons block in your config) and see if that changes anything. It does sound like there is a bug here somewhere. 0.8.6 is quite a huge update with lots of changes, but no direct impact on the icon code. It's also worth a try though, even if only to confirm that the issue is still there...

pabloyoyoista commented 2 years ago

I'll have a look at this when I am no longer travelling (maybe next week)

Thanks a lot, it is not something terribly urgent. And sorry I cannot send a PR instead of opening an issue.

does anything change if you use the --force option?

Not really. This time I run with that flag and removing the icon policy and recorded the logs: aarch run first (icon present) and x86_64 run first (icon missing). The behavior is still the same, although this time there are tarballs for all the different icon sizes.

Maybe a suite is skipped because it wasn't updated, and the other one was skipped because it had no components, and that's how you ended up with no icons.

Looking at logs and the code it looks like the archive is generated for the first architecture, even before the other architectures are processed, and that is the reason for the icon missing in the reproducer when x86_64 was run first. But not sure how to properly fix it without creating an architecture-dependent archive.

0.8.6 is quite a huge update with lots of changes, but no direct impact on the icon code. It's also worth a try though, even if only to confirm that the issue is still there...

I will see if I can get it upgraded in alpine. I will post here if I rerun it with 0.8.6

ximion commented 2 years ago

@pabloyoyoista Can you test the patch / Git master version of asgen and see if that fixes the issue? I couldn't reproduce your initial problem, but the code was clearly wrong and this change highly likely has fixed your issue (and hopefully hasn't introduced any regressions ^^ - in my tests everything appears to run just as well).

pabloyoyoista commented 2 years ago

Thank you very much! I'll try it out next week and report back :)

pabloyoyoista commented 2 years ago

@ximion I can confirm that the commit you mentioned fixes the issue I was experimenting! However, to build latest master in alpine, I had to revert this commit. The compiler wasn't able to find "setCainfo" in Compose. I guess maybe it is not in a release yet or maybe it is and we haven't gotten it in Alpine yet.

In any case, thank you very much for fixing this issue!

ximion commented 2 years ago

The former :-) Once there's a release, that issue will be gone. Nice that it's working well now! :-)