[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#51838] [PATCH v6 00/41] guix: node-build-system: Support compiling
From: |
Ryan Sundberg |
Subject: |
[bug#51838] [PATCH v6 00/41] guix: node-build-system: Support compiling add-ons with node-gyp. |
Date: |
Thu, 30 Dec 2021 12:03:15 -0800 |
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(-)
>
OpenPGP_signature
Description: OpenPGP digital signature
- [bug#51838] [PATCH v7 27/41] gnu: Add node-bindings., (continued)
- [bug#51838] [PATCH v7 27/41] gnu: Add node-bindings., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v7 26/41] gnu: Add node-file-uri-to-path., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v7 32/41] gnu: Add node-serialport-parser-delimiter., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v7 41/41] gnu: Add node-serialport., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v7 40/41] gnu: Add node-serialport-stream., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v7 39/41] gnu: Add node-serialport-parser-byte-length., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v7 29/41] gnu: Add node-ms., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v7 34/41] gnu: Add node-serialport-bindings., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v6 40/41] gnu: Add node-serialport-stream., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v6 38/41] gnu: Add node-serialport-parser-cctalk., Philip McGrath, 2021/12/30
- [bug#51838] [PATCH v6 00/41] guix: node-build-system: Support compiling add-ons with node-gyp.,
Ryan Sundberg <=
- [bug#51838] [PATCH v5 07/45] guix: node-build-system: Add #:absent-dependencies argument., Liliana Marie Prikler, 2021/12/20
- [bug#51838] [PATCH v5 07/45] guix: node-build-system: Add #:absent-dependencies argument., Jelle Licht, 2021/12/20
- [bug#51838] [PATCH v5 07/45] guix: node-build-system: Add #:absent-dependencies argument., Philip McGrath, 2021/12/20
- [bug#51838] [PATCH v5 12/45] gnu: node-llparse-builder-bootstrap: Use #:absent-dependencies., Philip McGrath, 2021/12/16
- [bug#51838] [PATCH v5 13/45] gnu: node-llparse-frontend-bootstrap: Use #:absent-dependencies., Philip McGrath, 2021/12/16
- [bug#51838] [PATCH v5 09/45] gnu: node-ms-bootstrap: Use #:absent-dependencies., Philip McGrath, 2021/12/16
- [bug#51838] [PATCH v5 08/45] gnu: node-semver-bootstrap: Use #:absent-dependencies., Philip McGrath, 2021/12/16
- [bug#51838] [PATCH v5 14/45] gnu: node-llparse-bootstrap: Use #:absent-dependencies., Philip McGrath, 2021/12/16
- [bug#51838] [PATCH v5 15/45] gnu: node-semver: Use #:absent-dependencies., Philip McGrath, 2021/12/16
- [bug#51838] [PATCH v5 17/45] gnu: node-once: Use #:absent-dependencies., Philip McGrath, 2021/12/16