[bug#51838] [PATCH v5 06/45] guix: node-build-system: Refactor patch-dependencies phase.

[Liliana Marie Prikler]

Hi,

Am Samstag, dem 18.12.2021 um 12:03 -0500 schrieb Philip McGrath:
> [...]
> 
> > 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?
> 
> I agree that assoc-set! is best avoided. (I am a Racketeer: we don't 
> mutate pairs.) However, this code was already using assoc-set!: the 
> change in this patch is merely to use it correctly.
> 
> AFAIK neither `info "(guile)Association Lists"` nor SRFI-1 (`info 
> "(guile)SRFI-1 Association Lists"`) provide a non-destructive assoc-
> set operation. Note in particular that acons and alist-cons do not
> work, since they don't remove existing entries for the same key: they
> would result in duplicate keys being written to the JSON object. (In
> theory this has undefined semantics; in practice, I believe Node.js
> would use the wrong entry.)
> 
> Of course, I know how to write a little library of purely functional 
> association list operations---but that seems vastly out of scope for 
> this patch series (which has already grown quite large). Furthermore,
> if we were going to make such changes, I think it might be better to
> change the representation of JSON objects to use a real immutable
> dictionary type, probably VHash (though it looks like those would
> still need missing functions, at which point a wrapper type that
> validated keys and maintained a consistent hash-proc might be even
> better). Alternatively we could use guile-json, which at least avoids
> the need for improper alists to disambiguate objects from arrays, but
> we would have to address the issues in Guix commit
> a4bb18921099b2ec8c1699e08a73ca0fa78d0486.
> 
> All of that reinforces my sense that we should not try to change this
> here.
I think you misread me here.  One thing that's bugging me is that you
(just like whoever wrote this before) strip the @ only to reintroduce
it.  I think it'd be better if (resolve-dependencies) simply took a
list and the let-block deconstructed the json.

As for the package-meta -> package-meta conversion, imo that could
perfectly be done with match or SXML transformation.  WDYT?