gtk/icons: (re)build icon theme caches #48116

romildo · 2018-10-09T19:19:51Z

Motivation for this change

(Re)build icon theme caches of all shared icon themes by the icons xdg service.

Up to now only the hicolor icon theme has its cache automatically built by the system-path service. This PR generalizes it for all icon themes in the system path.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option 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/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

michaelpj · 2018-10-10T01:43:19Z

This is not an xdg thing, it is a gtk thing, so I think it should be in a separate module.

I had thought that we would be able to make a shared cache, but it looks like we still have to do it per-theme, leading to the awkward dance with writing into the directory. That makes me think that we were wrong in saying that it's global state: it is actually per-theme state, so maybe we should actually be putting it in the individual theme packages.

michaelpj · 2018-10-10T01:47:47Z

Also, doing it in the package will make icon themes installed with nix-env work too.

romildo · 2018-10-10T02:19:12Z

Each icon theme has its cache. The difficult is that many packages may contribute with icons to the theme by installing some icons.

For instance, on my current NixOS system, the following packages (among many others) have icons installed into the hicolor icon theme: hicolor-icon-theme, psensor, wpa_gui, caja, etc.

As another example, the mate icon theme has icons installed by the packages mate-icon-theme, mate-settings-daemon, and libmateweather.

The HighContrast icon theme also has icons from different packages, like gnome-theme-extras and meld.

When the cache is built all of its icons has to be known. How to implement this?

michaelpj · 2018-10-10T02:21:37Z

Ah, I hadn't understood that, thanks! In that case I agree that this is the right thing to do.

I do think there's probably an issue with themes installed with nix-env (what happens if you have icons for the same theme spread across multiple XDG_DATA_DIRS anyway?), but arguably upstream should provide a better utility for that.

romildo · 2018-10-10T02:41:02Z

I think that most themes have all icons installed by only one package. On my system there are 71 themes installed. Only 3 of them have icons installed from more than one package.

If the main package of the theme provides a cache, presumably most of its icons will be available to applications without running a service. Only additional icons offered by other packages will not be available. Therefore I think that it is good that the main theme package installs a cache (although it does not completely fixes the situation for packages installed with nix-env).

The service solution presented here keeps the cache when there is only one package contributing with icons to the theme. Otherwise it rebuilds the cache taking all the icons into account.

matthewbauer · 2018-11-12T18:58:21Z

@michaelpj does this look ready to merge to you? Just want to make sure all potential issues have neen addressed here.

michaelpj · 2018-11-12T19:01:39Z

I still think this should be in a different module. Otherwise I'm happy, although it would be good if the rationale discussed in the comment thread was recorded in the code too.

matthewbauer · 2018-11-12T19:33:23Z

Yeah maybe leave it in system-path.nix for now? I’m not sure how else to handle it though… We want it to work in every gtk based de.

michaelpj · 2018-11-12T19:44:14Z

I think there should bea gtk module for this sort of stuff, like the xdg ones.

romildo · 2018-11-12T20:45:30Z

@michaelpj I can migrate it to a new gtk module if you prefer.

romildo · 2018-11-12T22:17:19Z

@michaelpj @matthewbauer I have written the new gtk module for setting the icon cache.

(Why was @infinisil asked to review? Is he really the owner of 3 files in this PR? It does not seem so.)

matthewbauer · 2018-11-12T22:18:26Z

Awesome! Looks good to me.

michaelpj · 2018-11-12T22:19:06Z

Lgtm, thanks!

romildo · 2018-11-12T22:30:54Z

@matthewbauer Maybe you can merge #47690 too. It is related.

aszlig · 2018-11-13T09:48:38Z

nixos/modules/config/gtk/gtk-icon-cache.nix

