From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id YPBKMuPH2GFwlwAAgWs5BA (envelope-from ) for ; Sat, 08 Jan 2022 00:08:19 +0100 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id QAjJL+PH2GF9CAEA9RJhRA (envelope-from ) for ; Sat, 08 Jan 2022 00:08:19 +0100 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 7A9DE2B975 for ; Sat, 8 Jan 2022 00:08:19 +0100 (CET) Received: from localhost ([::1]:44330 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n5yLK-0004v2-IX for larch@yhetil.org; Fri, 07 Jan 2022 18:08:18 -0500 Received: from eggs.gnu.org ([209.51.188.92]:56110) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n5yL4-0004uo-Si for guix-patches@gnu.org; Fri, 07 Jan 2022 18:08:02 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:53145) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1n5yL4-00046s-8r for guix-patches@gnu.org; Fri, 07 Jan 2022 18:08:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1n5yL4-0007Ym-3E for guix-patches@gnu.org; Fri, 07 Jan 2022 18:08:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#51838] [PATCH v8 00/41] guix: node-build-system: Support compiling add-ons with node-gyp. Resent-From: Jelle Licht Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Fri, 07 Jan 2022 23:08: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 , Timothy Sample Cc: 51838@debbugs.gnu.org, Pierre Langlois , Philip McGrath , Leo Famulari Received: via spool by 51838-submit@debbugs.gnu.org id=B51838.164159683128995 (code B ref 51838); Fri, 07 Jan 2022 23:08:02 +0000 Received: (at 51838) by debbugs.gnu.org; 7 Jan 2022 23:07:11 +0000 Received: from localhost ([127.0.0.1]:46048 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1n5yKF-0007XZ-B3 for submit@debbugs.gnu.org; Fri, 07 Jan 2022 18:07:11 -0500 Received: from mail2.fsfe.org ([213.95.165.55]:46612) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1n5yKD-0007XR-Hj for 51838@debbugs.gnu.org; Fri, 07 Jan 2022 18:07:10 -0500 From: Jelle Licht DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fsfe.org; s=2021081301; t=1641596827; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=GAW9grASYvZpkyNAdiRk8uVUBPNXAi250xdv8Ve0a5I=; b=hgKAWp4J6lmYnj0R7oF2R3mTNZUW22mDdcLYBKUlZlTP2RtZEiery4NGSwV5Hir4harDx1 r5Cyfy4+QFuuSsrMC7TQtargI5ltjgo8c6H2gf1KUrndUJrLsxc0QJ01Q68+LtLSSdZEQs sx/8a5dD6FXYYAZ2OXGRC7qoniNdkJo= In-Reply-To: <80c4882db596f0b439b9ecca0b59cfea3ec96499.camel@gmail.com> References: <64e08d3a1838ed8507f33fae895545372960522f.camel@gmail.com> <87v8yv1ofd.fsf@ngyro.com> <86o84n8dkj.fsf@fsfe.org> <80c4882db596f0b439b9ecca0b59cfea3ec96499.camel@gmail.com> Date: Sat, 08 Jan 2022 00:07:06 +0100 Message-ID: <86iluv87s5.fsf@fsfe.org> MIME-Version: 1.0 Content-Type: text/plain 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=1641596899; 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: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=GAW9grASYvZpkyNAdiRk8uVUBPNXAi250xdv8Ve0a5I=; b=rw22wHJuxtmQNK0d/mApa5kC63M7C+nNDZOR6zAFmnZLUk3QHItcNczwiI9wG8QxLXBfPT oBhMD19aRzFfDQ7Y34twStQhpPrKfa3KaNFkGbcGXznlKZdwcLCfPQTn7OVHdr3ZkNkVCk 0wmZUOF1b/ege6Z3X+J3KaCP4M9oXFr8UkVrRaRxfUGFNvOVNsIdP3x3y1tBbe6FAzu93a 7Kz0JEx9xqxL20hKLJTo4jkrwFLNsLlJP3vOyfqhleaMQ0X3otl3Wyc3Z9RQch8gj+W0pM DY2puxPdKFv+JuuXGxFj31JRjOhefbd66ccPIy81KVv8fjRKv6OHOt2+wVjbNg== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1641596899; a=rsa-sha256; cv=none; b=eg2MEOP/lDNRChMxf8AvopsiAlDzWoJ3RYldIA6Des40oDWRfufjw8oB/xxE72JOv7NVqL dJ7BSUH0FDMcWVNjRRgKX7lHp3rWnmKCikM1U4mYvU51YuDiZo6LVcos+tpGuHFNJckuTx O25FjJ+2kjUJIeM4X8HPm720xj2JVa30SjPkQYzdZeshHeCGWpeyV77ICKxGrMZe/0fWFp nCQlq7IpE81h6PaPruBXKZk520yU7ADdvmjL0W77fk4X1kFDaeaHcyIFnbUdljVUD0Ot/X FTEnK4Pgf/ASEylW9MaEiTGWsB9HITZYh4zOUbGggvvU8E0eUOvsc5FH4WEFHQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=fsfe.org header.s=2021081301 header.b=hgKAWp4J; dmarc=fail reason="SPF not aligned (relaxed)" header.from=fsfe.org (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: -0.50 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=fsfe.org header.s=2021081301 header.b=hgKAWp4J; dmarc=fail reason="SPF not aligned (relaxed)" header.from=fsfe.org (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: 7A9DE2B975 X-Spam-Score: -0.50 X-Migadu-Scanner: scn1.migadu.com X-TUID: hhUJ+3oN3w6j Liliana Marie Prikler writes: > Am Freitag, dem 07.01.2022 um 22:02 +0100 schrieb Jelle Licht: >> To put it another way: a package that has to delete about 50 >> dependencies before it can be packaged in guix proper is allowed to >> look ugly! It can serve as a very unambigous list of things to >> possibly improve in a package, while being easy to review as well if >> changes are later proposed (e.g., you see 'node-karma-runner' being >> added to inputs, as well as the "karma-runner" string being removed >> from the list of dependencies to patch out). With regex you always >> need to hunt down if what is actually happening is what was intended >> to happen. >> >> In addition, since the things-that-can-be-matched by the regex w.r.t. >> a particular package.json file are already known and clearly >> enumerated, the only advantage regex brings us here is brevity. The >> biggest advantage of not using regex is that after a later package >> update, you can't inadvertently patch out a dependency that was added >> by upstream. > I'm not sure this is a good argument. Even with a huge ass list (put > the hyphen where you want to), upstreams can strengthen and weaken > coupling with an update, both incidentally and purposefully. So let's > say we decided to delete node-karma for version 0.2.0 of a package and > that worked fine until 0.6.5, but it's a requirement in 0.7.0 -- not > that we'd run tests or anything, because those require tap -- I don't > think either form is particularly helpful and in fact, I'd urge > reviewers to take a close look at the package.json before and after > regardless form. Rather the other way around; if we do not allow regex, we will see that there is a new dependency that we are currently neither patching nor adding to the inputs. We should celebrate any accurate build time failures we can make happen! >> Before the DRY brigade comes to take my guix REPL, I'd argue that any >> duplication here is incidental. What happens if we want to patch out >> "karma" and "karma-chrome-launcher", but for some reason want to keep >> "karma-browserify"?[1] Either way, the reviewer has to do a double >> take to see whether a change from "karma.*" to the two specifications >> "karma-chrome-launcher" and "karma" actually gives the expected >> output w.r.t. the package.json file that is being patched. > I think node packages do generally follow a pattern here, but let's > assume they don't and your example works that way. In that case, I'd > argue to make that regexp "karma(-browserify)?". And if later on > karma-chrome-launcher is to be added to that list, but karma-icecat is > allowed, "karma(-(browserify|chrome-launcher))?" In other words, we ^ I would object to this (constructed example) in a patch review, fwiw. At this stage, we expect the author to determine which dependencies to leave in and out, then correctly encode these choices as a regex, which is then decoded again by a reviewer who then must manually verify that this decoded intent makes sense with regards to both the listed inputs and the actual unpatched contents of package.json. Without regex, there's just a few details that need to be kept in mind for each step, for both author and reviewer(s). Adding something to the deleted dependencies can either: - fix a (newly) broken build - remove an optional dependency, and can most likely be removed from existing inputs. Removing something from the deleted dependencies can either: - be a no-op: so the existing deleted dependency was (made) redundant - enable an optional dependency, which should most likely be added to the inputs. That's it! Some creative types can put this in fancy decision graph and we have our "You Too Can Review Node Packages" campaign! This all just to clarify my position, but perhaps also demonstrate that it's a subjective preference, so 'agree to disagree' is a fine position to take here. > can as a community discourage the usage of ".*" without condemning all > regexp. > > Now I'm personally not convinced that disallowing "karma.*" altogether > is useful if we don't even have a karma package to go with -- and I'm > very sure we'd notice karma being packaged and check our karma dropping > packages -- but I'm willing to accept de gustibus here. This is a very pragmatic example indeed! If some other folks could still weigh in, I'd be okay with the resulting decision either way. - Jelle