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

[Liliana Marie Prikler]

Hi,

Am Freitag, dem 07.01.2022 um 17:11 -0500 schrieb Philip McGrath:
> I think this is not quite correct.
> 
> (Actually, I suspect more broadly that node-build-system's handling
> of peerDependencies is not quite correct, but wrapping my head around
> the semantics of peerDependencies is on my to-do list for after these
> patches are applied. Here's one thing I want to read and understand: 
> https://pnpm.io/how-peers-are-resolved)
> 
> NPM does not try to install packages in "peerDependencis" during 'npm
> install' (out 'configure' phase). The problem arises because because
> our 'patch-dependencies' phase adds the "peerDependencies" as
> additional "dependencies". (Why? I don't fully understand, but I
> guess because it wants them to be installed.) We want absent
> "peerDependencies" to not be listed in "dependencies", but I don't
> think we want to delete them from "peerDependencies": at a minimum,
> we do not need to, and it seems like it might cause problems that I
> don't fully understand.
> 
> (This is one of the reasons I preferred to handle absent dependencies
> in the 'patch-dependencies' phase.)
I'd like to be able to understand that too, but npm still boggles my
mind.  I think node-build-system's implementation is a rather pragmatic
one; it forces you to use just a single combination of versions for all
of those rather than relying on node trickery (on a related note,
perhaps we ought to make inputs in node-build-system propagated-inputs
to be on the safe side).

> > 4. Regexps :)
> 
> Hopefully addressed in my previous email :) Jelle makes good
> arguments  for the no-regexps side. I'm genuinely on the fence, which
> suggests to me the best course might be to leave it as a possible
> future extension (as we're doing with '#:absent-dependencies').
We do already have threads on the regexp thing, so I'm not going to
respond here to keep it manageable.  The change is a rather small one
inside node-build-system itself, but you have to expand those strings
again ;)

> For the patches where you've made substantive changes to the 
> implementation or the commit message, maybe there should be more of
> an annotation than just "Signed-off-by". I'm not super familiar with
> the Git etiquette here, but one suggestion I've seen is something
> like:
> 
>      Signed-off-by: Random J Developer <random@developer.example.org>
>      [lucky@maintainer.example.org: struct foo moved from foo.c to
> foo.h]
>      Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org>
> 
> It's not a big deal, I could just imagine getting confused some
> months or years from now about what exactly I wrote or didn't write.
> (TBH the numerous versions of this series are already a bit
> unwieldy.)
If you don't like my phrasing in a particular message, you are free to
change it for v9 (or if you don't want a v9 to be reviewed, just reply
to the individual one with how you'd reword it and I'll try to keep my
changes to your version minimal).  I simply tried to adapt your
messages to the generally accepted style, which means moving long-form
discussions out of the ChangeLog into the message body and correcting
every variable to variable.

Now I do admit that I used some more flowery parts for 3-5 and that's
completely on me, but 3 already has me as Co-Author anyway.

> Time permitting, I'll send some more comments, but the only things I 
> think need to be addressed before merging are peerDependencies and
> regexps.
Cool.  Let's just not forget to send a v9 once we have what looks like
to be a reasonable action (assuming it's not a do-nothing, in which
case I just need to reword some of your commit messages again).  In my
personal experience, patches help a stalled discussion tremendously.

Cheers