From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id YOilKhdAqWFWVwEAgWs5BA (envelope-from ) for ; Thu, 02 Dec 2021 22:52:23 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id KBVCJhdAqWHrGgAAbx9fmQ (envelope-from ) for ; Thu, 02 Dec 2021 21:52:23 +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 494B1295D1 for ; Thu, 2 Dec 2021 22:52:23 +0100 (CET) Received: from localhost ([::1]:45392 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1msu06-0005fd-AK for larch@yhetil.org; Thu, 02 Dec 2021 16:52:22 -0500 Received: from eggs.gnu.org ([209.51.188.92]:58756) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mstzm-0005cK-8Q for guix-patches@gnu.org; Thu, 02 Dec 2021 16:52:03 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:37940) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mstzm-0001z7-0f for guix-patches@gnu.org; Thu, 02 Dec 2021 16:52:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mstzm-0007CI-00 for guix-patches@gnu.org; Thu, 02 Dec 2021 16:52:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#51838] [PATCH 00/11] guix: node-build-system: Support compiling add-ons with node-gyp. Resent-From: Philip McGrath Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 02 Dec 2021 21:52: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: Timothy Sample Cc: 51838@debbugs.gnu.org, Liliana Marie Prikler Received: via spool by 51838-submit@debbugs.gnu.org id=B51838.163848187127604 (code B ref 51838); Thu, 02 Dec 2021 21:52:01 +0000 Received: (at 51838) by debbugs.gnu.org; 2 Dec 2021 21:51:11 +0000 Received: from localhost ([127.0.0.1]:49486 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mstyw-0007B9-PM for submit@debbugs.gnu.org; Thu, 02 Dec 2021 16:51:11 -0500 Received: from mail-vk1-f182.google.com ([209.85.221.182]:44969) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mstyq-0007AT-S3 for 51838@debbugs.gnu.org; Thu, 02 Dec 2021 16:51:09 -0500 Received: by mail-vk1-f182.google.com with SMTP id u68so526970vke.11 for <51838@debbugs.gnu.org>; Thu, 02 Dec 2021 13:51:04 -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=40zKOFAJXpC22fTOQwznN3MMLG1e+IqwiWd6R1GBiWM=; b=UddifdzLyv/yxjgxXLq0rqNGXrnWbUuB06rQLLKWP7+PKO2ldHURhSbkteGzrWcZwQ Jv4+jY/O8+AIVRkVpwcLsH/69ZC6lU9NNoZ6CLGLY+nR3+3d3h3O3c24wf294SX8qATK 3920SggOhhSLTUdsNWG5sSc57fxfXxy3Lp2CBOMJdlCRz8nU9Bdi9VxDwdr/Cv0zpvjJ Q+waBRGMXLg9fu6/l8xFIoCOZDoyQUKfslap/lKl+koOiJcK+Mp2QFZeeWVsMwxH+uhc Kwe20fJtzaS8Y7LKkXctd0SVypJnkMyXrYsLRhHUwTajnMpVnGHQZfOeulsMM74MhWuc fmfA== 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=40zKOFAJXpC22fTOQwznN3MMLG1e+IqwiWd6R1GBiWM=; b=neiEu7TQivvyVpnezqhPGOHo1zIboIGGAZVq7hXYxYtfcXCn96hDk5xzXrL0vFPK2j AGuQB4X5m6vk6S/B0mNqWXqkgEDWSbRAVfzN8dr2tOz2hbF9Av1gqBhEUGZ9jgwPPhVI QnNCoyU+MLbgMZfRBHAcwj427C2rGXTcAeMXctjWFR8IsygW4ft+moewGKOf5tLnny6X JfH02/lez2jUrYd2Owpv83iEIO17pRj9BWS0vY15eVjmk8j0NjWAdQmQCZo53loj4tqw xe+bN2y45VmAtU1lPePJ2Uo0X1M3LfAESnN2KQfEhsaOYzp3faa6ChO4/QmWM3P2Oe9G S9qw== X-Gm-Message-State: AOAM533Jyfyut0xbqKJnx/oA81PZk0KByvCcXrlsnFEbJziKxF/MR1mj mdaJ+WEbLhzRy2DxFiYQi+IKrQ== X-Google-Smtp-Source: ABdhPJy9fbv/3UN+JSiTytBI82fo1BUPmZtSLE/dqQyO8hQUmuQ3ZhKbUxK/FhDfLOORS83OP/L/CA== X-Received: by 2002:a05:6122:64b:: with SMTP id h11mr18502820vkp.16.1638481859218; Thu, 02 Dec 2021 13:50:59 -0800 (PST) Received: from [192.168.45.36] (c-73-125-89-242.hsd1.fl.comcast.net. [73.125.89.242]) by smtp.gmail.com with ESMTPSA id t189sm211358vsb.13.2021.12.02.13.50.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 02 Dec 2021 13:50:59 -0800 (PST) Message-ID: Date: Thu, 2 Dec 2021 16:50:58 -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: <5a04aa92-e80d-e11b-235c-b7f5e3a92d00@philipmcgrath.com> <20211120043406.952350-1-philip@philipmcgrath.com> <20211120043406.952350-5-philip@philipmcgrath.com> <87tufwm66b.fsf_-_@ngyro.com> From: Philip McGrath In-Reply-To: <87tufwm66b.fsf_-_@ngyro.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=1638481943; 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=40zKOFAJXpC22fTOQwznN3MMLG1e+IqwiWd6R1GBiWM=; b=gdoLEMyRkwqBU4smSgoeeexTtERnFW4H9ulX8pmd097bsYkyQE2Cvl5whxWBvUE10ca49H ruDoGNWotudoodQAum8rAT+Ik3YL7MppTo40wLqUOWghy7NkA77Q+18LgHPnBnAm3cc53z 5mtA7Ns7Okg6ugyGqXtxaEMqm1R3fVi97zHtjGRdUNICCUM3kzBEXCb3hZHf4bWZYqQdwL d65mimib/6VyrG6uEay1EsHZMm8ub30pk6dJm3tyX/SCEhfvBlnCYd3Sx7HzVDVvM5xLIu l1ahGYJJi4s68eOXE6N27Wv91xiJoZI4CniMi+P6Gv0qVpqTIEt4VPepTiSipw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1638481943; a=rsa-sha256; cv=none; b=IjQswtgHcUdQksT9/zvRm+pAgo0zkO2fxqVGH+kepJsG/np+w0+kZY4BIJi8EsNGBraHHH 3ZklkC+EPUM4USfP+1xGd+mYIt1Yw8DVVqpGVx6a5zHbajqx6RLr3sUlVkH5Vr4hYbei2+ qHgIrbC/0d+dl3jyrVPMmx1ZcIQuxuhsq+/mvVmR0rvzXyuMDYsppkXGv7vFBCiP+ixT4k 5/+wmddNnQLsJtI/f+Rcy7sES0W8AvJLtDVrKiXXuZSQ0kf7LlqW5zo+GsaxPSFnZRWtks nnDuPw/y/oktOA2PZCKagJQOF2vjbmOH2hysVcHYfT4py8Qqhs8n99//fD7MLw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=UddifdzL; 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: -1.92 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=UddifdzL; 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: 494B1295D1 X-Spam-Score: -1.92 X-Migadu-Scanner: scn0.migadu.com X-TUID: 5c4Dh7E696zg Hi! On 11/28/21 14:27, Timothy Sample wrote: > Philip McGrath 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). I could split it into two commits, if that would be more clear. But see below. >> (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! :) The (guix build json) module represents a JSON object as a Scheme pair with the symbol @ in the car and an alist in the cdr. I'd guess this is because, if JSON objects were represented as Scheme alists, it would be impossible to distinguish the empty JSON object from the empty JSON array. (Racket's json library instead represents JSON objects with immutable hash tables, a disjoint type.) I found it confusing that: 1. this format does not seem to be documented; 2. `resolve-dependencies` has the same shape as `assoc-ref`, but returns an unwrapped alist where `assoc-ref` would return a wrapped pair with @; and 3. prior to this patch, the implementation of `resolve-dependencies` was written as though the symbol @ could appear at arbitrary positions in the list, which made it less than obvious that it would actually occur exactly once, at the head. I tried to address the last point when refactoring, but I see that the patch I sent made matters extra confusing by leaving in dead code from the old approach (maybe due to a rebase conflict I vaguely remember?). I should fix that, and probably also explain some of this in comments. -Philip