[bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd.

[ludo@gnu.org (Ludovic Courtès)]

Hello,

Christopher Baines <mail@cbaines.net> skribis:

> * gnu/services/web.scm (<apache-httpd-load-module>,
>   <apache-httpd-config-file>, <apache-httpd-virtualhost>
>   <apache-httpd-configuration>): New record types.
>   (%default-apache-httpd-modules, %apache-httpd-accounts,
>   apache-httpd-service-type): New variables.
>   (apache-httpd-shepherd-services, apache-httpd-activation,
>   apache-httpd-process-extensions): New procedures.
> * gnu/tests/web.scm (run-apache-httpd-test): New procedure.
>   (%apache-httpd-os, %tests-apache-httpd): New variables.

Nice!  Documentation will be welcome.  :-)

> +  #:export (<apache-httpd-configuration>
> +            apache-httpd-configuration
> +            apache-httpd-configuration?
> +            apache-httpd-configuration-package
> +            apache-httpd-configuration-pid-file
> +            apache-httpd-configuration-config

In this context I think ‘httpd-’ would be good enough as a prefix.

> +(define %default-apache-httpd-modules
> +  (map (match-lambda
> +         ((name file)
> +          (apache-httpd-load-module
> +           (name name)
> +           (file file))))
> +       '(("authn_file_module" "modules/mod_authn_file.so")
> +         ("authn_core_module" "modules/mod_authn_core.so")
> +         ("authz_host_module" "modules/mod_authz_host.so")

I think having this list here is the right approach.  However could you
write where it comes from?  I guess it’s equal to the current default in
httpd, right?

How often do you expect this to be updated?

> +(define-record-type* <apache-httpd-config-file>
> +  apache-httpd-config-file make-apache-httpd-config-file
> +  apache-httpd-config-file?
> +  (load-modules   apache-httpd-config-file-load-modules
> +                  (default %default-apache-httpd-modules))

Or “loaded-modules” or just “modules”?

> +(define* (run-apache-httpd-test #:optional (http-port 8080))
> +  "Run tests in %APACHE-HTTPD-OS, which has apache-httpd running and listening on
> +HTTP-PORT."

I wonder if we could abstract ‘run-nginx-test’ just enough so it can be
used for both.  It’d need to take a service name, log file, and PID file
as arguments.

Otherwise LGTM, thanks!

Ludo’.