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 > and , > 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 , because: > > On 12/20/21 18:10, Jelle Licht wrote: >> Liliana Marie Prikler 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(-) >