From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id kEeBFSpKvmEW5AAAgWs5BA (envelope-from ) for ; Sat, 18 Dec 2021 21:52:58 +0100 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id kNUuESpKvmFEOAAAB5/wlQ (envelope-from ) for ; Sat, 18 Dec 2021 20:52:58 +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 BAC47D035 for ; Sat, 18 Dec 2021 21:52:57 +0100 (CET) Received: from localhost ([::1]:53396 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1myghM-0008DQ-Ux for larch@yhetil.org; Sat, 18 Dec 2021 15:52:57 -0500 Received: from eggs.gnu.org ([209.51.188.92]:49442) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mygfo-0005LL-V6 for guix-patches@gnu.org; Sat, 18 Dec 2021 15:51:21 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:60891) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mygfo-0003ci-M0 for guix-patches@gnu.org; Sat, 18 Dec 2021 15:51:20 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mygfo-0004pJ-KA for guix-patches@gnu.org; Sat, 18 Dec 2021 15:51:20 -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: Sat, 18 Dec 2021 20:51:20 +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.163986060617721 (code B ref 51838); Sat, 18 Dec 2021 20:51:20 +0000 Received: (at 51838) by debbugs.gnu.org; 18 Dec 2021 20:50:06 +0000 Received: from localhost ([127.0.0.1]:44065 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mygec-0004bk-0m for submit@debbugs.gnu.org; Sat, 18 Dec 2021 15:50:06 -0500 Received: from mail-wr1-f66.google.com ([209.85.221.66]:35578) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mygea-0004aQ-0B for 51838@debbugs.gnu.org; Sat, 18 Dec 2021 15:50:05 -0500 Received: by mail-wr1-f66.google.com with SMTP id j18so11198625wrd.2 for <51838@debbugs.gnu.org>; Sat, 18 Dec 2021 12:50:03 -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=Akfv1H6J4TYuecC7lVywpteMNCw9KntKGIe7Dfa3g9U=; b=k5iFFrXqmr1E8iQMhYAYbUvY24d8nnWdxZoi76Y8RYzVZ9zPqkt4VxpeVUu9/yDuUN 9MBZDwI8WaaBLU48KKBtYRuOi8JQatWuYPOLVNFuq9jzzd5/uDuUgfwsZANeIxTvucVj EbXnJBejVJaytcbENkqY1JxVDuqny4ANuw1Y0vRsOAyeSyo1V/EOsLobo0dPW3z2/nI2 KWdlO7KfrRSXsu8BumAEIEV40s6WHZAj7hkzdUHjrzTYULxww17kDFr5tKJ2EE7t2I0z PLBp/h4df9ik8ANaMHcNk6XfjLSx7/P622bpdRkwhYxxx2YY8UZwqNz3AxV2KVgN+IDE jJFw== 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=Akfv1H6J4TYuecC7lVywpteMNCw9KntKGIe7Dfa3g9U=; b=CuAsyi2zZ8y1Dne5CE+y2QEgsXVgWUR9+4kT0pttjy/3x3NjPjilMHVL4gVVzgUUIb SWWjTAvzEgp595MXcwwjb3GX5KQSKXLN8swPyn+ExhGuwxFEiT3NuqGAapYTIR36qDDl hc5YFFjxjP1bmMPqLzzZhKIYScE4ep7YC0Mx0M9/mwE4MNz6IlrLCQIic95LBOaWIl8X 4dTzEYSKQuL9qQ4ODX8gvCJGTQjRnR+iQEXTQnnR6VJY/4N2tGcrEio44Bz1xH6Lj7em KmqK9Lecy7SnOi5a17w0vvQu6GOIUmsNv5E/ROwBBm2s9AZT6eszsgDQS9z6UQ6b/KU1 TlNw== X-Gm-Message-State: AOAM530lybJdtozmo8AZgLzW+FmOBT3eqWLbTSZwCLqhTNwEOUXdVL7e +xv2wiRcwjDOFOxR4AxpLIQ= X-Google-Smtp-Source: ABdhPJzKi7dRYZJ0CkpZ7XD5SgnAIdBcp6+4lz/3XQu0wrlTd6mLWrbJH+kR60NIOEomuJ3nRW2QMw== X-Received: by 2002:a05:6000:1625:: with SMTP id v5mr7331973wrb.196.1639860597919; Sat, 18 Dec 2021 12:49:57 -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 c12sm13048722wrr.108.2021.12.18.12.49.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Dec 2021 12:49:57 -0800 (PST) Message-ID: <2c803ea8f5c5c16c892f5528f63a4c7e98470c5b.camel@gmail.com> From: Liliana Marie Prikler Date: Sat, 18 Dec 2021 21:49:56 +0100 In-Reply-To: <33696592-8a3a-8260-bf27-652cf78727a6@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> 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=1639860777; 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=Akfv1H6J4TYuecC7lVywpteMNCw9KntKGIe7Dfa3g9U=; b=FoC/B1pADzPaKmzmBLZNy/D3jJCMbB1GxjLSoB8iPzSl5pHgZfIgbBPHv1wxyVNvJmqF8l VNWbY9Rf3BAm9Z55ApHt01GzTvKrW2TcIpZFqfGoNdc3rpPV+cVrm5mrhbW25KHj4J2cqx igCZ9bBFkS47XINQj7kjouL315T/yP5SsYk61XXkJTPL9z5IuR8EefcI9GDx2s2uYnA4u9 NQOAxK355esERNFGH73JCQnyjM1+mhV93FdK0zWOxwCf7E2S3f7Vm6eBTEm1sXCh91xgaB VD2vRN3MiD301SfpXqi/e7S53tmN5RbmhLDI+1K05A8tEBg0dyuyFofuQZwL9A== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1639860777; a=rsa-sha256; cv=none; b=aKx4fVnopULElxRVI6bWA3wfru9hZPl3uEtQ2XeLIxJigv/rqpxUAMKVjtyf3CGuOXNAqL 2tBtR35rdgBQ3GMZAH2sQ3JqinGNNhU6tbt2ST9wNyWadcd54bVW619OtUMGxzbUiEOR/8 SRGUa0riNMtgALnHGOLOelRJSvP+YRGhRFoeWUzKo+RB7Mlmq5xIAC2ntwCFuDQZWojkQr AGA5xDI5HqBhlcG4TlfuKJrQcw5TXjqEnQ1pgb2oWD467IdDiesW0dLQ7S5LL4zah1DCp9 qjpK7avloeH+QtJcIhzBnkPsu5F/nAiumnQQ8xzLFVoAval+5q4qYx6EZg4Pkg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20210112 header.b=k5iFFrXq; 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=k5iFFrXq; 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: BAC47D035 X-Spam-Score: -2.41 X-Migadu-Scanner: scn0.migadu.com X-TUID: 5bvFWtU7j+ov 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. 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. > > > Another benefit is that it would help us gain knowledge as to > > > which NPM packages are often used but not actually required > > > (e.g., NPM publishing tools).  With this knowledge, we could > > > write a clever NPM importer that ignored obviously inessential > > > dependencies. > > I agree with this, too: likewise, we could see packages that are > often wanted but aren't in Guix and prioritize adding them! Part of > the benefit of #:absent-dependencies, IMO, is as a form of > communication with humans. We do have a wishlist... > > > I guess I’m starting to sound like a broken record now – this is > > > basically what we covered before!  :)  Maybe we’re in need of a > > > fresh perspective.  (If anyone is reading along and has thoughts, > > > feel free to chime in!) > > I think the NPM convention is to put everything you need "at build > > time, but not at runtime" into dev-dependencies, no?  In any case, > > one approach I could offer is to sanity-check by searching for > > require() statements and trying them in a controlled node > > environment.  Thiscould look something like > > > > eval("try { var dep = require('" + dependency + "'); true } > > catch (e) { false; }") > > > > Once we know where require statements are made and whether they > > succeed, we can start estimating the impact of a missing > > dependency. > > For this, it'd be nice to have a full function call graph, in which > > a node is coloured dirty if it has a failing require statement, > > lies within a module that has one or calls into a dirty node.  > > However, as a primitive approximation we can also count the node > > modules with failing requires against those that don't.  We set an > > arbitrary threshold of allowed failures, e.g. 0.42, and then check > > that whatever value we obtain from the above is lower than that. > > This could be interesting, and I think some of the JavaScript > blunders we don't have packaged for Guix yet try to do something like > this, but such analysis is not tractable in the general case, > especially with CommonJS `require`, which is just a function and can > be given arbitrary arguments computed at runtime. (And some packages > really use it that way!) Of course I forgot about computed goto... > 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? > > While that would be nice and all, I think the overall issue with > > the current node implementation in Guix is that 'configure' and > > 'sanity-check' are the same phase, so you have to disable both or > > none.  I think we could easily do with a configure phase that does > > nothing, not even warn about a missing dependency, and a sanity- > > check phase that requires every dependency mentioned in > > package.json to be met. Packagers would then outright delete > > sanity-check as they do for python and as they did for configure > > (but not have configure fail due to it!) > > or deliberately rewrite the package.json for the sanity check and > > dropping absent dependencies, i.e. what you do minus the keyword.  > > If later needed for the purposes of an importer, we would then > > still have that database and could at some point introduce the key > > #:insane-requirements.  WDYT? > > I don't understand the benefit of this, and I'm also confused about > the proposed implementation specifics. Why even have "a configure > phase that does nothing"? What phase would run `npm install`? > Presumably, we would have to delete all missing dependencies before > that. I meant "does nothing" in the sense of "does nothing with missing dependencies", i.e. does not even warn. It would of course still run npm whatever. On that note, I did typo there, so it would be patch-dependencies. So patch-dependencies would be implemented using and=> or and-let* to decide whether to patch an entry or not. Another implementation of the sanity check phase would be to read the package json once more as we already do for patch-dependencies, but this time only to check that we have an input that provides it. The benefit of doing that would the same as the strict/warn switch from before, but without adding a new keyword at all. The downside would be that it still invites (delete 'sanity-check) without the comments we'd tend to write around #:tests? #f. > 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. Given the reality of XKCD 1172, I do think that implementing a sanity- check phase that checks for existence of all packages is the sanest option here, together with safeguarding against missing dependencies in patch-dependencies. Does that sound reasonable to everyone else here? Cheers