Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gtk3: make GTK depend on gsettings_desktop_schemas #31891

Merged
merged 1 commit into from Nov 27, 2017
Merged

gtk3: make GTK depend on gsettings_desktop_schemas #31891

merged 1 commit into from Nov 27, 2017

Conversation

ghost
Copy link

@ghost ghost commented Nov 21, 2017

Motivation for this change

So dconf settings will be applied to GTK3 programs even if running outside of Gnome. See #31293

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@ghost
Copy link
Author

ghost commented Nov 21, 2017

/cc @jtojnar

@@ -168,7 +168,7 @@ in stdenv.mkDerivation rec {
ln -s $out/lib/libreoffice/program/$a $out/bin/$a
wrapProgram "$out/bin/$a" \
--prefix XDG_DATA_DIRS : \
"$out/share:$GSETTINGS_SCHEMAS_PATH" \
"$out/share" \
Copy link
Member

@jtojnar jtojnar Nov 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already part of wrapGAppsHook:

if [ -d "$prefix/share" ]; then
gappsWrapperArgs+=(--prefix XDG_DATA_DIRS : "$prefix/share")
fi

@@ -22,7 +22,7 @@ stdenv.mkDerivation rec {

nativeBuildInputs = [ pkgconfig ];
buildInputs = [ intltool file openssl curl libevent zlib ]
++ optionals enableGTK3 [ gtk3 makeWrapper ]
++ optionals enableGTK3 [ gtk3 wrapGAppsHook ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preferably, the hook would be added to nativeBuildInputs.

Copy link
Member

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm gsettings_desktop_schemas are correctly picked up by GTK in Transmission in Sway.

I also considered adding hicolor_icon_theme as a hook dependency but since it is probably only needed by apps with CSD, file chooser dialogue or similar advanced constructs, it is probably wiser to add it to the respective applications’ dependencies.

@jtojnar
Copy link
Member

jtojnar commented Nov 21, 2017

Could you please squash the third commit into the first one and add some description to the second one (at least the issue link)?

@ghost
Copy link
Author

ghost commented Nov 21, 2017

@jtojnar Done, thank you!

@gebner
Copy link
Member

gebner commented Nov 21, 2017

We should probably clean up the now-superfluous gsettings_desktop_schemas in various packages like dconf-editor.

@jtojnar
Copy link
Member

jtojnar commented Nov 21, 2017

Actually, would it make sense to make GTK rather than the hook depend on the schemas?

diff --git a/pkgs/development/libraries/gtk+/3.x.nix b/pkgs/development/libraries/gtk+/3.x.nix
index fe6d37ec692..40231edf40e 100644
--- a/pkgs/development/libraries/gtk+/3.x.nix
+++ b/pkgs/development/libraries/gtk+/3.x.nix
@@ -4,7 +4,7 @@
 , waylandSupport ? stdenv.isLinux, wayland, wayland-protocols
 , xineramaSupport ? stdenv.isLinux
 , cupsSupport ? stdenv.isLinux, cups ? null
-, darwin
+, darwin, gnome3
 }:
 
 assert cupsSupport -> cups != null;
@@ -31,7 +31,7 @@ stdenv.mkDerivation rec {
 
   patches = [ ./3.0-immodules.cache.patch ];
 
-  buildInputs = [ libxkbcommon epoxy json_glib ];
+  buildInputs = [ libxkbcommon epoxy json_glib gnome3.gsettings_desktop_schemas ];
   propagatedBuildInputs = with xorg; with stdenv.lib;
     [ expat glib cairo pango gdk_pixbuf atk at_spi2_atk
       libXrandr libXrender libXcomposite libXi libXcursor libSM libICE ]
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index 08f04faf041..23a9bb27fe0 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -346,7 +346,7 @@ with pkgs;
   findXMLCatalogs = makeSetupHook { } ../build-support/setup-hooks/find-xml-catalogs.sh;
 
   wrapGAppsHook = makeSetupHook {
-    deps = [ gnome3.dconf.lib gnome3.gtk gnome3.gsettings_desktop_schemas librsvg makeWrapper ];
+    deps = [ gnome3.dconf.lib gnome3.gtk librsvg makeWrapper ];
   } ../build-support/setup-hooks/wrap-gapps-hook.sh;
 
   separateDebugInfo = makeSetupHook { } ../build-support/setup-hooks/separate-debug-info.sh;

@jtojnar
Copy link
Member

jtojnar commented Nov 21, 2017

/cc GTK maintainers @raskin, @vcunat, @lethalman

@ghost
Copy link
Author

ghost commented Nov 22, 2017

@jtojnar With your patch dconf settings are not applied to e.g. transmission when testing this way:

cd nixpkgs
nix-build -A transmission_gtk
./result/bin/transmission-gtk

Am I missing anything?

@jtojnar
Copy link
Member

jtojnar commented Nov 22, 2017

Sorry, should have added it to propagatedBuildInputs for glib envHook to run.

diff --git a/pkgs/development/libraries/gtk+/3.x.nix b/pkgs/development/libraries/gtk+/3.x.nix
index fe6d37ec692..57366bc3582 100644
--- a/pkgs/development/libraries/gtk+/3.x.nix
+++ b/pkgs/development/libraries/gtk+/3.x.nix
@@ -4,7 +4,7 @@
 , waylandSupport ? stdenv.isLinux, wayland, wayland-protocols
 , xineramaSupport ? stdenv.isLinux
 , cupsSupport ? stdenv.isLinux, cups ? null
-, darwin
+, darwin, gnome3
 }:
 
 assert cupsSupport -> cups != null;
@@ -33,7 +33,7 @@ stdenv.mkDerivation rec {
 
   buildInputs = [ libxkbcommon epoxy json_glib ];
   propagatedBuildInputs = with xorg; with stdenv.lib;
-    [ expat glib cairo pango gdk_pixbuf atk at_spi2_atk
+    [ expat glib cairo pango gdk_pixbuf atk at_spi2_atk gnome3.gsettings_desktop_schemas
       libXrandr libXrender libXcomposite libXi libXcursor libSM libICE ]
     ++ optionals waylandSupport [ wayland wayland-protocols ]
     ++ optionals stdenv.isDarwin (with darwin.apple_sdk.frameworks; [ AppKit Cocoa ])
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index 08f04faf041..23a9bb27fe0 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -346,7 +346,7 @@ with pkgs;
   findXMLCatalogs = makeSetupHook { } ../build-support/setup-hooks/find-xml-catalogs.sh;
 
   wrapGAppsHook = makeSetupHook {
-    deps = [ gnome3.dconf.lib gnome3.gtk gnome3.gsettings_desktop_schemas librsvg makeWrapper ];
+    deps = [ gnome3.dconf.lib gnome3.gtk librsvg makeWrapper ];
   } ../build-support/setup-hooks/wrap-gapps-hook.sh;
 
   separateDebugInfo = makeSetupHook { } ../build-support/setup-hooks/separate-debug-info.sh;

@ghost
Copy link
Author

ghost commented Nov 22, 2017

@jtojnar This one works, thank you!

@jtojnar jtojnar added the 6.topic: GNOME GNOME desktop environment and its underlying platform label Nov 23, 2017
@jtojnar
Copy link
Member

jtojnar commented Nov 23, 2017

@gnidorah could you update it then, we got okayed by @vcunat on IRC.

@ghost
Copy link
Author

ghost commented Nov 24, 2017

@jtojnar Done, thanks!

So dconf settings will be applied to GTK3 programs even if running outside of Gnome. See #31293
@ghost ghost changed the title wrapGAppsHook: add gsettings_desktop_schemas to dependencies gtk3: make GTK depend on gsettings_desktop_schemas Nov 24, 2017
@orivej
Copy link
Contributor

orivej commented Nov 26, 2017

Why do you propagate this dependency from GTK and not from some Gnome package? (glib?) AFAIK GTK applications can not use GSettings without compiling with it.

@orivej
Copy link
Contributor

orivej commented Nov 26, 2017

Oh, I was confused, gsettings is an inherent part of libgio, which is one of glib libraries, which is an inherent dependency of GTK. I'll have to study GTK on NixOS more to be able to review such changes.

@jtojnar
Copy link
Member

jtojnar commented Nov 26, 2017

@orivej As described in the issue, GSettings is required when running on Wayland. We could apply the patch from the upstream bug but it would still not work while applications depend on dconf – we would also need to fix #11239 – and it still would not work for applications inherently require dconf (i.e. most gnome apps).

@vcunat vcunat changed the base branch from master to staging November 27, 2017 12:56
@vcunat vcunat self-assigned this Nov 27, 2017
@vcunat vcunat merged commit 8e03cda into NixOS:staging Nov 27, 2017
@ghost ghost deleted the gapps branch November 27, 2017 15:41
@ghost ghost mentioned this pull request Nov 30, 2017
8 tasks
@jtojnar jtojnar mentioned this pull request Dec 18, 2017
8 tasks
@jtojnar jtojnar mentioned this pull request Feb 1, 2018
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 10.rebuild-darwin: 11-100 10.rebuild-linux: 501+
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants