guix-patches
[Top][All Lists]
Advanced

[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: Philip McGrath
Subject: [bug#51838] [PATCH v8 00/41] guix: node-build-system: Support compiling add-ons with node-gyp.
Date: Fri, 7 Jan 2022 23:14:04 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1

Hi,

On 1/7/22 18:47, Liliana Marie Prikler wrote:
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 mean, I did start writing this patch series because I find it more understandable than just using npm :)

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

I will send a v9 that doesn't delete "peerDependencies" and just rely on doing the ordering properly. Hopefully, we can improve the situation later, once we understand how "peerDependencies" are actually supposed to work (and/or adopt '#:absent-dependencies' :) ). I don't know of any concrete problems that would be caused by overzealously deleting "peerDependencies", but I wouldn't know of them, since that's not the behavior I've been testing all this time.

(on a related note,
perhaps we ought to make inputs in node-build-system propagated-inputs
to be on the safe side).

I think that might not actually lead Node.js to find all of the modules, and some things I've been reading with interest (but not yet fully understanding) suggests that the opposite, i.e. creating a more strict "node_modules", is actually useful. [1] [2]


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 ;)

I am not 100% clear---if I'm wrong, please speak up!---but my sense from the previous thread is that:

 1. some people have reservations about some regexp proposals;

 2. everyone who has liked any of the regexp proposals has been ok
    with one of the options for distinguishing regexps from non-regexp
    strings, either '(regexp "foo") or (make-regexp "foo"), with another
    dimension being whether or not to require full matches; and

 3. with either of the options from #2---as long as regexps are using
    some representation that answers #f to 'string?'---regexp support
    could be added as a fully compatible future extension.

So I think---I hope!---a version without regex support could get everyone's assent. Unless someone tells me otherwise first, that's what I'll do in v9.

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.

Ok!

-Philip

[1]: https://pnpm.io/blog/2020/05/27/flat-node-modules-is-not-the-only-way
[2]: https://medium.com/pnpm/pnpms-strictness-helps-to-avoid-silly-bugs-9a15fb306308





reply via email to

[Prev in Thread] Current Thread [Next in Thread]