Created attachment 363057 [details]
patch2

IRC comments:
1. using syslog is bad
I use it for local testing, I shall remove most of it, maybe add 1-2 g_debug() calls for the final version.
2. coding style
do you have a guide?
I have mostly tried to follow the rest of gsettings.c

(14:47:15) borschty: you are checking g_settings_schema_get_path for NULL multiple times, the NULL check should probably be before you do the more expensive strcmp check, you call strdup on the result of g_settings_schema_get_path which at this point is known to be NULL
(14:48:12) borschty: the success return value in g_settings_write_sid_to_backend is never used and I'm not sure how it should be used in g_settings_write_to_backend
(14:49:16) borschty: what is the return value of g_settings_write_to_backend supposed to be if writing the actual setting works, but writing the schema-id fails?
(14:50:18) borschty: are there any programs out there that might already be using the name "schema-id" for one of their keys in relocatable schemas?
(14:53:27) cppspinfo: 1. right, I'll change that.
(14:53:27) cppspinfo: 2. true, me neither. So far it is only useful for testing.
(14:53:27) cppspinfo: 3. Same as usual. schema-id is a convenience, it shouldn't interfere with the currents workings of gsettings.
(14:53:27) cppspinfo: 4. Yeah, good point. Kinda impossible to know.
(14:55:15) cppspinfo: 4. Probably no, as AFAIK the only application that would benefit from that is dconf-editor
(14:55:28) cppspinfo: hence this patch
(14:56:16) cppspinfo: an application knows the schema id, you have to give it when creating a gsettings
(14:56:43) cppspinfo:     GSettings *settings;
(14:56:43) cppspinfo:     settings = g_settings_new_with_path ("org.mate.sensors-applet", applet_path);
(14:56:53) cppspinfo: from the mate sensors applet
(14:57:09) cppspinfo: this is a reloc schema connection
(14:58:01) cppspinfo: so probably no programs that would store gsetting schema-id
(14:58:24) cppspinfo: may be some that use the same key for another purpose though
(14:59:05) cppspinfo: I guess it could be renamed like: schema-idxjhdrkjxhdrgd
(14:59:13) cppspinfo: just to be sure

Created attachment 363063 [details] [review]
cleaned up patch

I have made some suggested changes.
removed syslog, fixed styling

Review of attachment 363063 [details] [review]:

Thanks for the patch; there's still some work left to do.

The coding style for GLib and GTK is available here: https://git.gnome.org/browse/gtk+/tree/docs/CODING-STYLE

It would be good to have a proper commit message, explaining what are you trying to achieve, and why — including a reference to this bug.

::: gio/gsettings.c
@@ +1136,3 @@
+/* write schema id to backend for relocatable schemas */
+static gboolean
+g_settings_write_sid_to_backend (GSettings *settings)

This should probably be called `add_schema_id()`.

@@ +1141,3 @@
+  gchar *path = NULL;
+
+g_settings_write_sid_to_backend (GSettings *settings)

There's no need to set this to `NULL`: `g_strdup()` will return `NULL` if it gets an empty or `NULL` string as an input.

