From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id yHD5Nh95mGGOzAAAgWs5BA (envelope-from ) for ; Sat, 20 Nov 2021 05:27: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 mp2 with LMTPS id WCixMh95mGE3RAAAB5/wlQ (envelope-from ) for ; Sat, 20 Nov 2021 04:27: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 593522F937 for ; Sat, 20 Nov 2021 05:27:11 +0100 (CET) Received: from localhost ([::1]:59612 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1moHy2-0008BK-IE for larch@yhetil.org; Fri, 19 Nov 2021 23:27:10 -0500 Received: from eggs.gnu.org ([209.51.188.92]:57482) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1moHxu-0008Au-Uc for guix-patches@gnu.org; Fri, 19 Nov 2021 23:27:02 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:57838) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1moHxu-0006Wz-M1 for guix-patches@gnu.org; Fri, 19 Nov 2021 23:27:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1moHxu-0007cF-Cz for guix-patches@gnu.org; Fri, 19 Nov 2021 23:27:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#51838] [PATCH 03/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: Sat, 20 Nov 2021 04:27:02 +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: Pierre Langlois Received: via spool by 51838-submit@debbugs.gnu.org id=B51838.163738237429212 (code B ref 51838); Sat, 20 Nov 2021 04:27:02 +0000 Received: (at 51838) by debbugs.gnu.org; 20 Nov 2021 04:26:14 +0000 Received: from localhost ([127.0.0.1]:41151 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1moHx8-0007b6-Av for submit@debbugs.gnu.org; Fri, 19 Nov 2021 23:26:14 -0500 Received: from mail-qv1-f41.google.com ([209.85.219.41]:35518) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1moHx6-0007aq-8C for 51838@debbugs.gnu.org; Fri, 19 Nov 2021 23:26:13 -0500 Received: by mail-qv1-f41.google.com with SMTP id g1so8518244qvd.2 for <51838@debbugs.gnu.org>; Fri, 19 Nov 2021 20:26:12 -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=FvpHoCQ6pAYGr59ljfr0iZSpe4T/ujl1rktP93DEteg=; b=Gt3AdPvN7BYOrr19jh0+BMIQpIx/z3607nl/6CmViypfL6UVvbiz1zHAdAIw5GPOM3 jfQSsiz1BoRJSdaBDm7E2ZrKeIM/jNnqawQoCDlSPdML1dvJ1XUC53VGHeheSV4YesWS oJHL375Lo8HBR5XMl0wMA8p/D0JSFXkKnXByaZPjGTxZ43nMBxd3d31uuqjK3qSbsUbm qml3z8q1GMNNFLhYwWofxujiitpkFBXiYcRT8D+HDVfWt/3168JJFFnwA+8gy1Cls26k IIuDQ5vUJQKPGTD3rGriNwsGBAI+IQ5D+dIri4/qG/vOPAzcCRAoIPedVQeBAXn3fMxl DTpA== 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=FvpHoCQ6pAYGr59ljfr0iZSpe4T/ujl1rktP93DEteg=; b=0JBaahkjiqWANUazAecQQQzqHSzYHejF7LgIuYfXy02U1spMF9mYdsFA0VJP+2gfjF 4IuzeqiX+XB783B7isTcbE27PfJ1MHVZAJ+raYhguQcb2SDfnh0pn7u06ZVl7+ttQ8G5 A6Qjy1/TrIYK5hhBCDABvJ5OF44KLA/51IresqVi8xQ4VuSMEBWVIQbISU4DXeyK/svB 82yNczoPdK+NvAVXWixPojG8ZZUlv4rmlFvqt8xTFNdN6dwQXLsUZh+QTOlqxuEEBWpj JJKEnZbUQeMii0V4qqtGg8pV6m03eyLAAsG18JliYfqNGZ5eNTObt3bG3KdcoLJYZSwj wNfw== X-Gm-Message-State: AOAM533G8kXlcvVsZF9QqsXwubWdBJKZ5fuUBE0Clv6o2R7YNghCa2Ow 2ogJKxJ/NRVLj4u/Brwsg7xRNA== X-Google-Smtp-Source: ABdhPJyIIYiqC6X0daUp0/C3XBmvXHV/2gb0Wst70UEsxorWgQY84ckbjDlruDJSB/tmwHuR4d6vOA== X-Received: by 2002:a0c:edb0:: with SMTP id h16mr79942924qvr.17.1637382366726; Fri, 19 Nov 2021 20:26:06 -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 t11sm993037qkp.56.2021.11.19.20.26.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 19 Nov 2021 20:26:06 -0800 (PST) Message-ID: <5a04aa92-e80d-e11b-235c-b7f5e3a92d00@philipmcgrath.com> Date: Fri, 19 Nov 2021 23:26:05 -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: <20211114125830.45427-1-philip@philipmcgrath.com> <20211114130409.49241-1-philip@philipmcgrath.com> <20211114130409.49241-2-philip@philipmcgrath.com> <48018e12484d19466d9c6f253a8d7ebeae93e947.camel@gmail.com> From: Philip McGrath In-Reply-To: <48018e12484d19466d9c6f253a8d7ebeae93e947.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=1637382431; 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=FvpHoCQ6pAYGr59ljfr0iZSpe4T/ujl1rktP93DEteg=; b=sVX1/lern+W8skYRVS34FuzERVQBVe1bOq2Sf9PB85woZCrNWbKnSZ1v9mhEQFL+SRQiqy xFKvFa6Z3b1b0YUi+8pURUbp1779beAvq5eQUlWigVGL+ok7h170GG87G5ekuRnckIUkHy F7p9phbPRSzHzXpTKLnjoG8AKJ+pXytcfmdpq5Wenkgx+v5/0hvGtPseEUkKsfpR9uBlsE loCK3CuipyBpBs9rI7jnGIg8QEAqVFF4GZxbJa/0H81PwnoUQEpqfmeHV/TQvYa2ErDcKg 70UHjDWwJiXCycDghtCftdwW+Yu2T/1bcHDgHrNY1rK6D+yo3J2azfkfx0MghQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1637382431; a=rsa-sha256; cv=none; b=mFlSyeThmUWXGcOBZgABangoc4zwx/9MjwIPHr4WU9/sFPb/TtDHL/n2GHsJW1/HjTgYM+ qN7AxMZQw9Sj7bDEBi2fqeYU7vAT8/1zbch54t6MncUBhOriDHg3H5DSR30jKgRoWbzF8N /7wgT4mpRyaCZSqyjafq70UPhxvIh460Pjadg+k/JoDUOjgF1R6uHzcsO6LOYWG0JDrpnl 1p/+SlVMk5m4RA1tLxeoofyB9d5/3DzKt5mD3WzcNmu9R9VM3y1fZH3g32+NJB+csu3dn7 Kz6JNFXO43QtODGE8xizWntF2XJVEy3EiVB0B0u05y5Tt9X52KoB9CV8xS7gaw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=Gt3AdPvN; 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.87 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=Gt3AdPvN; 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: 593522F937 X-Spam-Score: -1.87 X-Migadu-Scanner: scn0.migadu.com X-TUID: l+gOMGTnqSO2 Hi! On 11/14/21 15:44, Liliana Marie Prikler wrote: > Looking at this patch, it does *a lot* at once and could probably be > separated into more than one. Particularly, I'd suggest providing > capabilities in node-build-system first, then switching over to the new > thing in node. Thanks for these comments. Some of the things to which you drew my attention seem to have been workarounds for problems that had since been solved at a deeper level. Then, in particular, this comment: >> +(define* (configure-gyp #:key inputs #:allow-other-keys) >> + "Run 'node-gyp configure' if we see a 'binding.gyp' file." >> + (if (file-exists? "binding.gyp") >> + (invoke (which "node-gyp") "configure") >> + #t)) >> + > You might want to make this part of configure itself, though I'm not > sure what the consensus is there when mixing different build system > styles. (invoke (which ...) ) is also a pretty rare pattern, used in > only four locations so far. prompted me to look more closely at why so much manual work was needed in the first place. It turns out that the `npm install` in the `'configure` phase should have handled most of it automatically, but the Guix packages were deleting the configure phase to avoid checking for devDependencies that aren't in Guix (or that we just don't want, e.g. some dependencies of node-sqlite3). In v2 of this series (which will follow this email), I've removed all of the `node-gyp`-specific build-side code and tried a more general solution, adding an `#:absent-packages` argument to instruct the `'patch-dependencies` phase to remove the specified packages from the "package.json" file. This means that the Guix package can still run `'configure`/`npm install`, checking properly for the packages that we *do* have/want and doing all of the other automatic work `npm install` does. I also like that listing the missing packages in the Guix package definition provides a sort of documentation of what is missing: for example, it is clear which packages could have their tests enabled with the addition of a `node-tap` package, without having to inspect all of the individual "package.json" files. I've updated all of the existing Node.js packages that deleted their `'configure` phase to use `#:absent-dependencies` instead. >> @@ -58,10 +62,13 @@ (define private-keywords >> `(("source" ,source)) >> '()) >> ,@inputs >> - >> ;; Keep the standard inputs of 'gnu-build- >> system'. >> ,@(standard-packages))) >> (build-inputs `(("node" ,node) >> + ("python" ,python) >> + ;; We don't always need libuv, but the libuv >> and >> + ;; node versions need to match: >> + ("libuv" ,@(assoc-ref (package-inputs node) >> "libuv")) >> ,@native-inputs)) >> (outputs outputs) >> (build node-build) > Will this python input always or often enough be needed? What's the > build system ratio on node like, gyp vs. anything else, particularly > with packages close to the node core? GYP is a Python program, and it (or at least node's fork of it) expects to have a python executable available to invoke with sub-programs. Since npm depends on node-gyp and GYP, it is pretty close to the core. In v2, I've just had packages that use node-gyp add Python to their inputs. IIRC, that used to not be enough, but it seems underlying problems were fixed in the mean time. An alternative approach would be to configure it using the npmrc file, as I do for `nodedir` in v2. I'm not sure that makes much difference either way, but in v2 I've tried to minimize the amount of `node-gyp`-specific handling. -Philip