Subject: Re: [bug#74542] [PATCH 04/11] build: Add ‘--development’ option.

From: Simon Tournier <[email-masked]>
Date: Tue, 26 Nov 2024 16:26:25 +0100

Hi,

Here the oneline commit message is:

     build: Add ‘--development’ option.

when elsewhere in the same series, it reads:

     guix build: Add ‘--dependents’.

I suggest consistency; ’build:’ or ’guix build:’ for both.

On Tue, 26 Nov 2024 at 11:33, Ludovic Courtès <[email-masked]> wrote:

> +  ;; Process OPTS in "the right order", meaning that if the user typed
> +  ;; "-D hello", arrange to see the 'development? option before the "hello"
> +  ;; spec.
> +  (let loop ((opts (reverse opts))
> +             (type 'regular)
> +             (result '()))
> +    (define (for-type obj)
> +      ;; Return a list of objects corresponding to OBJ adjusted for TYPE.
> +      (match type
> +        ('regular
> +         (list obj))
> +        ('development
> +         (if (package? obj)
> +             (map manifest-entry-item
> +                  (manifest-entries
> +                   (package->development-manifest obj system)))
> +             obj))))

Well, the loop seems overcomplicated but nothing better to propose. ;-)

> +    (match opts
> +      (()
> +       (reverse result))
> +      ((head . tail)
> +       (match head
> +         (('argument . (? string? spec))
> +          (cond ((derivation-path? spec)
> +                 (catch 'system-error
> +                   (lambda ()
> +                     ;; Ask for absolute file names so that .drv file
> +                     ;; names passed from the user to 'read-derivation'
> +                     ;; are absolute when it returns.
> +                     (let ((spec (canonicalize-path spec)))
> +                       (loop tail 'regular
> +                             (cons (read-derivation-from-file spec)
> +                                   result))))
> +                   (lambda args
> +                     ;; Non-existent .drv files can be substituted down
> +                     ;; the road, so don't error out.
> +                     (if (= ENOENT (system-error-errno args))
> +                         (loop tail 'regular result)
> +                         (apply throw args)))))
> +                ((store-path? spec)
> +                 ;; Nothing to do; maybe for --log-file.
> +                 (loop tail type result))
> +                (else
> +                 (loop tail 'regular
> +                       (append (for-type (specification->package spec))
> +                               result)))))
> +         (('file . file)
> +          (let ((file (or (and (string-suffix? ".json" file)
> +                               (json->scheme-file file))
> +                          file)))
> +            (loop tail 'regular
> +                  (append (append-map
> +                           for-type
> +                           (ensure-list (load* file (make-user-module '()))))
> +                          result))))
> +         (('manifest . manifest)
> +          (loop tail 'regular
> +                (append (map manifest-entry-item
> +                             (manifest-entries
> +                              (load* manifest
> +                                     (make-user-module '((guix profiles)
> +                                                         (gnu))))))
> +                        result)))
> +         (('expression . str)
> +          (loop tail 'regular
> +                (append (append-map for-type (ensure-list (read/eval str)))
> +                        result)))
> +         (('argument . (? derivation? drv))
> +          (loop tail 'regular (cons drv result)))

Since you are at it, why not move this pattern close to the other
pattern matching ’argument’ too?

Cheers,
simon