Re: Improving how NGINX modules are used and built

[Bruno Victal <mirai@makinata.eu>]

On 2023-01-09 10:51, Ludovic Courtès wrote:
> Hello!
> 
> mirai <mirai@makinata.eu> skribis:
> 
>> An oddity of how nginx modules are packaged in guix is that they all place
>> the .so file under /etc/nginx/modules which is an odd directory to place library object files.
> 
> To me that should be treated as a bug.  Those .so files should go to
> $PKG/lib/nginx instead, or something similar.

Fixing this bug is likely to cause pain to existing module users, as the path to these .so
files is explicitly passed to <nginx-configuration> and is what's documented in the Guix manual.
<... continued below ...>

>> Looking at how network-manager-configuration handles its vpn-plugins field, it seems doable
>> that a similar approach can be used here.
>> The existing nginx-modules should be changed to install their .so files under lib{64}/nginx
>> instead and they should drop a etc/nginx/modules/foo_module.conf file responsible for loading
>> the module from the .so file. Including modules through a .conf should be preferred as
>> there's no guarantee that a module is a .so file or that it is always a
>> _single_ .so file but in general this file should typically be a one-line .conf file containing:
>>
>> load_module "/gnu/store/......nginx-foo-module/usr/lib64/nginx/ngx_foo_module.so";
>>
>>
>> And nginx-configuration should serialize the modules field as a series of lines including
>> the module .conf files, that is:
>>
>> include "/gnu/store/......nginx-foo-module/etc/nginx/modules/ngx_foo_module.conf";
>> include "/gnu/store/......nginx-bar-module/etc/nginx/modules/ngx_bar_module.conf";
>> ...
>>
>> (note: a directory union could be used here as an alternative)
> 
> I’d say that ideally, one could extend <nginx-configuration> with
> modules, and that would automatically create the ‘load_module’
> statements.

A change here would really improve how modules are used (the current way of things in Guix is:
for beginners: "guess the module path", for the seasoned: "build and list the package output
directory").

Inevitably, this won't be a backward compatible change unless we yet add another
deprecation warning and a new field "temporarily" (read as: permanently). 
(or maybe using match to differentiate between strings and file-like objects?)

I'm not convinced that <nginx-configuration> should be generating load_module statements here,
these should be generated by the module-package itself into a .conf file and <nginx-configuration>
generates a include statement for it. Reason being that nothing stops a module being comprised
of several .so files or be a "pseudo-module", that is, it's a .conf snippet to be included.

I haven't encountered modules like these yet but there's nothing saying that they can't be done this way.


Cheers,
Bruno