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 662168 - Some doubts about the recent "change the way we do dconf" commit
Some doubts about the recent "change the way we do dconf" commit
Status: RESOLVED FIXED
Product: gdm
Classification: Core
Component: general
3.2.x
Other Linux
: Normal normal
: ---
Assigned To: GDM maintainers
GDM maintainers
Depends on:
Blocks:
 
 
Reported: 2011-10-19 08:08 UTC by Vincent Untz
Modified: 2011-10-19 17:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
data: only run dconf update if DESTDIR is unset (944 bytes, patch)
2011-10-19 16:20 UTC, Ray Strode [halfline]
committed Details | Review
data: drop weird dconf script (13.79 KB, patch)
2011-10-19 16:20 UTC, Ray Strode [halfline]
committed Details | Review

Description Vincent Untz 2011-10-19 08:08:43 UTC
I have some doubts about the new way we do dconf for gdm.

I understand it's an improvement, but I'm worried about the gdm-update-dconf-db script.

First, it feels like anyone adding dconf change sfor gdm should run this script. Except that it's completely unclear how this script should be called. I'd prefer to have the script hardcode things (like using the gdm profile).

The script also runs a dbus, which feels, hrm, weird. And there's no documentation on how to handle this for packagers, which worries me even more :-)

Btw, tiny bug: "dconf update" shouldn't be called on "make install" if $DESTDIR is set, I guess.
Comment 1 Ionut Biru 2011-10-19 08:24:36 UTC
is confusing for me as well. i added in post install and post update, dconf update but i'm not sure if is the right thing to do. 

does it have to be run as gdm user?

gdm-update-dconf-db assumes that dbus is running since it creates a session but when installing packages, dbus doesn't run and i'm pretty sure settings are lost
Comment 2 Ray Strode [halfline] 2011-10-19 13:01:35 UTC
I'm definitely open to other ideas. I don't really like the script.

The main motivations, though, are:

1) allow the admin of a system to make changes to gdm configuration without them getting blown away during a package update
2) prevent an admins changes to gdm configuration from accidentally overwriting gdm's lockdown configuration
3) provide an extra level of asssurance that a new settings daemon plugin (or whatever) won't get enabled unintentionally at the login screen

You don't *have* to call gdm-update-dconf-db in the package scripts, just dconf update.  All the script does is make the setting overrides "current" against the installed system's configuration.

I'm basically open to 1) making it take no arguments and working by just running it or 2) making it a build time thing like make-override-db.sh used to be or 3) other ideas.

Ryan (or anyone), what do you think is the best way forward?
Comment 4 Allison Karlitskaya (desrt) 2011-10-19 13:18:11 UTC
There are two ways you could do this, really (which I think roughly correspond to your 'old' and 'new' ways):

1) Install a binary 'gdm' database.

2) Install a keyfile in the gdm.d/ directory and run dconf update a lot.


Discussion on each:

1) This is pretty nice, actually, except for two things

 - it's pretty opaque to the user in terms of what's in the blob
   - I plan to fix that with better tool support (for dumping blobs)

 - the build-time setup is really annoying
   - I plan to fix that with better tool support (for building blobs)

 - the mechanism for the user to override this is by adding their own db
   to the 'gdm' profile in between the user and the 'gdm' db and populating
   it as they wish.

   This means that the user doesn't clash your db files, but they do clash
   the 'gdm' profile file.

 - locked-down values cannot be changed by the intermediate override by the
   rule that the lowest-level lock takes priority.  That's maybe bad.

2) Also pretty nice.  Perhaps nicer.

 - quite transparent to the user in terms of what the settings are

 - the user can add their own keyfile to the same directory to do overrides

 - 'dconf update' already exists and works great.  You run this from 'make
    install' or postinsts and the user runs it when they make changes.

 - sore point is that it is not immediately clear to the user what they
   should name their file so that it takes precedence over the installed
   file.  that could easily be addressed with a comment at the top of the
   installed file.

 - user would be able to provide their own settings for locked-down values
   because they are operating at the same level as the lock itself.
Comment 5 Allison Karlitskaya (desrt) 2011-10-19 13:19:46 UTC
one more thing: 'dconf update' does not need a session dbus -- it only connects to the system bus (if it exists).

