unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#29741] [PATCH] gnu: services: web: Add service for apache-httpd.
@ 2017-12-16 20:12           ` Christopher Baines
  2018-01-11  9:46             ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for httpd Ludovic Courtès
  0 siblings, 1 reply; 11+ messages in thread
From: Christopher Baines @ 2017-12-16 20:12 UTC (permalink / raw)
  To: 29741


I've got as far as writing the service type, and a simple system
test. I've also managed to deploy this with mod_wsgi (which isn't
packaged for Guix yet), which involved extending the service and adding
some virtualhosts.

There isn't any documentation yet, and I'm not very sure of the
way the configuration is organised. In particular:

 - There is a record type for modules, which are handled specifically
   handled in the configuration file. I guess this might be helpful if
   you want to remove modules, but I'm not sure if this is useful.
 - There is some default extra configuration, which is related to the
   mime_module, which makes me wonder if there should be something
   relating these two things.

Christopher Baines (2):
  gnu: tests: web: Generalise the test data for the nginx test.
  gnu: services: web: Add service for apache-httpd.

 gnu/services/web.scm | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 gnu/tests/web.scm    |  90 ++++++++++++++++++--
 2 files changed, 311 insertions(+), 6 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the test data for the nginx test.
       [not found] <handler.29741.B.15134551667792.ack@debbugs.gnu.org>
@ 2017-12-16 20:16 ` Christopher Baines
  2017-12-16 20:17   ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd Christopher Baines
  2017-12-18 10:02   ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the test data for the nginx test Ludovic Courtès
  0 siblings, 2 replies; 11+ messages in thread
From: Christopher Baines @ 2017-12-16 20:16 UTC (permalink / raw)
  To: 29741

So that it can also be used for other web servers.

* gnu/tests/web.scm (%index.html-contents): Change nginx to guix.
  (%make-http-root): Move the index.html file from /srv to /srv/http.
  (%nginx-servers): Remove the setting of root.
---
 gnu/tests/web.scm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gnu/tests/web.scm b/gnu/tests/web.scm
