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

[Philip McGrath]

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

-- 
2.32.0