[bug#51838] [PATCH v8 00/41] guix: node-build-system: Support compiling add-ons with node-gyp.

[Liliana Marie Prikler]

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.

> 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
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.

Cheers