* [bug#68946] [RFC PATCH 1/1] guix: Add logging module.
2024-02-06 4:12 [bug#68946] [RFC PATCH 0/1] Add logging capability to Guix Maxim Cournoyer
@ 2024-02-06 4:12 ` Maxim Cournoyer
2024-02-11 4:43 ` [bug#68946] [PATCH v2] " Maxim Cournoyer
2024-11-01 21:00 ` [bug#68946] Add logging capability to Guix Steve George
2 siblings, 0 replies; 12+ messages in thread
From: Maxim Cournoyer @ 2024-02-06 4:12 UTC (permalink / raw)
To: 68946
Cc: Maxim Cournoyer, Christopher Baines, Josselin Poiret,
Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
Simon Tournier, Tobias Geerinckx-Rice
* 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [bug#68946] [PATCH v2] guix: Add logging module.
2024-02-06 4:12 [bug#68946] [RFC PATCH 0/1] Add logging capability to Guix Maxim Cournoyer
2024-02-06 4:12 ` [bug#68946] [RFC PATCH 1/1] guix: Add logging module Maxim Cournoyer
@ 2024-02-11 4:43 ` Maxim Cournoyer
2024-02-14 14:47 ` Simon Tournier
2024-02-18 22:17 ` Ludovic Courtès
2024-11-01 21:00 ` [bug#68946] Add logging capability to Guix Steve George
2 siblings, 2 replies; 12+ messages in thread
From: Maxim Cournoyer @ 2024-02-11 4:43 UTC (permalink / raw)
To: 68946
Cc: Maxim Cournoyer, Christopher Baines, Josselin Poiret,
Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
Simon Tournier, Tobias Geerinckx-Rice
* 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [bug#68946] [PATCH v2] guix: Add logging module.
2024-02-11 4:43 ` [bug#68946] [PATCH v2] " Maxim Cournoyer
@ 2024-02-14 14:47 ` Simon Tournier
2024-02-16 19:03 ` Maxim Cournoyer
2024-02-18 22:17 ` Ludovic Courtès
1 sibling, 1 reply; 12+ messages in thread
From: Simon Tournier @ 2024-02-14 14:47 UTC (permalink / raw)
To: Maxim Cournoyer, 68946
Cc: Josselin Poiret, Maxim Cournoyer, Mathieu Othacehe,
Ludovic Courtès, Tobias Geerinckx-Rice, Ricardo Wurmus,
Christopher Baines
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)
^ permalink raw reply [flat|nested] 12+ messages in thread* [bug#68946] [PATCH v2] guix: Add logging module.
2024-02-14 14:47 ` Simon Tournier
@ 2024-02-16 19:03 ` Maxim Cournoyer
2024-02-19 15:57 ` Simon Tournier
0 siblings, 1 reply; 12+ messages in thread
From: Maxim Cournoyer @ 2024-02-16 19:03 UTC (permalink / raw)
To: Simon Tournier
Cc: 68946, Mathieu Othacehe, Ludovic Courtès,
Tobias Geerinckx-Rice, Josselin Poiret, Ricardo Wurmus,
Christopher Baines
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
^ permalink raw reply [flat|nested] 12+ messages in thread* [bug#68946] [PATCH v2] guix: Add logging module.
2024-02-16 19:03 ` Maxim Cournoyer
@ 2024-02-19 15:57 ` Simon Tournier
0 siblings, 0 replies; 12+ messages in thread
From: Simon Tournier @ 2024-02-19 15:57 UTC (permalink / raw)
To: Maxim Cournoyer
Cc: 68946, Mathieu Othacehe, Ludovic Courtès,
Tobias Geerinckx-Rice, Josselin Poiret, Ricardo Wurmus,
Christopher Baines
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]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [bug#68946] [PATCH v2] guix: Add logging module.
2024-02-11 4:43 ` [bug#68946] [PATCH v2] " Maxim Cournoyer
2024-02-14 14:47 ` Simon Tournier
@ 2024-02-18 22:17 ` Ludovic Courtès
2024-02-23 1:13 ` Maxim Cournoyer
1 sibling, 1 reply; 12+ messages in thread
From: Ludovic Courtès @ 2024-02-18 22:17 UTC (permalink / raw)
To: Maxim Cournoyer
Cc: 68946, Simon Tournier, Mathieu Othacehe, Tobias Geerinckx-Rice,
Josselin Poiret, Ricardo Wurmus, Christopher Baines
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’.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [bug#68946] [PATCH v2] guix: Add logging module.
2024-02-18 22:17 ` Ludovic Courtès
@ 2024-02-23 1:13 ` Maxim Cournoyer
2024-02-24 11:44 ` Ludovic Courtès
0 siblings, 1 reply; 12+ messages in thread
From: Maxim Cournoyer @ 2024-02-23 1:13 UTC (permalink / raw)
To: Ludovic Courtès
Cc: 68946, Simon Tournier, Mathieu Othacehe, Tobias Geerinckx-Rice,
Josselin Poiret, Ricardo Wurmus, Christopher Baines
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [bug#68946] [PATCH v2] guix: Add logging module.
2024-02-23 1:13 ` Maxim Cournoyer
@ 2024-02-24 11:44 ` Ludovic Courtès
2024-02-25 14:32 ` Maxim Cournoyer
2024-11-16 10:57 ` Simon Tournier
0 siblings, 2 replies; 12+ messages in thread
From: Ludovic Courtès @ 2024-02-24 11:44 UTC (permalink / raw)
To: Maxim Cournoyer
Cc: 68946, Simon Tournier, Mathieu Othacehe, Tobias Geerinckx-Rice,
Josselin Poiret, Ricardo Wurmus, Christopher Baines
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’.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [bug#68946] [PATCH v2] guix: Add logging module.
2024-02-24 11:44 ` Ludovic Courtès
@ 2024-02-25 14:32 ` Maxim Cournoyer
2024-11-16 10:57 ` Simon Tournier
1 sibling, 0 replies; 12+ messages in thread
From: Maxim Cournoyer @ 2024-02-25 14:32 UTC (permalink / raw)
To: Ludovic Courtès
Cc: 68946, Simon Tournier, Mathieu Othacehe, Tobias Geerinckx-Rice,
Josselin Poiret, Ricardo Wurmus, Christopher Baines
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [bug#68946] [PATCH v2] guix: Add logging module.
2024-02-24 11:44 ` Ludovic Courtès
2024-02-25 14:32 ` Maxim Cournoyer
@ 2024-11-16 10:57 ` Simon Tournier
1 sibling, 0 replies; 12+ messages in thread
From: Simon Tournier @ 2024-11-16 10:57 UTC (permalink / raw)
To: Ludovic Courtès, Maxim Cournoyer
Cc: 68946, Mathieu Othacehe, Tobias Geerinckx-Rice, Josselin Poiret,
Ricardo Wurmus, Christopher Baines
Hi,
On Sat, 24 Feb 2024 at 12:44, Ludovic Courtès <ludo@gnu.org> wrote:
>> 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. :-)
I propose a candidate: guix pull. ;-) See:
toward a plan? (was Re: Reducing "You found a bug" reports)
Simon Tournier <zimon.toutoune@gmail.com>
Tue, 10 Sep 2024 16:51:09 +0200
id:87wmjjd0ki.fsf@gmail.com
https://lists.gnu.org/archive/html/guix-devel/2024-09
https://yhetil.org/guix/87wmjjd0ki.fsf@gmail.com
Cheers,
simon
^ permalink raw reply [flat|nested] 12+ messages in thread
* [bug#68946] Add logging capability to Guix
2024-02-06 4:12 [bug#68946] [RFC PATCH 0/1] Add logging capability to Guix Maxim Cournoyer
2024-02-06 4:12 ` [bug#68946] [RFC PATCH 1/1] guix: Add logging module Maxim Cournoyer
2024-02-11 4:43 ` [bug#68946] [PATCH v2] " Maxim Cournoyer
@ 2024-11-01 21:00 ` Steve George
2 siblings, 0 replies; 12+ messages in thread
From: Steve George @ 2024-11-01 21:00 UTC (permalink / raw)
To: 68946
Hi,
Giving this a little nudge. I think the ball was with you Maxim to send a patch adding some initial logging to see if it becomes useful. Still happy to do so?
Steve / Futurile
^ permalink raw reply [flat|nested] 12+ messages in thread