From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id 0DMbHavSwGFBzgAAgWs5BA (envelope-from ) for ; Mon, 20 Dec 2021 19:59:55 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id sCDBGKvSwGFmbwAAB5/wlQ (envelope-from ) for ; Mon, 20 Dec 2021 18:59:55 +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 29E5C5879 for ; Mon, 20 Dec 2021 19:59:55 +0100 (CET) Received: from localhost ([::1]:57920 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mzNt4-0008Vm-9n for larch@yhetil.org; Mon, 20 Dec 2021 13:59:54 -0500 Received: from eggs.gnu.org ([209.51.188.92]:58656) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mzN10-0003u3-PK for guix-patches@gnu.org; Mon, 20 Dec 2021 13:04:04 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:39841) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mzN10-0000cd-7V for guix-patches@gnu.org; Mon, 20 Dec 2021 13:04:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mzN0z-0007wI-SL for guix-patches@gnu.org; Mon, 20 Dec 2021 13:04:01 -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: Mon, 20 Dec 2021 18:04:01 +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.164002340530470 (code B ref 51838); Mon, 20 Dec 2021 18:04:01 +0000 Received: (at 51838) by debbugs.gnu.org; 20 Dec 2021 18:03:25 +0000 Received: from localhost ([127.0.0.1]:51387 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mzN0P-0007vO-6v for submit@debbugs.gnu.org; Mon, 20 Dec 2021 13:03:25 -0500 Received: from mail-vk1-f171.google.com ([209.85.221.171]:41903) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mzN0K-0007v4-Bx for 51838@debbugs.gnu.org; Mon, 20 Dec 2021 13:03:23 -0500 Received: by mail-vk1-f171.google.com with SMTP id s144so6635390vkb.8 for <51838@debbugs.gnu.org>; Mon, 20 Dec 2021 10:03:20 -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:from:subject:to:cc :references:content-language:in-reply-to:content-transfer-encoding; bh=fM9tdaBgXGkB7lAJxi8tbmf1xrGc2z8F9ZDh6cb1G5M=; b=YfBiwvXT5n9HxuFDq4/kPX3ut5pnPnTV10pqm8OeyJD5osFzyXKzdkvxocuKw6tyV4 AKsiRzI8EMLQzWV3W9e4KhOxvQ/ZGeVfOp+917D4y5L02P/kzJ7EVJFv0F5irKZY+Ri5 0D46NhzfesZnz6VYjpZO6631LDeORafK/oIi74EY1GVJV2/jbvJIuT9CRupVxq4BnY8/ spy22XuAQEAbEUFIjRpelZtP/G42Wqx/YiU4qcMdps6ox5/xeV3r8/e/W/ekKADpWznP OmVYvmB5jrcBUWAtS6DISgfl7owYuTjOJHAh561pKwMDXeWSdprHPSSuBWKqvkK253uO Oa3g== 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:from :subject:to:cc:references:content-language:in-reply-to :content-transfer-encoding; bh=fM9tdaBgXGkB7lAJxi8tbmf1xrGc2z8F9ZDh6cb1G5M=; b=e4ivCOa0yEWwzaTny7nLYpyRQPvkI2dp71RmC8QUcZcaXvPpDgozMSljaoTLjU/y4m Cr4Glb+VAUPoh7+c5cIcBX9oQgHCshv2nnHff2/hEL+KGvOj71nU+dnunScHlGWe8rIP B3iKziuUIPlUgCxlnumfEwF13M0SPx5nYKoWh9qfm/OutshP/oGUxnvraZe8z2jBbST3 5nneZ/4ZInzsiX5V64saO0UpOTO5jwl1xYkKgyMzS6Q2hhtJMiq2YM+uysf343DN4tJJ nuNemun1BqXJHws9alFF0SOX/lKZdJaKtJOGfrM03jDCuxpNnWg/3tH32rGyAE4JkIDI /jxg== X-Gm-Message-State: AOAM531+kckfATZBtoE9F41MKI6pM1yt61ZctYHdOfkxUeVPoLtlkSA+ VT40zCxefTA6zqI61opVx74duQ== X-Google-Smtp-Source: ABdhPJzztKTujHrQ2B7bnx7KhwMmN5UCSRYxYaCXBIt6ARItCfH7rCvMKZqiKDAQNNBRnPyqJwj63Q== X-Received: by 2002:a1f:b615:: with SMTP id g21mr6116637vkf.35.1640023394694; Mon, 20 Dec 2021 10:03:14 -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 w22sm3442240vsk.11.2021.12.20.10.03.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 20 Dec 2021 10:03:14 -0800 (PST) Message-ID: <4fead57b-66f0-a0e8-cf3b-a65a8ecba1b2@philipmcgrath.com> Date: Mon, 20 Dec 2021 13:03:13 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 From: Philip McGrath References: <20211213060107.129223-1-philip@philipmcgrath.com> <20211217020325.520821-1-philip@philipmcgrath.com> <20211217020325.520821-7-philip@philipmcgrath.com> <17f63a58ea9b462e67847f9c7698a119e3915a08.camel@gmail.com> <650d1c43-8106-e7e9-5855-29cfa5f9d147@philipmcgrath.com> <9fe83c79f796bb323816d2c02e45b4277680eda0.camel@gmail.com> Content-Language: en-US In-Reply-To: <9fe83c79f796bb323816d2c02e45b4277680eda0.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=1640026795; 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=fM9tdaBgXGkB7lAJxi8tbmf1xrGc2z8F9ZDh6cb1G5M=; b=F4HEfz+aaJJLcBesgOXAyEqXZhSzNxbLWWX9RWg/+qZqqAKfwmSOXAJlEQJyN+n2DZ1LSa FfIm4bGOQewo0vveah+TL6A8GYN3FjZJN27g7kIdQxx6O3A/9pxITWjeuxXqjXcXqxOMFB nu7/H+b2DWXBHt7FluTeM/3ePnLuYq7dmQEh5yOd2XGYIli/SubdhdshQcTCurmF7kTnY3 JgrsllWnMSRk88lfygzV73SkTsTO/zOG8qAMzDFFhLwLqEUyh5SLlqSBF2Qs25xjPsguY0 Us88II7vKmcPFAMuEUBKitZN+4ZC8lUwfNCphqUAAmxXPs2IEzdmc3ZGB0yoUQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1640026795; a=rsa-sha256; cv=none; b=KxguRbcVHmITxVfUq7nEnsQnVMcUDaHtDYaflkThj8w+ipq4iOXQNm1QhgCri1fdk1A8wu c8qlaUY2ZgMnmOdKn5oiIIZJnAjtZ3AaniQ4xGxPFYi4CM9fSJ3H9nSEeGrs56oQjwt+n/ b4jZe6CL/Sn6T4yj0D8WMffQT9q1DvAJ+tp35l219teoVGDf02lCNH8MgUTQqnvv5y+nsP e6QVlthynNoi+n5wuIu459m1A3auZrpmkwW2DmxR2PqQ8SR47zIgUR0p4VERObvd0O2A7F LTELjZjGWswS64oTILo/VhIHrlRyR2cjPv5GxLhgBniub98Ke+es/3GUJVVdQw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=YfBiwvXT; 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: 0.48 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=YfBiwvXT; 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: 29E5C5879 X-Spam-Score: 0.48 X-Migadu-Scanner: scn0.migadu.com X-TUID: so0dr5gReF0s Hi, On 12/18/21 12:52, Liliana Marie Prikler wrote: > 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? > I definitely am not understanding what you have in mind here. When you write "strip the @", I'm not sure what you're referring to, because there are multiple "@" tags here, one beginning each JSON object. (Maybe this is obvious, but it hadn't been obvious to me.) So, the (guix build json) representation of a "package.json" file might look like this: ``` `(@ ("name" . "apple") ("version" . "1.0") ("dependencies". (@ ("banana" . "*") ("pear" . "*"))) ("devDependencies" . (@ ("peach" . "*") ("orange" . "*"))) ("peerDependencies" . (@ ("node-gyp" . "7.x"))) ("peerDependenciesMeta" . (@ ("node-gyp" . (@ ("optional" . #t))))) ("optionalDependencies" . (@ ("node-gyp" . "7.x")))) ``` An unfortunate consequence of this representation is that JSON objects are not usable directly as association lists: some procedures expecting association lists seem to silently ignore the non-pair at the head of the list, but I don't think that's guaranteed, and other procedures just don't work. In particular, `append` applied to two JSON objects does not produce a JSON object, even ignoring the problem of duplicate keys. Given that the current code adds "peerDependencies" as additional "dependencies", the choice (as I see it) is between the current approach, in which `resolve-dependencies` returns genuine association lists and the `let*` block turns them JSON objects, or changing `resolve-dependencies` to return JSON objects and implementing `json-object-append`, which doesn't seem obviously better, unless it were part of broader changes. (As an aside, I am not convinced that the current handling of "peerDependencies" is right, but I think reevaluating that behavior is out of scope for this patch series, and particularly for this patch, in which, as Tim said in , my goal was merely to make the use of `assoc-set!` safe.) I definitely think the broader situation should be improved! But I think those improvements are out of scope for this patch series. It seems like much more discussion would be needed on what the improvements should be, and potentially coordination with other users of (guix build json). Personally, I'd want to represent JSON objects with a real immutable dictionary type that gave us more guarantees about correctness by construction. If we continue with tagged association lists, we should write a little library of purely functional operations on JSON objects. But that all seems very far afield. -Philip