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

[Simon Tournier <zimon.toutoune@gmail.com>]

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)