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

[Liliana Marie Prikler]

Hi Jelle,

Am Samstag, dem 08.01.2022 um 00:07 +0100 schrieb Jelle Licht:
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> > [...]
> > 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!
We're arguing past each other.  What I'm saying is that with neither
you can assure your exclude list is actually reasonable in any sense of
the word.
> > 

> 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.
Again de gustibus, but I'd rather read one very well thought out
"typescript.*" than dozens of lines excluding typescript-something.
Also, with longer/well-known prefixes, it typically becomes easier to
reason about a single regexp rather than seeing again 3+ lines of any
given package family and missing the forest for the trees. 

> 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!
That's not at all the complete list of possibilities for node packages.
Unless you define "optional" in the loosest sense, meaning "yeah, you
can build it, but try calling a simple function and you'll get an
import error of doom", which is possible because node doesn't
statically verify zilch (*).  That's the main reason Philip rejected my
"just restrict ourselves to what we find in inputs" suggestion.

Cheers

> 
> 
(*) double negative meant as actual negative