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 UEZSHDXuUGEjKQEAgWs5BA (envelope-from ) for ; Mon, 27 Sep 2021 00:03:33 +0200 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 CI7jFzXuUGEnXgAAB5/wlQ (envelope-from ) for ; Sun, 26 Sep 2021 22:03:33 +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 7D35A36B4C for ; Mon, 27 Sep 2021 00:03:32 +0200 (CEST) Received: from localhost ([::1]:34616 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mUcF9-0000zN-99 for larch@yhetil.org; Sun, 26 Sep 2021 18:03:31 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:57272) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mUcEg-0000xX-U6 for guix-patches@gnu.org; Sun, 26 Sep 2021 18:03:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:55614) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mUcEg-0001ob-LX for guix-patches@gnu.org; Sun, 26 Sep 2021 18:03:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mUcEg-0004Uc-84 for guix-patches@gnu.org; Sun, 26 Sep 2021 18:03:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#49946] [PATCH 08/31] gnu: node: Patch /usr/bin/env in node-gyp. Resent-From: Philip McGrath Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 26 Sep 2021 22:03:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 49946 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Pierre Langlois Cc: 49946@debbugs.gnu.org, Maxime Devos Received: via spool by 49946-submit@debbugs.gnu.org id=B49946.163269377617258 (code B ref 49946); Sun, 26 Sep 2021 22:03:02 +0000 Received: (at 49946) by debbugs.gnu.org; 26 Sep 2021 22:02:56 +0000 Received: from localhost ([127.0.0.1]:38927 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mUcEa-0004UH-7Z for submit@debbugs.gnu.org; Sun, 26 Sep 2021 18:02:56 -0400 Received: from mail-qk1-f171.google.com ([209.85.222.171]:33669) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mUcEY-0004U4-Cl for 49946@debbugs.gnu.org; Sun, 26 Sep 2021 18:02:55 -0400 Received: by mail-qk1-f171.google.com with SMTP id d207so35052735qkg.0 for <49946@debbugs.gnu.org>; Sun, 26 Sep 2021 15:02:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=philipmcgrath.com; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=QjLw4Mn90chDV10lYjLz2lOQVsQu0l/I9vkZk8FXYwc=; b=Ox1HjIJeRKq8qAQAA/Cz2cBrPq4zJ5LDoBBPhUes+YozhS7u6UK47Vnks4YIVXfLHB wn5nnhn+Gb57bvkUTvWnp+nzt65gWGQd4VHZFBB7+7O3+k2CbJa09I8XfKpEQAgc4p9E 0776V7jzfJJfq8pvap76+zSV9OH6ZEJ+hOBfywRRdVuWuJySaQDxlO7erqg4lKWGPVON Hf0AYJHsbwIbZj20H1I3ICp6XU4HCYOIUJiHUeIsBHqQABIHAya8NIRRLtwDUnVLE1yX MHMpVFamNE1WIHp/lzDReFTrtuGy5xSBU0Br5XqfdrrNx+XV78kn/7oFxlYSXRF1q1au hF8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=QjLw4Mn90chDV10lYjLz2lOQVsQu0l/I9vkZk8FXYwc=; b=BXTtoA7oj8YEZU2uRKOV88boJeQUUkoYqYHeQDe8OMuuMRboZYHEx/e/YtZaVd1Gsv UWC+Dqux+E9Z/Avr6xqOAdJ6PJee3m4ZlS7I1T3maO/lmJZCplU06NvGN75X46rEY/BA F+DD/WCSQ45DtO7PdWe1VABKTrO9VQt4sg4gqZOZ3MyXyOfuL8Tb/NX1rN3SbVSoJuSw EeVrtvbIShqtGzsao17ntx2ay97fs5OEgpNX2snweJ+6Gdj2mpUgyZFnd1ljy6YaPCN2 ydC0jFXo8gpiiKX06xTaDQaJKqePrPRZjhaH1RsWawlmI9ltO6v2BLMrzykkyYudMfRv il7A== X-Gm-Message-State: AOAM531c6mpwwrCV6a5q1lKjTFwWq0PypgK2SBAqmQtbZAvsn0Shdqdi mvNIgfUMlUkQy+Nhfk0rK0xvtj1t1dOWq9ZS X-Google-Smtp-Source: ABdhPJyy+0EyoMXraDZXdutiYuQeHCjROva80csaHoGC59/5rNIk+lMfKaJgt3n9GDxAsDSK3P5hdQ== X-Received: by 2002:ae9:ed45:: with SMTP id c66mr21729605qkg.336.1632693768505; Sun, 26 Sep 2021 15:02:48 -0700 (PDT) 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 w7sm317578qtc.29.2021.09.26.15.02.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 26 Sep 2021 15:02:48 -0700 (PDT) References: <87h7fztt60.fsf@gmx.com> <20210808233354.6745-1-pierre.langlois@gmx.com> <20210808233354.6745-8-pierre.langlois@gmx.com> <42e10baddb6afe308f67c3240bf5da8159e6f118.camel@telenet.be> <87o88gq5p5.fsf@gmx.com> From: Philip McGrath Message-ID: Date: Sun, 26 Sep 2021 18:02:47 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <87o88gq5p5.fsf@gmx.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US 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=1632693812; 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=QjLw4Mn90chDV10lYjLz2lOQVsQu0l/I9vkZk8FXYwc=; b=DXUucd8Cgd2R2O/JLUyQDhxfq0tySGIhI1SJyjcZXpcCid+KX21HP9bwYXqGx7pUKSPKda AICV83jdUXI+uPezVTb+hHWzu6kKGHpLQo3MFiDp24GiWva9vt39zGG+LgT/HG7frSf9iz r1CbYhTbveXxVGt4Y3dooo632G/8V+2cQY6EYvc4xIMiXyIJtr31j/i+ZXmnxp90uF7tzE Tk4fFEbWTIHlO8/7/ids8u32aM2DYvxUTf0TniUI+phB5jxjSjnZVqxT30JBwRqK159Hrh vabygndmVImzaRaqJtHic4HZrNttmcCKoeF4ea0pX9DQgGKkTpz0/rPEqYgU/g== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1632693812; a=rsa-sha256; cv=none; b=aZbE9AiizbS+IXG0cuHDN35JnF4zHYbKuToEJYEGGhUumcvo6ZyteOTEadYS9cLcH5yEo8 6KTucmd+pneGLeEc41csiP7xqAIolaJ+iyhRD+4iBQTqHR8VfDUsqAF3ILlawmZVa5oAac tY/DQrzdlkRRMRr212/8XlEzmq7nn1ryhecTXLB6EwxrTlDayuS2C4g0BdguAlu869Ryvb O8i5IL2H7wn6DpEiyIc3cqW3JDh9BIqktmvtDo6YZbM9EHZR/1X5cAYfch9Yopy7BrgGsY OUCXqw4s7AdaoOclMRP82fT0pncMeF3efVgAf6qXduTj5cZ2qnM45PkmhJyEIw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=Ox1HjIJe; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Spam-Score: -1.39 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=google header.b=Ox1HjIJe; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Queue-Id: 7D35A36B4C X-Spam-Score: -1.39 X-Migadu-Scanner: scn0.migadu.com X-TUID: RGGhDh6bKzCD Hi Pierre, On 9/25/21 6:24 AM, Pierre Langlois wrote: > Philip McGrath writes: >> Since the shebang line for node-gyp is specifically "#!/usr/bin/env node", I >> wonder if it should use the node built by this package, rather than a dynamic >> node. > > Yeah we could do that, although I generally prefer to follow whatever > the script already does, there could be a good reason for them to use > `env' no I think it might be better to use `patch-shebang` from `(guix build utils)` rather than `substitute*` these by hand, and it seems that `patch-shebang` removed the indirection through `env`. My guess is most of these cases are to accommodate the fact that `node` and `python` are often installed to places other than `/usr/bin`. >> I'd guess node-gyp may not be the only one with shebangs that ought to >> be patched. > > Yeah there could be others, although normally the patching phase from > the gnu build system should have taken care of most of them, hopefully > all, I'm not sure why it didn't work for /usr/bin/env though. > > I would suggest we patch things as we encounter them, did you find > anymore issues when working on your package? Looking at `gnu-build-system`, it seems that the `'patch-shebangs` phase only operates on files installed in the "/bin" and "/sbin" subdirectories of the package's outputs. That restriction doesn't make sense to me in general: for instance, what about "/libexec"? For Node specifically, this misses a lot of stuff under "/lib/node_modules" and "/lib/node_modules/npm/node_modules". I think I more general fix could subsume the `'patch-npm-shebang` and `'patch-node-shebang` phases in building Node, too. > For instance, while working on a newer version of one of the packages in > this series, I saw we may need to patch GYP's python reference as well, > like so: > > (substitute* "deps/npm/node_modules/node-gyp/gyp/gyp_main.py" > (("#!/usr/bin/env python") > (string-append "#!" (assoc-ref inputs "python") "/bin/python3"))) > > Only for node 14+. The reason seems to be that gyp still refers to > "python", but python2 is no longer a dependency for newer nodes. And it > seems GYP is perfectly happy with python3, and the shebang is fixed > upstream so a never node will be fine: > https://github.com/nodejs/node-gyp/pull/2355/files I think in some places (but perhaps not enough places) Guix uses `python-wrapper` to work around this ... > > Maybe updating node would be better than this fix though. I'm not totally clear on whether the upstream fix is in 14.17.6 LTS, but, if so, that seems great! > >> More generally, I see that there are 355 directories installed under >> "lib/node_modules/npm/node_modules" (which corresponds to the "deps" >> path above). Most of them don't seem to be available as Guix packages that could >> be depended upon by other Guix node packages. > > Yeah that's tricky, ideally we should remove all the node_modules deps > and package them separately, I wonder if anybody tried to do that > already. I would suspect it to be quite a lot of work, sometimes > unbundling stops being worth and when it's hard to maintain dependencies > manually. > > Hopefully we can get there one day though! I don't want to deter anybody > from trying :-), I might give it a go on a raindy day. Since these are developed and released with Node, and apparently we can build them as part of the Node build process, I was thinking we could just make packages that point to these versions we're already building. It might be good to hear from someone who develops with node/npm, though ... I just use it to install software that I can't find packaged elsewhere. > >> On 8/8/21 6:29 PM, Pierre Langlois wrote: >> >>> ... `node-gyp' needs >> >>> node headers to compile against, packaged as a tarball, which it tries >> >>> to download. Instead, we can run a `node-gyp --tarball <> configure' >> >>> step to manually provide the tarball, which we can package separately >> >>> for any given node version. >> >> There is also a --nodedir option, which I found could work with something like: >> >> (string-append "--nodedir=" (assoc-ref inputs "node")) >> >> That seems like it might be better, though I don't know all the considerations >> for cross-compilation and such. > > Oh that's a good idea, I didn't really like having to download the > headers separately from the main package, especially given we run > snippet on the source to remove bundled dependencies. > > Trying this out this approach does work, but I needed to: > > - Create a union directory with both node and libuv. The node package > only has headers for V8/node, but we also need libuv, so doing > something like this works: > > (union-build node-sources > (list (assoc-ref inputs "node") > (assoc-ref inputs "libuv")) > #:create-all-directories? #t > #:log-port (%make-void-port "w")) I found it worked to just add libuv as an input of packages built with node-gyp. I hadn't tried to change `node-build-system`, but I think that would be the place to do it. > > - For some reason, --nodedir didn't really "configure" gyp to use that > node directory, I think it's meant to be passed everytime you run > any gyp command. Instead I found that you can use and environment > variable: > > (setenv "npm_config_nodedir" node-sources) That seems right. I believe there's a similar "npm_config_python" for the Python executable to use. Alternatively, I think it's possible to configure these in $PREFIX/etc/npmrc: > > And that works for the packages in this series! That'll be much better > than before, I'll do it this way. > > Thanks again for taking a look, I'll see if I can send updated patches > sometimes this weekend. Glad it was useful! For patching the shebangs, here's a variant of node-lts that worked for me, though I think it would be even better to combine it with the existing phases: ``` (define-public patched-node (let ((node node-lts)) (package (inherit node) (arguments (substitute-keyword-arguments (package-arguments node) ((#:phases standard-phases) `(modify-phases ,standard-phases (add-after 'patch-npm-shebang 'patch-more-shebangs (lambda* (#:key inputs outputs #:allow-other-keys) (define (append-map f lst) (apply append (map f lst))) ;; from patch-shebangs (define bin-directories ;;(match-lambda ;; ((_ . dir) (lambda (pr) (let ((dir (cdr pr))) (list (string-append dir "/bin") (string-append dir "/sbin"))))) (define output-bindirs (append-map bin-directories outputs)) (define input-bindirs ;; Shebangs should refer to binaries of the target system---i.e., from ;; "inputs", not from "native-inputs". (append-map bin-directories inputs)) (define path (append output-bindirs input-bindirs)) (with-directory-excursion (string-append (assoc-ref outputs "out") "/lib/node_modules/npm/node_modules") (for-each ;;(cut patch-shebang <> path) (lambda (file) (patch-shebang file path)) ;; from patch-generated-file-shebangs (find-files "." (lambda (file stat) (and (eq? 'regular (stat:type stat)) (not (zero? (logand (stat:mode stat) #o100))))) #:stat lstat)))))))))))) ``` -Philip