+  options = {
+    gtk.iconCache.enable = mkOption {
+      type = types.bool;
+      default = true;


Okay, so this broke the installer tests and while it would be trivial to just add gtk3 to the system closure, I think this should only default to true if the user really has a graphical setup, eg. not on a headless server system.

We could set it to true in xserver.nix. I did something similar for the XDG modules, but I wasn't brave enough to make the default be false, because no doubt some people are running with weird systems but still want GTK icons.

Could we not just set it to false in the installer?

Before this PR, in the system-path module, there was not an explicit dependency on gtk3, and the hicolor icon theme cache was being built with

if [ -x $out/bin/gtk-update-icon-cache -a -f $out/share/icons/hicolor/index.theme ]; then $out/bin/gtk-update-icon-cache $out/share/icons/hicolor fi

That is, the cache was being built only if gtk-update-icon-cache was available on the system.

Would it be reasonable to keep this approach in the new gtk module? I am not sure.

No, please don't go back to that, making the dependency explicit is the main reason I like this!

Having this enabled by default also breaks NixOS on armv6l, because gtk eventually depends on valgrind, which doesn't build on armv6l.

Probably the armv6 stuff is a separate issue which should be addressed in the appropriate place.

aszlig · 2018-11-13T09:54:38Z

nixos/modules/config/gtk/gtk-icon-cache.nix

+    # The module solution presented here keeps the cache when there is
+    # only one package contributing with icons to the theme. Otherwise it
+    # rebuilds the cache taking into account the icons provided all
+    # packages.


I think this whole comment block really belongs to the commit message (which should describe the reasons why you introduced that change) rather than here and I'd only keep the first and last paragraph here as a comment.

I suppose this change would need a git amend or rebase. I do not know how to accomplish it once the PR has been committed.

matthewbauer · 2018-11-16T00:59:07Z

Ok so this adds gtk3 to lots of places where it wasn't needed before. I think we can still use this new module but need to make it optional using the -x trick:

if [ -x $out/bin/gtk-update-icon-cache -a -f $out/share/icons/hicolor/index.theme ]; then
   $out/bin/gtk-update-icon-cache $out/share/icons/hicolor
fi

For it to only do this when you already have gtk in your system.

michaelpj · 2018-11-16T09:07:00Z

I really don't think we should do that - half the point of this is to make the gtk dep explicit. I think we should either disable it by default and enable it in various wms, or manually disable it when we don't want it.

lopsided98 · 2018-11-16T17:15:44Z

We have the environment.noXlibs option, which may be a good way to disable this if we want to keep it enabled by default.

romildo · 2018-11-16T20:59:28Z

The icon caches are needed by GTK applications (which are graphical applications) and may run in one of the GDK backends. Running GTK+ Applications mentions the following backends:

quartz the native Quartz backend
win32 the native backend for Microsoft Windows
x11 the native backend for connecting to X11 servers.
broadway the Broadway backend for display in web browsers
wayland the Wayland backend for connecting to Wayland display servers
mir the Mir backend for connecting to Mir display servers

Which of these backends are supported in NixOS? Is there a module for each supported backend?

We could set it to true in xserver.nix. I did something similar for the XDG modules, but I wasn't brave enough to make the default be false, because no doubt some people are running with weird systems but still want GTK icons.

Is it plausible to disable it by default and enable it in module xserver.nix?

Those running with weird systems without xserver but still wanting GTK icons could enable it explicitly.

It's a quick approximation to unblock unstable channels after #48116. This commit isn't ideal, as I suspect most wayland users won't have xserver.enable, so they will lose the icon cache in case they had gtk in system path (otherwise they didn't get cache anyway). I considered using environment.noXlibs, but the nixos tests installing headless systems do *not* get that option, so we would still be pulling gtk in many cases where it's clearly not desired. We need to design this more carefully.

vcunat · 2018-11-17T10:54:05Z

Right, for now I pushed the approximation via xserver.enable.

Wayland probably works on NixOS. Quartz and win32 are irrelevant for NixOS; mir and broadway seem unlikely in this context.

Config design

Perhaps we need some profile or explicit option for most common "headless vs. desktop" code, but I'm afraid that needs more time to design and discuss, and perhaps it also needs nontrivial changes (possibly backwards incompatible).

There's nixos/modules/config/xdg/* which pulls shared-mime-info and desktop-file-utils, there's been some discussion around udisks. I expect that the two most common use cases of NixOS are (1) pure server, i.e. non-interactive, no need for any X or XDG, and (2) desktop/workstation which might additionally provide server-like functions, i.e. you do want graphical display, all XDG, etc. Our default ATM is halfway, and that seems bad – we do not enable X (nor wayland) by default and yet we pull XDG utilities and don't use noXlibs.

michaelpj · 2018-11-17T11:06:12Z

I did propose disabling the XDG modules by default (#45920), but I was worried about the breakage potential.

vcunat · 2018-11-18T13:23:54Z

Yes, I wouldn't do that by itself, as "desktop" users typically want these IMHO. I'm still not sure in what way exactly these configs should best be "triggered". ATM I suppose it might look like factoring out the common desktop code into one place (a profile or option) and have all display managers trigger that code by default. I'd expect these would be mkDefault lines, so they should be easily overridden individually, even as a result of other options, as there can't be one configuration to suit everyone.

romildo mentioned this pull request Oct 9, 2018

build GTK icon caches for some packages #47690

Merged

9 tasks

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Oct 9, 2018

gtk: new service giving support to GTK+ applications

39a9b86

romildo requested a review from infinisil as a code owner November 12, 2018 22:07

romildo changed the title ~~xdg/icons: (re)build icon theme caches~~ gtk/icons: (re)build icon theme caches Nov 12, 2018

matthewbauer merged commit 964d01a into NixOS:master Nov 12, 2018

romildo deleted the fix.xdg-icons branch November 12, 2018 22:34

aszlig reviewed Nov 13, 2018

View reviewed changes

srhb mentioned this pull request Nov 16, 2018

nixos/gtk.iconCache: Disable by default #50453

Closed

9 tasks

jtojnar mentioned this pull request Mar 9, 2019

sublime3: fix icon #57130

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gtk/icons: (re)build icon theme caches #48116

gtk/icons: (re)build icon theme caches #48116

romildo commented Oct 9, 2018

michaelpj commented Oct 10, 2018

michaelpj commented Oct 10, 2018

romildo commented Oct 10, 2018

michaelpj commented Oct 10, 2018

romildo commented Oct 10, 2018

matthewbauer commented Nov 12, 2018

michaelpj commented Nov 12, 2018

matthewbauer commented Nov 12, 2018

michaelpj commented Nov 12, 2018

romildo commented Nov 12, 2018

romildo commented Nov 12, 2018

matthewbauer commented Nov 12, 2018

michaelpj commented Nov 12, 2018

romildo commented Nov 12, 2018

aszlig Nov 13, 2018

michaelpj Nov 13, 2018

romildo Nov 13, 2018

michaelpj Nov 13, 2018

lopsided98 Nov 13, 2018

romildo Nov 16, 2018

aszlig Nov 13, 2018

romildo Nov 13, 2018

michaelpj Nov 13, 2018

matthewbauer commented Nov 16, 2018

michaelpj commented Nov 16, 2018

lopsided98 commented Nov 16, 2018

romildo commented Nov 16, 2018

vcunat commented Nov 17, 2018

michaelpj commented Nov 17, 2018

vcunat commented Nov 18, 2018 •

edited

gtk/icons: (re)build icon theme caches #48116

gtk/icons: (re)build icon theme caches #48116

Conversation

romildo commented Oct 9, 2018

Motivation for this change

Things done

michaelpj commented Oct 10, 2018

michaelpj commented Oct 10, 2018

romildo commented Oct 10, 2018

michaelpj commented Oct 10, 2018

romildo commented Oct 10, 2018

matthewbauer commented Nov 12, 2018

michaelpj commented Nov 12, 2018

matthewbauer commented Nov 12, 2018

michaelpj commented Nov 12, 2018

romildo commented Nov 12, 2018

romildo commented Nov 12, 2018

matthewbauer commented Nov 12, 2018

michaelpj commented Nov 12, 2018

romildo commented Nov 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewbauer commented Nov 16, 2018

michaelpj commented Nov 16, 2018

lopsided98 commented Nov 16, 2018

romildo commented Nov 16, 2018

vcunat commented Nov 17, 2018

Config design

michaelpj commented Nov 17, 2018

vcunat commented Nov 18, 2018 • edited

vcunat commented Nov 18, 2018 •

edited