index e975cb830..b84f072ac 100644
--- a/gnu/tests/web.scm
+++ b/gnu/tests/web.scm
@@ -32,21 +32,21 @@
             %test-php-fpm))
 
 (define %index.html-contents
-  ;; Contents of the /index.html file served by nginx.
-  "Hello, nginx!")
+  ;; Contents of the /index.html file served by apache-httpd.
+  "Hello, guix!")
 
 (define %make-http-root
   ;; Create our server root in /srv.
   #~(begin
       (mkdir "/srv")
-      (call-with-output-file "/srv/index.html"
+      (mkdir "/srv/http")
+      (call-with-output-file "/srv/http/index.html"
         (lambda (port)
           (display #$%index.html-contents port)))))
 
 (define %nginx-servers
   ;; Server blocks.
   (list (nginx-server-configuration
-         (root "/srv")
          (http-port 8042))))
 
 (define %nginx-os
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd.
  2017-12-16 20:16 ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the test data for the nginx test Christopher Baines
@ 2017-12-16 20:17   ` Christopher Baines
  2017-12-18 10:10     ` Ludovic Courtès
  2017-12-18 10:02   ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the test data for the nginx test Ludovic Courtès
  1 sibling, 1 reply; 11+ messages in thread
From: Christopher Baines @ 2017-12-16 20:17 UTC (permalink / raw)
  To: 29741

* 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.
---
 gnu/services/web.scm | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 gnu/tests/web.scm    |  82 ++++++++++++++++++-
 2 files changed, 307 insertions(+), 2 deletions(-)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 582cf535c..8bfa1bc9b 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -33,8 +33,36 @@
   #:use-module ((guix utils) #:select (version-major))
   #:use-module ((guix packages) #:select (package-version))
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-9)
   #:use-module (ice-9 match)
-  #:export (<nginx-configuration>
+  #:export (<apache-httpd-configuration>
+            apache-httpd-configuration
+            apache-httpd-configuration?
+            apache-httpd-configuration-package
+            apache-httpd-configuration-pid-file
+            apache-httpd-configuration-config
+
+            <apache-httpd-virtualhost>
+            apache-httpd-virtualhost
+            apache-httpd-virtualhost?
+            apache-httpd-virtualhost-addresses-and-ports
+            apache-httpd-virtualhost-contents
+
+            <apache-httpd-config-file>
+            apache-httpd-config-file
+            apache-httpd-config-file?
+            apache-httpd-config-file-load-modules
+            apache-httpd-config-file-server-root
+            apache-httpd-config-file-server-name
+            apache-httpd-config-file-listen
+            apache-httpd-config-file-pid-file
+            apache-httpd-config-file-error-log
+            apache-httpd-config-file-user
+            apache-httpd-config-file-group
+
+            apache-httpd-service-type
+
+            <nginx-configuration>
             nginx-configuration
             nginx-configuration?
             nginx-configuartion-nginx
@@ -132,6 +160,203 @@
 ;;;
 ;;; Code:
 
+(define-record-type* <apache-httpd-load-module>
+  apache-httpd-load-module make-apache-httpd-load-module
+  apache-httpd-load-module?
+  (name apache-httpd-load-module-name)
+  (file apache-httpd-load-module-file))
+
+(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")
+         ("authz_groupfile_module" "modules/mod_authz_groupfile.so")
+         ("authz_user_module" "modules/mod_authz_user.so")
+         ("authz_core_module" "modules/mod_authz_core.so")
+         ("access_compat_module" "modules/mod_access_compat.so")
+         ("auth_basic_module" "modules/mod_auth_basic.so")
+         ("reqtimeout_module" "modules/mod_reqtimeout.so")
+         ("filter_module" "modules/mod_filter.so")
+         ("mime_module" "modules/mod_mime.so")
+         ("log_config_module" "modules/mod_log_config.so")
+         ("env_module" "modules/mod_env.so")
+         ("headers_module" "modules/mod_headers.so")
+         ("setenvif_module" "modules/mod_setenvif.so")
+         ("version_module" "modules/mod_version.so")
+         ("unixd_module" "modules/mod_unixd.so")
+         ("status_module" "modules/mod_status.so")
+         ("autoindex_module" "modules/mod_autoindex.so")
+         ("dir_module" "modules/mod_dir.so")
+         ("alias_module" "modules/mod_alias.so"))))
+
+(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))
+  (server-root    apache-httpd-config-file-server-root
+                  (default httpd))
+  (server-name    apache-httpd-config-file-server-name
+                  (default #f))
+  (document-root  apache-httpd-config-file-document-root
+                  (default "/srv/http"))
+  (listen         apache-httpd-config-file-listen
+                  (default '("80")))
+  (pid-file       apache-httpd-config-file-pid-file
+                  (default "/var/run/apache-httpd"))
+  (error-log      apache-httpd-config-file-error-log
+                  (default "/var/log/apache-httpd/error_log"))
+  (user           apache-httpd-config-file-user
+                  (default "apache-httpd"))
+  (group          apache-httpd-config-file-group
+                  (default "apache-httpd"))
+  (extra-config   apache-httpd-config-file-extra-config
+                  (default
+                    `("TypesConfig " ,(file-append httpd "/etc/httpd/mime.types") "\n"))))
+
+(define-gexp-compiler (apache-httpd-config-file-compiler
+                       (file <apache-httpd-config-file>) system target)
+  (match file
+    (($ <apache-httpd-config-file> load-modules server-root server-name
+                                   document-root listen pid-file error-log
+                                   user group extra-config)
+     (gexp->derivation
+      "httpd.conf"
+      #~(call-with-output-file (ungexp output "out")
+          (lambda (port)
+            (display
+             (string-append
+              (ungexp-splicing
+               `(,@(append-map
+                    (match-lambda
+                      (($ <apache-httpd-load-module> name module)
+                       `("LoadModule " ,name " " ,module "\n")))
+                    load-modules)
+                 ,@`("ServerRoot " ,server-root "\n")
+                 ,@(if server-name
+                       `("ServerName " ,server-name "\n")
+                       '())
+                 ,@`("DocumentRoot " ,document-root "\n")
+                 ,@(append-map
+                    (lambda (listen-value)
+                      `("Listen " ,listen-value "\n"))
+                    listen)
+                 ,@(if pid-file
+                       `("Pidfile " ,pid-file "\n")
+                       '())
+                 ,@(if error-log
+                       `("ErrorLog " ,error-log "\n")
+                       '())
+                 ,@(if user
+                       `("User " ,user "\n")
+                       '())
+                 ,@(if group
+                       `("Group " ,group "\n")
+                       '())
+                 "\n\n"
+                 ,@extra-config)))
+             port)))
+      #:local-build? #t))))
+
+(define-record-type <apache-httpd-virtualhost>
+  (apache-httpd-virtualhost addresses-and-ports contents)
+  apache-httpd-virtualhost?
+  (addresses-and-ports apache-httpd-virtualhost-addresses-and-ports)
+  (contents            apache-httpd-virtualhost-contents))
+
+(define-record-type* <apache-httpd-configuration>
+  apache-httpd-configuration make-apache-httpd-configuration
+  apache-httpd-configuration?
+  (package  apache-httpd-configuration-package
+            (default httpd))
+  (pid-file apache-httpd-configuration-pid-file
+            (default "/var/run/apache-httpd"))
+  (config   apache-httpd-configuration-config
+            (default (apache-httpd-config-file))))
+
+(define %apache-httpd-accounts
+  (list (user-group (name "apache-httpd") (system? #t))
+        (user-account
+         (name "apache-httpd")
+         (group "apache-httpd")
+         (system? #t)
+         (comment "Apache HTTPD server user")
+         (home-directory "/var/empty")
+         (shell (file-append shadow "/sbin/nologin")))))
+
+(define apache-httpd-shepherd-services
+  (match-lambda
+    (($ <apache-httpd-configuration> package pid-file config)
+     (list (shepherd-service
+            (provision '(apache-httpd))
+            (documentation "The Apache HTTP Server")
+            (requirement '(networking))
+            (start #~(make-forkexec-constructor
+                      `(#$(file-append package "/bin/httpd")
+                        #$@(if config
+                               (list "-f" config)
+                               '()))
+                      #:pid-file #$pid-file))
+            (stop #~(make-kill-destructor)))))))
+
+(define apache-httpd-activation
+  (match-lambda
+    (($ <apache-httpd-configuration> package pid-file config)
+     (match-record
+      config
+      <apache-httpd-config-file>
+      (error-log document-root)
+      #~(begin
+          (use-modules (guix build utils))
+
+          (mkdir-p #$(dirname error-log))
+          (mkdir-p #$document-root))))))
+
+(define (apache-httpd-process-extensions original-config extension-configs)
+  (let ((config (apache-httpd-configuration-config
+                 original-config)))
+    (if (apache-httpd-config-file? config)
+        (apache-httpd-configuration
+         (inherit original-config)
+         (config
+          (apache-httpd-config-file
+           (inherit config)
+           (extra-config
+            (append (apache-httpd-config-file-extra-config config)
+                    (append-map
+                     (match-lambda
+                       (($ <apache-httpd-virtualhost>
+                           addresses-and-ports
+                           contents)
+                        `(,(string-append
+                            "<VirtualHost " addresses-and-ports ">\n")
+                          ,@contents
+                          "\n</VirtualHost>\n"))
+                       ((? string? x)
+                        `("\n" ,x "\n"))
+                       ((? list? x)
+                        `("\n" ,@x "\n")))
+                     extension-configs)))))))))
+
+(define apache-httpd-service-type
+  (service-type (name 'apache-httpd)
+                (extensions
+                 (list (service-extension shepherd-root-service-type
+                                          apache-httpd-shepherd-services)
+                       (service-extension activation-service-type
+                                          apache-httpd-activation)
+                       (service-extension account-service-type
+                                          (const %apache-httpd-accounts))))
+                (compose concatenate)
+                (extend apache-httpd-process-extensions)
+                (default-value
+                  (apache-httpd-configuration))))
+
 (define-record-type* <nginx-server-configuration>
   nginx-server-configuration make-nginx-server-configuration
   nginx-server-configuration?
diff --git a/gnu/tests/web.scm b/gnu/tests/web.scm
index b84f072ac..1f4ab697c 100644
--- a/gnu/tests/web.scm
+++ b/gnu/tests/web.scm
@@ -28,7 +28,8 @@
   #:use-module (gnu services networking)
   #:use-module (guix gexp)
   #:use-module (guix store)
-  #:export (%test-nginx
+  #:export (%test-apache-httpd
+            %test-nginx
             %test-php-fpm))
 
 (define %index.html-contents
@@ -44,6 +45,85 @@
         (lambda (port)
           (display #$%index.html-contents port)))))
 
+(define %apache-httpd-os
+  ;; Operating system under test.
+  (simple-operating-system
+   (dhcp-client-service)
+   (service apache-httpd-service-type
+            (apache-httpd-configuration
+             (config (apache-httpd-config-file
+                      (listen '("8080"))))))
+   (simple-service 'make-http-root activation-service-type
+                   %make-http-root)))
+
+(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."
+  (define os
+    (marionette-operating-system
+     %apache-httpd-os
+     #:imported-modules '((gnu services herd)
+                          (guix combinators))))
+
+  (define vm
+    (virtual-machine
+     (operating-system os)
+     (port-forwardings `((,http-port . 8080)))))
+
+  (define test
+    (with-imported-modules '((gnu build marionette))
+      #~(begin
+          (use-modules (srfi srfi-11) (srfi srfi-64)
+                       (gnu build marionette)
+                       (web uri)
+                       (web client)
+                       (web response))
+
+          (define marionette
+            (make-marionette (list #$vm)))
+
+          (mkdir #$output)
+          (chdir #$output)
+
+          (test-begin "apache-httpd")
+
+          ;; Wait for apache-httpd to be up and running.
+          (test-assert "service running"
+            (marionette-eval
+             '(begin
+                (use-modules (gnu services herd))
+                (match (start-service 'apache-httpd)
+                  (#f #f)
+                  (('service response-parts ...)
+                   (match (assq-ref response-parts 'running)
+                     ((pid) (number? pid))))))
+             marionette))
+
+          ;; Retrieve the index.html file we put in /srv.
+          (test-equal "http-get"
+            '(200 #$%index.html-contents)
+            (let-values (((response text)
+                          (http-get "http://localhost:8080/index.html"
+                                    #:decode-body? #t)))
+              (list (response-code response) text)))
+
+          ;; There should be a log file in here.
+          (test-assert "log file"
+            (marionette-eval
+             '(file-exists? "/var/log/apache-httpd/error_log")
+             marionette))
+
+          (test-end)
+          (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
+
+  (gexp->derivation "apache-httpd-test" test))
+
+(define %test-apache-httpd
+  (system-test
+   (name "apache-httpd")
+   (description "Connect to a running APACHE-HTTPD server.")
+   (value (run-apache-httpd-test))))
+
 (define %nginx-servers
   ;; Server blocks.
   (list (nginx-server-configuration
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the test data for the nginx test.
  2017-12-16 20:16 ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the test data for the nginx test Christopher Baines
  2017-12-16 20:17   ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd Christopher Baines
@ 2017-12-18 10:02   ` Ludovic Courtès
  1 sibling, 0 replies; 11+ messages in thread
From: Ludovic Courtès @ 2017-12-18 10:02 UTC (permalink / raw)
  To: Christopher Baines; +Cc: 29741

Christopher Baines <mail@cbaines.net> skribis:

> So that it can also be used for other web servers.
>
> * gnu/tests/web.scm (%index.html-contents): Change nginx to guix.
>   (%make-http-root): Move the index.html file from /srv to /srv/http.
>   (%nginx-servers): Remove the setting of root.

OK!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd.
  2017-12-16 20:17   ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd Christopher Baines
@ 2017-12-18 10:10     ` Ludovic Courtès
  2017-12-19  8:03       ` Christopher Baines
                         ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Ludovic Courtès @ 2017-12-18 10:10 UTC (permalink / raw)
  To: Christopher Baines; +Cc: 29741

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’.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd.
  2017-12-18 10:10     ` Ludovic Courtès
@ 2017-12-19  8:03       ` Christopher Baines
  2017-12-24 18:01       ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the nginx test Christopher Baines
  2017-12-24 18:08       ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd Christopher Baines
  2 siblings, 0 replies; 11+ messages in thread
From: Christopher Baines @ 2017-12-19  8:03 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 29741

[-- Attachment #1: Type: text/plain, Size: 1988 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

>> +  #: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.

Ok.

>> +(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?

Yeah, I think I copied the configuration from the httpd.conf file in the
httpd package.

> How often do you expect this to be updated?

I'm not sure, but probably infrequently.

>> +(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”?

Yeah, I'll look at changing this to 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.

That sounds good, I'll take a look.

> Otherwise LGTM, thanks!

Thanks for reviewing :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the nginx test.
  2017-12-18 10:10     ` Ludovic Courtès
  2017-12-19  8:03       ` Christopher Baines
@ 2017-12-24 18:01       ` Christopher Baines
  2017-12-24 18:01         ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for httpd Christopher Baines
  2017-12-24 18:08       ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd Christopher Baines
  2 siblings, 1 reply; 11+ messages in thread
From: Christopher Baines @ 2017-12-24 18:01 UTC (permalink / raw)
  To: 29741

So that it can also be used for other web servers.

* gnu/tests/web.scm (%index.html-contents): Change nginx to guix.
  (%make-http-root): Move the index.html file from /srv to /srv/http.
  (%nginx-servers): Remove the setting of root.
  (run-nginx-test, run-webserver-test): Rename run-nginx-test to
  run-webserver-test and generalise its behaviour
  (%test-nginx): Change to use run-webserver-test, rather than run-nginx-test.
---
 gnu/tests/web.scm | 97 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 52 insertions(+), 45 deletions(-)

diff --git a/gnu/tests/web.scm b/gnu/tests/web.scm
index 336f25b3c..5595e9ddf 100644
--- a/gnu/tests/web.scm
+++ b/gnu/tests/web.scm
@@ -33,47 +33,33 @@
             %test-php-fpm))
 
 (define %index.html-contents
-  ;; Contents of the /index.html file served by nginx.
-  "Hello, nginx!")
+  ;; Contents of the /index.html file.
+  "Hello, guix!")
 
 (define %make-http-root
   ;; Create our server root in /srv.
   #~(begin
       (mkdir "/srv")
-      (call-with-output-file "/srv/index.html"
+      (mkdir "/srv/http")
+      (call-with-output-file "/srv/http/index.html"
         (lambda (port)
           (display #$%index.html-contents port)))))
 
-(define %nginx-servers
-  ;; Server blocks.
-  (list (nginx-server-configuration
-         (root "/srv")
-         (listen '("8042" "443 ssl")))))
-
-(define %nginx-os
-  ;; Operating system under test.
-  (simple-operating-system
-   (dhcp-client-service)
-   (service nginx-service-type
-            (nginx-configuration
-             (log-directory "/var/log/nginx")
-             (server-blocks %nginx-servers)))
-   (simple-service 'make-http-root activation-service-type
-                   %make-http-root)))
-
-(define* (run-nginx-test #:optional (http-port 8042))
+(define* (run-webserver-test name test-os #:key (log-file #f) (http-port 8080))
   "Run tests in %NGINX-OS, which has nginx running and listening on
 HTTP-PORT."
   (define os
     (marionette-operating-system
-     %nginx-os
+     test-os
      #:imported-modules '((gnu services herd)
                           (guix combinators))))
 
+  (define forwarded-port 8080)
+
   (define vm
     (virtual-machine
      (operating-system os)
-     (port-forwardings `((8080 . ,http-port)))))
+     (port-forwardings `((,http-port . ,forwarded-port)))))
 
   (define test
     (with-imported-modules '((gnu build marionette))
@@ -90,48 +76,69 @@ HTTP-PORT."
           (mkdir #$output)
           (chdir #$output)
 
-          (test-begin "nginx")
+          (test-begin #$name)
 
-          ;; Wait for nginx to be up and running.
-          (test-eq "service running"
-            'running!
+          (test-assert #$(string-append name " service running")
             (marionette-eval
              '(begin
                 (use-modules (gnu services herd))
-                (start-service 'nginx)
-                'running!)
-             marionette))
-
-          ;; Make sure the PID file is created.
-          (test-assert "PID file"
-            (marionette-eval
-             '(file-exists? "/var/run/nginx/pid")
+                (match (start-service '#$(string->symbol name))
+                  (#f #f)
+                  (('service response-parts ...)
+                   (match (assq-ref response-parts 'running)
+                     ((#t) #t)
+                     ((pid) (number? pid))))))
              marionette))
 
           ;; Retrieve the index.html file we put in /srv.
           (test-equal "http-get"
             '(200 #$%index.html-contents)
-            (let-values (((response text)
-                          (http-get "http://localhost:8080/index.html"
-                                    #:decode-body? #t)))
+            (let-values
+                (((response text)
+                  (http-get #$(simple-format
+                               #f "http://localhost:~A/index.html" forwarded-port)
+                            #:decode-body? #t)))
               (list (response-code response) text)))
 
-          ;; There should be a log file in here.
-          (test-assert "log file"
-            (marionette-eval
-             '(file-exists? "/var/log/nginx/access.log")
-             marionette))
+          #$@(if log-file
+                 `((test-assert ,(string-append "log file exists " log-file)
+                     (marionette-eval
+                      '(file-exists? ,log-file)
+                      marionette)))
+                 '())
 
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
 
-  (gexp->derivation "nginx-test" test))
+  (gexp->derivation (string-append name "-test") test))
+
+\f
+;;;
+;;; NGINX
+;;;
+
+(define %nginx-servers
+  ;; Server blocks.
+  (list (nginx-server-configuration
+         (listen '("8080")))))
+
+(define %nginx-os
+  ;; Operating system under test.
+  (simple-operating-system
+   (dhcp-client-service)
+   (service nginx-service-type
+            (nginx-configuration
+             (log-directory "/var/log/nginx")
+             (server-blocks %nginx-servers)))
+   (simple-service 'make-http-root activation-service-type
+                   %make-http-root)))
 
 (define %test-nginx
   (system-test
    (name "nginx")
    (description "Connect to a running NGINX server.")
-   (value (run-nginx-test))))
+   (value (run-webserver-test name %nginx-os
+                              #:log-file "/var/log/nginx/access.log"))))
 
 \f
 ;;;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#29741] [PATCH 2/2] gnu: services: web: Add service for httpd.
  2017-12-24 18:01       ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the nginx test Christopher Baines
@ 2017-12-24 18:01         ` Christopher Baines
  2017-12-16 20:12           ` [bug#29741] [PATCH] gnu: services: web: Add service for apache-httpd Christopher Baines
  0 siblings, 1 reply; 11+ messages in thread
From: Christopher Baines @ 2017-12-24 18:01 UTC (permalink / raw)
  To: 29741

* gnu/services/web.scm (<httpd-module>,
  <httpd-config-file>, <httpd-virtualhost>
  <httpd-configuration>): New record types.
  (%default-httpd-modules, %httpd-accounts,
  httpd-service-type): New variables.
  (httpd-shepherd-services, httpd-activation,
  httpd-process-extensions): New procedures.
* gnu/tests/web.scm (run-httpd-test): New procedure.
  (%httpd-os, %tests-httpd): New variables.
* doc/guix.texi (Web Services): Document the Apache HTTP Server.
---
 doc/guix.texi        | 158 ++++++++++++++++++++++++++++++++++-
 gnu/services/web.scm | 229 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 gnu/tests/web.scm    |  26 +++++-
 3 files changed, 409 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index cb6754a4b..21b6ce003 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -14814,8 +14814,162 @@ Local accounts with lower values will silently fail to authenticate.
 @cindex web
 @cindex www
 @cindex HTTP
-The @code{(gnu services web)} module provides the nginx web server and
-also a fastcgi wrapper daemon.
+The @code{(gnu services web)} module provides the Apache HTTP Server,
+the nginx web server, and also a fastcgi wrapper daemon.
+
+@subsubheading Apache HTTP Server
+
+@deffn {Scheme Variable} httpd-service-type
+Service type for the @uref{https://httpd.apache.org/,Apache HTTP}
+server.  The value for this service type is a @code{https-configuration}
+record.
+
+A simple example configuration is given below.
+
+@example
+(service httpd-service-type
+         (httpd-configuration
+           (config
+             (httpd-config-file
+               (server-name "www.example.com")
+               (document-root "/srv/http/www.example.com")))))
+@end example
+
+Other services can also extend the @code{httpd-service-type} to add to
+the configuration.
+
+@example
+(simple-service 'my-extra-server httpd-service-type
+                (list
+                  (httpd-virtualhost
+                    "*:80"
+                    (list (string-append
+                           "ServerName "www.example.com
+                            DocumentRoot \"/srv/http/www.example.com\"")))))
+@end example
+@end deffn
+
+The details for the @code{httpd-configuration}, @code{httpd-module},
+@code{httpd-config-file} and @code{httpd-virtualhost} record types are
+given below.
+
+@deffn {Data Type} httpd-configuration
+This data type represents the configuration for the httpd service.
+
+@table @asis
+@item @code{package} (default: @code{httpd})
+The httpd package to use.
+
+@item @code{pid-file} (default: @code{"/var/run/httpd"})
+The pid file used by the shepherd-service.
+
+@item @code{config} (default: @code{(httpd-config-file)})
+The configuration file to use with the httpd service. The default value
+is a @code{httpd-config-file} record, but this can also be a different
+G-expression that generates a file, for example a @code{plain-file}. A
+file outside of the store can also be specified through a string.
+
+@end table
+@end deffn
+
+@deffn {Data Type} httpd-module
+This data type represents a module for the httpd service.
+
+@table @asis
+@item @code{name}
+The name of the module.
+
+@item @code{file}
+The file for the module. This can be relative to the httpd package being
+used, the absolute location of a file, or a G-expression for a file
+within the store, for example @code{(file-append mod-wsgi
+"/modules/mod_wsgi.so")}.
+
+@end table
+@end deffn
+
+@deffn {Data Type} httpd-config-file
+This data type represents a configuration file for the httpd service.
+
+@table @asis
+@item @code{modules} (default: @code{%default-httpd-modules})
+The modules to load. Additional modules can be added here, or loaded by
+additional configuration.
+
+@item @code{server-root} (default: @code{httpd})
+The @code{ServerRoot} in the configuration file, defaults to the httpd
+package. Directives including @code{Include} and @code{LoadModule} are
+taken as relative to the server root.
+
+@item @code{server-name} (default: @code{#f})
+The @code{ServerName} in the configuration file, used to specify the
+request scheme, hostname and port that the server uses to identify
+itself.
+
+This doesn't need to be set in the server config, and can be specifyed
+in virtual hosts. The default is @code{#f} to not specify a
+@code{ServerName}.
+
+@item @code{document-root} (default: @code{"/srv/http"})
+The @code{DocumentRoot} from which files will be served.
+
+@item @code{listen} (default: @code{'("80")})
+The list of values for the @code{Listen} directives in the config
+file. The value should be a list of strings, when each string can
+specify the port number to listen on, and optionally the IP address and
+protocol to use.
+
+@item @code{pid-file} (default: @code{"/var/run/httpd"})
+The @code{PidFile} to use. This should match the @code{pid-file} set in
+the @code{httpd-configuration} so that the Shepherd service is
+configured correctly.
+
+@item @code{error-log} (default: @code{"/var/log/httpd/error_log"})
+The @code{ErrorLog} to which the server will log errors.
+
+@item @code{user} (default: @code{"httpd"})
+The @code{User} which the server will answer requests as.
+
+@item @code{group} (default: @code{"httpd"})
+The @code{Group} which the server will answer requests as.
+
+@item @code{extra-config} (default: @code{(list "TypesConfig etc/httpd/mime.types")})
+A flat list of strings and G-expressions which will be added to the end
+of the configuration file.
+
+Any values which the service is extended with will be appended to this
+list.
+
+@end table
+@end deffn
+
+@deffn {Data Type} httpd-virtualhost
+This data type represents a virtualhost configuration block for the httpd service.
+
+These should be added to the extra-config for the httpd-service.
+
+@example
+(simple-service 'my-extra-server httpd-service-type
+                (list
+                  (httpd-virtualhost
+                    "*:80"
+                    (list (string-append
+                           "ServerName "www.example.com
+                            DocumentRoot \"/srv/http/www.example.com\"")))))
+@end example
+
+@table @asis
+@item @code{addresses-and-ports}
+The addresses and ports for the @code{VirtualHost} directive.
+
+@item @code{contents}
+The contents of the @code{VirtualHost} directive, this should be a list
+of strings and G-expressions.
+
+@end table
+@end deffn
+
+@subsubheading NGINX
 
 @deffn {Scheme Variable} nginx-service-type
 Service type for the @uref{https://nginx.org/,NGinx} web server.  The
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 2371ddb6d..c1ffe3e05 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -34,8 +34,36 @@
   #:use-module ((guix utils) #:select (version-major))
   #:use-module ((guix packages) #:select (package-version))
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-9)
   #:use-module (ice-9 match)
-  #:export (<nginx-configuration>
+  #:export (<httpd-configuration>
+            httpd-configuration
+            httpd-configuration?
+            httpd-configuration-package
+            httpd-configuration-pid-file
+            httpd-configuration-config
+
+            <httpd-virtualhost>
+            httpd-virtualhost
+            httpd-virtualhost?
+            httpd-virtualhost-addresses-and-ports
+            httpd-virtualhost-contents
+
+            <httpd-config-file>
+            httpd-config-file
+            httpd-config-file?
+            httpd-config-file-modules
+            httpd-config-file-server-root
+            httpd-config-file-server-name
+            httpd-config-file-listen
+            httpd-config-file-pid-file
+            httpd-config-file-error-log
+            httpd-config-file-user
+            httpd-config-file-group
+
+            httpd-service-type
+
+            <nginx-configuration>
             nginx-configuration
             nginx-configuration?
             nginx-configuartion-nginx
@@ -133,6 +161,205 @@
 ;;;
 ;;; Code:
 
+(define-record-type* <httpd-module>
+  httpd-module make-httpd-module
+  httpd-module?
+  (name httpd-load-module-name)
+  (file httpd-load-module-file))
+
+;; Default modules for the httpd-service-type, taken from etc/httpd/httpd.conf
+;; file in the httpd package.
+(define %default-httpd-modules
+  (map (match-lambda
+         ((name file)
+          (httpd-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")
+         ("authz_groupfile_module" "modules/mod_authz_groupfile.so")
+         ("authz_user_module" "modules/mod_authz_user.so")
+         ("authz_core_module" "modules/mod_authz_core.so")
+         ("access_compat_module" "modules/mod_access_compat.so")
+         ("auth_basic_module" "modules/mod_auth_basic.so")
+         ("reqtimeout_module" "modules/mod_reqtimeout.so")
+         ("filter_module" "modules/mod_filter.so")
+         ("mime_module" "modules/mod_mime.so")
+         ("log_config_module" "modules/mod_log_config.so")
+         ("env_module" "modules/mod_env.so")
+         ("headers_module" "modules/mod_headers.so")
+         ("setenvif_module" "modules/mod_setenvif.so")
+         ("version_module" "modules/mod_version.so")
+         ("unixd_module" "modules/mod_unixd.so")
+         ("status_module" "modules/mod_status.so")
+         ("autoindex_module" "modules/mod_autoindex.so")
+         ("dir_module" "modules/mod_dir.so")
+         ("alias_module" "modules/mod_alias.so"))))
+
+(define-record-type* <httpd-config-file>
+  httpd-config-file make-httpd-config-file
+  httpd-config-file?
+  (modules        httpd-config-file-modules
+                  (default %default-httpd-modules))
+  (server-root    httpd-config-file-server-root
+                  (default httpd))
+  (server-name    httpd-config-file-server-name
+                  (default #f))
+  (document-root  httpd-config-file-document-root
+                  (default "/srv/http"))
+  (listen         httpd-config-file-listen
+                  (default '("80")))
+  (pid-file       httpd-config-file-pid-file
+                  (default "/var/run/httpd"))
+  (error-log      httpd-config-file-error-log
+                  (default "/var/log/httpd/error_log"))
+  (user           httpd-config-file-user
+                  (default "httpd"))
+  (group          httpd-config-file-group
+                  (default "httpd"))
+  (extra-config   httpd-config-file-extra-config
+                  (default
+                    (list "TypesConfig etc/httpd/mime.types"))))
+
+(define-gexp-compiler (httpd-config-file-compiler
+                       (file <httpd-config-file>) system target)
+  (match file
+    (($ <httpd-config-file> load-modules server-root server-name
+                                   document-root listen pid-file error-log
+                                   user group extra-config)
+     (gexp->derivation
+      "httpd.conf"
+      #~(call-with-output-file (ungexp output "out")
+          (lambda (port)
+            (display
+             (string-append
+              (ungexp-splicing
+               `(,@(append-map
+                    (match-lambda
+                      (($ <httpd-module> name module)
+                       `("LoadModule " ,name " " ,module "\n")))
+                    load-modules)
+                 ,@`("ServerRoot " ,server-root "\n")
+                 ,@(if server-name
+                       `("ServerName " ,server-name "\n")
+                       '())
+                 ,@`("DocumentRoot " ,document-root "\n")
+                 ,@(append-map
+                    (lambda (listen-value)
+                      `("Listen " ,listen-value "\n"))
+                    listen)
+                 ,@(if pid-file
+                       `("Pidfile " ,pid-file "\n")
+                       '())
+                 ,@(if error-log
+                       `("ErrorLog " ,error-log "\n")
+                       '())
+                 ,@(if user
+                       `("User " ,user "\n")
+                       '())
+                 ,@(if group
+                       `("Group " ,group "\n")
+                       '())
+                 "\n\n"
+                 ,@extra-config)))
+             port)))
+      #:local-build? #t))))
+
+(define-record-type <httpd-virtualhost>
+  (httpd-virtualhost addresses-and-ports contents)
+  httpd-virtualhost?
+  (addresses-and-ports httpd-virtualhost-addresses-and-ports)
+  (contents            httpd-virtualhost-contents))
+
+(define-record-type* <httpd-configuration>
+  httpd-configuration make-httpd-configuration
+  httpd-configuration?
+  (package  httpd-configuration-package
+            (default httpd))
+  (pid-file httpd-configuration-pid-file
+            (default "/var/run/httpd"))
+  (config   httpd-configuration-config
+            (default (httpd-config-file))))
+
+(define %httpd-accounts
+  (list (user-group (name "httpd") (system? #t))
+        (user-account
+         (name "httpd")
+         (group "httpd")
+         (system? #t)
+         (comment "Apache HTTPD server user")
+         (home-directory "/var/empty")
+         (shell (file-append shadow "/sbin/nologin")))))
+
+(define httpd-shepherd-services
+  (match-lambda
+    (($ <httpd-configuration> package pid-file config)
+     (list (shepherd-service
+            (provision '(httpd))
+            (documentation "The Apache HTTP Server")
+            (requirement '(networking))
+            (start #~(make-forkexec-constructor
+                      `(#$(file-append package "/bin/httpd")
+                        #$@(if config
+                               (list "-f" config)
+                               '()))
+                      #:pid-file #$pid-file))
+            (stop #~(make-kill-destructor)))))))
+
+(define httpd-activation
+  (match-lambda
+    (($ <httpd-configuration> package pid-file config)
+     (match-record
+      config
+      <httpd-config-file>
+      (error-log document-root)
+      #~(begin
+          (use-modules (guix build utils))
+
+          (mkdir-p #$(dirname error-log))
+          (mkdir-p #$document-root))))))
+
+(define (httpd-process-extensions original-config extension-configs)
+  (let ((config (httpd-configuration-config
+                 original-config)))
+    (if (httpd-config-file? config)
+        (httpd-configuration
+         (inherit original-config)
+         (config
+          (httpd-config-file
+           (inherit config)
+           (extra-config
+            (append (httpd-config-file-extra-config config)
+                    (append-map
+                     (match-lambda
+                       (($ <httpd-virtualhost>
+                           addresses-and-ports
+                           contents)
+                        `(,(string-append
+                            "<VirtualHost " addresses-and-ports ">\n")
+                          ,@contents
+                          "\n</VirtualHost>\n"))
+                       ((? string? x)
+                        `("\n" ,x "\n"))
+                       ((? list? x)
+                        `("\n" ,@x "\n")))
+                     extension-configs)))))))))
+
+(define httpd-service-type
+  (service-type (name 'httpd)
+                (extensions
+                 (list (service-extension shepherd-root-service-type
+                                          httpd-shepherd-services)
+                       (service-extension activation-service-type
+                                          httpd-activation)
+                       (service-extension account-service-type
+                                          (const %httpd-accounts))))
+                (compose concatenate)
+                (extend httpd-process-extensions)
+                (default-value
+                  (httpd-configuration))))
+
 (define-record-type* <nginx-server-configuration>
   nginx-server-configuration make-nginx-server-configuration
   nginx-server-configuration?
diff --git a/gnu/tests/web.scm b/gnu/tests/web.scm
index 5595e9ddf..1912f8f79 100644
--- a/gnu/tests/web.scm
+++ b/gnu/tests/web.scm
@@ -29,7 +29,8 @@
   #:use-module (gnu services networking)
   #:use-module (guix gexp)
   #:use-module (guix store)
-  #:export (%test-nginx
+  #:export (%test-httpd
+            %test-nginx
             %test-php-fpm))
 
 (define %index.html-contents
@@ -113,6 +114,29 @@ HTTP-PORT."
   (gexp->derivation (string-append name "-test") test))
 
 \f
+;;;
+;;; HTTPD
+;;;
+
+(define %httpd-os
+  (simple-operating-system
+   (dhcp-client-service)
+   (service httpd-service-type
+            (httpd-configuration
+             (config
+              (httpd-config-file
+               (listen '("8080"))))))
+   (simple-service 'make-http-root activation-service-type
+                   %make-http-root)))
+
+(define %test-httpd
+  (system-test
+   (name "httpd")
+   (description "Connect to a running HTTPD server.")
+   (value (run-webserver-test name %httpd-os
+                              #:log-file "/var/log/httpd/error_log"))))
+
+\f
 ;;;
 ;;; NGINX
 ;;;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd.
  2017-12-18 10:10     ` Ludovic Courtès
  2017-12-19  8:03       ` Christopher Baines
  2017-12-24 18:01       ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the nginx test Christopher Baines
@ 2017-12-24 18:08       ` Christopher Baines
  2 siblings, 0 replies; 11+ messages in thread
From: Christopher Baines @ 2017-12-24 18:08 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 29741

[-- Attachment #1: Type: text/plain, Size: 2882 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> 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.  :-)

Great, I've sent a couple of updated patches, which now include
documentation.

>> +  #: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.

Ok, I've changed the prefix to 'httpd-'.

>> +(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?

I got this from the etc/httpd/httpd.conf file in the httpd package, and
I've now added a comment in to the code saying this. I don't expect it
will need updating often.

>> +(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”?

I've gone with modules, and changed this field and the corresponding
record type.

>> +(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.

That sounds good to me. I've now updated the first patch to generalise
the test, so that it can be used for both the nginx and httpd service.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#29741] [PATCH 2/2] gnu: services: web: Add service for httpd.
  2017-12-16 20:12           ` [bug#29741] [PATCH] gnu: services: web: Add service for apache-httpd Christopher Baines
@ 2018-01-11  9:46             ` Ludovic Courtès
  2018-01-17  8:45               ` bug#29741: " Christopher Baines
  0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2018-01-11  9:46 UTC (permalink / raw)
  To: Christopher Baines; +Cc: 29741

Hi Chris!

Sorry for the delay.

Christopher Baines <mail@cbaines.net> skribis:

> * gnu/services/web.scm (<httpd-module>,
>   <httpd-config-file>, <httpd-virtualhost>
>   <httpd-configuration>): New record types.
>   (%default-httpd-modules, %httpd-accounts,
>   httpd-service-type): New variables.
>   (httpd-shepherd-services, httpd-activation,
>   httpd-process-extensions): New procedures.
> * gnu/tests/web.scm (run-httpd-test): New procedure.
>   (%httpd-os, %tests-httpd): New variables.
> * doc/guix.texi (Web Services): Document the Apache HTTP Server.

[...]

> +The @code{(gnu services web)} module provides the Apache HTTP Server,
> +the nginx web server, and also a fastcgi wrapper daemon.
> +
> +@subsubheading Apache HTTP Server
> +
> +@deffn {Scheme Variable} httpd-service-type
> +Service type for the @uref{https://httpd.apache.org/,Apache HTTP}
> +server.  The value for this service type is a @code{https-configuration}
        ^
Maybe add “(@dfn{httpd})”.

Apart from this super-nitpicky comment, this LGTM!  :-)

Thank you,
Ludo’.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#29741: [PATCH 2/2] gnu: services: web: Add service for httpd.
  2018-01-11  9:46             ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for httpd Ludovic Courtès
@ 2018-01-17  8:45               ` Christopher Baines
  0 siblings, 0 replies; 11+ messages in thread
From: Christopher Baines @ 2018-01-17  8:45 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 29741-done

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> Hi Chris!
>
> Sorry for the delay.
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> * gnu/services/web.scm (<httpd-module>,
>>   <httpd-config-file>, <httpd-virtualhost>
>>   <httpd-configuration>): New record types.
>>   (%default-httpd-modules, %httpd-accounts,
>>   httpd-service-type): New variables.
>>   (httpd-shepherd-services, httpd-activation,
>>   httpd-process-extensions): New procedures.
>> * gnu/tests/web.scm (run-httpd-test): New procedure.
>>   (%httpd-os, %tests-httpd): New variables.
>> * doc/guix.texi (Web Services): Document the Apache HTTP Server.
>
> [...]
>
>> +The @code{(gnu services web)} module provides the Apache HTTP Server,
>> +the nginx web server, and also a fastcgi wrapper daemon.
>> +
>> +@subsubheading Apache HTTP Server
>> +
>> +@deffn {Scheme Variable} httpd-service-type
>> +Service type for the @uref{https://httpd.apache.org/,Apache HTTP}
>> +server.  The value for this service type is a @code{https-configuration}
>         ^
> Maybe add “(@dfn{httpd})”.
>
> Apart from this super-nitpicky comment, this LGTM!  :-)

Great, I've made that change and pushed. Thanks for reviewing Ludo :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-01-17  8:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <handler.29741.B.15134551667792.ack@debbugs.gnu.org>
2017-12-16 20:16 ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the test data for the nginx test Christopher Baines
2017-12-16 20:17   ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd Christopher Baines
2017-12-18 10:10     ` Ludovic Courtès
2017-12-19  8:03       ` Christopher Baines
2017-12-24 18:01       ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the nginx test Christopher Baines
2017-12-24 18:01         ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for httpd Christopher Baines
2017-12-16 20:12           ` [bug#29741] [PATCH] gnu: services: web: Add service for apache-httpd Christopher Baines
2018-01-11  9:46             ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for httpd Ludovic Courtès
2018-01-17  8:45               ` bug#29741: " Christopher Baines
2017-12-24 18:08       ` [bug#29741] [PATCH 2/2] gnu: services: web: Add service for apache-httpd Christopher Baines
2017-12-18 10:02   ` [bug#29741] [PATCH 1/2] gnu: tests: web: Generalise the test data for the nginx test Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).