Make gdk-pixbuf support avif

[mkg20001]

Motivation for this change

Staying up-to-date with technical progress

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• 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 nixpkgs-review --run "nixpkgs-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.

[mkg20001]

Build fails as it tries to copy to ${GDK_PIXBUF_MODULEDIR} which is inside the pixbuf package

Which directory should it store the loader in?

[mkg20001]

Also should libavif be propagated in any way so that it's automatically "just there"? Adding it to gdk-pixbuf causes infinite recursion, since that's a dep of libavif

[SuperSandro2000]

Also should libavif be propagated in any way so that it's automatically "just there"? Adding it to gdk-pixbuf causes infinite recursion, since that's a dep of libavif

This should be detected automatically if a so is linked against it. If this does not happen add it to propagatedBuildInputs.

Build fails as it tries to copy to ${GDK_PIXBUF_MODULEDIR} which is inside the pixbuf package

You can replace that path with ${gdk-pixbuf}/some/path.

Which directory should it store the loader in?

It should be copied to $out. I am not sure where it lives under systems like Debian but /var/share should be $out/share.

[mkg20001]

We're building libavif here, we can't write to gdk-pixbuf because that's already built

Or are you saying I'm supposed to build the loader in gdk-pixbuf?

[SuperSandro2000]

We're building libavif here, we can't write to gdk-pixbuf because that's already built

Than I was misunderstanding you. Sorry.

Or are you saying I'm supposed to build the loader in gdk-pixbuf?

I am not sure. I am not very familiar with this library. A third option would be to add another package and build it there.

[jtojnar]

You can change the path using environment variable:

PKG_CONFIG_GDK_PIXBUF_2_0_GDK_PIXBUF_MODULEDIR = "${placeholder "out"}/${gdk-pixbuf.moduleDir}";

But loading will still be a problem, see the docs in https://github.com/NixOS/nixpkgs/pull/101537/files

[mkg20001]

<package>gdk-pixbuf</package> setup hook will populate <envar>GDK_PIXBUF_MODULE_FILE</envar> with the path to biggest <filename>loaders.cache</filename> file from the dependencies containing <link xlink:href="ssec-gnome-gdk-pixbuf-loaders">GdkPixbuf loaders</link>.

WHAT THE FUCK? That's like counting corona cases based on the weight of the paper.

Isn't there some method to merge all the loader caches?

[jtojnar]

Sure, the docs mention the module

nixpkgs/nixos/modules/services/x11/gdk-pixbuf.nix

Lines 11 to 25 in 1ecc5c7

	# Generate the cache file by running gdk-pixbuf-query-loaders for each
	# package and concatenating the results.
	loadersCache = pkgs.runCommand "gdk-pixbuf-loaders.cache" { preferLocalBuild = true; } ''
	(
	for package in ${concatStringsSep " " effectivePackages}; do
	module_dir="$package/${pkgs.gdk-pixbuf.moduleDir}"
	if [[ ! -d $module_dir ]]; then
	echo "Warning (services.xserver.gdk-pixbuf): missing module directory $module_dir" 1>&2
	continue
	fi
	GDK_PIXBUF_MODULEDIR="$module_dir" \
	${pkgs.stdenv.hostPlatform.emulator pkgs.buildPackages} ${pkgs.gdk-pixbuf.dev}/bin/gdk-pixbuf-query-loaders
	done
	) > "$out"
	'';

, we would need to make wrapGAppsHook do that and include the cache file in each derivation using wrapGAppsHook.

[mkg20001]

Setting services.xserver.gdk-pixbuf.modulePackages = with pkgs; [ libavif ]; should be enough to get it working, right?

What is the cache then used for in the package itself?

Why should we have both? (cache in package and generated cache per nixOS options)

[jtojnar]

Setting services.xserver.gdk-pixbuf.modulePackages = with pkgs; [ libavif ]; should be enough to get it working, right?

No, packages using wrapGAppsHook will override the environment variable so they will not see it.

What is the cache then used for in the package itself?

The environment variable is set in the wrapper to make it more hermetic. Images would not work otherwise on systems that do not set the environment variable globally.

Why should we have both? (cache in package and generated cache per nixOS options)

I do not think we should have the NixOS option but some people disagree. Edit: #42562

[mkg20001]

I see gdk-pixbuf modules from the prespective of a nixOS user installing avif support (for ex) globally, so that the file-manager can render thumbnails.

Having it per package makes it weird for the edge case that there's multiple cache files in the system.

Setting the loaders cache by default with wrapGAppsHook doesn't make too much sense if no extra loaders are included, assuming default ones always loaded.

It could be changed so that only packages that have a hard-requirement on specific loaders have their own cache shipped that doesn't interfere with the global configuration.

[jtojnar]

I see gdk-pixbuf modules from the prespective of a nixOS user installing avif support (for ex) globally, so that the file-manager can render thumbnails.

Yeah, this is hard.

Having it per package makes it weird for the edge case that there's multiple cache files in the system.

🤷‍♀️

Setting the loaders cache by default with wrapGAppsHook doesn't make too much sense if no extra loaders are included, assuming default ones always loaded.

We are loading the librsvg wrapper, which all GTK apps need to render window close button and other icons.

It could be changed so that only packages that have a hard-requirement on specific loaders have their own cache shipped that doesn't interfere with the global configuration.

🤷‍♀️

Also with global flag you get delightful bugs like #54278 and #60254.

[jtojnar]

As discussed on IRC, you will need to generate a loaders.cache file and pass it to the program through environment.

[jtojnar]

Environment variables typically go down, but this expression is already a bit lopsided.

[SuperSandro2000]

@mkg20001 ping

[stale]

I marked this as stale due to inactivity. → More info

[nixos-discourse]

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

https://discourse.nixos.org/t/webp-support-in-nautilus-and-eog-eye-of-gnome/16158/4

[ncfavier]

I've resurrected this in #244723