glib: Add variable for overriding schemas #31683
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For some reason, the GNOME 3.26 update broke the overrides. It turns out the overrides now need to come before the overriden schemas in the XDG_DATA_DIRS variable. This is not possible in general due to applications prefixing the variable (e.g. in wrapGAppsHook). To fix this, a new environment variable NIX_GSETTINGS_OVERRIDES_DIR was introduced. It has greater priority than XDG_DATA_DIRS but lower than GSETTINGS_SCHEMA_DIR. A separate variable was chosen in order not to block the built-in one for users.
For some reason, the GNOME 3.26 update broke the overrides. It turns out the overrides now need to come before the overriden schemas in the XDG_DATA_DIRS variable. This is not possible in general due to applications prefixing the variable (e.g. in wrapGAppsHook). To fix this, a new environment variable NIX_GSETTINGS_OVERRIDES_DIR was introduced. It has greater priority than XDG_DATA_DIRS but lower than GSETTINGS_SCHEMA_DIR. A separate variable was chosen in order not to block the built-in one for users.
I'll have to look in to why the eval check thinks this is OK, but the estimator thinks it isn't. Also, I need to fix the estimator to exit non-zero on eval errors :) |
GrahamcOfBorg
added
10.rebuild-darwin: 501+
10.rebuild-linux: 501+
and removed
10.rebuild-darwin: 0
10.rebuild-linux: 0
labels
7c6f434c
reviewed
Nov 24, 2017
|
|
||
| + if ((path = g_getenv ("NIX_GSETTINGS_OVERRIDES_DIR")) != NULL) | ||
| + try_prepend_dir (path); | ||
| + |
There was a problem hiding this comment.
If you call it overrides, shouldn't it be prepended last, to have maximal priority? And if these are just additions, maybe call them NIX_EXTRA_GSETTINGS_DIR or something like that?
There was a problem hiding this comment.
I assumed the GSETTINGS_SCHEMA_DIR is for debugging, which is why I made kept it the highest priority. I named it overrides since its purpose is to load defaults overridden by the gnome3 NixOS module.
There was a problem hiding this comment.
I expect this option to be used to add and not override anything, so I would use something like NIX_GSETTINGS_SCHEMA_DIR.
I am not convinced, but I agree it is not worth further bikeshedding. The change looks good in general.
There was a problem hiding this comment.
Well, glib acts like if it traversed all the directories on the list and added the default values for each key described in the directory to a dictionary, but did not override the keys already in the dictionary – basically the first match is used, hence the override.
|
@grahamc are you sure the two checks were not done w.r.t. different |
7c6f434c
approved these changes
Nov 25, 2017
vcunat
reviewed
Nov 27, 2017
| @@ -150,7 +150,7 @@ in { | |||
| export XDG_DATA_DIRS=$XDG_DATA_DIRS''${XDG_DATA_DIRS:+:}${mimeAppsList}/share | |||
|
|
|||
| # Override gsettings-desktop-schema | |||
| export XDG_DATA_DIRS=${nixos-gsettings-desktop-schemas}/share/gsettings-schemas/nixos-gsettings-overrides''${XDG_DATA_DIRS:+:}$XDG_DATA_DIRS | |||
| export NIX_GSETTINGS_OVERRIDES_DIR=${nixos-gsettings-desktop-schemas}/share/gsettings-schemas/nixos-gsettings-overrides/glib-2.0/schemas | |||
There was a problem hiding this comment.
Could there be a use case where $NIX_GSETTINGS_OVERRIDES_DIR was set already before this command?
There was a problem hiding this comment.
Anyway, that can be amended later without a larger rebuild.
vcunat
approved these changes
Nov 27, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a part of GNOME 3.26 update. I would like to get input from glib maintainers (@lovek323, @raskin) & co.
I assume we prefer introducing custom enviroment variable to blocking an existing one? Though, if we used the existing one, user could always override it, only losing the defaults from NixOS module.
Motivation for this change
For some reason, the GNOME 3.26 update broke the overrides. It turns out the overrides now need to come before the overridden schemas in the
XDG_DATA_DIRSvariable. This is not possible in general due to applications prefixing the variable (e.g. inwrapGAppsHook).To fix this, a new environment variable
NIX_GSETTINGS_OVERRIDES_DIRwas introduced. It has greater priority thanXDG_DATA_DIRSbut lower thanGSETTINGS_SCHEMA_DIR. A separate variable was chosen in order not to block the built-in one for users.Things done
I tested the module with
GSETTINGS_SCHEMA_DIRvariable, which is basically the same. Will test with the proper one once it builds.build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)