GNU bug report logs - #62490
[PATCH] services: nginx: Make logging level configurable.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 62490 in the body.
You can then email your comments to 62490 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: guix-patches <at> gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH] services: nginx: Make logging level configurable.
Date: Mon, 27 Mar 2023 19:51:04 +0100

* gnu/services/web.scm (<nginx-configuration>)[log-level]: New field.
(assert-valid-log-level): New procedure.
(default-nginx-config): Make log-level configurable.
* doc/guix.texi (Web Services): Document it.
---

Note: Hardcoding this value to 'info is extremely bad for
flash endurance, and results in very large logs.

 doc/guix.texi        |  5 +++++
 gnu/services/web.scm | 24 +++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index c49e51b72e..8df3c285ad 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -29838,6 +29838,11 @@ Web Services
 @item @code{log-directory} (default: @code{"/var/log/nginx"})
 The directory to which NGinx will write log files.
 
+@item @code{log-level} (default: @code{'error}) (type: symbol)
+Logging level, which can be any of the following values: @code{'debug},
+@code{'info}, @code{'notice}, @code{'warn}, @code{'error}, @code{'crit},
+@code{'alert}, or @code{'emerg}.
+
 @item @code{run-directory} (default: @code{"/var/run/nginx"})
 The directory in which NGinx will create a pid file, and write temporary
 files.
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index d56e893527..aef694e145 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -51,6 +51,9 @@ (define-module (gnu services web)
   #:use-module (gnu packages logging)
   #:use-module (gnu packages mail)
   #:use-module (gnu packages rust-apps)
+  #:autoload   (guix i18n) (G_)
+  #:use-module (guix combinators)
+  #:use-module (guix diagnostics)
   #:use-module (guix packages)
   #:use-module (guix records)
   #:use-module (guix modules)
@@ -61,6 +64,8 @@ (define-module (gnu services web)
   #:use-module ((guix packages) #:select (package-version))
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-9)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
   #:export (httpd-configuration
@@ -96,6 +101,7 @@ (define-module (gnu services web)
             nginx-configuration-nginx
             nginx-configuration-shepherd-requirement
             nginx-configuration-log-directory
+            nginx-configuration-log-level
             nginx-configuration-run-directory
             nginx-configuration-server-blocks
             nginx-configuration-upstream-blocks
@@ -562,6 +568,9 @@ (define-record-type* <nginx-configuration>
                         (default '()))              ;list of symbols
   (log-directory nginx-configuration-log-directory  ;string
                  (default "/var/log/nginx"))
+  (log-level     nginx-configuration-log-level
+                 (sanitize assert-valid-log-level)
+                 (default 'error))
   (run-directory nginx-configuration-run-directory  ;string
                  (default "/var/run/nginx"))
   (server-blocks nginx-configuration-server-blocks
@@ -584,6 +593,18 @@ (define-record-type* <nginx-configuration>
   (file          nginx-configuration-file         ;#f | string | file-like
                  (default #f)))
 
+(define-compile-time-procedure (assert-valid-log-level (level symbol?))
+  "Ensure @var{level} is one of @code{'debug}, @code{'info}, @code{'notice},
+@code{'warn}, @code{'error}, @code{'crit}, @code{'alert}, or @code{'emerg}."
+  (unless (memq level '(debug info notice warn error crit alert emerg))
+    (raise
+     (make-compound-condition
+      (formatted-message (G_ "unknown log level '~a'") level)
+      (condition (&error-location
+                  (location
+                   (source-properties->location procedure-call-location)))))))
+  level)
+
 (define (config-domain-strings names)
  "Return a string denoting the nginx config representation of NAMES, a list
 of domain names."
@@ -692,6 +713,7 @@ (define (default-nginx-config config)
   (match-record config
                 <nginx-configuration>
                 (nginx log-directory run-directory
+                 log-level
                  server-blocks upstream-blocks
                  server-names-hash-bucket-size
                  server-names-hash-bucket-max-size
@@ -704,7 +726,7 @@ (define (default-nginx-config config)
           (flatten
            "user nginx nginx;\n"
            "pid " run-directory "/pid;\n"
-           "error_log " log-directory "/error.log info;\n"
+           "error_log " log-directory "/error.log " (symbol->string log-level) ";\n"
            (map emit-load-module modules)
            (map emit-global-directive global-directives)
            "http {\n"
-- 
2.39.1

Message #8 received at 62490 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 62490 <at> debbugs.gnu.org, ludo <at> gnu.org, mail <at> cbaines.net
Subject: Re: [bug#62490] [PATCH] services: nginx: Make logging level
 configurable.
Date: Tue, 28 Mar 2023 10:12:30 -0400

Hello,

Bruno Victal <mirai <at> makinata.eu> writes:

> * gnu/services/web.scm (<nginx-configuration>)[log-level]: New field.
> (assert-valid-log-level): New procedure.
> (default-nginx-config): Make log-level configurable.
> * doc/guix.texi (Web Services): Document it.

Thanks!

> ---
>
> Note: Hardcoding this value to 'info is extremely bad for
> flash endurance, and results in very large logs.
>
>  doc/guix.texi        |  5 +++++
>  gnu/services/web.scm | 24 +++++++++++++++++++++++-
>  2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index c49e51b72e..8df3c285ad 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -29838,6 +29838,11 @@ Web Services
>  @item @code{log-directory} (default: @code{"/var/log/nginx"})
>  The directory to which NGinx will write log files.
>  
> +@item @code{log-level} (default: @code{'error}) (type: symbol)
> +Logging level, which can be any of the following values: @code{'debug},
> +@code{'info}, @code{'notice}, @code{'warn}, @code{'error}, @code{'crit},
> +@code{'alert}, or @code{'emerg}.
> +
>  @item @code{run-directory} (default: @code{"/var/run/nginx"})
>  The directory in which NGinx will create a pid file, and write temporary
>  files.
> diff --git a/gnu/services/web.scm b/gnu/services/web.scm
> index d56e893527..aef694e145 100644
> --- a/gnu/services/web.scm
> +++ b/gnu/services/web.scm
> @@ -51,6 +51,9 @@ (define-module (gnu services web)
>    #:use-module (gnu packages logging)
>    #:use-module (gnu packages mail)
>    #:use-module (gnu packages rust-apps)
> +  #:autoload   (guix i18n) (G_)
> +  #:use-module (guix combinators)
> +  #:use-module (guix diagnostics)
>    #:use-module (guix packages)
>    #:use-module (guix records)
>    #:use-module (guix modules)
> @@ -61,6 +64,8 @@ (define-module (gnu services web)
>    #:use-module ((guix packages) #:select (package-version))
>    #:use-module (srfi srfi-1)
>    #:use-module (srfi srfi-9)
> +  #:use-module (srfi srfi-34)
> +  #:use-module (srfi srfi-35)
>    #:use-module (ice-9 match)
>    #:use-module (ice-9 format)
>    #:export (httpd-configuration
> @@ -96,6 +101,7 @@ (define-module (gnu services web)
>              nginx-configuration-nginx
>              nginx-configuration-shepherd-requirement
>              nginx-configuration-log-directory
> +            nginx-configuration-log-level
>              nginx-configuration-run-directory
>              nginx-configuration-server-blocks
>              nginx-configuration-upstream-blocks
> @@ -562,6 +568,9 @@ (define-record-type* <nginx-configuration>
>                          (default '()))              ;list of symbols
>    (log-directory nginx-configuration-log-directory  ;string
>                   (default "/var/log/nginx"))
> +  (log-level     nginx-configuration-log-level
> +                 (sanitize assert-valid-log-level)
> +                 (default 'error))
>    (run-directory nginx-configuration-run-directory  ;string
>                   (default "/var/run/nginx"))
>    (server-blocks nginx-configuration-server-blocks
> @@ -584,6 +593,18 @@ (define-record-type* <nginx-configuration>
>    (file          nginx-configuration-file         ;#f | string | file-like
>                   (default #f)))
>  
> +(define-compile-time-procedure (assert-valid-log-level (level symbol?))
> +  "Ensure @var{level} is one of @code{'debug}, @code{'info}, @code{'notice},
> +@code{'warn}, @code{'error}, @code{'crit}, @code{'alert}, or @code{'emerg}."
> +  (unless (memq level '(debug info notice warn error crit alert emerg))
> +    (raise
> +     (make-compound-condition
> +      (formatted-message (G_ "unknown log level '~a'") level)
> +      (condition (&error-location
> +                  (location
> +                   (source-properties->location procedure-call-location)))))))
> +  level)

It's the first time I've seen define-compile-time-procedure in actual
use.  Is it really necessary?  What happens if you omit wrapping
assert-valid-log-level with it?

>  (define (config-domain-strings names)
>   "Return a string denoting the nginx config representation of NAMES, a list
>  of domain names."
> @@ -692,6 +713,7 @@ (define (default-nginx-config config)
>    (match-record config
>                  <nginx-configuration>
>                  (nginx log-directory run-directory
> +                 log-level
>                   server-blocks upstream-blocks
>                   server-names-hash-bucket-size
>                   server-names-hash-bucket-max-size
> @@ -704,7 +726,7 @@ (define (default-nginx-config config)
>            (flatten
>             "user nginx nginx;\n"
>             "pid " run-directory "/pid;\n"
> -           "error_log " log-directory "/error.log info;\n"
> +           "error_log " log-directory "/error.log " (symbol->string log-level) ";\n"
>             (map emit-load-module modules)
>             (map emit-global-directive global-directives)
>             "http {\n"

The rest LGTM.

-- 
Thanks,
Maxim

Message #11 received at 62490 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 62490 <at> debbugs.gnu.org, ludo <at> gnu.org, mail <at> cbaines.net
Subject: Re: [bug#62490] [PATCH] services: nginx: Make logging level
 configurable.
Date: Tue, 28 Mar 2023 15:32:56 +0100

Hi Maxim,

On 2023-03-28 15:12, Maxim Cournoyer wrote:
> Bruno Victal <mirai <at> makinata.eu> writes:
>>  
>> +(define-compile-time-procedure (assert-valid-log-level (level symbol?))
>> +  "Ensure @var{level} is one of @code{'debug}, @code{'info}, @code{'notice},
>> +@code{'warn}, @code{'error}, @code{'crit}, @code{'alert}, or @code{'emerg}."
>> +  (unless (memq level '(debug info notice warn error crit alert emerg))
>> +    (raise
>> +     (make-compound-condition
>> +      (formatted-message (G_ "unknown log level '~a'") level)
>> +      (condition (&error-location
>> +                  (location
>> +                   (source-properties->location procedure-call-location)))))))
>> +  level)
> 
> It's the first time I've seen define-compile-time-procedure in actual
> use.  Is it really necessary?  What happens if you omit wrapping
> assert-valid-log-level with it?

It will still work, provided the declaration is adjusted accordingly.
As for the reasons and benefits of using define-compile-time-procedure,
it's best explained at <https://logs.guix.gnu.org/guix/2023-03-20.log#131047>.


Cheers,
Bruno

Message #14 received at 62490 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 62490 <at> debbugs.gnu.org, ludo <at> gnu.org, mail <at> cbaines.net
Subject: Re: [bug#62490] [PATCH] services: nginx: Make logging level
 configurable.
Date: Tue, 28 Mar 2023 11:03:45 -0400

OK,

Bruno Victal <mirai <at> makinata.eu> writes:

> Hi Maxim,
>
> On 2023-03-28 15:12, Maxim Cournoyer wrote:
>> Bruno Victal <mirai <at> makinata.eu> writes:
>>>  
>>> +(define-compile-time-procedure (assert-valid-log-level (level symbol?))
>>> +  "Ensure @var{level} is one of @code{'debug}, @code{'info}, @code{'notice},
>>> +@code{'warn}, @code{'error}, @code{'crit}, @code{'alert}, or @code{'emerg}."
>>> +  (unless (memq level '(debug info notice warn error crit alert emerg))
>>> +    (raise
>>> +     (make-compound-condition
>>> +      (formatted-message (G_ "unknown log level '~a'") level)
>>> +      (condition (&error-location
>>> +                  (location
>>> +                   (source-properties->location procedure-call-location)))))))
>>> +  level)
>> 
>> It's the first time I've seen define-compile-time-procedure in actual
>> use.  Is it really necessary?  What happens if you omit wrapping
>> assert-valid-log-level with it?
>
> It will still work, provided the declaration is adjusted accordingly.
> As for the reasons and benefits of using define-compile-time-procedure,
> it's best explained at <https://logs.guix.gnu.org/guix/2023-03-20.log#131047>.

I guess it's not actually useful here, since none of the fields thunked?

As another note, the nginx-configuration record looks simple enough that
perhaps it'd best be migrated to use the define-configuration method,
which could be made as a prior commit to this change.

As a benefit it'd validate the other field types as well.

-- 
Thanks,
Maxim

Message #17 received at 62490 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 62490 <at> debbugs.gnu.org, mail <at> cbaines.net, maxim.cournoyer <at> gmail.com
Subject: Re: bug#62490: [PATCH] services: nginx: Make logging level
 configurable.
Date: Sun, 02 Apr 2023 17:39:41 +0200

Hi,

Bruno Victal <mirai <at> makinata.eu> skribis:

> +(define-compile-time-procedure (assert-valid-log-level (level symbol?))
> +  "Ensure @var{level} is one of @code{'debug}, @code{'info}, @code{'notice},
> +@code{'warn}, @code{'error}, @code{'crit}, @code{'alert}, or @code{'emerg}."

As it turns out, ‘define-compile-time-procedure’ cannot work with
symbols.  In short, that’s because in the end the generated macro
checks:

  (symbol? #'(quote debug))

which doesn’t do what we want.

Anyway, you can either make it a regular procedure instead, or use a
trick found in R6RS and used in some places in Guix, Guile-Gcrypt, etc.,
which is to define a macro that validates things:

  (endianness little)  ;R6RS

  (operating-id valid-path?) ;(guix store), with ‘define-enumerate-type’

Making it a procedure is prolly good enough.  The compiler can optimize
it out at compile time, FWIW:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,optimize (unless (memq 'debug '(debug info)) (throw 'x))
$13 = (if #f #f)
--8<---------------cut here---------------end--------------->8---

Ludo’.

Message #20 received at 62490 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 62490 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>, maxim.cournoyer <at> gmail.com
Subject: [PATCH v2 1/2] services: nginx: Make logging level configurable.
Date: Mon,  3 Apr 2023 12:58:02 +0100

* gnu/services/web.scm (<nginx-configuration>)[log-level]: New field.
(assert-valid-log-level): New procedure.
(default-nginx-config): Make log-level configurable.
* doc/guix.texi (Web Services): Document it.
---
 doc/guix.texi        |  5 +++++
 gnu/services/web.scm | 19 ++++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 4f72e2f34a..6a2380aa3e 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -29838,6 +29838,11 @@ Web Services
 @item @code{log-directory} (default: @code{"/var/log/nginx"})
 The directory to which NGinx will write log files.
 
+@item @code{log-level} (default: @code{'error}) (type: symbol)
+Logging level, which can be any of the following values: @code{'debug},
+@code{'info}, @code{'notice}, @code{'warn}, @code{'error}, @code{'crit},
+@code{'alert}, or @code{'emerg}.
+
 @item @code{run-directory} (default: @code{"/var/run/nginx"})
 The directory in which NGinx will create a pid file, and write temporary
 files.
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index d56e893527..4fe9c2d9ab 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -15,6 +15,7 @@
 ;;; Copyright © 2020 Oleg Pykhalov <go.wigust <at> gmail.com>
 ;;; Copyright © 2020, 2021 Alexandru-Sergiu Marton <brown121407 <at> posteo.ro>
 ;;; Copyright © 2022 Simen Endsjø <simendsjo <at> gmail.com>
+;;; Copyright © 2023 Bruno Victal <mirai <at> makinata.eu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -51,6 +52,8 @@ (define-module (gnu services web)
   #:use-module (gnu packages logging)
   #:use-module (gnu packages mail)
   #:use-module (gnu packages rust-apps)
+  #:autoload   (guix i18n) (G_)
+  #:use-module (guix diagnostics)
   #:use-module (guix packages)
   #:use-module (guix records)
   #:use-module (guix modules)
@@ -61,6 +64,7 @@ (define-module (gnu services web)
   #:use-module ((guix packages) #:select (package-version))
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-9)
+  #:use-module (srfi srfi-34)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
   #:export (httpd-configuration
@@ -96,6 +100,7 @@ (define-module (gnu services web)
             nginx-configuration-nginx
             nginx-configuration-shepherd-requirement
             nginx-configuration-log-directory
+            nginx-configuration-log-level
             nginx-configuration-run-directory
             nginx-configuration-server-blocks
             nginx-configuration-upstream-blocks
@@ -562,6 +567,9 @@ (define-record-type* <nginx-configuration>
                         (default '()))              ;list of symbols
   (log-directory nginx-configuration-log-directory  ;string
                  (default "/var/log/nginx"))
+  (log-level     nginx-configuration-log-level
+                 (sanitize assert-valid-log-level)
+                 (default 'error))
   (run-directory nginx-configuration-run-directory  ;string
                  (default "/var/run/nginx"))
   (server-blocks nginx-configuration-server-blocks
@@ -584,6 +592,14 @@ (define-record-type* <nginx-configuration>
   (file          nginx-configuration-file         ;#f | string | file-like
                  (default #f)))
 
+(define (assert-valid-log-level level)
+  "Ensure @var{level} is one of @code{'debug}, @code{'info}, @code{'notice},
+@code{'warn}, @code{'error}, @code{'crit}, @code{'alert}, or @code{'emerg}."
+  (unless (memq level '(debug info notice warn error crit alert emerg))
+    (raise
+     (formatted-message (G_ "unknown log level '~a'~%") level)))
+  level)
+
 (define (config-domain-strings names)
  "Return a string denoting the nginx config representation of NAMES, a list
 of domain names."
@@ -692,6 +708,7 @@ (define (default-nginx-config config)
   (match-record config
                 <nginx-configuration>
                 (nginx log-directory run-directory
+                 log-level
                  server-blocks upstream-blocks
                  server-names-hash-bucket-size
                  server-names-hash-bucket-max-size
@@ -704,7 +721,7 @@ (define (default-nginx-config config)
           (flatten
            "user nginx nginx;\n"
            "pid " run-directory "/pid;\n"
-           "error_log " log-directory "/error.log info;\n"
+           "error_log " log-directory "/error.log " (symbol->string log-level) ";\n"
            (map emit-load-module modules)
            (map emit-global-directive global-directives)
            "http {\n"
-- 
2.39.2

Message #23 received at 62490 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 62490 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>, maxim.cournoyer <at> gmail.com
Subject: [PATCH v2 2/2] services: nginx: Add reopen action.
Date: Mon,  3 Apr 2023 12:58:03 +0100

This is required to allow log file rotations using rottlog, etc.

* gnu/services/web.scm (nginx-shepherd-service): Add reopen shepherd action.
---
 gnu/services/web.scm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 4fe9c2d9ab..45897d7d6f 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -840,7 +840,11 @@ (define (nginx-shepherd-service config)
 the same configuration file.  It is useful for situations where the same nginx
 configuration file can point to different things after a reload, such as
 renewed TLS certificates, or @code{include}d files.")
-                 (procedure (nginx-action "-s" "reload"))))))))))
+                 (procedure (nginx-action "-s" "reload")))
+               (shepherd-action
+                (name 'reopen)
+                (documentation "Re-open log files.")
+                (procedure (nginx-action "-s" "reopen"))))))))))
 
 (define nginx-service-type
   (service-type (name 'nginx)
-- 
2.39.2

Message #28 received at 62490-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 62490-done <at> debbugs.gnu.org, Bruno Victal <mirai <at> makinata.eu>,
 mail <at> cbaines.net
Subject: Re: bug#62490: [PATCH] services: nginx: Make logging level
 configurable.
Date: Tue, 11 Apr 2023 13:12:16 -0400

Hi,

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

> Hi,
>
> Bruno Victal <mirai <at> makinata.eu> skribis:
>
>> +(define-compile-time-procedure (assert-valid-log-level (level symbol?))
>> +  "Ensure @var{level} is one of @code{'debug}, @code{'info}, @code{'notice},
>> +@code{'warn}, @code{'error}, @code{'crit}, @code{'alert}, or @code{'emerg}."
>
> As it turns out, ‘define-compile-time-procedure’ cannot work with
> symbols.  In short, that’s because in the end the generated macro
> checks:
>
>   (symbol? #'(quote debug))
>
> which doesn’t do what we want.
>
> Anyway, you can either make it a regular procedure instead, or use a
> trick found in R6RS and used in some places in Guix, Guile-Gcrypt, etc.,
> which is to define a macro that validates things:
>
>   (endianness little)  ;R6RS
>
>   (operating-id valid-path?) ;(guix store), with ‘define-enumerate-type’
>
> Making it a procedure is prolly good enough.  The compiler can optimize
> it out at compile time, FWIW:
>
> scheme@(guile-user)> ,optimize (unless (memq 'debug '(debug info)) (throw 'x))
> $13 = (if #f #f)

I've installed Bruno's v2, which reworked the above into a simple
procedure.

Closing!

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.