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 0FF0NwtnvmFcJQEAgWs5BA (envelope-from ) for ; Sat, 18 Dec 2021 23:56:11 +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 yFkAMwtnvmFmfwAAbx9fmQ (envelope-from ) for ; Sat, 18 Dec 2021 22:56:11 +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 2BA832A525 for ; Sat, 18 Dec 2021 23:56:11 +0100 (CET) Received: from localhost ([::1]:33154 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1myicb-00055D-VL for larch@yhetil.org; Sat, 18 Dec 2021 17:56:09 -0500 Received: from eggs.gnu.org ([209.51.188.92]:41846) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1myicU-00054q-1H for guix-patches@gnu.org; Sat, 18 Dec 2021 17:56:02 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:32882) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1myicT-000503-Op for guix-patches@gnu.org; Sat, 18 Dec 2021 17:56:01 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1myicT-0001kD-Jv for guix-patches@gnu.org; Sat, 18 Dec 2021 17:56:01 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#51838] [PATCH v5 07/45] guix: node-build-system: Add #:absent-dependencies argument. Resent-From: Philip McGrath Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sat, 18 Dec 2021 22:56: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 , Timothy Sample Cc: 51838@debbugs.gnu.org, Pierre Langlois , Jelle Licht Received: via spool by 51838-submit@debbugs.gnu.org id=B51838.16398681236658 (code B ref 51838); Sat, 18 Dec 2021 22:56:01 +0000 Received: (at 51838) by debbugs.gnu.org; 18 Dec 2021 22:55:23 +0000 Received: from localhost ([127.0.0.1]:44428 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1myibq-0001jK-Ik for submit@debbugs.gnu.org; Sat, 18 Dec 2021 17:55:23 -0500 Received: from mail-qv1-f41.google.com ([209.85.219.41]:43688) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1myibo-0001j7-QF for 51838@debbugs.gnu.org; Sat, 18 Dec 2021 17:55:21 -0500 Received: by mail-qv1-f41.google.com with SMTP id m6so5844771qvh.10 for <51838@debbugs.gnu.org>; Sat, 18 Dec 2021 14:55: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:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=ifJOZEdOe9sYa2elb1M4LrCSSfBM04dwPVOVE0nAQpA=; b=VDRCCBcAxk9K4VAJGre/ulrQjfrnaSB/j9Z2c2a9T4bpYS69Pyne5M4D4jDZ3WbokU IhcqfTYr3Gpu2O8BfdVGoVqb3gVhxGgDND5P+wiuXa3U6nYpDjznJCMpt8aOPaNNCkYw 0nCfB3Y9fk49/el1O3recd8ktbzH2RwHMmPTt9birpIdK7tZaWfMujq/AjzaTpMl+KcB nwlkfjfZK3Otvl5vdoffKZOFEOXDlVmZq9qTuTLlob7JdTFehu59rs7XWiCAXjDpwczb 49JcTQp3LYSSak+SYGhNh0TSnDdP/zSD5uLJUPVbrF4Fb3tCew4W7EavFhZSmclTQxmP 0LoQ== 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=ifJOZEdOe9sYa2elb1M4LrCSSfBM04dwPVOVE0nAQpA=; b=0ehZnWB9zQ/C9juYj2tAzuLSMwnFBAk737kl+AVEB7t5eVrJwqvcVMxMzAb0q3xyX8 z8/ILlZp/ax7AjXkB+gmPqerQ1vcSBqdgyQThHT88D9HoLpaMRunlCOu+sB96smBWe1P UvYNHRbOlU3ovaqxp8o2NfN0NeXWNDIrmJqatOC0jA6SEP+4SvegllpeoK53gyR8laNm zAJ3GKDIL7zeP3CBxYsktBS6Eqn1aoarbvcg86GGym/XtJn2/8eUU5r0n6gPfkpjvVPU 8u+XNi6QFop3+el2WJp4p0WuqZPgwYZ0N0oqpBE5B0ky21L6Gln1mJ/f+/KarGo2MS0N lgSg== X-Gm-Message-State: AOAM5325Ve/XKBXAkmbcm1HE8PW02qkysNv4d1PUaH0ueIzkBV9xMstR i1mHyZNGBy7Oz6rWL7hHMchZNg== X-Google-Smtp-Source: ABdhPJx8hr1VfTQ/hNAUZebF51obEB+Rr5oBqXcTViDx+bh+nzaAI/97Yod+ZaPu37mEHupMIB0bEQ== X-Received: by 2002:a05:6214:c83:: with SMTP id r3mr7516045qvr.110.1639868115134; Sat, 18 Dec 2021 14:55:15 -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 u16sm7646370qke.127.2021.12.18.14.55.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 18 Dec 2021 14:55:14 -0800 (PST) Message-ID: <314a0ea4-a851-6642-0a59-d4c61d65c242@philipmcgrath.com> Date: Sat, 18 Dec 2021 17:55:13 -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-8-philip@philipmcgrath.com> <87k0g36xp3.fsf@ngyro.com> <871r2a7hme.fsf@ngyro.com> <815f327e36ecd24066179586997947ecd8f31150.camel@gmail.com> <33696592-8a3a-8260-bf27-652cf78727a6@philipmcgrath.com> <2c803ea8f5c5c16c892f5528f63a4c7e98470c5b.camel@gmail.com> From: Philip McGrath In-Reply-To: <2c803ea8f5c5c16c892f5528f63a4c7e98470c5b.camel@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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=1639868171; 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=ifJOZEdOe9sYa2elb1M4LrCSSfBM04dwPVOVE0nAQpA=; b=KAxZiPRdUpqmiND8udLUtLOmKPVsRjphZ45vfZp3JpOjuKpjBnT8yWj+ZHLY2yN6A+ATHJ /6eYTn9TdBlnJRXJkZoCMaFnPzRyChMUe9oVfrVuq90XHYhaewgpHMhX7O/8/HuPYJQWoq meRXVPfXEYf7ddeevqN9SeDdHZOURKXJfbUPrxuhuu87rZqm66vVYotgw+rDgyZSHxnn1o Zsc/r0ojgLl9U9w3fhHcwITqpXSeWRHRTw+wLVypT9fm1bDfxG5cfi0Z0fH9Lo6+dE6O3t KeZOBhvO8Lmt3pPhsZlraybIobKGOnFaEAoyznfxmcPzMnij4U5CmeXJHqCGlg== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1639868171; a=rsa-sha256; cv=none; b=JdkWUkCRb1UJTLNcWcxeNeaXZCM5ANiJgoPKh+pm3rMrYqcWp1TPLzXhk5dczUGmsvBZTP ADJHkFPt8EiDY0J5LzAS8WdrgTpHY71du7GlgaULIUZjxEyxy4Ok6mJ4QyCqAD9Ea0s6DK rYt6eBFLdRZ0dQdPUHee6ywAX0aGkejDb7ideDmEiff4mBMoIQZ2AJ4IYnrZXvZQGkpbPd mdgubMVd8hgJo4IdifAqA+NHYlqdYc8rZ7xpflghs69WOH62e+rUvpcJ6W5FB10LLbYHV4 r21PM563TSZaKJJEpk6JppbdyVs/c4m/q2F7xUZCEh0zZIPEQRhpRzdbKGjj/Q== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=VDRCCBcA; 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.51 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=VDRCCBcA; 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: 2BA832A525 X-Spam-Score: -1.51 X-Migadu-Scanner: scn0.migadu.com X-TUID: SNFSJhyfnNJu Hi, On 12/18/21 15:49, Liliana Marie Prikler wrote: > Hi, > > Am Samstag, dem 18.12.2021 um 13:31 -0500 schrieb Philip McGrath: >> I also feel like I'm missing something, though maybe I just disagree. >> >> To try to be concrete, here's a real example. Between v3 and v4 of >> this patch series, I discovered that leaving out node-debug could >> actually cause runtime problems for some of the node-serialport >> packages. (It turns out it's really a logging library, which wasn't >> what I'd thought at first.) After I added the node-debug, I could >> easily search node-xyz.scm for the packages that listed it among >> their #:absent-dependencies and add it to them as an input. > Wouldn't we get a huge stack trace of doom with the failed require in > that case if it was truly non-negotiable, though? > >> It seems like this would be much less convenient if node-build-system >> were to silently delete such dependencies and simply print a warning. >> I guess I would have to search through the build logs for all Node.js >> packages, right? > Since node packages do have the tendency to pull in the entire > internet, you might not be too far off with that statement, but again > if you have a particular issue in a single package due to an omitted > dependency, the offending package ought to be close to the most recent > call on the stack. I think this is part of where our expectations are diverging, because I think the failure mode for CommonJS require (ES6 import may be at least somewhat better) is much less clean than this seems to suggest. Returning to my concrete example, my motivation for this has been trying to set up a Guix System service to run the Webthings Gateway (formerly Mozilla IoT).[1] (Currently I have it on a branch of my Guix fork at [2]; since there are many yacks left to shave before it could be upstreamed, I plan to turn it into a channel once it can build against Guix master, i.e. once this patch series is applied.) I discovered the problem with the missing node-debug only when webthings-service caused webthings-gateway to load webthings-addon-zwave-adapter at runtime. (Those are both node-build-system packages, but there are also webthings-addon-* packages in Python and Rust, and in principle any language could be used.) The webthings-addon-zwave-adapter has node-serialport as a package.json dependency. As I remember it, even then, the lack of node-debug only caused a runtime error on a certain code path, perhaps triggered by the presence or absence of Z-Wave adapter hardware: I had used all of the packages without problems before hitting the issue. There was a stack trace, and it did help somewhat, but it was immensely helpful to be able to find in node-xyz.scm all of the packages that wanted, but did not have, node-debug. I imagine it would be even more useful in a more tangled dependency graph. In particular, we don't have any JavaScript test frameworks packaged for Guix yet, so most of the time we aren't actually running the code in our Node.js packages, just putting the right files in the right places. So the runtime error may not come until some application/tool has been created, potentially very far along the dependency graph, and may not be revealed by tests (that we can actually run) at all. I think we should optimize for the kind of high-quality packages we aspire to have in mainline Guix, where eventually we hope to have all sufficiently useful libre Node.js packages available. In that case, #:absent-dependencies makes it explicit when Guix packagers are overriding an upstream decision. While we work to achieve that reality, I think #:absent-dependencies is a much better place for a to-do list than having to search build logs or source "package.json"s. However ... > > An alternative to searching through the build logs would also be to > build all the sources and grepping their package.jsons ;) > >> More generally, I think truly "optional dependencies" (in the Node.js >> sense, to the extent I understand it) and dependencies we actively >> don't want (e.g. because node-sqlite3 shouldn't transitively depend >> on node-aws-sdk) are relatively rare cases. >> >> The most common cases seem to be dependencies we really would like to >> have, such as test frameworks or Typescript, but haven't packaged >> yet, and thus need to work around. Many packages that have >> #:absent-dependencies for such reasons also need to use `#:tests? #f` >> or to replace the build phase with some kind of manual alternative. >> >> I guess I don't understand what case the warn-and-drop approach is >> optimizing for. For both the case when dependencies aren't packaged >> for Guix yet and the case when Guix packagers have actively decided >> to skip some upstream dependencies, I think #:absent-dependencies is >> more useful. Having to look for that information in warnings in the >> build log seems much less ergonomic. > That's why my original suggestion was to have a switch between "strict" > meaning we fail as before and "warn" meaning "we know we have unmet > dependencies". The "warn" case would be annotated similar to #:tests? > #f and the strict case default. So you could still communicate > important missing packages like typescript et al., but people who hack > on their own channels with lower standards can wing it without having > to delete configure. I can see the use of a "warn" mode for hacking things together quickly without having to totally delete configure. I think this could coexist with #:absent-dependencies and could be done in a follow-on to this patch series. Right now, the #:absent-dependencies argument must be a list of strings. To support a "warn" mode, we could loosen that contract a bit: we can bikeshed about details, but let's say that we're in "warn" mode if the list contains the symbol 'infer. We change this code: --- diff --git a/guix/build/node-build-system.scm b/guix/build/node-build-system.scm index b74e593838..892104b6d2 100644 --- a/guix/build/node-build-system.scm +++ b/guix/build/node-build-system.scm @@ -69,7 +69,8 @@ (define (list-modules directory) input-paths) index)) -(define* (patch-dependencies #:key inputs #:allow-other-keys) +(define* (patch-dependencies #:key inputs absent-dependencies + #:allow-other-keys) (define index (index-modules (map cdr inputs))) @@ -86,7 +87,9 @@ (define (resolve-dependencies meta-alist meta-key) (('@ . orig-deps) (fold (match-lambda* (((key . value) acc) - (acons key (hash-ref index key value) acc))) + (if (member key absent-dependencies) + acc + (acons key (hash-ref index key value) acc)))) '() orig-deps)))) -- 2.32.0 to do something like this: --8<---------------cut here---------------start------------->8--- (if (or (member key absent-dependencies) (and (memq 'infer absent-dependencies) (not (hash-ref index key #f)))) acc (acons key (hash-ref index key value) acc)) --8<---------------cut here---------------end--------------->8--- Would that meet your objective? >> Also, currently node-build-system doesn't seem to be removing those >> files which `npm pack` is supposed to exclude, which would probably >> be a prerequisite for addressing this. > For the record, which files would that be? Could we do emacs-build- > system style #:include and #:exclude lists? It at least includes files listed in ".npmignore", but I think there are entries in "package.json" that control the behavior, too. We should adjust our repack phase to ignore those files---but I, at least, would need to look into it further to know exactly what the correct behavior is. >> I think there is room for improvement in node-build-system. One thing >> I've been thinking about is some articles I've seen (but not fully >> thought through yet) from the developers of PNPM, an alternative >> package manager for Node.js, which seems to have some similarities to >> Guix in symlinking things to a "store".[1][2][3] (It could be >> especially interesting for bootstrapping npm! And the approach to >> "monorepos" also seems relevant.) I also think an importer is very >> important, even if it's an imperfect one: `guix import npm-binary` >> was indispensable in developing these patches. I have some ideas >> about improving it, in particular that we should assume the newer "^" >> semantics for dependencies everywhere (i.e. that major versions and >> only major versions have incompatible changes: a common case recently >> seems to be moving from CommonJS modules to ES6 modules). >> >> As I understand it, node-build-system is undocumented, with no >> guarantees of compatibility. If #:absent-dependencies is at least an >> improvement over the status quo---which I think it is, since the new >> packages wouldn't build with the status quo---could we apply this and >> replace it later if someone implements a better strategy? I don't >> think I can implement control-flow analysis for JavaScript within the >> scope of this patch series. > It's sadly not that easy. See XKCD 1172. Certainly that's true in general, but I thought this warning from [3] would apply here: When you maintain package definitions outside Guix, we, Guix developers, consider that *the compatibility burden is on you.* Remember that package modules and package definitions are just Scheme code that uses various programming interfaces (APIs). We want to remain free to change these APIs to keep improving Guix, possibly in ways that break your channel. We never change APIs gratuitously, but we will *not* commit to freezing APIs either. -Philip [1]: https://webthings.io/ [2]: https://gitlab.com/philip1/guix-patches/-/tree/wip-webthings [3]: https://guix.gnu.org/manual/devel/en/html_node/Creating-a-Channel.html