Sorry for the late reply, this got lost in my queue. I think this is reasonable compromise between performance and convenience for packaging.

Primary concerns would be that gdk-pixbuf does not normally allow this so I am not sure how well does gdk_pixbuf_io_init_modules function handle being called multiple times, potentially with overlapping cache files.

Some basic tests along the line of

I share your concern about the robustness of the patch. With some tests, hopefully we'll have a better idea of its vulnerabilities.
Though it seems to me like the code is somewhat resilient to loading multiple files. The module loading procedure loads a number of builtin modules before reading loaders.cache. And in gdk_pixbuf_io_init_modules, it expects modules to possibly already exist in file_formats prior to invocation.
Of course, upstream still assumes a single loaders.cache, so the internal interfaces can change at any time.

About --suffix, I think (?) later entries in $GDK_PIXBUF_MODULE_FILE have precedence.
The loop calls gdk_pixbuf_io_init_modules for each file in order, which then prepends loaders from the file to file_formats. So the order in file_formats is reversed from that of $GDK_PIXBUF_MODULE_FILE.
_gdk_pixbuf_get_module then selects which module to load by iterating through file_formats in order.
If it's built with GDK_PIXBUF_USE_GIO_MIME, it selects the first module in file_formats with a matching MIME type.
If it's built without MIME support, then it searches the entire list for the module with the best "score". Comparison is done with a > rather than a >=, so if multiple modules provide the highest score, the earliest encountered will remain the selected maximum.

In any case, the opposite precedence behavior seems to be the standard, and I agree modules included with the package should have precedence. So I'll change it to --prefix, and edit the patch to process the path in reverse order.

I wonder if packaging procedures will have to change.
I can think of these behaviours that packages rely on:

• when gdk-pixbuf or librsvg is added to buildInputs, its setupHook loads its loaders.cache to the build environment $GDK_PIXBUF_MODULE_FILE
• when both are added to buildInputs, their setupHooks work out the conflict, and the functionality of both appears
• wrapGAppsHook takes $GDK_PIXBUF_MODULE_FILE in the build environment as what it should include with the executable.
  • There may be an expectation that including the package in buildInputs is all that's needed to gain loader support, disregarding the role of wrapGAppsHook
• the librsvg loaders.cache contains the standard gdk-pixbuf loaders as well.
  • Though apps typically include both as dependencies, when manually constructing a wrapper, only librsvg's loaders.cache is set.
• obviously, the library should work with only one loaders.cache in $GDK_PIXBUF_MODULE_FILE

Seeing how many gtk programs there are, I imagine they behaviours have to stay. But it doesn't really indicate how we should design the interface for packaging more-than-one loaders.cache.

Should we make all loader packages have setupHooks that append to $GDK_PIXBUF_MODULE_FILE, just like shared libraries do to $LD_LOADERS_PATH? But unlike shared libraries, modules often overlap: do we then need a way to specify their order?

Though the vast majority of programs just need the standard librsvg/gdk-pixbuf to render GTK icons. Perhaps wrapGAppsHook can include them automatically, packages not directly calling gdk-pixbuf no longer need to explicitly include them in buildInputs (but it doesn't hurt), packages needing other loaders can manually wrap them (maintaining the librsvg-containing-gdk-pixbuf behaviour), and setupHooks can be eliminated. In any case, users will benefit from the ability to specify their own loaders.

• There may be an expectation that including the package in buildInputs is all that's needed to gain loader support, disregarding the role of wrapGAppsHook

It is ¿well? documented that wrappers are always required.

• Though apps typically include both as dependencies, when manually constructing a wrapper, only librsvg's loaders.cache is set.

Again, I would not worry about that. It is documented what wrapGAppsHook does and people are strongly encourage to use that. If they want to roll their own, they can, but then they are on their own.

obviously, the library should work with only one loaders.cache in $GDK_PIXBUF_MODULE_FILE

I think wrapGAppsHook should have gdk-pixbuf in deps, like it has librsvg.

(Alternately, we might compile the gdk-pixbuf’s loaders.cache path into the gdk-pixbuf library – not sure if that is a good idea since it would not be possible to disable them then.)

Should we make all loader packages have setupHooks that append to $GDK_PIXBUF_MODULE_FILE, just like shared libraries do to $LD_LOADERS_PATH? But unlike shared libraries, modules often overlap: do we then need a way to specify their order?

Yeah, with the ordering you mention, that will be unpredictable, especially since loaders can get into inputs via propagatedBuildInputs of dependencies. Though I wonder how common overlaps actually are, most of the modules focus on a single format/family. And when there are some overlaps, we do not package those modules anyway.

Though the vast majority of programs just need the standard librsvg/gdk-pixbuf to render GTK icons. Perhaps wrapGAppsHook can include them automatically, packages not directly calling gdk-pixbuf no longer need to explicitly include them in buildInputs (but it doesn't hurt), packages needing other loaders can manually wrap them (maintaining the librsvg-containing-gdk-pixbuf behaviour), and setupHooks can be eliminated.

Actually, that sounds reasonable too. For convenience, wrapGAppsHook could pick up packages from extraGdkPixbufModules attribute and completely ignore the inputs. The issue would occur when one would want to use the loaders outside the context of a wrapper (e.g. in Nix shell) so we would need to expose the variable to avoid the need for hacks like https://discourse.nixos.org/t/python-qt-woes/11808/2?u=jtojnar.

Sorry for the delay. I've made some corrections, and added a simple test for gdk-pixbuf.

Still need to modify services.xserver.gdk-pixbuf.modulePackages module, as it no longer needs to generate an amalgamated loaders.cache, and add tests for it.

@cwyc Could you resolve the conflicts?

Sorry I have not managed to review this early enough for this to make it into 2022.11. I have created a workaround for now #197029