After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 747209 - glib-compile-schemas ought to reject repeated <summary> and <description>
glib-compile-schemas ought to reject repeated <summary> and <description>
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gsettings
unspecified
Other All
: Normal normal
: ---
Assigned To: Allison Karlitskaya (desrt)
gtkdev
Depends on:
Blocks:
 
 
Reported: 2015-04-01 22:38 UTC by Allison Karlitskaya (desrt)
Modified: 2015-04-08 09:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
glib-compile-schema: Don't accept duplicate docs (6.61 KB, patch)
2015-04-01 22:57 UTC, Matthias Clasen
committed Details | Review

Description Allison Karlitskaya (desrt) 2015-04-01 22:38:02 UTC
A somewhat common mistake when using GSettings is to set up intltool's XML merging support.

That results in a schema that looks something like so:

  <schema path="/org/gnome/gnome-screenshot/" id="org.gnome.gnome-screenshot">
    <key type="b" name="take-window-shot">
      <default>false</default>
      <summary>Window-specific screenshot (deprecated)</summary>
      <summary xml:lang="an">Captura especifica de finestra (obsoleto)</summary>
      <summary xml:lang="ar">لقطة شاشة مختصّة بنافذة (مُبطل)</summary>
      <summary xml:lang="as">উইন্ডোৰ ক্ষেত্ৰত নিৰ্দিষ্ট স্ক্ৰিনশ্বট (স্খলিত)</summary>
      <summary xml:lang="ast">Captura específica de ventana (obsoleto)</summary>
etc.

This is not a valid schema file, and has never been a valid schema file.  The translated descriptions have never been used.  Further, they cause problems with dconf-editor: see bug 747125 about that.

The reason that we don't throw an error in this case is because <summary> and <description> are completely ignored by the compiler.  They don't end up in the binary output, so why handle them at all?

The compiler should be modified to at least detect multiple <summary> and <description> lines and warn about them.
Comment 1 Matthias Clasen 2015-04-01 22:57:54 UTC
Created attachment 300781 [details] [review]
glib-compile-schema: Don't accept duplicate docs

This schema compiler was completely ignoring <summary> and
<description> tags. Unfortunately, there are modules out there
who merge translations for these back in, with xml:lang. And
this is giving dconf-editor a hard time. Since this is not
how translations of schemas are meant to be done, just
reject such schema files.

Also add tests exercising the new error handling.
Comment 2 Allison Karlitskaya (desrt) 2015-04-01 23:37:05 UTC
Review of attachment 300781 [details] [review]:

Looks good, and thanks for the tests.

I guess making this an error at this point means we have an entire cycle to mop up the mess....
Comment 3 Matthias Clasen 2015-04-01 23:41:58 UTC
Attachment 300781 [details] pushed as b2734d7 - glib-compile-schema: Don't accept duplicate docs
Comment 4 Que Quotion 2015-04-07 18:50:54 UTC
I opened bug 747472 against the solution to this one.

This is a really bad idea: it prevents applications with bad schema from opening at all (they segfault in glib).

This could cripple an entire desktop depending on what a user has installed when they upgrade glib.

Rather, glib should output some warnings (at runtime, errors on compile) and allow the user to continue using programs.
Comment 5 Emmanuele Bassi (:ebassi) 2015-04-07 19:04:22 UTC
(In reply to Que Quotion from comment #4)
> I opened bug 747472 against the solution to this one.
> 
> This is a really bad idea: it prevents applications with bad schema from
> opening at all (they segfault in glib).

That's not what this commit does.

Neither GLib nor applications call glib-compile-schemas during their normal operations: it is only called at application install time.
 
I think you're confusing this issue GSettings aborting because of missing or invalid compiled schemas — something that has always happened since GSettings was added to GIO.

If you're experiencing issues after this commit is because you're trying to change the schema, compile it, and discard the errors that glib-compile-schemas outputs. The old compiled schema is still available, but the application code is using the new schema keys and layout — and it explodes, like it always did with a mismatched schema use.

> This could cripple an entire desktop depending on what a user has installed
> when they upgrade glib.

Not really; this would be caught during build time, and appropriately fixed by the application maintainer, assuming the maintainer is not ignoring errors during the build and installation process of their own application.

> Rather, glib should output some warnings (at runtime, errors on compile) and
> allow the user to continue using programs.

That's what happens with glib-compile-schemas after this bug.