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 651240 - Check for schemas before trying to load them
Check for schemas before trying to load them
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: extensions
3.0.x
Other Linux
: Normal major
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 651225 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-05-27 12:33 UTC by ecyrbe
Modified: 2011-11-19 17:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
main: Add safe wrapper function for g_settings_new() (1.23 KB, patch)
2011-05-27 13:17 UTC, Florian Müllner
none Details | Review
use the previous patch to check for schemas (2.89 KB, patch)
2011-05-27 13:59 UTC, ecyrbe
none Details | Review
main: Add safe wrapper function for g_settings_new() (1.24 KB, patch)
2011-05-27 14:31 UTC, Florian Müllner
reviewed Details | Review

Description ecyrbe 2011-05-27 12:33:59 UTC
currently when a schemas is loaded, the gsettings behaviour can make the entire gnome-shell goaway and the user as no choice but reboot.
Here is a workaround. for every gnome-shell code that needs to load a schema we should check that it's present, and if not advertise the user.

here a code sample for this :


has_schema: function(schema){
 let schemas = Gio.Settings.list_schemas();
 for(let i=0;i<schemas.length;i++){
   if(schemas[i]==schema){
     return true;
   }
  }
  return false;
}

and before loading the schema do :

//check for the schemas before loading
if(!this.has_schema(YOUR_SETTINGS_SCHEMA)){
  global.log('xml schemas not installed');
  <here add code to advertise the user of a broken installation>
  return;
}
Comment 1 Dan Winship 2011-05-27 12:50:46 UTC
*** Bug 651225 has been marked as a duplicate of this bug. ***
Comment 2 Florian Müllner 2011-05-27 13:17:46 UTC
Created attachment 188747 [details] [review]
main: Add safe wrapper function for g_settings_new()

g_settings_new() aborts if the requested schema is not found, so
extensions which rely on optional schemas crash the shell.
Provide a convenience function which checks the existance of a
schema before creating the GSettings object and throw an exception
if it does not exist.
Comment 3 ecyrbe 2011-05-27 13:59:11 UTC
Created attachment 188753 [details] [review]
use the previous patch to check for schemas

Patch for gnome-shell-extensions use the previous patch to check for schemas before loading them.
Comment 4 ecyrbe 2011-05-27 14:23:02 UTC
Review of attachment 188747 [details] [review]:

event if this is the way to go, this patch is broken, in operator is not working like that for arrays, cf https://developer.mozilla.org/en/JavaScript/Reference/Operators/Special/in
Comment 5 Florian Müllner 2011-05-27 14:31:50 UTC
Created attachment 188761 [details] [review]
main: Add safe wrapper function for g_settings_new()

(In reply to comment #4)
> in operator is not working like that for arrays, cf
> https://developer.mozilla.org/en/JavaScript/Reference/Operators/Special/in

Woops, my bad ...
Comment 6 ecyrbe 2011-05-27 15:24:41 UTC
Review of attachment 188761 [details] [review]:

it's okay for me. Applied in my branch and working like a charm
Comment 7 Owen Taylor 2011-05-27 15:27:00 UTC
(In reply to comment #5)
> Created an attachment (id=188761) [details] [review]
> main: Add safe wrapper function for g_settings_new()
> 
> (In reply to comment #4)
> > in operator is not working like that for arrays, cf
> > https://developer.mozilla.org/en/JavaScript/Reference/Operators/Special/in
> 
> Woops, my bad ...

IMO, should be fixed at the GSettings and/or GJS level. Add something at the GSettings level that has a GError *, and use rename annotations or Javascript overrides to make new Gio.Settings() use that.
Comment 8 ecyrbe 2011-05-27 15:35:58 UTC
I know, but i proposed this type of workaround as i fear that glib core developpers are not willing to change their minds.
I hope that other voices will make them reconsider this.
Comment 9 John Stowers 2011-05-27 22:27:39 UTC
(In reply to comment #7)
> (In reply to comment #5)
> > Created an attachment (id=188761) [details] [review] [details] [review]
> > main: Add safe wrapper function for g_settings_new()
> > 
> > (In reply to comment #4)
> > > in operator is not working like that for arrays, cf
> > > https://developer.mozilla.org/en/JavaScript/Reference/Operators/Special/in
> > 
> > Woops, my bad ...
> 
> IMO, should be fixed at the GSettings and/or GJS level. Add something at the
> GSettings level that has a GError *, and use rename annotations or Javascript
> overrides to make new Gio.Settings() use that.

yeah, this is just a workaround for bug 649717 and/or bug 645254
Comment 10 ecyrbe 2011-05-29 02:55:57 UTC
i submitted a patch for correcting glib just in case glib dev change their minds:
https://bugzilla.gnome.org/show_bug.cgi?id=651366
Comment 11 John Stowers 2011-05-29 03:41:50 UTC
(In reply to comment #10)
> i submitted a patch for correcting glib just in case glib dev change their
> minds:
> https://bugzilla.gnome.org/show_bug.cgi?id=651366

This changes existing behaviour. I don't think this will be received well. How about adding g_settings_try_new...

I think a patch implementing the solutions discussed on bug 649717 would be more likey accepted.
Comment 12 ecyrbe 2011-05-29 05:24:57 UTC
yes of course, i can implement such a patch. thanks for the suggestion.
Comment 13 Florian Müllner 2011-11-19 17:04:34 UTC
Bug 649717 has landed, which obsoletes the patch; closing.