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

[Ryan Sundberg via Guix-patches via <guix-patches@gnu.org>]

[-- Attachment #1.1: Type: text/plain, Size: 8525 bytes --]

I think the #:absent-dependencies pattern is very straightforward
argument and is clearly a common enough occurrence to merit a shorthand
expression. I have not been following the developments in the
node-build-system here recently but it looks like lots of progress is
being made. Thanks all for contributing!

--
Sincerely,
Ryan Sundberg

On 12/29/21 11:38 PM, Philip McGrath wrote:
> Hi Liliana (and everyone),
> 
> Thanks for taking care of the earlier patches from the series!
> 
> It sounds like (guix build json) is going to be around for a reasonably
> long time, and I'm ok with that.
> 
> In the hope of clearing a path to move forward, I'm sending v6 of this
> patch series, immediately followed by a closely-related v7: I strongly
> prefer v7, but I would be ok with either version being applied if one of
> them can achieve consensus. (If v6 is merged, I'll send a separate patch
> series to propose '#:absent-dependencies'.)
> 
> I've put the two versions up on GitLab
> as <https://gitlab.com/philip1/guix-patches/-/tags/guix-issue-51838-v6>
> and <https://gitlab.com/philip1/guix-patches/-/tags/guix-issue-51838-v7>,
> respectively.
> 
> I've re-organized the patches in the series somewhat to facilitate a
> minimal, side-by-side comparison between '#:absent-dependencies' and this
> approach:
> 
> On 12/20/21 17:00, Liliana Marie Prikler wrote:
>> (add-after 'patch-dependencies 'drop-junk
>>    (lambda _
>>      (with-atomic-json-replacement "package.json"
>>        (lambda (json) (delete-dependencies json '("node-tap"))))))
> 
> The series is now organized as follows:
> 
>   - The first 4 patches are identical in v6 and v7:
> 
>       - Patches 01 & 02/41 are non-controversial build system changes
>         ('delete-lockfiles' and libuv).
> 
>       - Patch 03/41 adds to (guix build node-build-system) several utility
>         functions for transforming JSON in the representation used by (guix
>         build json), especially functional update of tagged JSON
>         objects. It also adjusts the rest of (guix build node-build-system)
>         to make use of those functions, eliminating all uses of
>         'assoc-set!' and other procedures that mutate assosciation lists.
> 
>         Of the new functions, only 'with-atomic-json-file-replacement' is
>         exported for now: my hope is that further (valuable and important!)
>         discussion about the API design of these functions or their
>         implementations need not block either v6 or v7 of this series.
> 
>       - Patch 04/41 adds the 'avoid-node-gyp-rebuild' phase. I've re-worked
>         the implementation to use the new helper functions.
> 
>   - Patch 05/41 is the truly significant difference between v6 and v7: it
>     implements the strategy for dealing with missing dependencies.
> 
>     In v6, it exports a new public function 'delete-dependencies' from
>     (guix build node-build-system).
> 
>     In v7, it adds '#:absent-dependencies'. One difference from previous
>     versions of this series is that it handles '#:absent-dependencies' in a
>     new 'delete-dependencies' phase, which makes the change even more
>     self-contained.
> 
>   - Patches 06 through 17/41 adjust existing packages to make use of the
>     approach to missing dependencies from patch 05/41 of each series.
> 
>   - Patches 18 through 41/41 add the new packages excercising native addon
>     support, again differing based on the approach from patch 05/41. The
>     other slight difference from previous versions of this series is that,
>     where applicable, I've adjusted the new package definitions to use
>     'with-atomic-json-file-replacement' and never to use 'assoc-set!' or
>     other procedures that mutate assosciation lists.
> 
> I continue to think that '#:absent-dependencies' is a good approach, in
> brief, as Jelle put it in <https://issues.guix.gnu.org/51838#247>, because:
> 
> On 12/20/21 18:10, Jelle Licht wrote:
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>>> Am Montag, dem 20.12.2021 um 14:33 -0500 schrieb Philip McGrath:
>>>> If we took your final suggestion above, I think we'd have something
>>>> like this:
>>>>
>>>> ```
>>>> #:phases
>>>> (modify-phases %standard-phases
>>>>     (add-after 'unpack 'delete-dependencies
>>>>       (make-delete-dependencies-phase '("node-tap"))))
>>>> ```
>>>>
>>>> That seems pretty similar to, under the current patch series:
>>>>
>>>> ```
>>>> #:absent-dependencies '("node-tap")
>>>> ```
>>> That is the point, but please don't add a function called "make-delete-
>>> dependencies-phase".  We have lambda.  We can easily add with-atomic-
>>> json-replacement.  We can also add a "delete-dependencies" function
>>> that takes a json and a list of dependencies if you so want.
>>>
>>> So in short
>>>
>>> (add-after 'patch-dependencies 'drop-junk
>>>    (lambda _
>>>      (with-atomic-json-replacement "package.json"
>>>        (lambda (json) (delete-dependencies json '("node-tap"))))))
>>>
>>> would be the "verbose" style of disabling a list of dependencies.
>>>
>>
>> I think you are _really_ underestimating how many packages will need a
>> phase like this in the future. I would agree with this approach if it
>> were only needed incidentally, similar to the frequency of patching
>> version requirements in setup.py or requirements.txt for python
>> packages.
>>
>> Pretty much everything except the '("node-tap") list will be identical
>> between packages; how do you propose we reduce this duplication? At this
>> point I feel like I'm rehasing the opposite of your last point, so let
>> me rephrase; how many times do you want to see/type/copy+paste the above
>> snippet before you would consider exposing this functionality on a
>> higher level?
>>
> 
> Additionally, I think having a phase in '%standard-phases' is a good way of
> addressing the need to use 'delete-dependencies' after the
> 'patch-dependencies' phase, which I explain in patch v6 05/41.
> 
> But, again, I could live with either v6 or v7 being applied if one of them
> can achieve consensus.
> 
> So, without further ado, here is v6!
> 
>   -Philip
> 
> Philip McGrath (41):
>   guix: node-build-system: Add delete-lockfiles phase.
>   guix: node-build-system: Add implicit libuv input.
>   guix: node-build-system: Add JSON utilities.
>   guix: node-build-system: Add avoid-node-gyp-rebuild phase.
>   guix: node-build-system: Add 'delete-dependencies' helper function.
>   gnu: node-semver-bootstrap: Use 'delete-dependencies'.
>   gnu: node-ms-bootstrap: Use 'delete-dependencies'.
>   gnu: node-binary-search-bootstrap: Use 'delete-dependencies'.
>   gnu: node-debug-bootstrap: Use 'delete-dependencies'.
>   gnu: node-llparse-builder-bootstrap: Use 'delete-dependencies'.
>   gnu: node-llparse-frontend-bootstrap: Use 'delete-dependencies'.
>   gnu: node-llparse-bootstrap: Use 'delete-dependencies'.
>   gnu: node-semver: Use 'delete-dependencies'.
>   gnu: node-wrappy: Use 'delete-dependencies'.
>   gnu: node-once: Use 'delete-dependencies'.
>   gnu: node-irc-colors: Use 'delete-dependencies'.
>   gnu: node-irc: Use 'delete-dependencies'.
>   gnu: Add node-inherits.
>   gnu: Add node-safe-buffer.
>   gnu: Add node-string-decoder.
>   gnu: Add node-readable-stream.
>   gnu: Add node-nan.
>   gnu: Add node-openzwave-shared.
>   gnu: Add node-addon-api.
>   gnu: Add node-sqlite3.
>   gnu: Add node-file-uri-to-path.
>   gnu: Add node-bindings.
>   gnu: Add node-segfault-handler.
>   gnu: Add node-ms.
>   gnu: Add node-debug.
>   gnu: Add node-serialport-binding-abstract.
>   gnu: Add node-serialport-parser-delimiter.
>   gnu: Add node-serialport-parser-readline.
>   gnu: Add node-serialport-bindings.
>   gnu: Add node-serialport-parser-regex.
>   gnu: Add node-serialport-parser-ready.
>   gnu: Add node-serialport-parser-inter-byte-timeout.
>   gnu: Add node-serialport-parser-cctalk.
>   gnu: Add node-serialport-parser-byte-length.
>   gnu: Add node-serialport-stream.
>   gnu: Add node-serialport.
> 
>  gnu/packages/node-xyz.scm        | 1013 +++++++++++++++++++++++++++++-
>  gnu/packages/node.scm            |   78 ++-
>  gnu/packages/zwave.scm           |   64 ++
>  guix/build-system/node.scm       |    9 +-
>  guix/build/node-build-system.scm |  355 ++++++++++-
>  5 files changed, 1464 insertions(+), 55 deletions(-)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]