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

Previous Next

Full log

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

From: Timothy Sample <samplet <at> ngyro.com>
To: Philip McGrath <philip <at> philipmcgrath.com>
Cc: 51838 <at> debbugs.gnu.org, Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Subject: Re: bug#51838: [PATCH 00/11] guix: node-build-system: Support
 compiling add-ons with node-gyp.
Date: Sun, 28 Nov 2021 14:27:56 -0500

Hi Philip,

Philip McGrath <philip <at> philipmcgrath.com> writes:

> -  (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))))
> +  (define (resolve-dependencies meta-alist meta-key)
> +    (match (assoc-ref meta-alist meta-key)
> +      (#f
> +       '())
> +      (('@ . orig-deps)
> +       (fold (match-lambda*
> +               (('@ acc)
> +                acc)
> +                (((key . value) acc)
> +                 (if (member key absent-dependencies)
> +                     acc
> +                     (acons key (hash-ref index key value) acc))))
>            '()
> -          (or (assoc-ref package-meta meta-key) '())))
> +          orig-deps))))

There’s a lot of refactoring going here in addition to change at hand.
To me, it needlessly obscures the actual change (which is just the check
for membership in 'absent-dependencies', AFAICS).

>    (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"'.

Good catch!

> +      (let* ((package-meta (read-json in))
> +             (alist (match package-meta
> +                      ((@ . alist) alist)))

Why do we have to unwrap (and then re-wrap later on) the alist?  It
would be nice to explain that in the changelog.  Maybe it’s just me, but
I can’t figure it out!  :)

Thanks!


-- Tim

Previous Next

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