tree-wide: add missing parent icon themes #85229

romildo · 2020-04-14T14:22:21Z

Motivation for this change

Icon themes may inherit from other icon themes. The inheritance is specified using the Inherits key in the index.theme file.

According to the icon theme specification that means that icons not provided by the theme are looked for in its parents. The idea is that missing icons are looked for in the parent icon themes, in the order specified.

Therefore the parent themes should be installed as dependencies for a more complete experience regarding the icon sets used.

Things done

Mic92 · 2020-04-14T14:43:07Z

pkgs/data/icons/papirus-maia-icon-theme/default.nix

+  buildInputs = [
+    breeze-icons
+    gnome-icon-theme
+    papirus-icon-theme


Do they get referenced in the build?

No. They get referenced in the installed file share/icons/Papirus-Adapta-Maia/index.theme (and others):

Inherits=Papirus,breeze,gnome,hicolor

And it expects just that those themes are installed.

Ok. The problem is that just adding them to buildInputs won't make them available at runtime or install them into the user's profile.
cc @jtojnar

Yeah, I think the inherited themes need to go to propagatedBuildInputs. Recently encountered this issue in #84983.

jtojnar · 2020-04-14T18:34:50Z

pkgs/desktops/cinnamon/mint-y-icons/default.nix

@@ -18,6 +19,7 @@ stdenv.mkDerivation rec {

  propagatedUserEnvPkgs = [


I think propagatedBuildInputs should be enough but I am not completely sure.

This derivation was already using propagatedUserEnvPkgs and I have just added missing dependencies. I will change it to propagatedBuildInputs, handling the dependencies uniformly.

Is propagatedUserEnvPkgs documented somewhere? I am not finding it in the nixpkgs manual.

It is not documented because it is frowned upon since it breaks purity.

But reading #34657 (comment), propagatedBuildInputs might not work.

Relevant: #43049

romildo · 2020-04-15T00:00:35Z

I have tested with propagatedBuildInputs and with propagatedUserEnvPkgs in a VM configured with the startx display manager and the fluxbox window manager. I have listed the files in /run/current-system/sw/share/icons. I have also looked at the themes with the application lxappearance. The only theme explicitly installed was papirus-maia-icon-theme (using environment.systemPackages).

Only with propagatedUserEnvPkgs the parent themes were made available automatically.

The following screenshots illustrates the results.

With propagatedBuildInputs:

With propagatedUserEnvPkgs:

I think this is conclusive.

jtojnar · 2020-04-15T00:10:05Z

I think it might be preferable (even though ugly) to symlink the inherited themes into the output. Relying on propagatedUserEnvPkgs is just too big of a violation of encapsulation and purity for my taste.

Mic92 · 2020-04-15T09:28:24Z

If symlink works, than this should be the preferred solution. In future a setup-hook that scans index.theme performs the symlink on its own might be the way to go.

michaelpj · 2020-04-15T09:31:10Z

I think it might be preferable (even though ugly) to symlink the inherited themes into the output.

Won't this result in collisions if the parent theme is installed separately? (That might be fine, they're the same content after all).

romildo · 2020-04-15T12:23:22Z

Testing with symbolic links in papirus-maia-icon-theme with both papirus-maia-icon-theme and breeze-icons in environment.systemPackages:

Collisions are reported at build time:

$ nixos-rebuild build-vm --show-trace --option sandbox true -I nixpkgs=/alt/nixpkgs -I nixos-config=/alt/nixos/configuration.fluxbox.nix 2>&1 | tee /var/tmp/nixos-rebuild-vm.log
building Nix...
building the system configuration...
these derivations will be built:
  /nix/store/1cspvw79b958fsw7073d2cxw55ylj6bc-system-path.drv
[...]
building '/nix/store/1cspvw79b958fsw7073d2cxw55ylj6bc-system-path.drv'...
collision between `/nix/store/bq8hnk8n1xyy18nmwd7mf78kirs2s0d9-breeze-icons-5.68.0/share/icons/breeze-dark/actions/16@2x/stateshape.svg' and `/nix/store/89bsafcnz0a6r1wfaiifvmsnbh6ccnxz-papirus-maia-icon-theme-2019-07-26/share/icons/breeze-dark/actions/16@2x/stateshape.svg'
collision between `/nix/store/bq8hnk8n1xyy18nmwd7mf78kirs2s0d9-breeze-icons-5.68.0/share/icons/breeze-dark/actions/16@2x/dialog-object-properties.svg' and `/nix/store/89bsafcnz0a6r1wfaiifvmsnbh6ccnxz-papirus-maia-icon-theme-2019-07-26/share/icons/breeze-dark/actions/16@2x/dialog-object-properties.svg'
collision between `/nix/store/bq8hnk8n1xyy18nmwd7mf78kirs2s0d9-breeze-icons-5.68.0/share/icons/breeze-dark/actions/16@2x/document-save-all.svg' and `/nix/store/89bsafcnz0a6r1wfaiifvmsnbh6ccnxz-papirus-maia-icon-theme-2019-07-26/share/icons/breeze-dark/actions/16@2x/document-save-all.svg'
collision between `/nix/store/bq8hnk8n1xyy18nmwd7mf78kirs2s0d9-breeze-icons-5.68.0/share/icons/breeze-dark/actions/16@2x/kruler-south.svg' and `/nix/store/89bsafcnz0a6r1wfaiifvmsnbh6ccnxz-papirus-maia-icon-theme-2019-07-26/share/icons/breeze-dark/actions/16@2x/kruler-south.svg'
collision between `/nix/store/bq8hnk8n1xyy18nmwd7mf78kirs2s0d9-breeze-icons-5.68.0/share/icons/breeze-dark/actions/16@2x/highlight-pointer-spot.svg' and `/nix/store/89bsafcnz0a6r1wfaiifvmsnbh6ccnxz-papirus-maia-icon-theme-2019-07-26/share/icons/breeze-dark/actions/16@2x/highlight-pointer-spot.svg'
[...]
collision between `/nix/store/bq8hnk8n1xyy18nmwd7mf78kirs2s0d9-breeze-icons-5.68.0/share/icons/breeze/places/64/folder-downloads.svg' and `/nix/store/89bsafcnz0a6r1wfaiifvmsnbh6ccnxz-papirus-maia-icon-theme-2019-07-26/share/icons/breeze/places/64/folder-downloads.svg'
created 39920 symlinks in user environment
gtk-update-icon-cache: Cache file created successfully.
gtk-update-icon-cache: Cache file created successfully.
gtk-update-icon-cache: Cache file created successfully.
building '/nix/store/ylzwcnfziq0sh9h2cdcxdxq89s5i4app-dbus-1.drv'...
building '/nix/store/0ggf9zbx6ackvk98fqkyr4ccyy1lx747-unit-polkit.service.drv'...
building '/nix/store/q6n9wsd55dvpqnr3d3xh7fj1jprf8pzm-unit-systemd-fsck-.service.drv'...
building '/nix/store/93i3jmhp60v3xxavm22fn8d571ljy4dz-unit-dbus.service.drv'...
building '/nix/store/scjf25hizm5c8lyl9fdbsjnaaxclgxyx-unit-dbus.service.drv'...
building '/nix/store/f73af23ffjps3iw9l7m48k39fqm8dqir-system-units.drv'...
building '/nix/store/kwnp8n3jczl3wmim25mixbl178478xa8-user-units.drv'...
building '/nix/store/44rvwlz38pjra1bryds6qr0dy0svmlk0-etc.drv'...
building '/nix/store/1crsvb182anj0b9was6bqgdjxqs1yv9v-nixos-system-nixos-20.09pre-git.drv'...
building '/nix/store/rg3qf72c3xcvzygc16050jf68q7rrfsw-closure-info.drv'...
building '/nix/store/zxzpd7zcpmskib7yjcg3xi1h0havalkw-run-nixos-vm.drv'...
building '/nix/store/afqj6dvskdb2hf0962yiz76qv6mkm0mq-nixos-vm.drv'...

Done.  The virtual machine can be started by running /nix/store/088zz2r26xw8vj61kmrrcfcp3h173vzn-nixos-vm/bin/run-nixos-vm

romildo · 2020-04-15T12:25:23Z

Should I go on and implement this solution for all icon themes?

romildo · 2020-04-16T14:47:05Z

Using symlinks has one complexity: indirect inheritance. It would be needed to symlink also the parent themes from the parents, and so on, up to the inheritance root. This is not implemented yet in this PR.

Also I am not liking the number of collisions.

romildo · 2020-04-16T14:48:02Z

cc @worldofpeace

romildo · 2020-04-18T02:47:46Z

Now there is a setup hook in hicolor-icon-themes that, for each installed icon theme, finds its parent themes by looking at theme.index. Then each parent theme is searched among the dependencies. If found the parent theme directory is linked at $out/share/icons.

The dependencies providing parent icon themes should be propagated.

It works for indirect inheritance.

I have also tried to fix all packages providing icon themes in nixpkgs in order to indicate the dependencies needed to find the parent themes.

worldofpeace · 2020-04-18T17:50:28Z

I like the approach @jtojnar has suggested here, I also really don't favor using propagatedUserEnv. But I'm honestly not sure, what was the margin of encapsulation and purity we even wanted out of icon themes in nixpkgs?

The dependencies providing parent icon themes should be propagated.
It works for indirect inheritance.
I have also tried to fix all packages providing icon themes in nixpkgs in order to indicate the dependencies needed to find the parent themes.

I think this is something we should document in the gnome section in the nixpkgs manual.

@jtojnar do you think backporting 682734d would be a good idea for 20.03?

Also I am not liking the number of collisions.

These collision warnings have always been annoying, 😄 even if this introduced some.
I don't think there is a way to avoid that, unless we just make it be quiet in general.

Also: - add runHook calls - move postFixup actions to installPhase - gtk-update-icon-cache does accepts only one icon path

nixos-discourse · 2020-06-09T11:26:27Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/proliferation-of-symbolic-links-in-run-current-system-sw-share-icons/7587/1

ofborg bot added 6.topic: cinnamon 6.topic: xfce The Xfce Desktop Environment 10.rebuild-darwin: 0 10.rebuild-linux: 11-100 labels Apr 14, 2020

Mic92 reviewed Apr 14, 2020

View reviewed changes

jtojnar reviewed Apr 14, 2020

View reviewed changes

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 and removed 10.rebuild-darwin: 0 labels Apr 16, 2020

ofborg bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 10.rebuild-darwin: 11-100 10.rebuild-linux: 501+ 10.rebuild-linux: 501-1000 and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Apr 18, 2020

romildo added 4 commits April 19, 2020 21:29

hicolor-icon-theme: add hook to symlink inherited icon themes

bbeb6ac

arc-icon-theme: add missing parent icon themes

368c434

cinnamon.mint-y-icons: add missing parent icon themes

b491cae

Also: - add runHook calls - move postFixup actions to installPhase - gtk-update-icon-cache does accepts only one icon path

deepin.deepin-icon-theme: add missing parent icon themes

b097c95

Merge branch 'staging' into fix.icon-theme-inheritance

adcb685

romildo merged commit 663d995 into NixOS:staging Apr 21, 2020

romildo deleted the fix.icon-theme-inheritance branch April 21, 2020 13:31

ofborg bot removed 6.topic: golang 6.topic: haskell 6.topic: nixos 6.topic: python 6.topic: qt/kde 8.has: documentation 8.has: module (update) 2.status: merge conflict labels Apr 21, 2020

romildo mentioned this pull request Apr 28, 2020

gnome: document packaging icon themes #86198

Merged

10 tasks

romildo mentioned this pull request Jun 10, 2020

buildenv: paths to link resolving to the same absolute path is not a conflict #90004

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tree-wide: add missing parent icon themes #85229

tree-wide: add missing parent icon themes #85229

romildo commented Apr 14, 2020 •

edited

Mic92 Apr 14, 2020

romildo Apr 14, 2020

Mic92 Apr 14, 2020 •

edited

jtojnar Apr 14, 2020

jtojnar Apr 14, 2020

romildo Apr 14, 2020

romildo Apr 14, 2020 •

edited

jtojnar Apr 14, 2020 •

edited

romildo commented Apr 15, 2020

jtojnar commented Apr 15, 2020

Mic92 commented Apr 15, 2020 •

edited

michaelpj commented Apr 15, 2020

romildo commented Apr 15, 2020

romildo commented Apr 15, 2020

romildo commented Apr 16, 2020

romildo commented Apr 16, 2020

romildo commented Apr 18, 2020 •

edited

worldofpeace commented Apr 18, 2020

nixos-discourse commented Jun 9, 2020

		@@ -18,6 +19,7 @@ stdenv.mkDerivation rec {

		propagatedUserEnvPkgs = [

tree-wide: add missing parent icon themes #85229

tree-wide: add missing parent icon themes #85229

Conversation

romildo commented Apr 14, 2020 • edited

Motivation for this change

Things done

Mic92 Apr 14, 2020

Choose a reason for hiding this comment

romildo Apr 14, 2020

Choose a reason for hiding this comment

Mic92 Apr 14, 2020 • edited

Choose a reason for hiding this comment

jtojnar Apr 14, 2020

Choose a reason for hiding this comment

jtojnar Apr 14, 2020

Choose a reason for hiding this comment

romildo Apr 14, 2020

Choose a reason for hiding this comment

romildo Apr 14, 2020 • edited

Choose a reason for hiding this comment

jtojnar Apr 14, 2020 • edited

Choose a reason for hiding this comment

romildo commented Apr 15, 2020

jtojnar commented Apr 15, 2020

Mic92 commented Apr 15, 2020 • edited

michaelpj commented Apr 15, 2020

romildo commented Apr 15, 2020

romildo commented Apr 15, 2020

romildo commented Apr 16, 2020

romildo commented Apr 16, 2020

romildo commented Apr 18, 2020 • edited

worldofpeace commented Apr 18, 2020

nixos-discourse commented Jun 9, 2020

romildo commented Apr 14, 2020 •

edited

Mic92 Apr 14, 2020 •

edited

romildo Apr 14, 2020 •

edited

jtojnar Apr 14, 2020 •

edited

Mic92 commented Apr 15, 2020 •

edited

romildo commented Apr 18, 2020 •

edited