[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#51838] [PATCH v8 00/41] guix: node-build-system: Support compiling
From: |
Liliana Marie Prikler |
Subject: |
[bug#51838] [PATCH v8 00/41] guix: node-build-system: Support compiling add-ons with node-gyp. |
Date: |
Sat, 08 Jan 2022 00:47:46 +0100 |
User-agent: |
Evolution 3.42.1 |
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
[bug#51838] [PATCH v8 00/41] guix: node-build-system: Support compiling add-ons with node-gyp., Philip McGrath, 2022/01/07
- [bug#51838] [PATCH v8 00/41] guix: node-build-system: Support compiling add-ons with node-gyp.,
Liliana Marie Prikler <=
- [bug#51838] [PATCH v8 00/41] guix: node-build-system: Support compiling add-ons with node-gyp., Philip McGrath, 2022/01/07
- [bug#51838] [PATCH v9 00/41] guix: node-build-system: Support compiling add-ons with node-gyp., Philip McGrath, 2022/01/08
- [bug#51838] [PATCH v9 01/41] guix: node-build-system: Add delete-lockfiles phase., Philip McGrath, 2022/01/08
- [bug#51838] [PATCH v9 02/41] guix: node-build-system: Add implicit libuv input., Philip McGrath, 2022/01/08
- [bug#51838] [PATCH v9 03/41] guix: node-build-system: Add JSON utilities., Philip McGrath, 2022/01/08
- [bug#51838] [PATCH v9 13/41] gnu: node-semver: Use 'delete-dependencies'., Philip McGrath, 2022/01/08
- [bug#51838] [PATCH v9 05/41] guix: node-build-system: Add 'delete-dependencies' helper function., Philip McGrath, 2022/01/08
- [bug#51838] [PATCH v9 06/41] gnu: node-semver-bootstrap: Use 'delete-dependencies'., Philip McGrath, 2022/01/08
- [bug#51838] [PATCH v9 09/41] gnu: node-debug-bootstrap: Use 'delete-dependencies'., Philip McGrath, 2022/01/08
- [bug#51838] [PATCH v9 17/41] gnu: node-irc: Use 'delete-dependencies'., Philip McGrath, 2022/01/08