GNOME Bugzilla – Bug 747472
Don't ignore already-installed schemas with multiple <summary> and <description>
Last modified: 2015-05-08 16:16:08 UTC
Please revert this change. Rather than break dozens of packages with no warning and cripple entire desktops based on their allegedly improper implementation of something for which I can't even find documentation, maybe... don't?
A much better idea would be to ignore the improper syntax; maybe spit out some terminal output--not prevent programs from opening.
The documentation is here: https://developer.gnome.org/gio/stable/GSettings.html You can also find a link to the DTD of the gschema XML format linked from that very page: https://git.gnome.org/browse/glib/tree/gio/gschema.dtd The main problem that the commit in question attempts to solve is that developers have been doing things wrong for a long while, and it was our fault that we didn't implement a more strict validation. As linked by the bug in that commit (bug 747209) this lax approach caused real world problems like bug 747125. The documentation for using GSettings inside build systems has also been amended to raise the issue of developers improperly using intltool: https://wiki.gnome.org/HowDoI/GSettings I understand the frustration, but the whole idea of introducing this stricter validation was to catch applications mangling the schema format during a development cycle, and give application developers 6 months to fix their build system.
(In reply to Que Quotion from comment #1) > A much better idea would be to ignore the improper syntax; maybe spit out > some terminal output--not prevent programs from opening. That's not what the commit in question does. The commit you asked to revert validates the schema during compilation, and emits an error on the terminal, like you ask. The only thing that terminates is a tool provided by GLib and called during the build process — the GSettings API does not use the XML schema, so "programs" are not prevented from opening. Care to explain what it is you're experiencing?
Indeed, the point of doing this early in the cycle is to let people catch up with this. Just 'ignoring' the invalid syntax would be one thing, but the invalid schemas are causing real bugs, like Chinese translations showing up for key names in dconf-editor - I can't use the tool if I can't read the keys. Grepping through my schemas, I find only a handful of applications that are affected: gnome-calendar, gnome-dictionary, file-roller, gedit, gnome-screenshot and gnome-maps. At least 2 of these have already been fixed. If you want to copy the fix I committed for gnome-screenshot, you can find it here: https://git.gnome.org/browse/gnome-screenshot/commit/?id=65473b2912bb696d7b7da9d0943aef6c6e26254f
The commit in question prevents programs from opening. If the programs have invalid schema, when the schema are recompiled, the bad schema will be ignored (not installed); the next time a user tries to run that program it will crash with a segfault in glib because the schema are not installed. As Matthias has pointed out, a few gnome programs are affected; luckily not many. I am very concerned for the numerous third party programs that use gsettings and will be affected. For example, I am currently running a third-party DE that I am terrified to restart until I finished rebuilding glib with this patched out (good thing I have nano as a backup editor, because yeah: so much for gedit).
(In reply to Matthias Clasen from comment #4) > Indeed, the point of doing this early in the cycle is to let people catch up > with this. Just 'ignoring' the invalid syntax would be one thing, but the > invalid schemas are causing real bugs, like Chinese translations showing up > for key names in dconf-editor - I can't use the tool if I can't read the > keys. > > Grepping through my schemas, I find only a handful of applications that are > affected: gnome-calendar, gnome-dictionary, file-roller, gedit, > gnome-screenshot and gnome-maps. At least 2 of these have already been fixed. Indeed, gnome-dictionary has been fixed as well with these two, separate, commits that show the two bad cargo-culted practices: https://git.gnome.org/browse/gnome-dictionary/commit/?id=622735a9f6f7ca7745d48b280d5efa22833fcada https://git.gnome.org/browse/gnome-dictionary/commit/?id=a8a0565f5f27304d7c1a1566bf3e4d989b7c010f As I replied in bug 747209, though, this issue is a build/install time. Applications are not crashing because of it: they are aborting because the compiled, installed schema does not match the code using GSettings with it — likely because the glib-compile-schemas errors are ignored by the person doing the build and installation. This issue won't be solved by reverting the commit in question.
(In reply to Que Quotion from comment #5) > The commit in question prevents programs from opening. No, not really. > If the programs have invalid schema, when the schema are recompiled, the bad > schema will be ignored (not installed); the next time a user tries to run > that program it will crash with a segfault in glib because the schema are > not installed. It's not a segfault: it's a very explicit abort() call — one that has been in GSettings from day one. > As Matthias has pointed out, a few gnome programs are affected; luckily not > many. I am very concerned for the numerous third party programs that use > gsettings and will be affected. You should definitely file bugs against their build system. > For example, I am currently running a third-party DE that I am terrified to > restart until I finished rebuilding glib with this patched out (good thing I > have nano as a backup editor, because yeah: so much for gedit). Instead of patching GLib, you should really file bugs against those applications that error out during glib-compile-schemas calls. The errors during the build of these applications are not for show; they are meant to be fixed.
I understand about enforcing proper syntax on developers. There's cause here to break the compilation of programs using bad schema, but this is causing segfaults in programs at runtime; programs that were compiled and installed before updating glib (and were working until that moment). So, until those developers do update their syntax, in order to have a usable computer I am going to have to reverse this patch. I'm having to do that with terminal programs; just like I had to figure out what happened on the terminal. Lucky me I'm the kind of person who isn't discouraged by that kind of thing (plenty frustrated though).
Confirmed: patched out the commit, recompiled schemas: back doing to the work I was in the middle of with gedit.
(In reply to Que Quotion from comment #8) > I understand about enforcing proper syntax on developers. There's cause here > to break the compilation of programs using bad schema, but this is causing > segfaults in programs at runtime; programs that were compiled and installed > before updating glib (and were working until that moment). > > So, until those developers do update their syntax, in order to have a usable > computer I am going to have to reverse this patch. I'm having to do that > with terminal programs; just like I had to figure out what happened on the > terminal. Lucky me I'm the kind of person who isn't discouraged by that kind > of thing (plenty frustrated though). Its your choice to use a development branch of glib that's 6 months from the next stable release on your system. You will experience some temporary fallout every now and then. If you can't tolerate that, stick to a stable release, and a distribution that packages things for you - you'll never see this kind of issue then, because the failure here is at build time, so the packager will catch it before it ever reaches your system.
>>Comment 10 Thank you for the standard reply that I have been expecting since before I even posted the bug. My reasons for having my installation the way it is are complicated; suffice to say I wouldn't be doing it this way if I didn't have to (which is why I have a mix of stable release, bleeding-edge, and even legacy software installed, which isn't really so uncommon). I understand your motivation to enforce the proper syntax, but this implementation is breaking way more stuff than necessary. Please find a way for this to affect build-time only, so that people building packages and developers (the people who need to know about it and have both the means and responsibility to do something about it) get the message before hapless users end up with broken desktops.
(In reply to Que Quotion from comment #11) > I understand your motivation to enforce the proper syntax, but this > implementation is breaking way more stuff than necessary. Please find a way > for this to affect build-time only, You keep saying that — and at this point, after three explanations, I have to assume you're ignoring what we've been saying on purpose. The failure *is* at build time. The only reason why applications "crash" is because you *ignored* the build time error, went on to install the application anyway, and now are complaining that the application is broken. Sure: if you ignore build/install time errors, stuff will break. That's why we're confident developers and packagers will catch this issue before GLib 2.46 is released, in September.
I think what Que was saying is that he already has faulty schemas installed in /usr/share/glib-2/schemas, and then he's running the glib-compile-schemas from bleeding-edge glib, and it 'silently' breaks the installed app by skipping the schema when it previously would have included it. This problem is caused by a version skew between the glib that was used at application build time and the one that is used later to regenerate the compiled schema on the users system.
(In reply to Matthias Clasen from comment #13) > I think what Que was saying is that he already has faulty schemas installed > in /usr/share/glib-2/schemas, and then he's running the glib-compile-schemas > from bleeding-edge glib, and it 'silently' breaks the installed app by > skipping the schema when it previously would have included it. You should still see the error when installing the latest application, since that's what causing the compiled schema to fail. The glib-compile-schemas tool also prints out the file that caused the failure. We could tie this new strictness to the --strict command line option of glib-compile-schemas, but I'd still very much think we should do this by default during a development cycle.
Proposing to close this ticket as WONTFIX per explanations given above.
The problem here is pretty clear and it's a legitimate one: if a broken app [under the new definition of "broken"] is already installed then we will recompile its schema the next time _any_ new app is installed, as part of re-generating the cache. This will cause those apps to start aborting, as described above. Presently, on throwing an error, we have two possible outcomes: 1) the compile aborts [--strict was given]; or 2) the file is ignored [otherwise] We're hitting case 2 here, which is why the schema isn't being included in the cache, leading to the aborting apps. I fully believe that we ought to keep the --strict behaviour in place, at least for now. That will help apps get their acts cleaned up quickly. At the same time, we can't assume that distros will atomically update glib along with every package that had a (now) bad schema. We need to tolerate these schemas being installed, and we can't ignore them when building the cache. That means that we need to come up with a mechanism for the non-'--strict' case that reports the error, but includes the contents of the file anyway. We probably need to move away from reporting the error as a GError during the parsing, since that will abort the parse.
Created attachment 301169 [details] [review] gsettings: stay compatible with installed schemas Bug 747209 introduced an error when multiple <summary> or <description> tags are found for a single key in a GSettings schema. This check should have been present from the start, but it was left out because the schema compiler doesn't include these items in the cache file. Even still -- part of the schema compiler's job is validation, and it should be enforcing proper syntax here. Repeated <summary> and <description> tags are a semi-common problem when intltool has been misconfigured in the build system of a package, but it's possible to imagine mistakes being made by hand as well. The idea is that these problems would be caught during the build of a package and maintainers would be forced to fix their build systems. An unintended side-effect of this change, however, is that the schema compiler started ignoring already-installed schemas that contained these problems, when rebuilding the cache. This means that the installation of _any_ application would cause the regeneration of the entire cache, with these already-installed applications being excluded. Without the schema in the cache, the application would crash on next startup. The validation check in the gsettings m4 macro passes --strict to the compiler, which is not used when rebuilding the cache after installation. Pass this flag down into the parser and only throw the error in case --strict was given. This will result in the (desired) build failure without also causing already-installed apps to stop functioning. This means that we will not get even a warning about the invalid schema file in the already-installed case, but that's fine. There is no sense spamming the user with these messages when they are already quite fatal for the developer at build time.
Review of attachment 301169 [details] [review]: Looks ok to me.
Attachment 301169 [details] pushed as 2b8f131 - gsettings: stay compatible with installed schemas
(In reply to Ryan Lortie (desrt) from comment #16) Yes, precisely. Thank you. (In reply to Ryan Lortie (desrt) from comment #19) > pushed as 2b8f131 Thank you again!