From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:38309)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1eQsO2-0001uM-W1
	for guix-patches@gnu.org; Mon, 18 Dec 2017 05:11:07 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1eQsNy-0000NC-VR
	for guix-patches@gnu.org; Mon, 18 Dec 2017 05:11:06 -0500
Received: from debbugs.gnu.org ([208.118.235.43]:59240)
	by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)
	(Exim 4.71) (envelope-from <Debian-debbugs@debbugs.gnu.org>)
	id 1eQsNy-0000Mb-Ra
	for guix-patches@gnu.org; Mon, 18 Dec 2017 05:11:02 -0500
Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2)
	(envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1eQsNy-00018s-Dd
	for guix-patches@gnu.org; Mon, 18 Dec 2017 05:11:02 -0500
Subject: [bug#29741] [PATCH 2/2] gnu: services: web: Add service for
	apache-httpd.
Resent-Message-ID: <handler.29741.B29741.15135918084332@debbugs.gnu.org>
From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=)
References: <handler.29741.B.15134551667792.ack@debbugs.gnu.org>
	<20171216201700.23564-1-mail@cbaines.net>
	<20171216201700.23564-2-mail@cbaines.net>
Date: Mon, 18 Dec 2017 11:10:02 +0100
In-Reply-To: <20171216201700.23564-2-mail@cbaines.net> (Christopher Baines's
	message of "Sat, 16 Dec 2017 20:17:00 +0000")
Message-ID: <87zi6gtkhx.fsf@gnu.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
List-Id: <guix-patches.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-patches>,
	<mailto:guix-patches-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/guix-patches/>
List-Post: <mailto:guix-patches@gnu.org>
List-Help: <mailto:guix-patches-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-patches>,
	<mailto:guix-patches-request@gnu.org?subject=subscribe>
Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org
Sender: "Guix-patches" <guix-patches-bounces+kyle=kyleam.com@gnu.org>
To: Christopher Baines <mail@cbaines.net>
Cc: 29741@debbugs.gnu.org

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 =E2=80=98httpd-=E2=80=99 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=E2=80=99s equal to the current defau=
lt 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 =E2=80=9Cloaded-modules=E2=80=9D or just =E2=80=9Cmodules=E2=80=9D?

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

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

Otherwise LGTM, thanks!

Ludo=E2=80=99.