@@ +1142,3 @@
+
+  gchar *gsschemapath = NULL;
+{

Same as above.

@@ +1144,3 @@
+  gchar *gsschemaid = NULL;
+  gchar *retgsschemaid = NULL;
+  gboolean success = FALSE;

These variable names are really hard to read.

Maybe use:

  - schema_path
  - schema_id

The `retgsschemaid` variable is only used in the if block, so it should be declared there; same goes for `gvsid` and `path`.

@@ +1148,3 @@
+  /* is this a relocatable schema?
+  /* if yes g_settings_schema_get_path() returns NULL */
+

Coding style: missing space between function call and open parenthesis.

@@ +1151,3 @@
+
+  /* get schema-id property from gsettings object */
+  gchar *gsschemapath = NULL;

Coding style: missing space between function call and open parenthesis.

@@ +1153,3 @@
+  gsschemaid = g_strdup (g_settings_schema_get_id(settings->priv->schema));
+
+  gchar *gsschemaid = NULL;

Coding style:

 - missing space between `if` and open parenthesis
 - flip the sides of the equality around: gsschemapath == NULL && gsschemaid != NULL

@@ +1155,3 @@
+  if(NULL == gsschemapath && NULL != gsschemaid)
+    {
+  gchar *retgsschemaid = NULL;

Coding style: missing space between function call and open parenthesis.

The content of the debugging message is not really useful. What does `wsidtb` mean?

@@ +1163,3 @@
+      gvsid = g_settings_backend_read (settings->priv->backend, path, G_VARIANT_TYPE_STRING, FALSE);
+
+  /* if yes g_settings_schema_get_path() returns NULL */

Coding style: missing space between `if` and open parenthesis.

@@ +1168,3 @@
+          retgsschemaid = g_variant_dup_string (gvsid, NULL);
+
+  /* get schema-id property from gsettings object */

This debugging note is impressively opaque; you should write down something that can be understood 6 months down the line.

@@ +1174,3 @@
+        }
+
+  if(NULL == gsschemapath && NULL != gsschemaid)

Coding style:

  - missing space between `if` and open parenthesis
  - flip the terms of the equality around

There's no need to cast the arguments from `char*` to `const char*`.

This is unnecessarily wasteful. You could combine the block above and this one.

@@ +1178,3 @@
+          /* write schema-id to backend */
+          /* g_settings_backend_write() frees the new GVariant, as it is floating */
+      g_debug("gsettings.c wsidtb reloc schema found");

I honestly don't understand this.

You're checking if there's a value for `gsettings-relocatable-schema-id`, and if there is, and the value does not match it, then you replace it. Why would ever be the case?

@@ +1180,3 @@
+          success = g_settings_backend_write (settings->priv->backend, path, g_variant_new_string (gsschemaid), NULL);
+
+

Same as above: this debugging note should be understandable.

@@ +1184,3 @@
+    }
+
+      path = g_strconcat (settings->priv->path, "gsettings-relocatable-schema-id", NULL);

This comment does not help, really. Yes, `g_free()` is NULL safe.

@@ +1191,3 @@
+  g_free (path);
+
+      gvsid = g_settings_backend_read (settings->priv->backend, path, G_VARIANT_TYPE_STRING, FALSE);

You don't use `success` anywhere when calling this function, so this is either unnecessary or you need to handle a failure case.

@@ +1208,3 @@
+  /* if we don't want to write the "schema-id" drirectly, write it implicitly */
+  /* only for relocatable schemas */
+  if(success && (0 != g_strcmp0 ("gsettings-relocatable-schema-id", (const char *) key->name)))

Coding style:

 - missing space between `if` and open parenthesis
 - flip the terms of the equality

@@ +1211,3 @@
+    {
+      g_settings_write_sid_to_backend (settings);
+    }

Coding style: there's no need to use curly brackets for a single statement block.

@@ +2348,3 @@
       g_delayed_settings_backend_apply (delayed);
+
+      /* assume that the write was successful, write reloc schema-id */

This is a bit tricky, and I'm not sure it's something that can be assumed.

Thanks!
I have some questions:

1.
commit message:
Gsetting relocatable schema fix
commit description:
Save gsettings schema-id to the gsettings backend to allow dconf-editor or alike programs to display all settings properly for programs using relocatable schemas.
https://github.com/mate-desktop/mate-panel/issues/675
https://bugzilla.gnome.org/show_bug.cgi?id=789969

ok?

2. function name
style guide says: "GTK+ favours self-documenting function names"
In that regard g_settings_write_sid_to_backend or maybe g_settings_write_schema_id_to_backend would be better than add_schema_id.
no?
I have used the calling function's name as a base: g_settings_write_to_backend

3. debug message
The content of the debugging message is not really useful. What does `wsidtb` mean?
It is the abbreviated form of the function name.
Should I write g_settings_write_sid_to_backend instead?
gsettings.c narrows it down quite good I think.
If you are going to search for it, it doesn't even have to mean anything.

4. /* get schema-id property from gsettings object */
"This debugging note is impressively opaque; you should write down something that can be understood 6 months down the line."
I'm sorry, but I don't know what else to write. That is exactly what the next line does. I could add: this is what we would like to write to the backend.

5. "There's no need to cast the arguments from `char*` to `const char*`."
g_strcmp0():
https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html#g-strcmp0
It needs const char *, I'm giving it Gchar *.
The compiler doesn't seem to mind, so I have removed the typecast.

6. "This is unnecessarily wasteful. You could combine the block above and this one."
I don't understand this one. Which block to which one?

