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 yJxqHFRntmHvRwAAgWs5BA (envelope-from ) for ; Sun, 12 Dec 2021 22:19:16 +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 uK8lGFRntmGDBwAAB5/wlQ (envelope-from ) for ; Sun, 12 Dec 2021 21:19:16 +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 249A72434D for ; Sun, 12 Dec 2021 22:19:12 +0100 (CET) Received: from localhost ([::1]:55940 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mwWFT-0005L5-9S for larch@yhetil.org; Sun, 12 Dec 2021 16:19:11 -0500 Received: from eggs.gnu.org ([209.51.188.92]:54176) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mwWFK-0005Ke-PC for guix-patches@gnu.org; Sun, 12 Dec 2021 16:19:02 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:41757) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mwWFK-00085c-Da for guix-patches@gnu.org; Sun, 12 Dec 2021 16:19:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mwWFK-0003oZ-AP for guix-patches@gnu.org; Sun, 12 Dec 2021 16:19:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#51838] [PATCH v3 29/43] gnu: Add node-sqlite3. Resent-From: Philip McGrath Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 12 Dec 2021 21:19: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: Pierre Langlois Cc: 51838@debbugs.gnu.org X-Debbugs-Original-Cc: 51838@debbugs.gnu.org, guix-patches@gnu.org Received: via spool by 51838-submit@debbugs.gnu.org id=B51838.163934392814637 (code B ref 51838); Sun, 12 Dec 2021 21:19:02 +0000 Received: (at 51838) by debbugs.gnu.org; 12 Dec 2021 21:18:48 +0000 Received: from localhost ([127.0.0.1]:53302 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mwWF6-0003o0-B5 for submit@debbugs.gnu.org; Sun, 12 Dec 2021 16:18:48 -0500 Received: from mail-ua1-f45.google.com ([209.85.222.45]:40711) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mwWF1-0003nU-B2 for 51838@debbugs.gnu.org; Sun, 12 Dec 2021 16:18:44 -0500 Received: by mail-ua1-f45.google.com with SMTP id y5so26026311ual.7 for <51838@debbugs.gnu.org>; Sun, 12 Dec 2021 13:18:43 -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=2RhdTvEwvQxxE/zwUfFVz2h7628iRuBuX7YHOFRfcbM=; b=Zp6Jd16hksaz+e1Z5KQFFHIAcoQGuIms1uV1uCoUFr0YebLdISCI40cKODe90JgQ4A WHmzZYzNHI1bMWsDfuCwreQCdqMzi/KEmr4s8wQ1w3aDLe0fpLKHiIqTjqndNDEjlDCM P5TuBD3s36An1Y7+nIv+fht3+jMuS+9ZJFf5CuyyL4P0cqNBh61RPbACMOzy6OL9SudZ X29kJXmpWGp5jhOC1jV+TUlbmbt9Y1S8aOb+mQTapSSIBaSq40NSd3ONG6VFCiLcUR/k 08UJIq+UZRwA2jSpquL+7SIP2IwonuMfZi5FzaeE1FqLz+g1LHxefqMUJKutUOk5Ph/i hepQ== 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=2RhdTvEwvQxxE/zwUfFVz2h7628iRuBuX7YHOFRfcbM=; b=CvgwmdMrD+j9fLgWpeXJrz2p0N2rs+oiLj7u5ejiYEIutNuk9P01/ShIB/dRNJ/aKR JzLErKkFeX9Gx7rwF++n4wBEjPCw1n++hRM8SsdAxBun7c47hLTNDRoaHYmWiOS0sWJN j5T85SyS9ySsNJNl5UyqZyG6JO07EINK9X65UqS8A0iQCmrHfMj+OtgJdT3Jw4f5PAbd lAQM5FdYa6bcptHcjMrrbg1PDmfZyyEkQXgCqUTdVGskTxujK8cX1TRbfuEVvk4kewFN 5WoafO0cSVuBEe2uNvaV3dmN3RwimCClaeqbM9drDynqwosLxLb/2/xK43XBgE/Hmu+f tSdg== X-Gm-Message-State: AOAM531nkGxrZeT0I/XMKD6xYqrbQT2NpuXUgz5tjiRCEsS2BRPbUh4h a/SMZF+TffN01LDCwTOiovGM6w== X-Google-Smtp-Source: ABdhPJxM+tORmLp0R35GT+ro2jhqhnquLqJkqtZHmnz0xRlpAPCkF0wmfIEfjCFCoQj7Tq8hh9Nskw== X-Received: by 2002:a67:60c2:: with SMTP id u185mr24382044vsb.73.1639343917704; Sun, 12 Dec 2021 13:18:37 -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 x23sm1816007vsk.24.2021.12.12.13.18.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 12 Dec 2021 13:18:37 -0800 (PST) Message-ID: <5791d5f0-60c7-5f34-be4a-9d9c2333c703@philipmcgrath.com> Date: Sun, 12 Dec 2021 16:18:36 -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: <46a042bc08eb72a068b1e8c69bfe28cf2d4b2e53.camel@gmail.com> <20211208202838.752542-1-philip@philipmcgrath.com> <20211208202838.752542-30-philip@philipmcgrath.com> <87v8ztzu1a.fsf@gmx.com> From: Philip McGrath In-Reply-To: <87v8ztzu1a.fsf@gmx.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 ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1639343952; 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=2RhdTvEwvQxxE/zwUfFVz2h7628iRuBuX7YHOFRfcbM=; b=TSqoQioRtmC2G9bUrZPVgixaOnvjSM6gE0Jr34Cb1JneIwVofY0w8kPGK4eD3t9G0Y4T4x 7wC1Ko9zC4zyuOMSRU3nq5TyJuuY5nOS5aeL0mPM/HALf0/KG1T1X3Mosr6iRb/Bb+6aJ3 lCYl3ttBwZMqIXm4aARM8ixqiSve1zEq/eCvmsA12opiYLmkLfHkq7LqTuDUi9ODkfL1jW kPzdJa1O5dKvBgXuaYekg+6lIz2ZcfZP4Z5hbC7qg5DjnVK5oSJ0pDv8PXnbEugc3d+72O isEUGZOYPAJzPI5G9E/H/+Fk6EQVnFM3zQ+JkBufBxC8ZYYsegmUMv55JZL6EA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1639343952; a=rsa-sha256; cv=none; b=mR3ruQ9YOGsmV4nCm0WR9TuRtbk/tTxrXPqaBDLDcO89aj1ocqo0+n81gtvOfBPBEwu7P3 DEb9OkmpQC7QE+kmW0qpGtnA43mWrECy1K0SshdlCYgY6LDrqrvPQavIQ/L7BszPhX9eXm JL1NNE+kM/RJxSEpujDNoezX2qen43PTttqtuPe9ylhp+k32VJhi/W9jSHYD9XEzs0JUE2 0OqkQMK/gBQoQo8EtzgzSalvpqAAR/BbFbvAt5bAh2EUSDx2q58Zv912MSHyejbGoBghxF RXDIiuBiUKZYNKHjXjf4SkP9jF766IRKEt85DvurkfB7iKhCXP/kBAUGmSYuYg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=Zp6Jd16h; 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.97 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=Zp6Jd16h; 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: 249A72434D X-Spam-Score: -1.97 X-Migadu-Scanner: scn1.migadu.com X-TUID: QXALQTCDVPsS On 12/12/21 10:42, Pierre Langlois wrote: > > Philip McGrath writes: > >> * gnu/packages/node-xyz.scm (node-sqlite3): New variable. >> --- >> gnu/packages/node-xyz.scm | 118 +++++++++++++++++++++++++++++++++++++- >> 1 file changed, 115 insertions(+), 3 deletions(-) >> >> diff --git a/gnu/packages/node-xyz.scm b/gnu/packages/node-xyz.scm >> index 60dbfc163c..b979d0cd53 100644 >> --- a/gnu/packages/node-xyz.scm >> +++ b/gnu/packages/node-xyz.scm >> @@ -615,7 +615,8 @@ (define-public node-addon-api >> (sha256 >> (base32 "1bhvfi2m9nxfz418s619914vmidcnrzbjv6l9nid476c3zlpazch")))) >> (inputs >> - `(("python" ,python))) >> + `(("python" ,python) >> + ("node-safe-buffer" ,node-safe-buffer))) >> (build-system node-build-system) >> (arguments >> `(#:absent-dependencies >> @@ -630,8 +631,7 @@ (define-public node-addon-api >> "eslint-plugin-promise" >> "fs-extra" >> "path" >> - "pre-commit" >> - "safe-buffer") >> + "pre-commit") >> #:phases >> (modify-phases %standard-phases >> (add-after 'unpack 'skip-js-tests > > nit: Did you mean to include these changes in this patch? It seems they > should be part of the node-addon-api patch. Thanks! I've rebased, reorganized, and squashed this series a number of times: these must have ended up in the wrong place. >> + "aws-sdk" >> + "@mapbox/cloudfriend" >> + ;; Confusingly, this is only a dependency beceuse of > > typo: beceuse -> because Thanks, will fix! >> + (add-after 'patch-dependencies 'avoid-node-pre-gyp >> + (lambda args >> + (substitute* ".npmignore" >> + (("lib/binding") >> + "#lib/binding # <- patched for Guix")) > > Would this substitute* be more suited to live in the 'patch-binding-path > phase? No, at least not as currently written. The upstream .npmignore file excludes the compiled addon from `npm pack` and `npm install`, so patching it after Guix's install phase would be too late. (Except that, unfortunately, Guix doesn't seem to be respecting instructions about which files to include or not in built packages ...) The 'patch-binding-path phase, on the other hand, replaces code that dynamically finds the addon to load at runtime via the `@mapbox/node-pre-gyp` package (which we don't have) with one that simply uses the path we built directly. Because that path depends (or should---see below) on the expansion of the `module_path` configuration, including parameters like `napi_build_version`, the most reliable approach seems to be patching this after Guix's configure phase, so we can just find what path we actually did build, rather than having to accurately predict what we're going to build. Instead of all of this, Debian packages a patched version of `@mapbox/node-pre-gyp` that always builds from source. This has some appeal and might mean less patching overall. I started down that road in , but I found `@mapbox/node-pre-gyp` really did need its `npm-log` dependency: we have `npm-log` as a dependency of `npm`, but, when I tried to package it properly, I found (if I'm remembering the details correctly) that bootstrapping `@unicode/14.0.0` from https://github.com/node-unicode/node-unicode-data seemed to involve a dependency cycle. That seemed like a headache for another time. >> + (with-atomic-file-replacement "package.json" >> + (lambda (in out) >> + (let* ((js (read-json in)) >> + (alist (match js >> + (('@ . alist) alist))) >> + (scripts-alist (match (assoc-ref alist "scripts") >> + (('@ . alist) alist))) >> + (scripts-alist >> + ;; install script would use node-pre-gyp >> + (assoc-remove! scripts-alist "install")) >> + (alist >> + (assoc-set! alist "scripts" (cons '@ scripts-alist))) >> + (binary-alist (match (assoc-ref alist "binary") >> + (('@ . alist) alist))) >> + (js (cons '@ alist))) >> + ;; compensate for lack of @mapbox/node-pre-gyp >> + (setenv "GYP_DEFINES" >> + (string-append >> + "module_name=" >> + (assoc-ref binary-alist "module_name") >> + " " >> + "module_path=" >> + (assoc-ref binary-alist "module_path"))) >> + (write-json js >> + out))))))))) > > I was having a bit of a hard time understand this phase, let me know if > I have this right. We have this JSON input: > > --8<---------------cut here---------------start------------->8--- > "binary": { > "module_name": "node_sqlite3", > "module_path": "./lib/binding/napi-v{napi_build_version}-{platform}-{arch}", > "host": "https://mapbox-node-binary.s3.amazonaws.com", > "remote_path": "./{name}/v{version}/{toolset}/", > "package_name": "napi-v{napi_build_version}-{platform}-{arch}.tar.gz", > "napi_versions": [ > 3 > ] > }, > > "scripts": { > "install": "node-pre-gyp install --fallback-to-build", > "pretest": "node test/support/createdb.js", > "test": "mocha -R spec --timeout 480000", > "pack": "node-pre-gyp package" > }, > --8<---------------cut here---------------end--------------->8--- > > And we: > > - Read the "binary" entry to find the module_name and module_path to > give to node-gyp, so we can use our own GYP instead of a bundled one. > > - Delete the "scripts.install" phase, it's not using the correct GYP. > > Maybe a couple of comments would be helpful here :-). Yes, I'll add comments :) What you said is mostly right, but, to clarify, the upstream scripts.install is using node-PRE-gyp, which tries to download pre-built binaries from S3-compatible storage. Since we don't have a patched version like Debian, we definitely want to avoid that. When node-pre-gyp does decide to build from source, it arranges to supply module_name and module_path based on the package.json definitions, so the binding.gyp doesn't define them. Thus, we also have to pass them to node-gyp, and the GYP_DEFINES environment variable turns out to be the easiest way to make sure they get passed on from npm to node-gyp to gyp. What we do isn't quite consistent with node-pre-gyp because we don't currently perform substitution on the module_path, so there ends up being a directory literally named "napi-v{napi_build_version}-{platform}-{arch}". But that could be improved later. -Philip