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.

Sign up for GitHub

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

Jump to bottom

gtk3.setupHook: clear icon-theme.cache in preFixup #68552

Merged
merged 6 commits into from Sep 22, 2019
Merged

gtk3.setupHook: clear icon-theme.cache in preFixup #68552

merged 6 commits into from Sep 22, 2019

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Sep 12, 2019 •
edited by worldofpeace

Motivation for this change

Packages often run gtk-update-icon-cache to include their icons in themes’ icon cache. However, since each package is installed to its own prefix, the files will only collide. For that reason we are removing the icon-theme.cache from applications.

Previously, we did that in hicolor-icon-theme setup hook but, since it is actually gtk3’s utility that creates the cache, we thought it would be appropriate to let its setup hook handle the clearing.

Remaining tasks
  • Write up docs explaining icon-theme.cache
  • Evaluate packages still be using hicolor-icon-theme. They should not use it, unless they are an icon theme, in which case, they should propagate it.
  • Check for icon themes that have gtk3 in buildInputs and be sure to set dontDropIconThemeCache.
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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @worldofpeace

@worldofpeace
Copy link
Contributor

fc34a04 didn't really touch mate because I'm not sure what's being done there or the old xfce because we're removing it anyways.

@ofborg ofborg bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: pantheon The Pantheon desktop environment labels Sep 12, 2019
@worldofpeace
Copy link
Contributor

@jtojnar worldofpeace@bffc3be is to split the icon cache hook out of gtk3 so it can be used in gtk2. (gtk4 at some point as well)

@jtojnar
Copy link
Contributor Author

jtojnar commented Sep 12, 2019

worldofpeace@bffc3be looks good. Maybe preserve # shellcheck shell=bash in drop-icon-theme-cache.sh.

@worldofpeace
Copy link
Contributor

Maybe preserve # shellcheck shell=bash in drop-icon-theme-cache.sh.

Made sure it was everywhere 👍

This was referenced Sep 13, 2019
Merged
Merged
jtojnar and others added 4 commits September 18, 2019 22:45
@jtojnar
gtk3.setupHook: clear icon-theme.cache in preFixup
8f91e87 
Packages often run gtk-update-icon-cache to include their icons in themes’ icon cache. However, since each package is installed to its own prefix, the files will only collide. For that reason we are removing the icon-theme.cache from applications.

Previously, we did that in hicolor-icon-theme setup hook but, since it is actually gtk3’s utility that creates the cache, we thought it would be appropriate to let its setup hook handle the clearing.
@jtojnar
nixos/doc/gnome: clarify icon theme availability
22f7054
@worldofpeace @jtojnar
treewide: drop adding hicolor-icon-theme where possible
b0c2aea 
This was either for the setup-hook to remove caches or added
even though the respective icon theme propagated it.
@worldofpeace @jtojnar
treewide: icons propagate hicolor-icon-theme, dontDropIconThemeCache
b7ad6e1
@jtojnar jtojnar marked this pull request as ready for review September 18, 2019 23:45
jtojnar
jtojnar commented Sep 18, 2019
View reviewed changes
doc/languages-frameworks/gnome.xml
</para>

<para>
To avoid costly file system access when locating icons, GTK, <link xlink:href="https://woboq.com/blog/qicon-reads-gtk-icon-cache-in-qt57.html">as well as Qt</link>, can rely on <filename>icon-theme.cache</filename> files from the themes’ top-level directories. These files are generated using <command>gtk-update-icon-cache</command>, which is expected to be run whenever an icon is added or removed to an icon theme (typically an application icon into <literal>hicolor</literal> theme) and some programs do indeed run this after icon installation. However, since packages are installed into their own prefix by Nix, this would lead to conflicts. For that reason, <package>gtk3</package> provides a <link xlink:href="#ssec-gnome-hooks-gtk-drop-icon-theme-cache">setup hook</link> that will clean the file from installation. Since most applications only ship their own icon that will be loaded on start-up, it should not affect them too much. On the other hand, icon themes are much larger and more widely used so we need to cache them. Because we recommend installing icon themes globally, we will generate the cache files from all packages in a profile using a NixOS module. You can enable the cache generation using <option>gtk.iconCache.enable</option> option if your desktop environment does not already do that.
Copy link
Contributor Author

@jtojnar jtojnar Sep 18, 2019 •
edited

Choose a reason for hiding this comment

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

<option>gtk.iconCache.enable</option> should be something like <olink targetdoc="manual" targetptr="opt-gtk.iconCache.enable"/> but the olinkDB is probably not set-up properly.

Copy link
Contributor

Choose a reason for hiding this comment

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

when mentioning gtk-update-icon-cache can that be a link to https://developer.gnome.org/gtk3/stable/gtk-update-icon-cache.html also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to add <citerefentry><refentrytitle>gtk-update-icon-cache</refentrytitle> <manvolnum>1</manvolnum></citerefentry> but did not manage to fit it in.

@jtojnar
Copy link
Contributor Author

jtojnar commented Sep 18, 2019

Rendered: https://jtojnar.github.io/dumpling/#ssec-gnome-icons

@worldofpeace
Copy link
Contributor

Perhaps the bit about the NixOS module isn't suited for the nixpkgs manual?

@jtojnar
Copy link
Contributor Author

jtojnar commented Sep 19, 2019

Well most of gnome.xml is only meant for NixOS anyway. Too much global services and dependencies 😿

ttuegel
ttuegel approved these changes Sep 19, 2019
View reviewed changes
Copy link
Member

@ttuegel ttuegel left a comment

Choose a reason for hiding this comment

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

Changes to breeze-icons look good to me!

@jtojnar jtojnar merged commit f80e55f into NixOS:staging Sep 22, 2019
@jtojnar jtojnar deleted the gtk-no-icon-theme-cache branch September 22, 2019 14:30
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Sep 22, 2019
@jtojnar @dtzWill
gtk3.setupHook: clear icon-theme.cache in preFixup (NixOS#68552)
840d295 
gtk3.setupHook: clear icon-theme.cache in preFixup

(cherry picked from commit f80e55f)
@worldofpeace worldofpeace mentioned this pull request Oct 6, 2019
Closed
10 tasks
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Oct 22, 2019
@jtojnar @dtzWill
gtk3.setupHook: clear icon-theme.cache in preFixup (NixOS#68552)
e5c96f9 
gtk3.setupHook: clear icon-theme.cache in preFixup

(cherry picked from commit f80e55f)

FWIW:
-------
Conflicts:
	pkgs/applications/misc/tootle/default.nix
	pkgs/applications/networking/instant-messengers/fractal/default.nix
	pkgs/applications/search/catfish/default.nix
	pkgs/data/icons/elementary-xfce-icon-theme/default.nix
	pkgs/desktops/gnome-3/core/gnome-software/default.nix
	pkgs/development/libraries/gnome-online-accounts/default.nix
	pkgs/tools/misc/gparted/default.nix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttuegel ttuegel ttuegel approved these changes

@worldofpeace worldofpeace worldofpeace approved these changes

Assignees
No one assigned
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: pantheon The Pantheon desktop environment 6.topic: qt/kde 8.has: documentation 10.rebuild-darwin: 501-1000 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ 10.rebuild-linux: 2501-5000
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jtojnar @worldofpeace @ttuegel