From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id MCCCBVmzZ18YbAAA0tVLHw (envelope-from ) for ; Sun, 20 Sep 2020 19:54:01 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id iJFiAVmzZ19hSQAAbx9fmQ (envelope-from ) for ; Sun, 20 Sep 2020 19:54:01 +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 9B3C794023C for ; Sun, 20 Sep 2020 19:54:00 +0000 (UTC) Received: from localhost ([::1]:59536 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kK5PL-00041Y-IQ for larch@yhetil.org; Sun, 20 Sep 2020 15:53:59 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:41580) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kK5NT-0002rA-B8 for guix-patches@gnu.org; Sun, 20 Sep 2020 15:52:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:41191) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kK5NS-00080V-1j for guix-patches@gnu.org; Sun, 20 Sep 2020 15:52:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kK5NR-00027C-Ux for guix-patches@gnu.org; Sun, 20 Sep 2020 15:52:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#41219] [PATCH 2/2] guix: Enforce package.json "files" directive. Resent-From: Jelle Licht Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 20 Sep 2020 19:52:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 41219 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: To: Giacomo Leidi , 41219@debbugs.gnu.org Received: via spool by 41219-submit@debbugs.gnu.org id=B41219.16006314848067 (code B ref 41219); Sun, 20 Sep 2020 19:52:01 +0000 Received: (at 41219) by debbugs.gnu.org; 20 Sep 2020 19:51:24 +0000 Received: from localhost ([127.0.0.1]:52737 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kK5Mq-000263-C9 for submit@debbugs.gnu.org; Sun, 20 Sep 2020 15:51:24 -0400 Received: from mail1.fsfe.org ([217.69.89.151]:32974) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kK5Mo-00025u-1G for 41219@debbugs.gnu.org; Sun, 20 Sep 2020 15:51:23 -0400 From: Jelle Licht In-Reply-To: <20200512213131.28873-2-goodoldpaul@autistici.org> References: <20200512213131.28873-1-goodoldpaul@autistici.org> <20200512213131.28873-2-goodoldpaul@autistici.org> Date: Sun, 20 Sep 2020 21:51:18 +0200 Message-ID: <875z88jkg9.fsf@jlicht.xyz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -5.0 (-----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Spam-Score: -6.0 (------) 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-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=fsfe.org (policy=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-Spam-Score: -0.91 X-TUID: kRTajNu2+EAu Hey Giacomo,=20 Apologies for the delay! Better late than never, a review just for you. The other patch seems fine to me, but I'm not a 'guix glob' expert. Giacomo Leidi writes: > [snip] > --- a/guix/build/node-build-system.scm > +++ b/guix/build/node-build-system.scm > @@ -1,6 +1,7 @@ > ;;; GNU Guix --- Functional package management for GNU > ;;; Copyright =C2=A9 2015 David Thompson > ;;; Copyright =C2=A9 2016 Jelle Licht > +;;; Copyright =C2=A9 2020 Giacomo Leidi > ;;; > ;;; This file is part of GNU Guix. > ;;; > @@ -22,6 +23,7 @@ > #:use-module (guix build json) > #:use-module (guix build union) > #:use-module (guix build utils) > + #:use-module (guix glob) > #:use-module (ice-9 match) > #:use-module (ice-9 popen) > #:use-module (ice-9 regex) > @@ -110,18 +112,60 @@ the @file{bin} directory." > (#f #f))) > (dependencies (match (assoc-ref data "dependencies") > (('@ deps ...) deps) > - (#f #f)))) > + (#f #f))) > + (patterns (match (assoc-ref data "files") > + (() #f) > + ((? list? patrn-list) patrn-list) ^ Perhaps 'pattern-list'? I keep reading this as patron-list. We could also build the patterns here. Mapping over the pattern-list + 'default patterns' here might also be a wee bit faster. > + (#f #f))) > + (main (match (assoc-ref data "main") > + ("" #f) > + ((? string? main-module) main-module) > + (#f #f))) > + (install-dir (string-append target "/node_modules/" modulename)) > + (install-files (lambda (files directory) ^ You only use install-dir here: you could hard-code it in the lambda. > + (for-each (lambda (file) > + (install-file > + file > + (string-append directory "/" > + (dirname file)))) > + files)))) > (mkdir-p target) > - (copy-recursively "." (string-append target "/node_modules/" modulen= ame)) > - ;; Remove references to dependencies > - (delete-file-recursively > - (string-append target "/node_modules/" modulename "/node_modules")) > + (if patterns > + (install-files > + (filter (lambda (file) > + (any (lambda (pattern) > + (glob-match? > + (string->compiled-sglob pattern) > + file)) > + (append > + patterns > + '("package.json" > + ;; These files get installed no > + ;; matter the case or extension. > + "[rR][eE][aA][dD][mM][eE]*" > + "[cC][hH][aA][nN][gG][eE][sS]*" > + "[cC][hH][aA][nN][gG][eE][lL][oO][gG]*" > + "[hH][iI][sS][tT][oO][rR][yY]*" > + "[nN][oO][tT][iI][cC][eE]*")))) > + (map (lambda (path) > + (string-drop path 2)) ^ If this is meant to drop the "./" prefix, you should be able to leave it out. > + (find-files "."))) `find-files' accepts an optional second argument called PRED, so you can do that instead of the earlier 'filter'. > + install-dir) > + (begin > + (copy-recursively "." install-dir) > + ;; Remove references to dependencies > + (delete-file-recursively > + (string-append install-dir "/node_modules")))) > + (if (and main > + (not (file-exists? > + (string-append > + install-dir "/" (dirname main))))) > + (install-files (list main) install-dir)) ^ This should not be needed if we use the 'old' (=3Dnon-files) approach of installing. Do you think it makes sense to pull it into the previous block that only runs on using the 'files' directive? Thanks for you patience, and thanks again for working on this. HTH, - Jelle