7. "I honestly don't understand this.
You're checking if there's a value for `gsettings-relocatable-schema-id`, and if there is, and the value does not match it, then you replace it. Why would ever be the case?"
One can write to the backend without gsettings.
Normally that won't / shouldn't happen, that is true.
If for nothing else, it is good for testing.

8. "/* assume that the write was successful, write reloc schema-id */
This is a bit tricky, and I'm not sure it's something that can be assumed."

Well, yeah. It is possible that the settings will not be saved or at least not immediately. Still, if a program tries to write something to gsettings, chances are that it is going to be around, so even if the main write is not successful, the schema-id will be useful in the future. Unless of course it won't be written either, because of the same error / issue.
My problem here is that to be sure, it would need major modifications, like giving a return value to gdelayedsettingbackend.c g_delayed_settings_backend_apply(), but even in there it is possible, that the modifications won't immediately be written to the backend, so I would have to change the code that reacts to the signal sent from the backend, but things start to get pretty hazy for me around there...

Maintainer of GNOME’s head of dconf-editor here, so one of the main potential user of the function. I have already expressed various concerns about the hacky way it is designed, but some of them are quite theorical, and others might need quite more work than what people would accept to give to that to be done, so I’ll just let GLib designers do as they’re used to, and use the function if it is added.

But reading another time the proposed code, one thing might need more thinking. For now, the “special key” is added when a key is written one time to the relocatable schema. It would probably be better if it could be written when the schema is mapped to the path, so that all mapped schemas are visible (including the ones of new plugins of various applications, etc.), no just the ones that have been edited.

Created attachment 363595 [details] [review]
patch v3

included suggestions

Created attachment 364394 [details] [review]
patch v4

changed debug message

Does anyone know, when Assigned To: Allison Lortie (desrt) gets back from the extended vacation?
Isn't there anyone else, who could help me getting this patch checked and merged?

about the version
https://packages.ubuntu.com/search?keywords=libglib2.0-0
It would be nice to have this patch as far back as 2.40 for the Ubuntu 14.04 LTS

The patch is for the master.

I can't speak for desrt, but back when I was trying to create gsettinglist (bug 622126), I did propose introducing a '.schema-id' key for these instantiated relocatable schemas which would have worked like what you're proposing here; and IIRC desrt was not in favour of that.

I'm not in favour of my particular solution either. Mostly because this is the first time I have to do something with GLib.
BUT this problem has been around for many many years. It is time to fix it one way or the other.
If anyone has a better way to do this, please share.

As stated before, the problem comes from GLib, so it should be fixed there.
Sure, every application developer using reloc schemas could write keys which have not only the value, but a schema-id inside as well, but that feels way more stupid to me...

In the mate-sensors-applet saving one sensor means writing 16 keys. Multiply that by adding 'org.mate.sensors-applet.sensor' to every one of them...

For information, `dconf-editor` has patches wip for doing that from a list of mapping, it’s bug 762803, and as it’s an important function I’ll do my best to have it finished (by the initial author Davi or by myself) before next release.

As stated before, that is a workaround.
It doesn't fix the underlying issue and it creates the need to continuously update your mapping.
A schema id must be saved in the GSettings backend in order to be used for reverse lookup in other applications, like dconf-editor.

I haven't finished my work on d-e yet, but I could get the right schema to load and be displayed for reloc schema keys.

A schema id must be saved in the GSettings backend in order to be used for reverse lookup in other applications, like dconf-editor.
OR
apps have to write to your mapping

