[bug#68946] [RFC PATCH 0/1] Add logging capability to Guix

[bug#68946] [RFC PATCH 0/1] Add logging capability to Guix

[Maxim Cournoyer]

This adds the ability to simply log messages during execution, whose level can
be selected via a new '--log-level' option.

It relies on pending Guile features (SRFI 209 enums) [0] as well as some
modifications to Guile-Lib's logging library so that it can support source
properties [1].  To test, I symlink the 'guile' binary at the root of the
Guix checkout to my ~/src/guile/meta/guile built binary, and I build Guix with
'make GUILE=./guile', in an environment containing the modified guile-lib:

guix shell --pure -D guix --with-source=guile-lib=/home/maxim/src/guile-lib

Here's some dummy output just to show what the default format looks like:

2024-02-06 03:38:44 guix/derivations.scm:272:16: DEBUG: adding input #<<derivation-input> drv: #<derivation /gnu/store/2ll0ig7jn3w7rqsyfjl358r2q9lga15a-gcc-core-4.6.4.tar.gz.drv => /gnu/store/8fsrfzp53azpsb5n3rbh24n1bcrimzpj-gcc-core-4.6.4.tar.gz 7f60e5ea7af0> sub-derivations: ("out")> to list
2024-02-06 03:38:44 guix/derivations.scm:272:16: DEBUG: adding input #<<derivation-input> drv: #<derivation /gnu/store/5hmjcad7k2z2d8rc38p2jcsh88dvpqr6-gcc-g++-4.6.4.tar.gz.drv => /gnu/store/qbj186qvw9312ki8qqmxc5hx6izwp1km-gcc-g++-4.6.4.tar.gz 7f60e5ea7a00> sub-derivations: ("out")> to list

[0]  https://lists.gnu.org/archive/html/guile-devel/2023-12/msg00087.html
[1]  https://lists.gnu.org/archive/html/guile-devel/2024-02/msg00000.html


Maxim Cournoyer (1):
  guix: Add logging module.

 Makefile.am      |  5 ++-
 configure.ac     |  6 +--
 guix/logging.scm | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
 guix/ui.scm      | 17 ++++++++-
 4 files changed, 120 insertions(+), 6 deletions(-)
 create mode 100644 guix/logging.scm


base-commit: c76004c574acc3291e8d2526fe534719473a87ff
-- 
2.41.0

[bug#68946] [RFC PATCH 1/1] guix: Add logging module.

[Maxim Cournoyer]

* configure.ac: Require Guile-Lib.
* guix/logging.scm: New module.
* Makefile.am (MODULES): Register it.
* guix/ui.scm (show-guix-help): Document --log-level global option.
(%log-level): New parameter.
(run-guix-command): Init logging.
(run-guix): Parse new --log-level option.

Change-Id: I5026a0d62119615fec3cd0131309f9bcc346a7e9
---

 Makefile.am      |  5 ++-
 configure.ac     |  6 +--
 guix/logging.scm | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
 guix/ui.scm      | 17 ++++++++-
 4 files changed, 120 insertions(+), 6 deletions(-)
 create mode 100644 guix/logging.scm

diff --git a/Makefile.am b/Makefile.am
index cef972880c..6929ee5f69 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -14,7 +14,7 @@
 # Copyright © 2018 Oleg Pykhalov <go.wigust@gmail.com>
 # Copyright © 2018 Alex Vong <alexvong1995@gmail.com>
 # Copyright © 2019, 2023 Efraim Flashner <efraim@flashner.co.il>
-# Copyright © 2020, 2021, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+# Copyright © 2020, 2021, 2023, 2024 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 # Copyright © 2021 Chris Marusich <cmmarusich@gmail.com>
 # Copyright © 2021 Andrew Tropin <andrew@trop.in>
 # Copyright © 2023 Clément Lassieur <clement@lassieur.org>
@@ -125,7 +125,8 @@ MODULES =					\
   guix/substitutes.scm				\
   guix/upstream.scm				\
   guix/licenses.scm				\
-  guix/lint.scm				\
+  guix/lint.scm					\
+  guix/logging.scm				\
   guix/glob.scm					\
   guix/git.scm					\
   guix/git-authenticate.scm			\
diff --git a/configure.ac b/configure.ac
index ecbd596a34..133692c033 100644
--- a/configure.ac
+++ b/configure.ac
@@ -149,12 +149,12 @@ if test "x$guix_cv_have_recent_guile_git" != "xyes"; then
   AC_MSG_ERROR([A recent Guile-Git could not be found; please install it.])
 fi
 
-dnl Check for the optional Guile-Lib.
+dnl Require Guile-Lib, for logging and HTML parsing.
+GUILE_MODULE_REQUIRED([logging logger])
 GUILE_MODULE_EXPORTS([have_guile_lib], [(htmlprag)], [%strict-tokenizer?])
 AM_CONDITIONAL([HAVE_GUILE_LIB], [test "x$have_guile_lib" = "xyes"])
 AM_COND_IF(HAVE_GUILE_LIB,,
-  [AC_MSG_WARN([The Guile-Lib requirement was not satisfied (>= 0.2.7);
-Some features such as the Go importer will not be usable.])])
+  [AC_MSG_ERROR([The Guile-Lib requirement was not satisfied (>= 0.2.7)])])
 
 dnl Check for Guile-zlib.
 GUIX_CHECK_GUILE_ZLIB
diff --git a/guix/logging.scm b/guix/logging.scm
new file mode 100644
index 0000000000..913bc9ed3f
--- /dev/null
+++ b/guix/logging.scm
@@ -0,0 +1,98 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2023, 2024 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (guix logging)
+  #:use-module (logging logger)
+  #:use-module (logging port-log)
+
+  #:use-module (oop goops)
+  #:use-module (srfi srfi-209)
+
+  #:export (setup-logging
+            shutdown-logging
+
+            log-level
+            log-debug
+            log-info
+            log-warning
+            log-error
+            log-critical))
+
+(define-syntax define-log-level
+  ;; This macro defines a log-level enum type bound to ENUM-NAME for the
+  ;; provided levels.  The levels should be specified in increasing order of
+  ;; severity.  It also defines 'log-LEVEL' syntax to more conveniently log at
+  ;; LEVEL, with location information.
+  (lambda (x)
+    (define-syntax-rule (id parts ...)
+      ;; Assemble PARTS into a raw (unhygienic) identifier.
+      (datum->syntax x (symbol-append (syntax->datum parts) ...)))
+
+    (syntax-case x ()
+      ((_ enum-name (level ...))
+       #`(begin
+           (define enum-name
+             (make-enum-type '(level ...)))
+
+           #,@(map (lambda (lvl)
+                     (with-syntax ((log (id 'log- lvl))
+                                   (lvl lvl))
+                       #'(define-syntax log
+                           (lambda (y)
+                             (syntax-case y ()
+                               ((log . args)
+                                #`(log-msg
+                                   '#,(datum->syntax
+                                       y (syntax-source #'log))
+                                   'lvl #,@#'args)))))))
+                   #'(level ...)))))))
+
+(define-log-level log-level
+  (debug info warning error critical))
+
+(define* (setup-logging #:key (level 'warning))
+  "Configure and open logger at LEVEL)."
+  (let* ((level-enum (or (enum-name->enum log-level level)
+                         (begin
+                           (format (current-error-port)
+                                   "error: invalid log level~%")
+                           (exit 1))))
+         (lgr        (make <logger>))
+         (console    (make <port-log> #:port (current-error-port)))
+         (levels-count (enum-type-size log-level)))
+
+    ;; Register logging handlers.
+    (add-handler! lgr console)
+
+    ;; Configure the log level.
+    (let loop ((enum (enum-min log-level)))
+      (let ((lvl (enum-name enum)))
+        (unless (eq? level lvl)
+          (disable-log-level! lgr lvl)
+          (loop (enum-next enum)))))
+
+    ;; Activate the configured logger.
+    (set-default-logger! lgr)
+    (open-log! lgr)
+    (log-debug "logging initialized")))
+
+(define (shutdown-logging)
+  "Flush and destroy logger."
+  (flush-log)
+  (close-log!)
+  (set-default-logger! #f))
diff --git a/guix/ui.scm b/guix/ui.scm
index 962d291d2e..b42d620a0c 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -44,6 +44,7 @@ (define-module (guix ui)
   #:use-module (guix utils)
   #:use-module (guix store)
   #:use-module (guix config)
+  #:use-module (guix logging)
   #:use-module (guix packages)
   #:use-module (guix profiles)
   #:use-module (guix derivations)
@@ -2225,6 +2226,9 @@ (define (show-guix-help)
   -h, --help             display this helpful text again and exit"))
   (display (G_ "
   -V, --version          display version and copyright information and exit"))
+  (display (G_ "
+      --log-level=LEVEL  set the logging verbosity to LEVEL, one of
+                         debug, warning, error or critical"))
   (newline)
 
   (newline)
@@ -2248,6 +2252,8 @@ (define (show-guix-help)
               categories))
   (show-bug-report-information))
 
+(define %log-level (make-parameter 'warning))
+
 (define (run-guix-command command . args)
   "Run COMMAND with the given ARGS.  Report an error when COMMAND is not
 found."
@@ -2282,10 +2288,12 @@ (define (run-guix-command command . args)
                                   (symbol-append 'guix- command))))
     (parameterize ((program-name command))
       (dynamic-wind
-        (const #f)
+        (lambda ()
+          (setup-logging #:level (%log-level)))
         (lambda ()
           (apply command-main args))
         (lambda ()
+          (shutdown-logging)
           ;; Abuse 'exit-hook' (which is normally meant to be used by the
           ;; REPL) to run things like profiling hooks upon completion.
           (run-hook exit-hook))))))
@@ -2311,6 +2319,13 @@ (define (run-guix . args)
        (leave-on-EPIPE (show-guix-help)))
       ((or ("-V") ("--version"))
        (show-version-and-exit "guix"))
+      (((? (cut string-prefix? "--log-level=" <>) o) args ...)
+       (parameterize ((%log-level (string->symbol
+                                   (second (string-split o #\=)))))
+         (apply run-guix args)))
+      (("--log-level" level args ...)
+       (parameterize ((%log-level (string->symbol level)))
+         (apply run-guix args)))
       (((? option? o) args ...)
        (format (current-error-port)
                (G_ "guix: unrecognized option '~a'~%") o)
-- 
2.41.0

[bug#68946] [PATCH v2] guix: Add logging module.

[Maxim Cournoyer]

* configure.ac: Require Guile-Lib.
* guix/logging.scm: New module.
* Makefile.am (MODULES): Register it.
* guix/ui.scm (show-guix-help): Document --log-level global option.
(%log-level): New parameter.
(run-guix-command): Init logging.
(run-guix): Parse new --log-level option.

Change-Id: I5026a0d62119615fec3cd0131309f9bcc346a7e9
---

Changes in v2:
 - Honor the GUIX_LOG_LEVEL environment variable

 Makefile.am      |  5 ++-
 configure.ac     |  6 +--
 guix/logging.scm | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
 guix/ui.scm      | 19 +++++++++-
 4 files changed, 122 insertions(+), 6 deletions(-)
 create mode 100644 guix/logging.scm

diff --git a/Makefile.am b/Makefile.am
index cef972880c..6929ee5f69 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -14,7 +14,7 @@
 # Copyright © 2018 Oleg Pykhalov <go.wigust@gmail.com>
 # Copyright © 2018 Alex Vong <alexvong1995@gmail.com>
 # Copyright © 2019, 2023 Efraim Flashner <efraim@flashner.co.il>
-# Copyright © 2020, 2021, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+# Copyright © 2020, 2021, 2023, 2024 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 # Copyright © 2021 Chris Marusich <cmmarusich@gmail.com>
 # Copyright © 2021 Andrew Tropin <andrew@trop.in>
 # Copyright © 2023 Clément Lassieur <clement@lassieur.org>
@@ -125,7 +125,8 @@ MODULES =					\
   guix/substitutes.scm				\
   guix/upstream.scm				\
   guix/licenses.scm				\
-  guix/lint.scm				\
+  guix/lint.scm					\
+  guix/logging.scm				\
   guix/glob.scm					\
   guix/git.scm					\
   guix/git-authenticate.scm			\
diff --git a/configure.ac b/configure.ac
index ecbd596a34..133692c033 100644
--- a/configure.ac
+++ b/configure.ac
@@ -149,12 +149,12 @@ if test "x$guix_cv_have_recent_guile_git" != "xyes"; then
   AC_MSG_ERROR([A recent Guile-Git could not be found; please install it.])
 fi
 
-dnl Check for the optional Guile-Lib.
+dnl Require Guile-Lib, for logging and HTML parsing.
+GUILE_MODULE_REQUIRED([logging logger])
 GUILE_MODULE_EXPORTS([have_guile_lib], [(htmlprag)], [%strict-tokenizer?])
 AM_CONDITIONAL([HAVE_GUILE_LIB], [test "x$have_guile_lib" = "xyes"])
 AM_COND_IF(HAVE_GUILE_LIB,,
-  [AC_MSG_WARN([The Guile-Lib requirement was not satisfied (>= 0.2.7);
-Some features such as the Go importer will not be usable.])])
+  [AC_MSG_ERROR([The Guile-Lib requirement was not satisfied (>= 0.2.7)])])
 
 dnl Check for Guile-zlib.
 GUIX_CHECK_GUILE_ZLIB
diff --git a/guix/logging.scm b/guix/logging.scm
new file mode 100644
index 0000000000..913bc9ed3f
--- /dev/null
+++ b/guix/logging.scm
@@ -0,0 +1,98 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2023, 2024 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (guix logging)
+  #:use-module (logging logger)
+  #:use-module (logging port-log)
+
+  #:use-module (oop goops)
+  #:use-module (srfi srfi-209)
+
+  #:export (setup-logging
+            shutdown-logging
+
+            log-level
+            log-debug
+            log-info
+            log-warning
+            log-error
+            log-critical))
+
+(define-syntax define-log-level
+  ;; This macro defines a log-level enum type bound to ENUM-NAME for the
+  ;; provided levels.  The levels should be specified in increasing order of
+  ;; severity.  It also defines 'log-LEVEL' syntax to more conveniently log at
+  ;; LEVEL, with location information.
+  (lambda (x)
+    (define-syntax-rule (id parts ...)
+      ;; Assemble PARTS into a raw (unhygienic) identifier.
+      (datum->syntax x (symbol-append (syntax->datum parts) ...)))
+
+    (syntax-case x ()
+      ((_ enum-name (level ...))
+       #`(begin
+           (define enum-name
+             (make-enum-type '(level ...)))
+
+           #,@(map (lambda (lvl)
+                     (with-syntax ((log (id 'log- lvl))
+                                   (lvl lvl))
+                       #'(define-syntax log
+                           (lambda (y)
+                             (syntax-case y ()
+                               ((log . args)
+                                #`(log-msg
+                                   '#,(datum->syntax
+                                       y (syntax-source #'log))
+                                   'lvl #,@#'args)))))))
+                   #'(level ...)))))))
+
+(define-log-level log-level
+  (debug info warning error critical))
+
+(define* (setup-logging #:key (level 'warning))
+  "Configure and open logger at LEVEL)."
+  (let* ((level-enum (or (enum-name->enum log-level level)
+                         (begin
+                           (format (current-error-port)
+                                   "error: invalid log level~%")
+                           (exit 1))))
+         (lgr        (make <logger>))
+         (console    (make <port-log> #:port (current-error-port)))
+         (levels-count (enum-type-size log-level)))
+
+    ;; Register logging handlers.
+    (add-handler! lgr console)
+
+    ;; Configure the log level.
+    (let loop ((enum (enum-min log-level)))
+      (let ((lvl (enum-name enum)))
+        (unless (eq? level lvl)
+          (disable-log-level! lgr lvl)
+          (loop (enum-next enum)))))
+
+    ;; Activate the configured logger.
+    (set-default-logger! lgr)
+    (open-log! lgr)
+    (log-debug "logging initialized")))
+
+(define (shutdown-logging)
+  "Flush and destroy logger."
+  (flush-log)
+  (close-log!)
+  (set-default-logger! #f))
diff --git a/guix/ui.scm b/guix/ui.scm
index 962d291d2e..f5a6966854 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -44,6 +44,7 @@ (define-module (guix ui)
   #:use-module (guix utils)
   #:use-module (guix store)
   #:use-module (guix config)
+  #:use-module (guix logging)
   #:use-module (guix packages)
   #:use-module (guix profiles)
   #:use-module (guix derivations)
@@ -2225,6 +2226,9 @@ (define (show-guix-help)
   -h, --help             display this helpful text again and exit"))
   (display (G_ "
   -V, --version          display version and copyright information and exit"))
+  (display (G_ "
+      --log-level=LEVEL  set the logging verbosity to LEVEL, one of
+                         debug, warning, error or critical"))
   (newline)
 
   (newline)
@@ -2248,6 +2252,10 @@ (define (show-guix-help)
               categories))
   (show-bug-report-information))
 
+(define %log-level (make-parameter (or (and=> (getenv "GUIX_LOG_LEVEL")
+                                              string->symbol)
+                                       'warning)))
+
 (define (run-guix-command command . args)
   "Run COMMAND with the given ARGS.  Report an error when COMMAND is not
 found."
@@ -2282,10 +2290,12 @@ (define (run-guix-command command . args)
                                   (symbol-append 'guix- command))))
     (parameterize ((program-name command))
       (dynamic-wind
-        (const #f)
+        (lambda ()
+          (setup-logging #:level (%log-level)))
         (lambda ()
           (apply command-main args))
         (lambda ()
+          (shutdown-logging)
           ;; Abuse 'exit-hook' (which is normally meant to be used by the
           ;; REPL) to run things like profiling hooks upon completion.
           (run-hook exit-hook))))))
@@ -2311,6 +2321,13 @@ (define (run-guix . args)
        (leave-on-EPIPE (show-guix-help)))
       ((or ("-V") ("--version"))
        (show-version-and-exit "guix"))
+      (((? (cut string-prefix? "--log-level=" <>) o) args ...)
+       (parameterize ((%log-level (string->symbol
+                                   (second (string-split o #\=)))))
+         (apply run-guix args)))
+      (("--log-level" level args ...)
+       (parameterize ((%log-level (string->symbol level)))
+         (apply run-guix args)))
       (((? option? o) args ...)
        (format (current-error-port)
                (G_ "guix: unrecognized option '~a'~%") o)

base-commit: 9edbb2d7a40c9da7583a1046e39b87633459f656
-- 
2.41.0

[bug#68946] [PATCH v2] guix: Add logging module.

[Simon Tournier]

Hi,

On sam., 10 févr. 2024 at 23:43, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
> * configure.ac: Require Guile-Lib.
> * guix/logging.scm: New module.
> * Makefile.am (MODULES): Register it.
> * guix/ui.scm (show-guix-help): Document --log-level global option.
> (%log-level): New parameter.
> (run-guix-command): Init logging.
> (run-guix): Parse new --log-level option.

Cool!

How to synchronize this patch with the Guile-Lib counter-part

        [Guile-Lib PATCH v3 0/7] Make log-msg accept source properties for displaying source location
        Maxim Cournoyer <maxim.cournoyer@gmail.com>
        Thu, 08 Feb 2024 23:50:41 -0500
        id:20240209045150.17210-1-maxim.cournoyer@gmail.com

?

Aside, just to be sure to well understand, this introduces the logger
but it is not used yet, right?


> -  guix/lint.scm				\
> +  guix/lint.scm					\
> +  guix/logging.scm				\

Well, I assume being the nitpick person here. :-)

The line guix/lint.scm should not change here.


> +;;; Copyright © 2023, 2024 Maxim Cournoyer <maxim.cournoyer@gmail.com>

I think what makes the date of the Copyright is something publicly
declared (published) and not the date when the author started to work on
it.  At least, it is how it works for book in France.

So here the Copyright would be 2024 only, I guess.

> +(define-syntax define-log-level
> +  ;; This macro defines a log-level enum type bound to ENUM-NAME for the
> +  ;; provided levels.  The levels should be specified in increasing order of
> +  ;; severity.  It also defines 'log-LEVEL' syntax to more conveniently log at
> +  ;; LEVEL, with location information.

Why not also a docstring?


> +  (lambda (x)
> +    (define-syntax-rule (id parts ...)
> +      ;; Assemble PARTS into a raw (unhygienic) identifier.
> +      (datum->syntax x (symbol-append (syntax->datum parts) ...)))

> diff --git a/guix/ui.scm b/guix/ui.scm
> index 962d291d2e..f5a6966854 100644
> --- a/guix/ui.scm
> +++ b/guix/ui.scm
> @@ -44,6 +44,7 @@ (define-module (guix ui)
>    #:use-module (guix utils)
>    #:use-module (guix store)
>    #:use-module (guix config)
> +  #:use-module (guix logging)

[...]

> +      (((? (cut string-prefix? "--log-level=" <>) o) args ...)
> +       (parameterize ((%log-level (string->symbol
> +                                   (second (string-split o #\=)))))
> +         (apply run-guix args)))
> +      (("--log-level" level args ...)

This make “--log-level debug” valid, right?

I think the convention is --long-option[=PARAMETER] and that
--long-option PARAMTER is unconventional.  Although I do not find the
reference.

(also -sPARAMTER or -s PARAMETER)

[bug#68946] [PATCH v2] guix: Add logging module.

[Maxim Cournoyer]

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi,
>
> On sam., 10 févr. 2024 at 23:43, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>> * configure.ac: Require Guile-Lib.
>> * guix/logging.scm: New module.
>> * Makefile.am (MODULES): Register it.
>> * guix/ui.scm (show-guix-help): Document --log-level global option.
>> (%log-level): New parameter.
>> (run-guix-command): Init logging.
>> (run-guix): Parse new --log-level option.
>
> Cool!
>
> How to synchronize this patch with the Guile-Lib counter-part
>
>         [Guile-Lib PATCH v3 0/7] Make log-msg accept source properties for displaying source location
>         Maxim Cournoyer <maxim.cournoyer@gmail.com>
>         Thu, 08 Feb 2024 23:50:41 -0500
>         id:20240209045150.17210-1-maxim.cournoyer@gmail.com
>
> ?

I have commit access to guile-lib; I've pinged its maintainer to see if
they have any objection, and otherwise I'll merge it soon.

> Aside, just to be sure to well understand, this introduces the logger
> but it is not used yet, right?

Correct.

>
>> -  guix/lint.scm				\
>> +  guix/lint.scm					\
>> +  guix/logging.scm				\
>
> Well, I assume being the nitpick person here. :-)
>
> The line guix/lint.scm should not change here.

I fixed the number of tabs on the line above as it looked odd in Emacs
:-).

>> +;;; Copyright © 2023, 2024 Maxim Cournoyer <maxim.cournoyer@gmail.com>
>
> I think what makes the date of the Copyright is something publicly
> declared (published) and not the date when the author started to work on
> it.  At least, it is how it works for book in France.
>
> So here the Copyright would be 2024 only, I guess.

Most of that code also exists in guile-hall, was released in 2023, hence
the copyright year start [0].

[0]  https://gitlab.com/a-sassmannshausen/guile-hall/-/blob/master/hall/logging.scm

>> +(define-syntax define-log-level
>> +  ;; This macro defines a log-level enum type bound to ENUM-NAME for the
>> +  ;; provided levels.  The levels should be specified in increasing order of
>> +  ;; severity.  It also defines 'log-LEVEL' syntax to more conveniently log at
>> +  ;; LEVEL, with location information.
>
> Why not also a docstring?

Only procedures can have docstrings, unfortunately.

>> +  (lambda (x)
>> +    (define-syntax-rule (id parts ...)
>> +      ;; Assemble PARTS into a raw (unhygienic) identifier.
>> +      (datum->syntax x (symbol-append (syntax->datum parts) ...)))
>
>> diff --git a/guix/ui.scm b/guix/ui.scm
>> index 962d291d2e..f5a6966854 100644
>> --- a/guix/ui.scm
>> +++ b/guix/ui.scm
>> @@ -44,6 +44,7 @@ (define-module (guix ui)
>>    #:use-module (guix utils)
>>    #:use-module (guix store)
>>    #:use-module (guix config)
>> +  #:use-module (guix logging)
>
> [...]
>
>> +      (((? (cut string-prefix? "--log-level=" <>) o) args ...)
>> +       (parameterize ((%log-level (string->symbol
>> +                                   (second (string-split o #\=)))))
>> +         (apply run-guix args)))
>> +      (("--log-level" level args ...)
>
> This make “--log-level debug” valid, right?
>
> I think the convention is --long-option[=PARAMETER] and that
> --long-option PARAMTER is unconventional.  Although I do not find the
> reference.

It's not as much a convention as a limitation of the SRFI 37 option
parser.  GNU getopt, which SRFI 37 aims to emulate,  doesn't have such a
limitation, for example.  We should improve SRFI 37 to lift such
limitation, in my opinion.

-- 
Thanks,
Maxim

[bug#68946] [PATCH v2] guix: Add logging module.

[Simon Tournier]

Hi Maxim,

On ven., 16 févr. 2024 at 14:03, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> Most of that code also exists in guile-hall, was released in 2023, hence
> the copyright year start [0].

I think it would be best something like:

--8<---------------cut here---------------start------------->8---
;;; Copyright © 2024 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;;
;;; The procedures ’define-log-level', ’setup-logging’ and
;;; ’shutdown-logging’ are taken from the 'hall/logging.scm' file of
;;; Guile-lib (guile-hall). That file has the following copyright
;;; notice:
;;;
;;; Copyright © 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;;
;;; This file is part of Guix.
;;;
;;; GNU Guix is distributed in the hope that it will be useful, but
…
--8<---------------cut here---------------end--------------->8---

> [0]  https://gitlab.com/a-sassmannshausen/guile-hall/-/blob/master/hall/logging.scm
>
>>> +(define-syntax define-log-level
>>> +  ;; This macro defines a log-level enum type bound to ENUM-NAME for the
>>> +  ;; provided levels.  The levels should be specified in increasing order of
>>> +  ;; severity.  It also defines 'log-LEVEL' syntax to more conveniently log at
>>> +  ;; LEVEL, with location information.
>>
>> Why not also a docstring?
>
> Only procedures can have docstrings, unfortunately.

Ah?  For instance ’define-with-syntax-properties’ or ’define-diagnostic’
or ’leave’ from (guix diagnostics); other instance in (gnu build
accounts) or (gnu build secret-service) etc.

Do I miss something?


>> This make “--log-level debug” valid, right?
>>
>> I think the convention is --long-option[=PARAMETER] and that
>> --long-option PARAMTER is unconventional.  Although I do not find the
>> reference.
>
> It's not as much a convention as a limitation of the SRFI 37 option
> parser.  GNU getopt, which SRFI 37 aims to emulate,  doesn't have such a
> limitation, for example.  We should improve SRFI 37 to lift such
> limitation, in my opinion.

I think that’s a bad choice.  For two reasons,

 1. Let’s keep an uniform across subcommands

 2. It potentially opens bug as #50472; no space avoids ambiguous
    implementations as explained. :-)

    https://issues.guix.gnu.org/issue/50472

Cheers,
simon]

[bug#68946] [PATCH v2] guix: Add logging module.

[Ludovic Courtès]

Hi!

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> * configure.ac: Require Guile-Lib.
> * guix/logging.scm: New module.
> * Makefile.am (MODULES): Register it.
> * guix/ui.scm (show-guix-help): Document --log-level global option.
> (%log-level): New parameter.
> (run-guix-command): Init logging.
> (run-guix): Parse new --log-level option.
>
> Change-Id: I5026a0d62119615fec3cd0131309f9bcc346a7e9

Two quick notes: I think it’s be best to avoid depending on Guile-Lib
“just” for this (it’s not clear to me what it’s maintenance status is),
and it’d be great to have concrete use case.

My concern with logging is that it’s not a good fit for the functional
parts (unless it has a functional interface like ‘pk’, but that’s still
sorta “cluttering” otherwise functional code), and I’m not sure which
imperative part it could be used for (I don’t doubt there’s such a
thing, but I lack imagination).

Thanks,
Ludo’.

[bug#68946] [PATCH v2] guix: Add logging module.

[Maxim Cournoyer]

Hi Ludovic,

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

> Hi!
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> * configure.ac: Require Guile-Lib.
>> * guix/logging.scm: New module.
>> * Makefile.am (MODULES): Register it.
>> * guix/ui.scm (show-guix-help): Document --log-level global option.
>> (%log-level): New parameter.
>> (run-guix-command): Init logging.
>> (run-guix): Parse new --log-level option.
>>
>> Change-Id: I5026a0d62119615fec3cd0131309f9bcc346a7e9
>
> Two quick notes: I think it’s be best to avoid depending on Guile-Lib
> “just” for this (it’s not clear to me what it’s maintenance status is),
> and it’d be great to have concrete use case.

David Pirotte is doing great for Guile-Lib maintenance.  It's better
maintained, than say, Guile-Git, which we also depend on :-).  It has a
good test suite and is small.  We already depend on it for a couple of
our updaters, for its htmlprag HTML parser, so combined with this log
facility, that seems enough to justify hard depending on it.

> My concern with logging is that it’s not a good fit for the functional
> parts (unless it has a functional interface like ‘pk’, but that’s still
> sorta “cluttering” otherwise functional code), and I’m not sure which
> imperative part it could be used for (I don’t doubt there’s such a
> thing, but I lack imagination).

I understand the concern, but I think to be able to trace execution
easily at run time with a simple `--log-level=debug' would provide value
enough (as a developer aid) to justify it.

-- 
Thanks,
Maxim

[bug#68946] [PATCH v2] guix: Add logging module.

[Ludovic Courtès]

Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

>> My concern with logging is that it’s not a good fit for the functional
>> parts (unless it has a functional interface like ‘pk’, but that’s still
>> sorta “cluttering” otherwise functional code), and I’m not sure which
>> imperative part it could be used for (I don’t doubt there’s such a
>> thing, but I lack imagination).
>
> I understand the concern, but I think to be able to trace execution
> easily at run time with a simple `--log-level=debug' would provide value
> enough (as a developer aid) to justify it.

Again, I lack imagination; where would you use it today?

I think we should only add this facility once it has at least one user.
Otherwise people using ‘--log-level’ would be disappointed.  :-)

Ludo’.

[bug#68946] [PATCH v2] guix: Add logging module.

[Maxim Cournoyer]

Hi Ludo,

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

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>>> My concern with logging is that it’s not a good fit for the functional
>>> parts (unless it has a functional interface like ‘pk’, but that’s still
>>> sorta “cluttering” otherwise functional code), and I’m not sure which
>>> imperative part it could be used for (I don’t doubt there’s such a
>>> thing, but I lack imagination).
>>
>> I understand the concern, but I think to be able to trace execution
>> easily at run time with a simple `--log-level=debug' would provide value
>> enough (as a developer aid) to justify it.
>
> Again, I lack imagination; where would you use it today?
>
> I think we should only add this facility once it has at least one user.
> Otherwise people using ‘--log-level’ would be disappointed.  :-)

OK; I've added some basic logging to get the ball rolling; it's
currently limited to showing the parsed arguments used with the guix
command run, it looks like so:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix --log-level=debug build hello
2024-02-25 02:58:13 guix/logging.scm:92:5: (run-guix-command) DEBUG: logging initialized
2024-02-25 02:58:13 guix/ui.scm:2296:11: (run-guix-command) INFO: running guix command `build' with args: (hello)
2024-02-25 02:58:13 guix/scripts/build.scm:741:3: (guix-build) INFO: 'guix build' parsed options: ((argument . hello) (build-mode . 0) (graft? . #t) (substitutes? . #t) (offload? . #t) (print-build-trace? . #t) (print-extended-build-trace? . #t) (multiplexed-build-output? . #t) (verbosity . 3) (debug . 0))
/gnu/store/kf9cgiq5a2bbxf89h72scl6b5f4jmjpf-hello-2.12.1
--8<---------------cut here---------------end--------------->8---

While this is not super useful yet, we can see where execution started,
and which options were passed to the lower level command API, which
could be a helpful entry-point discoverability hint to a new hacker
wanting to add a switch or look into how a particular command is implemented.

I'll send a v3 with it soon.

-- 
Thanks,
Maxim