If there is a system bus, it should be permitted to talk to it since it needs to alert running programs (such as gdm) that the database has changed.

If there is no system bus, then there is no harm -- gdm is obviously not running in that case anyway, so no need to alert about changes.
Comment 6 Ray Strode [halfline] 2011-10-19 14:11:46 UTC
So the main problem with 2 is there is no way to provide wildcards so i can't say "only activate this 5 settings-daemon plugins" or whatever.  Other than that, I like 2 much better than the hack I did originally (which was 1).  And the entire point of the script is to work around that limitation.
Comment 7 Ray Strode [halfline] 2011-10-19 14:14:09 UTC
The session bus was an oversight. I copy-and-pasted the make-override-db script (which doesn't need a bus) and gutted the middle without thinking about it too hard.
Comment 8 Vincent Untz 2011-10-19 14:43:39 UTC
FWIW, if a simple "dconf update" can't work to keep settings current, I'd feel much better with a script that doesn't take any argument. At least, it's much harder to get wrong in that case.

Arguably, though, the "dconf update" should be called in the scriptlets of all packages installing schemas relevant to gdm, though :/.
Comment 9 Ray Strode [halfline] 2011-10-19 14:46:00 UTC
Okay, first let's revisit the "point" of the script.

The point is to take this file:

http://git.gnome.org/browse/gdm/tree/data/upstream-settings?id=25004

and convert it into a keyfile named 00-upstream-settings in /etc/dconf/db/gdm.d that users can then override with a file named 99-local-settings or so.

Then one "dconf update" call later and /etc/dconf/db/gdm is recreated to have a merged database.

So looking at the above file we see that most of the keys can be translated one to one into groups in the keyfile.  The two exceptions are:

org.gnome.settings-daemon.plugins..* active=false
org.gnome.settings-daemon.plugins.a11y-keyboard active=true
org.gnome.settings-daemon.plugins.background active=true
org.gnome.settings-daemon.plugins.cursor active=true
org.gnome.settings-daemon.plugins.media-keys active=true
org.gnome.settings-daemon.plugins.orientation active=true
org.gnome.settings-daemon.plugins.power active=true
org.gnome.settings-daemon.plugins.sound active=true
org.gnome.settings-daemon.plugins.xrandr active=true
org.gnome.settings-daemon.plugins.xsettings active=true

and

org.gnome.desktop.lockdown .*=true
org.gnome.desktop.lockdown disable-log-out=false

The idea being to white list the settings plugins we want and to lock down everything but log out (which we need for internal reasons).

We could write those out manually and then just ship the key file, but then we risk a new plugin getting added that gets enabled and shouldn't be.  It's certainly "safer" to force a white list.

I talked to Ryan a bit about this on irc. The consensus we came to is that we should just add an optional whitelist key to g-s-d and then set that. The lockdown keys are much less likely to change and we can just write those out.

In the mean time I'll drop the script from git, and change make to soley ship the generated file. When the white list gets added to g-s-d, we can change the file to use the white list instead of hard coding each plugin.
Comment 10 Ray Strode [halfline] 2011-10-19 14:47:32 UTC
I'll do a quick follow up release to, so distro don't have to change over to this script and then change back in a release.
Comment 11 Ray Strode [halfline] 2011-10-19 16:20:44 UTC
Created attachment 199450 [details] [review]
data: only run dconf update if DESTDIR is unset

There's really no point in updating the dconf db
if we haven't changed it.
Comment 12 Ray Strode [halfline] 2011-10-19 16:20:47 UTC
Created attachment 199451 [details] [review]
data: drop weird dconf script

It adds a lot of packaging pain and confusion for distributors,
and the bulk of the gain from it could be fixed by adding a
setting to gnome-settings-daemon.

Instead, just ship a static list of settings.

Distros will still need to run dconf update in their post transaction
hooks, however.
Comment 13 Ray Strode [halfline] 2011-10-19 16:21:25 UTC
okay 3.2.1.1 is released now.

make sure to run dconf update in post trans or so.
Comment 14 Ray Strode [halfline] 2011-10-19 17:06:22 UTC
i filed bug Bug 662220 to track handling the g-s-d whitelist feature.