GNU bug report logs - #51838
[PATCH 00/11] guix: node-build-system: Support compiling add-ons with node-gyp.

Previous Next

Full log

Message #1283 received at 51838 <at> debbugs.gnu.org (full text, mbox):

From: Philip McGrath <philip <at> philipmcgrath.com>
To: 51838 <at> debbugs.gnu.org
Cc: Timothy Sample <samplet <at> ngyro.com>,
 Pierre Langlois <pierre.langlois <at> gmx.com>, Jelle Licht <jlicht <at> fsfe.org>,
 Liliana Marie Prikler <liliana.prikler <at> gmail.com>,
 Leo Famulari <leo <at> famulari.name>
Subject: Re: [PATCH v8 00/41] guix: node-build-system: Support compiling
 add-ons with node-gyp.
Date: Fri, 7 Jan 2022 23:14:04 -0500

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.