<content_rating> handling

[jbrobst]

After running apt update today I got a fatal error from appstreamcli refresh-cache. I believe the cause is the new version of stellarium uploaded to Debian unstable yesterday, whose appdata XML contains a content_rating tag without any content_attribute children. It is unclear to me whether the appstream documentation forbids this:

The <content_rating/> must have <content_attribute/> children which each have an id property indicating the specific section that is rated.

If it is forbidden, I think "must have one or more <content_attribute/> children" would be clearer. The code does assume this is the case, as as_content_rating_to_variant calls g_variant_builder_end(&values_b), which is an error if the indefinite array variant values_b has not had anything added.

On the other hand, the user that submitted the PR to stellarium seems to have used the OARS generation tool linked by the documentation for content_rating, which does generate a childless content_rating tag if you answer "None" to all of the questions.

So, either appstream needs to be modified to accept a content_rating tag with no children or the OARS tool needs to change what it generates when all questions are answered with "None" (perhaps along with the clarification to the content_rating documentation above).

[ximion]

The specification actually indeed forbids this (a "must" is always binding) but it actually should not, in this case.
But even if this was a completely invalid XML files, the cache generation should never fail, at the absolute worst this respective metadata should just be dropped.

Therefore, this is a bug in both the implementation and the specification. I will work on a fix tomorrow.

[antoszka]

I'm running Kali Linux (which is pretty much Debian Testing with regards to this issue at least) and I'm still getting the error after manually upgrading appstreamcli to 0.12.2:

$ appstreamcli --version
AppStream version: 0.12.2
$ sudo appstreamcli refresh-cache

(appstreamcli:27992): GLib-CRITICAL **: 08:31:59.424: g_variant_builder_end: assertion '!GVSB(builder)->uniform_item_types || GVSB(builder)->prev_item_type != NULL || g_variant_type_is_definite (GVSB(builder)->type)' failed

(appstreamcli:27992): GLib-CRITICAL **: 08:31:59.425: g_variant_new_variant: assertion 'value != NULL' failed

(appstreamcli:27992): GLib-ERROR **: 08:31:59.425: g_variant_new_parsed: 11-13:invalid GVariant format string
Trace/breakpoint trap

[ximion]

@antoszka This is likely a new issue - can you help to debug it?
See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=906711#10 for details.

[jbrobst]

@ximion I think it might be that appstream 0.12.2-2 depends on libappstream4 >= 0.12.0, so some people who don't run a full sid and are just upgrading appstream only aren't automatically getting the new libappstream. This seems to be the case for Houmehr in the debian bug at least, since he still has libappstream4 0.12.2-1.

[ximion]

@jbrobst Good catch! I completely missed that - that could indeed be the reason why I can't reproduce the issue.
@antoszka can you please ensure libappstream is up to date on your system as well?

[antoszka]

Yes, certainly, I took care to upgrade both the client and the library:

~ dpkg -l | grep appstream
ii  appstream                                 0.12.2-1
ii  libappstream-glib8:amd64                  0.7.10-1
ii  libappstream4:amd64                       0.12.2-1
# redacted for brevity

Including the other information you requested:

~ appstreamcli status
AppStream Status:
Version: 0.12.2

Distribution metadata:
 /usr/share/app-info
  - XML:  1
  - No icons.

 /var/lib/app-info
  - YAML: 1
  - Iconsets:
     debian-sid-main

 /var/cache/app-info
  - Empty.

Metainfo files:
  - Found 80 components.
  - Found 7 components in legacy paths.

Summary:
We have information on 2230 software components.

~ find /var/lib/app-info/ /var/cache/app-info/ /usr/share/app-info/ -not -path "*/icons/*"
/var/lib/app-info/
/var/lib/app-info/icons
/var/lib/app-info/yaml
/var/lib/app-info/yaml/deb.debian.org_debian_dists_unstable_main_dep11_Components-amd64.yml.gz
/var/cache/app-info/
/var/cache/app-info/gv
/var/cache/app-info/gv/en_US.gvz
/usr/share/app-info/
/usr/share/app-info/xmls
/usr/share/app-info/xmls/org.gnome.Software.Featured.xml

[ximion]

@antoszka

Yes, certainly, I took care to upgrade both the client and the library

...and you still get the error when running appstreamcli refresh --force?
Your system state looks as standard as it gets... I'll try later today again to reproduce the issue.

[antoszka]

Yep:

~ sudo appstreamcli refresh-cache --force

(appstreamcli:5523): GLib-CRITICAL **: 18:12:41.572: g_variant_builder_end: assertion '!GVSB(builder)->uniform_item_types || GVSB(builder)->prev_item_type != NULL || g_variant_type_is_definite (GVSB(builder)->type)' failed

(appstreamcli:5523): GLib-CRITICAL **: 18:12:41.572: g_variant_new_variant: assertion 'value != NULL' failed

(appstreamcli:5523): GLib-ERROR **: 18:12:41.572: g_variant_new_parsed: 11-13:invalid GVariant format string
Trace/breakpoint trap

[antoszka]

Same thing with refresh as with refresh-cache, sorry about the noise, didn't notice the difference at first :)

