Package: guix-patches;
Reported by: Philip McGrath <philip <at> philipmcgrath.com>
Date: Sun, 14 Nov 2021 12:43:01 UTC
Severity: normal
Tags: patch
Done: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
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>, Philip McGrath <philip <at> philipmcgrath.com>, Liliana Marie Prikler <liliana.prikler <at> gmail.com> Subject: [bug#51838] [PATCH v6 00/41] guix: node-build-system: Support compiling add-ons with node-gyp. Date: Thu, 30 Dec 2021 02:38:38 -0500
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 <at> 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(-)
--
2.32.0
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.