(In reply to info-cppsp from comment #14)
> apps have to write to your mapping

That’s an interesting idea. It’s easy and doesn’t conceptually break things. I like that.

I think one issue with having GSettings save the schema ID to the backend is that it is done by GSettings, but it has to be discovered by using the backend level. 

Isn't there a way for the mapping to be exposed in the GSettings API somehow (e.g. g_settings_schema_list_paths)? Maybe, instead of storing the mapping in each path, put it in a well known place, like some gsettings owned key (e.g. /org/gnome/gio/settings/relocatable-map)?

(In reply to info-cppsp from comment #14)
> A schema id must be saved in the GSettings backend in order to be used for
> reverse lookup in other applications, like dconf-editor.
> OR
> apps have to write to your mapping

It could be decentralized, similar to the proposed approach, only not involving GSetings internals: apps could write the schema ID directly inside their own path, using a Dconf Editor specific relocatable schema.

Let me draw: :P
application -> GSettings -> GSettings backend (dconf in this case)

deconf-editor -> dconf

'it is done by GSettings'
Exactly. And normal applications use Gsettings and don't care about its backend.

'it has to be discovered by using the backend level'
Apps use GSettings and they know their own schema path and schema id.
d-e only has access to dconf (a backend) and to the gschema files of your system.
reloc schema gschema files don't have a path, only an id. As dconf stores path + key name -> GVariant pairs, no app can generate an association between r.s. path and r.s. schema id from the two alone. That is why the schema id has to be written under the path.

GSettings itself doesn't store anything. It is an interface. (AFAIK)
That is exactly the reason for my patch.

Using your words: 'put it in a well known place' = storing the mapping in each path

Of course, as stated before, dconf-editor or any other program directly communicating with a backend can store mappings.
BUT my patch provides a more universal solution.

Apps can't write their own schema id, as it is not in the gschema.
That would mean, that all apps using r.s. would have to rewrite their gschema files and add code to write the schema id from the app.
Applying my patch, which should solve this without the intervention of app devs, seem to be preferable.
If not, then the proposed 'mapping in d-e' would be the next best thing.

(In reply to Davi from comment #16)
> I think one issue with having GSettings save the schema ID to the backend is
> that it is done by GSettings, but it has to be discovered by using the
> backend level. 

Agree. It’s not the only issue, but it’s an important problem with the proposition in my mind.

> Isn't there a way for the mapping to be exposed in the GSettings API somehow
> (e.g. g_settings_schema_list_paths)? Maybe, instead of storing the mapping
> in each path, put it in a well known place, like some gsettings owned key
> (e.g. /org/gnome/gio/settings/relocatable-map)?

I’d say that this approach would need some real-life testing on how to store the mappings. Yes, that’s work for `dconf-editor`. A work you’ve already partly done. ^^

(In reply to Davi from comment #17)
> It could be decentralized, similar to the proposed approach, only not
> involving GSetings internals: apps could write the schema ID directly inside
> their own path, using a Dconf Editor specific relocatable schema.

Centralized or decentralized, if we ask applications devs to change their code, there should be a library or some common code to do that. And asking applications devs to do something is not exactly something I’d like, as it would be half done anyway.

(In reply to info-cppsp from comment #18)
> And normal applications use Gsettings and don't care about its backend.

It should be doable to write a “normal application” with the same scope of `dconf-editor`, that don’t rely on backend specificities. In fact, dconf-editor would do that, if:
 1) the relocatable schemas problem was solved;
 2) the dconf database had a way to be cleaned.

> 'it has to be discovered by using the backend level'
> reloc schema gschema files don't have a path, only an id. As dconf stores
> path + key name -> GVariant pairs, no app can generate an association
> between r.s. path and r.s. schema id from the two alone. That is why the
> schema id has to be written under the path.

Multiple relocatable schemas could be applied to the same path. (Multiple schemas could, even.) You cannot really trust cookies.

> Of course, as stated before, dconf-editor or any other program directly
> communicating with a backend can store mappings.
> BUT my patch provides a more universal solution.

The solution isn’t more universal, as that isn’t transparent for random application. It’s just placed deeper, and would be harder to make evolve.

Created attachment 364687 [details]
gsettings read

> I think one issue with having GSettings save the schema ID to the backend is
> that it is done by GSettings, but it has to be discovered by using the
> backend level. 

Agree. It’s not the only issue, but it’s an important problem with the
proposition in my mind.

On the contrary. That is the beauty of it. GSettings doesn't allow you to read keys that are not in the schema and NO other app needs the schema ID, only d-e and alike.
If you open apps settings (r.s.) through GSettings (for which you would already have to have the schema id and path), then you wouldn't need to read the schema ID from Gsettings, would you?

"It should be doable to write a “normal application” with the same scope of `dconf-editor`, that don’t rely on backend specificities."
But that wouldn't be called dconf-editor, but gsettings-editor. ;-)

1. It is of course possible to provide low level access to the schema-id written, for apps like gsettings-editor. I though about that myself. It wouldn't even be hard to write it.

2. It must be possible to delete keys. Or that one could write a function to do that.

"Multiple relocatable schemas"
Could you please explain? Isn't it a limitation of GSettings to have only one schema / path?

"The solution isn’t more universal, as that isn’t transparent for random application."
It doesn't have to be transparent.  A random app doesn't care about the backend or even relocatable schemas. They don't have to know about it.
All this is only important so that d-e can be used to set application settings, even if they use reloc schemas.

"It’s just placed deeper, and would be harder to make evolve."
Apart from 1. this should never have to be modified and it would work instantly for all r.s. apps without the app devs or you having to lift a finger.
((Well, apart from helping me patch d-e...))

2. After a quick check I would say, that one would have to mess with dconf (the dconf.Writer DBus service) or for a lower level access directly with gvdb.
I would gladly do that for you, if I get help from the GLib devs.
(At least so much, that they would accept my patch - otherwise it would be a wasted and HUGE effort.)

(In reply to info-cppsp from comment #23)
> "It should be doable to write a “normal application” with the same scope of
> `dconf-editor`, that don’t rely on backend specificities."
> But that wouldn't be called dconf-editor, but gsettings-editor. ;-)

“Internals” or something like that, more probably. I have a veto from desrt against “GNOME Registry”. :P

> 1. It is of course possible to provide low level access to the schema-id
> written, for apps like gsettings-editor. I though about that myself. It
> wouldn't even be hard to write it.
> 
> 2. It must be possible to delete keys. Or that one could write a function to
> do that.

I’m sure more and more things would be added to this proposal. Like a signal to be aware of schemas mapping removals, adds, etc. Waiting for an API.

> "Multiple relocatable schemas"
> Could you please explain? Isn't it a limitation of GSettings to have only
> one schema / path?

It isn’t. On a standard GNOME installation of Fedora, you have the problem of “/org/gnome/desktop/a11y/mouse/”, with two applications mapping to the same path, because why not. Even a single application could map two schemas at the same path. And with relocatable schemas, you can also map all you want where you want. Yay.

> All this is only important so that d-e can be used to set application
> settings, even if they use reloc schemas.

Don’t hack a library in a way that’s only useful for one application. Either help all users of the library, or hack in your application.

2. I can't imagine that anyone would support it to be a full blown API.
Probably the best thing would be to just use dconf for deletion. It shouldn't be needed 99.9% of the time. Even I have some leftover keys in dconf, but who cares...
In other words I probably wouldn't allow d-e or gsettings-editor to be able to delete keys.
In that regard this feature is not a must in either.
Also I am talking about occasionally deleting an unused key. For that no big API or callback is needed.

Multiple relocatable schemas
If you mean:
https://github.com/GNOME/gsettings-desktop-schemas/blob/master/schemas/org.gnome.desktop.a11y.mouse.gschema.xml.in
this is not an rs.

If you would want to support mrs, this patch would have to be rewritten to save an array of schema ids. Doable.
BUT the GLib devs would support this even less.
I can't imagine a situation, where you would want to have multiple rs to the same path. That feels stupid.
Even if, one would be better off making a new sub path for the other set of settings. It seems to be a very specialized case.

"Don’t hack a library in a way that’s only useful for one application. Either help all users of the library, or hack in your application."
I agree partly. This patch would be useful for all future applications, that would want use the now non existent association of schema id and path for rs.
Besides, in this case you can't reliably hack your application.
Either you would have to manage the mapping, the app devs, or both.

"help all users of the library"
I can only help those, who want to set sg in d-e, and it has rs...
I'm not that good yet. :)

(In reply to info-cppsp from comment #18)
> Let me draw: :P
> application -> GSettings -> GSettings backend (dconf in this case)
> 
> deconf-editor -> dconf

My reading of your proposal is actually:
app (write key value) -> GSettings (write key value + schema ID) -> dconf
dconf-editor (read schema ID) -> dconf
dconf-editor (read key value) -> GSettings

I think it should be:
application -> GSettings -> dconf
dconf-editor -> GSettings -> dconf

> 'it is done by GSettings'
> Exactly. And normal applications use Gsettings and don't care about its
> backend.
> 
> 'it has to be discovered by using the backend level'
> Apps use GSettings and they know their own schema path and schema id.
> d-e only has access to dconf (a backend) and to the gschema files of your
> system.
> reloc schema gschema files don't have a path, only an id. As dconf stores
> path + key name -> GVariant pairs, no app can generate an association
> between r.s. path and r.s. schema id from the two alone. That is why the
> schema id has to be written under the path.

Sorry, I wasn't clear. I understand the problem, but I think your solution is not a logical consequence of the problem (not the only one, at least). It does not "has to be" that way. My reading is that there is a deficiency in the GSettings API (i.e. "reloc schema gschema files don't have a path") that makes "settings crawlers" less useful, therefore the solution should be in the GSettings API (i.e. somehow associate paths to relocatable schemas). Your proposal does not help a GSettings user to find paths associated to a relocatable schema.

> GSettings itself doesn't store anything. It is an interface. (AFAIK)
> That is exactly the reason for my patch.

Your patch makes GSettings store things that don't show up in the interface. I don't think making GSettings store metadata on its own is a big issue (I might be wrong on that, though), but it should be related to some API function.

> Using your words: 'put it in a well known place' = storing the mapping in
> each path
> 
> Of course, as stated before, dconf-editor or any other program directly
> communicating with a backend can store mappings.
> BUT my patch provides a more universal solution.

Well known by who? We have to find the path before knowing the schema. It is the opposite of how GSettings works. Your proposal depends on Dconf Editor crawling the backend to find the paths. Crawling may not be possible for other backends, so that might be very dconf-specific.

(In reply to info-cppsp from comment #19)
> Apps can't write their own schema id, as it is not in the gschema.
> That would mean, that all apps using r.s. would have to rewrite their
> gschema files and add code to write the schema id from the app.
> Applying my patch, which should solve this without the intervention of app
> devs, seem to be preferable.
> If not, then the proposed 'mapping in d-e' would be the next best thing.

My opinion is either make it general (i.e. add some GSettings API and don't rely on crawling the backend), or move it out of GSettings.

(In reply to info-cppsp from comment #26)
> In other words I probably wouldn't allow d-e or gsettings-editor to be able
> to delete keys.

Users do not understand when you give them an application that has not a coherent set of functions. Have a look to opened and closed bugs in the “dconf-editor” product…

> If you would want to support mrs, this patch would have to be rewritten to
> save an array of schema ids. Doable.
> BUT the GLib devs would support this even less.

Yeps. That’s why I don’t like this solution. It’s a half-backed hack. It solves some problems, but will make solving others more difficult.

By the way, at one point, I’d like to save which application installed which schema. Should I use the same hack, again, with a new set of problems coming?

> I can't imagine a situation, where you would want to have multiple rs to the
> same path. That feels stupid.
> Even if, one would be better off making a new sub path for the other set of
> settings. It seems to be a very specialized case.

If you have correctly designed widgets coming from various libraries, you will have each widget install its own schemas, but you do not want them to install in subdirectories (users don’t care that widgets are offered by a library). Of course, you’ll only do that if the application that will be used support that; but it will come in `dconf-editor`.

(In reply to Davi from comment #27)
> (In reply to info-cppsp from comment #18)
> > Let me draw: :P
> > application -> GSettings -> GSettings backend (dconf in this case)
> > 
> > deconf-editor -> dconf
> 
> My reading of your proposal is actually:
> app (write key value) -> GSettings (write key value + schema ID) -> dconf
> dconf-editor (read schema ID) -> dconf
> dconf-editor (read key value) -> GSettings
> 
> I think it should be:
> application -> GSettings -> dconf
> dconf-editor -> GSettings -> dconf
> 

The first is how it is now.

My proposal has changed, thanks to AB.
Now it is:
app (open GSettings w/ g_settings_new_with_path()) -> GSettings (write schema ID) -> dconf

the d-e part: (so far only local on my dev env)
d-e load gschema files
new: d-e save even rs. under schema id, not path (there is none)
dconf-editor (read key value)
new: d-e lookup / search tree (schema-id key), get schema id
new: d-e lookup rs. from saved rs hashtable
new: set d-e ui

d-e DOES NOT USE GSETTINGS at least the gnome-3.18 branch I am working on.



> > 'it is done by GSettings'
> > Exactly. And normal applications use Gsettings and don't care about its
> > backend.
> > 
> > 'it has to be discovered by using the backend level'
> > Apps use GSettings and they know their own schema path and schema id.
> > d-e only has access to dconf (a backend) and to the gschema files of your
> > system.
> > reloc schema gschema files don't have a path, only an id. As dconf stores
> > path + key name -> GVariant pairs, no app can generate an association
> > between r.s. path and r.s. schema id from the two alone. That is why the
> > schema id has to be written under the path.
> 
> Sorry, I wasn't clear. I understand the problem, but I think your solution
> is not a logical consequence of the problem (not the only one, at least). It
> does not "has to be" that way. My reading is that there is a deficiency in
> the GSettings API (i.e. "reloc schema gschema files don't have a path") that
> makes "settings crawlers" less useful, therefore the solution should be in
> the GSettings API (i.e. somehow associate paths to relocatable schemas).
> Your proposal does not help a GSettings user to find paths associated to a
> relocatable schema.
> 

"therefore the solution should be in
> the GSettings API (i.e. somehow associate paths to relocatable schemas)"
I agree.
That is what this patch does.

"Your proposal does not help a GSettings user to find paths associated to a
> relocatable schema."

I can't do that with GSetting nor do I want to or need to.
At the creation of a GSettings object you must give the path.
You could only ask GSettings for reverse lookup (id -> list paths) if the backend would supports that. That would mean above my patch to extend the API of dconf and then GSettings.


> > GSettings itself doesn't store anything. It is an interface. (AFAIK)
> > That is exactly the reason for my patch.
> 
> Your patch makes GSettings store things that don't show up in the interface.
> I don't think making GSettings store metadata on its own is a big issue (I
> might be wrong on that, though), but it should be related to some API
> function.
> 

I don't want app devs to even know about this. (well...) It is supposed to be a mechanism internal to GSettings. Writing a 'public' function to be able to set the schema id value freely would be very bad.

> > Using your words: 'put it in a well known place' = storing the mapping in
> > each path
> > 
> > Of course, as stated before, dconf-editor or any other program directly
> > communicating with a backend can store mappings.
> > BUT my patch provides a more universal solution.
> 
> Well known by who? We have to find the path before knowing the schema. It is
> the opposite of how GSettings works. Your proposal depends on Dconf Editor
> crawling the backend to find the paths. Crawling may not be possible for
> other backends, so that might be very dconf-specific.
> 

Well known by anyone reading this bug.
For rs. you must give the schema id and path to make a GSettings object from an app.
The path is only looked up by GSettings for non rs based on the gschema files.


> (In reply to info-cppsp from comment #19)
> > Apps can't write their own schema id, as it is not in the gschema.
> > That would mean, that all apps using r.s. would have to rewrite their
> > gschema files and add code to write the schema id from the app.
> > Applying my patch, which should solve this without the intervention of app
> > devs, seem to be preferable.
> > If not, then the proposed 'mapping in d-e' would be the next best thing.
> 
> My opinion is either make it general (i.e. add some GSettings API and don't
> rely on crawling the backend), or move it out of GSettings.

I am not relying on crawling the backend. You are. (As a d-e dev.)
I am relying on looking up a key and then I am looking up another.
Surely you can agree, that without that much, you couldn't really call anything a GSettings backend.

As for other backends that don't support crawling, let's get back to that question, when it comes. Please don't mistake me for the GSettings maintainer just yet.
However I do appreciate, that you brought it up. Thank you.

(In reply to Arnaud B. from comment #28)
> (In reply to info-cppsp from comment #26)
> > In other words I probably wouldn't allow d-e or gsettings-editor to be able
> > to delete keys.
> 
> Users do not understand when you give them an application that has not a
> coherent set of functions. Have a look to opened and closed bugs in the
> “dconf-editor” product…
> 

Well, yeah. It is a moot point for sure. What nice is that you can reset from dconf. See pics.

> > If you would want to support mrs, this patch would have to be rewritten to
> > save an array of schema ids. Doable.
> > BUT the GLib devs would support this even less.
> 
> Yeps. That’s why I don’t like this solution. It’s a half-backed hack. It
> solves some problems, but will make solving others more difficult.
> 

You can't please everyone. And you shouldn't expect me to think of all the ways this patch could be extended in the future.

THE MOST IMPORTANT THING would be to get this merged as fast as possible. You can always revert it or add to it, change it later.

> By the way, at one point, I’d like to save which application installed which
> schema. Should I use the same hack, again, with a new set of problems coming?
> 

Please. That is a completely different matter. But to do that, you could use the exact same technique as this patch.

> > I can't imagine a situation, where you would want to have multiple rs to the
> > same path. That feels stupid.
> > Even if, one would be better off making a new sub path for the other set of
> > settings. It seems to be a very specialized case.
> 
> If you have correctly designed widgets coming from various libraries, you
> will have each widget install its own schemas, but you do not want them to
> install in subdirectories (users don’t care that widgets are offered by a
> library). Of course, you’ll only do that if the application that will be
> used support that; but it will come in `dconf-editor`.

See pics. Mate panel has objects. Each has its own rs. Problem solved.

Created attachment 364698 [details]
dconf reset1

this pic shows a badly places key

Created attachment 364699 [details]
dconf reset2

bad key removed with dconf reset

(In reply to info-cppsp from comment #30)
> And you shouldn't expect me to think of all the ways this patch could be
> extended in the future.

That looks like a big problem.

> THE MOST IMPORTANT THING would be to get this merged as fast as possible.
> You can always revert it or add to it, change it later.

The GLib has a stable interface. It’s not a library where you add and remove random functions when you want.

> > By the way, at one point, I’d like to save which application installed which
> > schema. Should I use the same hack, again, with a new set of problems coming?
> 
> Please. That is a completely different matter. But to do that, you could use
> the exact same technique as this patch.

That’s not a different matter. Your proposition is about adding some metadata in some cases in the backend. That’s a hard change with what’s done actually, and that’s why it has to be carefully thought, including possible extensions. Or done differently, or not done at all. Notably as other ways to do things exist.

> > If you have correctly designed widgets coming from various libraries, you
> > will have each widget install its own schemas, but you do not want them to
> > install in subdirectories (users don’t care that widgets are offered by a
> > library). Of course, you’ll only do that if the application that will be
> > used support that; but it will come in `dconf-editor`.
> 
> See pics. Mate panel has objects. Each has its own rs. Problem solved.

I don’t consider a list of keys folders called “object_n” a correct way to help users configure their applications.

Created attachment 364726 [details]
d-e dir names

"That looks like a big problem."
The problem is to solve the immediate issue.
This patch does that.
You expect me to think about rewriting the whole GSettings API, 'while I'm at it.'...

"The GLib has a stable interface. It’s not a library where you add and remove random functions when you want."

I am not modifying the GLib API. And it is not a random function. It fixes an internal issue.

"it has to be carefully thought"
It was. You helped me perfect my patch. If anything using GSettings accesses a path containing a rs. id key, it won't see it or be able to use it. It is completely hidden.

"I don’t consider a list of keys folders called “object_n” a correct way to help users configure their applications."

What the keys are called doesn't matter. Also, that has been partly changed already. See pic.

Review of attachment 364394 [details] [review]:

this approach isn't going to work, for a few reasons:

1) you can't add code to _new functions like this because it won't work with language bindings (which call g_object_new() directly)
2) i won't accept a patch that writes values to dconf on every *creation* of a gsettings object with a path.
3) i don't want a bunch of schema-id everywhere.  it's not even necessary for all relocatable schemas (for example, schemas that are at subpaths to other schemas).

this is the problem that GSettingsList was trying to solve, of course....

but for lack of that, here's something that we could do instead, which doesn't involve modifying GSettings at all:

1) add some new keys to gsettings to describe the likely place at which objects of a particular type are likely to appear
2) use the existing subclassing mechanism in gsettings schemas (extends) to make sure all items at a given place share a base class schema
3) in that base schema define a key (which you already have as your applet-type key) that identifies the correct subclass
4) have the subclasses define which applet-type they match, according to said key
5) put all of this logic in dconf-editor, and only in dconf-editor.

(In reply to Allison (desrt) from comment #35)
> Review of attachment 364394 [details] [review] [review]:

Happy to see you back. :)

> 1) add some new keys to gsettings to describe the likely place at which
> objects of a particular type are likely to appear
> 2) use the existing subclassing mechanism in gsettings schemas (extends) to
> make sure all items at a given place share a base class schema
> 3) in that base schema define a key (which you already have as your
> applet-type key) that identifies the correct subclass
> 4) have the subclasses define which applet-type they match, according to
> said key
> 5) put all of this logic in dconf-editor, and only in dconf-editor.

I don’t think that `dconf-editor`, that I maintain, has the role to define “a conventional way to do things”. It’s an end-user program, a consumer of the underlying libraries, that does its best to allow users to access their settings, but its development shouldn’t impact other programs work. And even if I’m trying hard to split its backend into a “dbus library” from it, there’s a long road before I can consider that this for-now-hypothetical backend program makes authority.

So I’d really prefer “some help” from GLib, a hint on such index keys or something like that.

(In reply to Allison (desrt) from comment #35)
> 1) add some new keys to gsettings to describe the likely place at which
> objects of a particular type are likely to appear

This would aid in the fix for bug #778225 too.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1299.