From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id uFsoBcAUvmF6cQAAgWs5BA (envelope-from ) for ; Sat, 18 Dec 2021 18:05:04 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id mILuAMAUvmE6fgAA1q6Kng (envelope-from ) for ; Sat, 18 Dec 2021 17:05:04 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 9BD2DD47A for ; Sat, 18 Dec 2021 18:05:03 +0100 (CET) Received: from localhost ([::1]:47602 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1myd8o-0000hv-PE for larch@yhetil.org; Sat, 18 Dec 2021 12:05:02 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37174) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1myd7r-0007u9-2y for guix-patches@gnu.org; Sat, 18 Dec 2021 12:04:03 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:60387) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1myd7q-0007hV-QG for guix-patches@gnu.org; Sat, 18 Dec 2021 12:04:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1myd7q-00050b-Mn for guix-patches@gnu.org; Sat, 18 Dec 2021 12:04:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#51838] [PATCH v5 06/45] guix: node-build-system: Refactor patch-dependencies phase. Resent-From: Philip McGrath Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sat, 18 Dec 2021 17:04:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 51838 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Liliana Marie Prikler , 51838@debbugs.gnu.org Cc: Timothy Sample , Pierre Langlois , Jelle Licht Received: via spool by 51838-submit@debbugs.gnu.org id=B51838.163984701519203 (code B ref 51838); Sat, 18 Dec 2021 17:04:02 +0000 Received: (at 51838) by debbugs.gnu.org; 18 Dec 2021 17:03:35 +0000 Received: from localhost ([127.0.0.1]:43698 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1myd7P-0004ze-7v for submit@debbugs.gnu.org; Sat, 18 Dec 2021 12:03:35 -0500 Received: from mail-qt1-f177.google.com ([209.85.160.177]:39779) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1myd7M-0004zR-3V for 51838@debbugs.gnu.org; Sat, 18 Dec 2021 12:03:33 -0500 Received: by mail-qt1-f177.google.com with SMTP id l8so5844533qtk.6 for <51838@debbugs.gnu.org>; Sat, 18 Dec 2021 09:03:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=philipmcgrath.com; s=google; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=9IhIYAEOKP+/vcbHDd2Ae1pc+p5sFAMA2+O+8Uzu/Sw=; b=LdO/sIhSg5QQBwDkBNDFq9gd8e4cZgX4AjtsjwRT26nV1cUmbXVWeo2pts5pdOIcHK vGuHXuXrNFFWdkBPMqeg/rQWMKoKGdxSBfS0LfWpZYT+RXfOWXtK6KKKF2GoOMecTYDB 9VAjqcbKMGBQo+g+0ACudR8nDiKzSwpK1labVfY+xAjzb6K63iavFpF7eXqNHkRRDWNo fPTjfW0E1KFt3FHWPj4BvV9tzU/JH3N07jj8OrJre92YX5ZnnIhwRP4a9po4qWqBGC2a S5+8U0zte+pi3u7Z/IALR6TOhd2FLpKHToltBS3hrCp/7N/6yO2aFFXh1W74hk7CHGIs vNBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=9IhIYAEOKP+/vcbHDd2Ae1pc+p5sFAMA2+O+8Uzu/Sw=; b=gTLQg37G5tzYb6458GEXzDP9KNR5z6sX2TqOJPh+hxCe8lqa2N+pkhij32hbXTo9fK 20wx9xfV3lJndaACT9W1sXwWO6Sn9wqVG8yv+bb3AXBKSXNRAyMIcYHAI9/R9D/U/dQw V1ZfT0VoeABwAo/G/wDcj/5nNZCna7zYd7N5sZzCpaWUopl1BFYvlr1aLm979LcMp2F9 tSc67YtIfRHLYz3z7dy5ejprdX7c3rgiwwbAbMfffFlhUmdTI8q3fhno7JROXcLds3AA Z8ebZSgy2VCBlBS4LDzq6hvhElQnNTZAmHh4HFR2gOCvUdFd69/leMceZxQ1B9IH1zk1 H4/g== X-Gm-Message-State: AOAM533R1YRaPIIvQivuFtlX5UFspIDKHkU/oKNezozgnU7DUbQw07xM UZOwjZmFU0Gx4VwYCTYvAwZGZQ== X-Google-Smtp-Source: ABdhPJzPQtOoqCgCMn+CjP93ggzh+mHe/kH1AZBR09ihNsq2xRQjMcyE8Xc6gEvwv3YnRjzZuLJAZw== X-Received: by 2002:ac8:5f0a:: with SMTP id x10mr6927815qta.607.1639847006609; Sat, 18 Dec 2021 09:03:26 -0800 (PST) Received: from [192.168.45.37] (c-73-125-89-242.hsd1.fl.comcast.net. [73.125.89.242]) by smtp.gmail.com with ESMTPSA id p10sm10653107qtw.97.2021.12.18.09.03.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 18 Dec 2021 09:03:26 -0800 (PST) Message-ID: <650d1c43-8106-e7e9-5855-29cfa5f9d147@philipmcgrath.com> Date: Sat, 18 Dec 2021 12:03:25 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 Content-Language: en-US References: <20211213060107.129223-1-philip@philipmcgrath.com> <20211217020325.520821-1-philip@philipmcgrath.com> <20211217020325.520821-7-philip@philipmcgrath.com> <17f63a58ea9b462e67847f9c7698a119e3915a08.camel@gmail.com> From: Philip McGrath In-Reply-To: <17f63a58ea9b462e67847f9c7698a119e3915a08.camel@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1639847103; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=9IhIYAEOKP+/vcbHDd2Ae1pc+p5sFAMA2+O+8Uzu/Sw=; b=aMQLqkYatpPUdB+fbhSiI9P03a1biSH1exkRPpUd0JqfgSf6pkzypUS42KVfCj/L8gvDIe MEaqN4WffDSJ1D5q2z1Zard92E0UCTcU06xOVmYlwm1TWeKTDOz9gzRbZw56z0SgICheCl 5WeZmnzG1i4CtU3H6b5TGCFn+DfQhP3Mfka6pEdcZS0b0xpuE3Exx1DZVBAgiyP1YJV8nO iIcVcmUKRRLaRkSXVvrHEIcxFDQ6DhBh/W2ahpbh4YTOHh5tiFrd2kbcBxF1v3Eb/N7vOS nxtUq+Rq94PlXsdkRUvsjccpDcvvyepdcQP2i2+WFPfgdhVTj3BtURUJ7BFfWw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1639847103; a=rsa-sha256; cv=none; b=o0ooBI731q+g1IUsgsx4CJn9Av4zQUcoV7IKp3DH1ZMMD+jJMFUDxL5capMjVlA9jHWU6q pIrdZmKI3p0gVC9F/sIlG5LJxoiiamFy0vXGnKcSYCOXBU7oCJpp5xblM7/REQ18ecFTi3 WQYwgjSvkGOPV0+imKwM4DCy8PSEyBj30nx8wI/6gAz3e2X23Ij6++ZpYocGXxiA23So0b NHSivMIIOAzyViPsI1LzgLZkVR/BGH72kPdaHh42P66fz65xwl/pMqI4rX93F+yGzYOsSU HUiAHHIYQI6kK/Zw+XQlSRsYZXLd/sKBx8erbpbu7A9zE8y+6nyJbU6fn3vVHA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b="LdO/sIhS"; dmarc=none; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: -2.01 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b="LdO/sIhS"; dmarc=none; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 9BD2DD47A X-Spam-Score: -2.01 X-Migadu-Scanner: scn0.migadu.com X-TUID: 6J+heLIinSUG Hi, On 12/16/21 23:29, Liliana Marie Prikler wrote: > 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? 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. -Philip