[ximion]

@antoszka You don't happen to be able to either compile AppStream from source or get the libas/ascli debug symbols from somewhere? ;-)
Then you could run:
G_DEBUG=fatal-warnings gdb --args appstreamcli refresh --force
and generate a nice backtrace for this error.

EDIT: refresh and refresh-cache are aliases to each other, so either one works ;-)

[antoszka]

I certainly can try compiling from source if there aren't too many pain-inducing deps :). Gimme a few minutes.

[ximion]

If you are using Debian, you can also use the automatically generated debug symbols: https://wiki.debian.org/HowToGetABacktrace#Installing_the_debugging_symbols

[antoszka]

@ximion I'm using Kali Rolling, which is almost Debian Testing, but not quite, so I'd prefer not to risk adding the Debian debug repo. I'm trying to build the package, with mixed results as of now :)

[antoszka]

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff7c87c41 in ?? () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
(gdb) bt
#0  0x00007ffff7c87c41 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#1  0x00007ffff7c88c7c in g_log_default_handler () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007ffff7c88f0d in g_logv () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007ffff7c8907f in g_log () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#4  0x00007ffff7cc2273 in g_variant_new_parsed_va () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#5  0x00007ffff7cc241b in g_variant_builder_add_parsed () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#6  0x00007ffff7f7f3f8 in  () at /usr/lib/x86_64-linux-gnu/libappstream.so.4
#7  0x00007ffff7f6fd39 in  () at /usr/lib/x86_64-linux-gnu/libappstream.so.4
#8  0x00007ffff7f74b86 in as_cache_file_save () at /usr/lib/x86_64-linux-gnu/libappstream.so.4
#9  0x00007ffff7f74f04 in as_pool_save_cache_file () at /usr/lib/x86_64-linux-gnu/libappstream.so.4
#10 0x00007ffff7f75072 in as_pool_refresh_cache () at /usr/lib/x86_64-linux-gnu/libappstream.so.4
#11 0x000055555555c9d4 in ascli_refresh_cache (cachepath=0x0, datapath=0x0, forced=1) at ../../tools/ascli-actions-mdata.c:60
#12 0x0000555555558f8b in as_client_run_refresh_cache (argv=0x7fffffffe558, argc=2) at ../../tools/appstream-cli.c:247
#13 0x000055555555a47a in as_client_run (argv=0x7fffffffe558, argc=3) at ../../tools/appstream-cli.c:818
#14 0x000055555555a7b4 in main (argc=3, argv=0x7fffffffe558) at ../../tools/appstream-cli.c:862
(gdb) 

Does that help? :)

[ximion]

@antoszka The most interesting part is missing, the library has no debug symbols. Maybe you can force GDB to load the libappstream library you have debug symbols for by also setting the LD_LIBRARY_PATH environment variable to the path where the library is located in post-build.

[antoszka]

Damn, when I use the locally built library I get no errors:

(gdb) run
Starting program: /home/antoni/programy/appstream/build/release_install/usr/local/bin/appstreamcli refresh --force
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
AppStream cache update completed successfully.
[Inferior 1 (process 10384) exited normally]

Not sure how to go about debugging that further. Can we catch up on IRC to discuss that live? I'm antoszka on Freenode.

[jbrobst]

@antoszka Are you sure you're on 0.12.2-2? Your dpkg -l output above says 0.12.2-1.

[ximion]

@jbrobst I just noticed that too - I think this is the problem here, as it was with the Debian bug.

[antoszka]

@jbrobst Yes, I'm on 0.12.2-1, but software-wise that's the same version, just the "first packing", shouldn't differ in core functionality, as far as my understanding of the Debian-like packaging scheme goes.

[ximion]

@antoszka That's not really how it works... Since you only have Debian revision 1, it means that the patch that fixes this bug is not actually included, so you still have the old version which has this bug.
That explains also why the issue is gone when you build Git master manually.

[antoszka]

OK, cool, I'll wait for my packager (Kali Linux) to pull in the newer Debian packages. I thought the fix is already in 0.12.2 (which I think is the version I built from the git checkout, not HEAD) and no additional patch was being added to -2.

[jbrobst]

@antoszka It's normal to see certain bug fixes which are not yet part of a new upstream version included as patches in the source package which just increase the debian revision number in the version; I think it might be the most common use case for source package patches aside from debian-specific changes.

In the case of this bug fix it won't hurt to manually install the .debs from unstable, once kali-rolling has a version exceeding 0.12.2-2 it will automatically be preferred by apt and thus updated as usual.

[antoszka]

Understood, thanks for all the help!

[hughsie]

spec: Clarify the need for content_attribute tags in content_rating

So what do we include in an OARS content rating section when it has no age restrictions? There was a lot of pushback about including ~40 lines of >none< in each metainfo file.

[ximion]

@hughsie You just include an empty content_rating tag. The previous spec required content_attribute children to be present in order for the file to be valid, with the patch you mentioned above it doesn't anymore.
So, nothing needs to be changed on OARS, the spec patch just clarifies what the status quo is :-)