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

  • Open
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Maxim Cournoyer
  • Simon Tournier
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
M
M
Maxim Cournoyer wrote on 6 Feb 05:12 +0100
(address . guix-patches@gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
cover.1707192720.git.maxim.cournoyer@gmail.com
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



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
M
M
Maxim Cournoyer wrote on 6 Feb 05:12 +0100
[RFC PATCH 1/1] guix: Add logging module.
(address . guix-patches@gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
8aa12707e491aa2fc5da48ba3877d368fe5710cf.1707192720.git.maxim.cournoyer@gmail.com
* 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

Toggle diff (208 lines)
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
M
M
Maxim Cournoyer wrote on 11 Feb 05:43 +0100
[PATCH v2] guix: Add logging module.
(address . 68946@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
0b81168e693a21f7fb08bd7677a5cc3120418fc9.1707626620.git.maxim.cournoyer@gmail.com
* 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

Toggle diff (212 lines)
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
S
S
Simon Tournier wrote on 14 Feb 15:47 +0100
87frxvm7ai.fsf@gmail.com
Hi,

On sam., 10 févr. 2024 at 23:43, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
Toggle quote (8 lines)
> * 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?


Toggle quote (4 lines)
> - 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.


Toggle quote (2 lines)
> +;;; 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.

Toggle quote (6 lines)
> +(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?


Toggle quote (15 lines)
> + (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)

[...]

Toggle quote (6 lines)
> + (((? (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)
M
M
Maxim Cournoyer wrote on 16 Feb 20:03 +0100
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
87le7ktenv.fsf@gmail.com
Hi Simon,

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

Toggle quote (22 lines)
> 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.

Toggle quote (3 lines)
> Aside, just to be sure to well understand, this introduces the logger
> but it is not used yet, right?

Correct.

Toggle quote (9 lines)
>
>> - 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
:-).

Toggle quote (8 lines)
>> +;;; 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].


Toggle quote (8 lines)
>> +(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.

Toggle quote (29 lines)
>> + (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
L
L
Ludovic Courtès wrote on 18 Feb 23:17 +0100
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87bk8dl8mm.fsf@gnu.org
Hi!

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

Toggle quote (10 lines)
> * 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’.
S
S
Simon Tournier wrote on 19 Feb 16:57 +0100
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
874je45tw7.fsf@gmail.com
Hi Maxim,

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

Toggle quote (3 lines)
> 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:

Toggle snippet (15 lines)
;;; 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

Toggle quote (12 lines)
>
>>> +(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?


Toggle quote (11 lines)
>> 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. :-)


Cheers,
simon]
M
M
Maxim Cournoyer wrote on 23 Feb 02:13 +0100
(name . Ludovic Courtès)(address . ludo@gnu.org)
87edd4oude.fsf@gmail.com
Hi Ludovic,

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

Toggle quote (18 lines)
> 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.

Toggle quote (6 lines)
> 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
L
L
Ludovic Courtès wrote on 24 Feb 12:44 +0100
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87ttlyaxyq.fsf@gnu.org
Hi,

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

Toggle quote (10 lines)
>> 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’.
M
M
Maxim Cournoyer wrote on 25 Feb 15:32 +0100
(name . Ludovic Courtès)(address . ludo@gnu.org)
87h6hwmx5z.fsf@gmail.com
Hi Ludo,

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

Toggle quote (19 lines)
> 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:

Toggle snippet (7 lines)
$ ./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

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
?