GNU bug report logs - #51838
[PATCH 00/11] guix: node-build-system: Support compiling add-ons with node-gyp.

Previous Next

Package: guix-patches;

Reported by: Philip McGrath <philip <at> philipmcgrath.com>

Date: Sun, 14 Nov 2021 12:43:01 UTC

Severity: normal

Tags: patch

Done: Liliana Marie Prikler <liliana.prikler <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


Message #716 received at 51838 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Philip McGrath <philip <at> philipmcgrath.com>, 51838 <at> debbugs.gnu.org
Cc: Timothy Sample <samplet <at> ngyro.com>,
 Pierre Langlois <pierre.langlois <at> gmx.com>, Jelle Licht <jlicht <at> fsfe.org>
Subject: Re: [PATCH v5 06/45] guix: node-build-system: Refactor
 patch-dependencies phase.
Date: Fri, 17 Dec 2021 05:29:54 +0100
Hi,

Am Donnerstag, dem 16.12.2021 um 21:02 -0500 schrieb Philip McGrath:
> * guix/build/node-build-system.scm (patch-dependencies): Strictly
> follow the linearity rules for `assoc-set!` and friends.
> Clarify the types of the arguments to and return value from the
> internal helper function `resolve-dependencies`.
> [...]
> -  (define (resolve-dependencies package-meta meta-key)
> -    (fold (lambda (key+value acc)
> -            (match key+value
> -              ('@ acc)
> -              ((key . value) (acons key (hash-ref index key value)
> acc))))
> -          '()
> -          (or (assoc-ref package-meta meta-key) '())))
> +  (define (resolve-dependencies meta-alist meta-key)
> +    ;; Given:
> +    ;;  - The alist from "package.json", with the '@ unwrapped
> +    ;;  - A string key, like "dependencies"
> +    ;; Returns: an alist (without a wrapping '@) like the entry in
> +    ;; meta-alist for meta-key, but with dependencies supplied
> +    ;; by Guix packages mapped to the absolute store paths to use.
> +    (match (assoc-ref meta-alist meta-key)
> +      (#f
> +       '())
> +      (('@ . orig-deps)
> +       (fold (match-lambda*
> +               (((key . value) acc)
> +                (acons key (hash-ref index key value) acc)))
> +             '()
> +             orig-deps))))
>  
>    (with-atomic-file-replacement "package.json"
>      (lambda (in out)
> -      (let ((package-meta (read-json in)))
> -        (assoc-set! package-meta "dependencies"
> -                    (append
> -                     '(@)
> -                     (resolve-dependencies package-meta
> "dependencies")
> -                     (resolve-dependencies package-meta
> "peerDependencies")))
> -        (assoc-set! package-meta "devDependencies"
> -                    (append
> -                     '(@)
> -                     (resolve-dependencies package-meta
> "devDependencies")))
> +      ;; It is unsafe to rely on 'assoc-set!' to update an
> +      ;; existing assosciation list variable:
> +      ;; see 'info "(guile)Adding or Setting Alist Entries"'.
> +      (let* ((package-meta (read-json in))
> +             (alist (match package-meta
> +                      ((@ . alist) alist)))
> +             (alist
> +              (assoc-set!
> +               alist "dependencies"
> +               (append
> +                '(@)
> +                (resolve-dependencies alist "dependencies")
> +                (resolve-dependencies alist "peerDependencies"))))
> +             (alist
> +              (assoc-set!
> +               alist "devDependencies"
> +               (append
> +                '(@)
> +                (resolve-dependencies alist "devDependencies"))))
> +             (package-meta (cons '@ alist)))
>          (write-json package-meta out))))
>    #t)
The Guix codebase is generally not the place to play around with
destructive semantics.  If you can avoid assoc-set!, I think you ought
to, especially if it helps making a two-step process into a single-step
one.  Anything I'm missing here?




This bug report was last modified 3 years and 195 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.