From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id 4EKtL9OEvmGUZwEAgWs5BA (envelope-from ) for ; Sun, 19 Dec 2021 02:03:15 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id 0LpsK9OEvmHXOwAA1q6Kng (envelope-from ) for ; Sun, 19 Dec 2021 01:03:15 +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 2297A2D896 for ; Sun, 19 Dec 2021 02:03:15 +0100 (CET) Received: from localhost ([::1]:49340 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mykba-00065U-0h for larch@yhetil.org; Sat, 18 Dec 2021 20:03:14 -0500 Received: from eggs.gnu.org ([209.51.188.92]:60470) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mykbO-000658-Gu for guix-patches@gnu.org; Sat, 18 Dec 2021 20:03:02 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:32997) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mykbO-0004Zy-9j for guix-patches@gnu.org; Sat, 18 Dec 2021 20:03:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mykbO-0004pu-0k for guix-patches@gnu.org; Sat, 18 Dec 2021 20:03:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#51838] [PATCH v5 07/45] guix: node-build-system: Add #:absent-dependencies argument. Resent-From: Liliana Marie Prikler Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 19 Dec 2021 01:03: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: Philip McGrath , Timothy Sample Cc: 51838@debbugs.gnu.org, Pierre Langlois , Jelle Licht Received: via spool by 51838-submit@debbugs.gnu.org id=B51838.163987574918551 (code B ref 51838); Sun, 19 Dec 2021 01:03:01 +0000 Received: (at 51838) by debbugs.gnu.org; 19 Dec 2021 01:02:29 +0000 Received: from localhost ([127.0.0.1]:44543 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mykaq-0004p9-Qz for submit@debbugs.gnu.org; Sat, 18 Dec 2021 20:02:29 -0500 Received: from mail-wm1-f66.google.com ([209.85.128.66]:35484) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mykao-0004ov-9R for 51838@debbugs.gnu.org; Sat, 18 Dec 2021 20:02:26 -0500 Received: by mail-wm1-f66.google.com with SMTP id bg2-20020a05600c3c8200b0034565c2be15so6899339wmb.0 for <51838@debbugs.gnu.org>; Sat, 18 Dec 2021 17:02:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:subject:from:to:cc:date:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=m4Lx3K7zZYzXShI7z2jiAFRwCb0TxGyzFgLjggp6zvQ=; b=iVnnfNHeRBqIt0YJe1IP5b2zgZJLWyS4Nox/9nFo01aVGG1SxdQv36r4opPtwdcqt/ f3B8fy2IEuPaIGAjm2246zTOxZcchPm/r8rdeTxqBUq6v7EWa0HYb8asX60ox7rfYETJ +iNplQ70HWl/N5M0QXPO1oTIfq3LN26+h9ZIpC//BrcxCRFF/37xElBcw3YHfLn0pu3r zqOXr3UqXNm53YxFY0ER9R21qlWv73wfKI9/7q9WQdos+qcY7oza1KB/djOywEAA+/4I imqgepE5tYljU69Nl0A9FGgOJvhPYjap0wbFh0kach4Rloj8PXmWnoKOf5/GiM7M7Mmq 6HDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=m4Lx3K7zZYzXShI7z2jiAFRwCb0TxGyzFgLjggp6zvQ=; b=315f6w/TgxyZQ85UR7VKUKTJnI+rHE6AWCCxQnPp4JXoTHRn8Z7oXuyxBL3UGqe4ln 08JA0zBUaoXyw+oxm1VZo7l2qeuZUWoEIAHBNCwPplSdo4JRXSMcEhQad3R+iaIToOUJ O+IjDKWSke+zL4HP0KJcj3dP8HzvLltorgGl8Xez14CdZG4dlrHhFjESrbXvIk0rr99A m1PyY6NMYL06pIHc1mXHsX8hfa8HCZH4Y5LGmPE/t+ZOY/FQb6SOf1mN17bSZd38ivjw im1HyZpJN19lJXIqyh8z7aCjl0NpLwLhPaFwmKRuRvOlpixSSggJcajbiPCvfKdUgoBC XJaw== X-Gm-Message-State: AOAM532JB2B3N2AdBvf7g1oQFkfEEVz/gG7COGs72aVvrT0sY3xLg02H WuDOjWP5gYBgyvfUpTKT3UA= X-Google-Smtp-Source: ABdhPJwOKRjIS91Pj9VjdWVhvxRaVP9tuZW+wTlgWatCFpdP4ecQrq99ibFRy1+zsNCP/QWs8e0Kwg== X-Received: by 2002:a05:600c:190c:: with SMTP id j12mr8382591wmq.166.1639875740208; Sat, 18 Dec 2021 17:02:20 -0800 (PST) Received: from nijino.fritz.box (85-127-52-93.dsl.dynamic.surfer.at. [85.127.52.93]) by smtp.gmail.com with ESMTPSA id h19sm12046845wmq.0.2021.12.18.17.02.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Dec 2021 17:02:19 -0800 (PST) Message-ID: <35164d79ab6ae1c0595d27f023376cf98ab1a39d.camel@gmail.com> From: Liliana Marie Prikler Date: Sun, 19 Dec 2021 02:02:18 +0100 In-Reply-To: <314a0ea4-a851-6642-0a59-d4c61d65c242@philipmcgrath.com> 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> <314a0ea4-a851-6642-0a59-d4c61d65c242@philipmcgrath.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.42.1 MIME-Version: 1.0 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=1639875795; 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=m4Lx3K7zZYzXShI7z2jiAFRwCb0TxGyzFgLjggp6zvQ=; b=eLShCFApsWXXDWYy1mceY8BAZMZTORUCW7rNT8bRdElmMeUGBaCEbSUUYXp0ynABCh/X82 SYsf5SqEkFLabSNAT28V+61qQrGhZsP7XXrv3SDRz3jhaevMRtC6U9l5f3opEwGzusfObo g289ExIZG/IHfpi/9UyWGoYpW+ldHleh2ItbgU9nIChXJ+lUhE7qtRfW1m2/WhQy16ei+8 dENOzxBPjrtr1/5qO1n8PEIxfhYaRoU2TFPCRkMSYSkkl1BMyp8DFDrlbYIbydOZRrY2PE iOhaIB2CC3DqjrtkmbIj8F+/mqfQvytNP12xU5tFjFqhTQRk2x5ODxACFO33Iw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1639875795; a=rsa-sha256; cv=none; b=GDqfRZrbP3H4ue7thICaJkWqP/q72t5Zxrx3DS7xNf5MFx6ceXJePKKXudnC6zVJseE118 ZEyzgw++cheC6rn7RYK6yD8ZsBH7SiAMLjdWMcHpsiY2tBygEyjqmznhvUUiH4lTk30x32 i/aP+oqHsgx8PQ/Tszfv8fTZiaXN/tbX4SwgBuWIA1jr3RDBIbeSvyeGljEjvwHSU9RZtm Guu+ao4vlcFS51n4tF3yQqq55oyWU1+0jUW3JYpBP+ToU50FeioNoDbChmgRicvYlGIpnx lgwEUMHTILejuQ9wMejxG8C5mprJjNAkQVVINmgnGOKMVWYMn+gpCh3zgF1O8Q== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20210112 header.b=iVnnfNHe; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=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.41 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20210112 header.b=iVnnfNHe; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=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: 2297A2D896 X-Spam-Score: -2.41 X-Migadu-Scanner: scn0.migadu.com X-TUID: XY6Q8Gl9VP8Z Am Samstag, dem 18.12.2021 um 17:55 -0500 schrieb Philip McGrath: > [T]he failure mode for CommonJS require (ES6 import may be at least > somewhat better) is much less clean than this seems to suggest. I think the less I know about JavaScript, the better I'll sleep at night. > 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. That might be beneficial in your particular use case here, but you also have to think about the maintenance burden your mechanism introduces. What if another leftpad happens and we have to speedrun our core- updates cycle to add #:absent-dependencies everywhere? > 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. The fix to not having test frameworks is adding them, not > 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... Compare this to tests. We have a keyword to disable all tests, which defaults to #f and we have other idioms for disabling particular tests. Your use case is no different. If at all, we should only have a keyword to disable the check completely and other idioms to e.g. patch the package.json file so that sanity-check/patch-dependencies/what- have-you doesn't error when it relies on its contents. > 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)))) > > 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--- You're actively making the code that resolves dependencies worse to look at only to keep the keyword. Don't. There are tools besides a hammer. > Would that meet your objective? No. As I repeatedly pointed out, I want either no keyword addition for this "feature" or a keyword that can be regarded as essentially boolean if not outright implemented as one. Reading this again, the existing lines already do what I want (hash-ref index key value) spits out value as a default if key is not found. So the solution is to not touch patch-dependencies at all; we don't have to abuse a function that's not meant for sanity checking to check sanity. > 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. Fair enough, that is probably out of scope for now, but we should revisit it before importing the node world and changing node-build- system becomes a core-updates task. > We never change APIs gratuitously. In my personal opinion, #:absent-dependencies would be a gratuitous change in API. There's no need to have this in the build system. We should rather look into ways that make it possible/easy for users to patch the package.json file between unpack and configure. This also calls back to my earlier point of the assoc-set! being out of place, which itself is also a manifestation of node-build-system not exposing adequate primitives towards rewriting essential files. For instance, the procedure (lambda (file proc) (with-atomic-file-replacement file (lambda (in out) (write-json (proc (read-json in)))))) would probably deserve its own name and export from node-build-system. Yes, you can export utility procedures from (guix build *-build- system), look at the Emacs and Python build systems for example. With this in place, we both get to have our cakes and eat it too. There would be no keyword added, and package maintainers would drop "absent" dependencies in a phase like so (add-after 'unpack 'drop-dependencies (lambda _ (with-atomic-json-replacement "package.json" (lambda (json) (map (match-lambda (('dependencies '@ . DEPENDENCIES) (filter away unwanted dependencies)) (('devDependencies '@ . DEPENDENCIES) (same)) (otherwise otherwise)) json))))) Of course, if that's too verbose, you can also expose that as a function from node-build-system. Then all we'd have to bikeshed is the name, which would be comparatively simple. Does that sound like a reasonable plan to you?