Subject: Re: [bug#67489] [PATCH] monad-repl: Add REPL commands to inspect package arguments.

From: Simon Tournier <[email-masked]>
Date: Tue, 28 Nov 2023 11:49:39 +0100

Hi,

TLDR: All LGTM. :-)

Minor comments.

On Mon, 27 Nov 2023 at 18:19, Ludovic Courtès <[email-masked]> wrote:

> Someone recently suggested we should have better debugging facilities
> for packagers, in particular when dealing with package variants,
> inheritance, and so on, which make it harder to understand what’s
> going on.

>From a quick test:

--8<---------------cut here---------------start------------->8---
scheme@(guix-user)> ,phases cava
$1 = (modify-phases %standard-phases
       (replace 'bootstrap
         (lambda _
           (setenv "HOME"
                   (getcwd))
           (invoke "sh" "autogen.sh")))
       (add-before 'build 'make-cava-ldflags
         (lambda _
           (mkdir-p (string-append ((@ (guile) getenv)
                                    "out") "/lib"))))
       (add-after 'install 'data
         (lambda _
           (for-each (lambda (file)
                       (install-file file
                                     (string-append ((@ (guile) getenv)
                                                     "out")
                                                    "/share/doc/examples")))
                     (find-files "example_files")))))
scheme@(guix-user)> ,configure-flags cava
$2 = (list (string-append "PREFIX="
                          ((@ (guile) getenv)
                           "out"))
           (string-append "FONT_DIR="
                          ((@ (guile) getenv)
                           "out") "/share/consolefonts"))
--8<---------------cut here---------------end--------------->8---

Cool!  Very helpful.  Much better than, something like:

--8<---------------cut here---------------start------------->8---
scheme@(guix-user)> ,pp (cdr (memq #:phases (package-arguments cava)))
$3 = (#<gexp (modify-phases %standard-phases (replace (quote bootstrap) (lambda _ (setenv "HOME" (getcwd)) (invoke "sh" "autogen.sh"))) (add-before (quote build) (quote make-cava-ldflags) (lambda _ (mkdir-p (string-append #<gexp-output out> "/lib")))) (add-after (quote install) (quote data) (lambda _ (for-each (lambda (file) (install-file file (string-append #<gexp-output out> "/share/doc/examples"))) (find-files "example_files"))))) gnu/packages/audio.scm:5161:11 7f7258403a50>)
--8<---------------cut here---------------end--------------->8---

Without drifting, why not the meta-command ,arguments?  For example, the
package could also have other specific keywords that maybe I would like
to inspect from the REPL.  For example, some packages have their own
keywords as #:julia-package-name.

Well, package-arguments partially does the job:

--8<---------------cut here---------------start------------->8---
scheme@(guix-user)> ,pp (package-arguments julia-uris)
$5 = (#:julia-package-name
 "URIs"
 #:julia-package-uuid
 "5c2747f8-b7ea-4ff2-ba2e-563bfd36b1d4"
 #:phases
 #<gexp (modify-phases %standard-phases (add-before (quote check) (quote change-dir) (lambda* (#:key source outputs #:allow-other-keys) (let ((out (assoc-ref outputs "out"))) (chdir (string-append out "/share/julia/loadpath/URIs/test")))))) gnu/packages/julia-xyz.scm:6482:6 7f725a441120>)
--8<---------------cut here---------------end--------------->8---

Other said, it could be nice to:

 1. pretty-print package-arguments by default,
 2. and compile the G-expressions.

Hence ,arguments. :-)

> +As a packager, you may be willing to inspect the build phases or flags
> +of a given package; this is particularly useful when relying a lot on
> +inheritance to define package variants (@pxref{Defining Package
> +Variants}) or when package arguments are a result of some computation,
> +both of which can make it harder to foresee what ends up in the package
> +arguments.  Additional commands let you inspect those package arguments:
> +
> +@example
> +scheme@@(guix-user)> ,phases grep
> +$1 = (modify-phases %standard-phases
> +       (add-after 'install 'fix-egrep-and-fgrep
> +         (lambda* (#:key outputs #:allow-other-keys)
> +           (let* ((out (assoc-ref outputs "out"))
> +                  (bin (string-append out "/bin")))
> +             (substitute* (list (string-append bin "/egrep")
> +                                (string-append bin "/fgrep"))
> +               (("^exec grep")
> +                (string-append "exec " bin "/grep")))))))
> +scheme@@(guix-user)> ,configure-flags findutils
> +$2 = (list "--localstatedir=/var")
> +scheme@@(guix-user)> ,make-flags binutils
> +$3 = '("MAKEINFO=true")
> +@end example
> +

Here all is clear.  What can be confusing is the expansion of #$output
as ((@ (guile) getenv) "out").  Maybe it could be worth to document this
under G-expression section.  Or provide a similar example using
G-expression under the section G-expression.

Well, that’s unrelated to this patch and could be done separately. :-)

>  ;;; GNU Guix --- Functional package management for GNU
> -;;; Copyright © 2014, 2015, 2016, 2022 Ludovic Courtès <[email-masked]>
> +;;; Copyright © 2014-2016, 2022-2023 Ludovic Courtès <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -21,13 +21,15 @@ (define-module (guix monad-repl)
>    #:use-module (guix monads)
>    #:use-module (guix utils)
>    #:use-module (guix packages)
> +  #:autoload   (guix build-system) (bag)
>    #:use-module (guix status)
> -  #:autoload   (guix gexp) (lower-object)
> +  #:autoload   (guix gexp) (gexp gexp? lower-gexp lowered-gexp-sexp lower-object)
>    #:use-module ((guix derivations)
>                  #:select (derivation?
>                            derivation->output-paths built-derivations))
> +  #:autoload   (guix read-print) (pretty-print-with-comments)
>    #:use-module (ice-9 match)
> -  #:use-module (ice-9 pretty-print)
> +  #:autoload   (ice-9 pretty-print) (pretty-print)

Out of curiosity, what is the advantage for #:autoload versus
#:use-module + #:select?

> +(define (keyword-argument-value args keyword default)
> +  "Return the value associated with KEYWORD in ARGS, a keyword/value sequence,
> +or DEFAULT if KEYWORD is missing from ARGS."
> +  (let loop ((args args))
> +    (match args
> +      (()
> +       default)
> +      ((kw value rest ...)
> +       (if (eq? kw keyword)
> +           value
> +           (loop rest))))))

    ( Aside.  Each time, I am surprised that common helpers are not
       Guile builtins, here for extracting keyword value.  It would help
       Guile to provide such builtin, by default or via some ice-9
       module.

     My 2 cents. ;-) )

Cheers,
simon