| 21 May 2023 |
 hexa | In reply to @lily:lily.flowers
There's no comment in the buildGoModule code about why it propagates nativeBuildInputs to the fetcher -- do you know why it would be doing that? no idea | 22:03:41 |
| hexa | In reply to @lily:lily.flowers
It looks like the evcc derivation has the exact same issue and fixes it via buildGoModule's overrideModAttrs to set a slimmer nativeBuildInputs oh, right 😲 | 22:04:10 |
 Lily Foster | In reply to @hexa:lossy.network
https://github.com/NixOS/nixpkgs/pull/230991#issuecomment-1552133508 I just realized that was before winter fixed the PR to actually work (which is why it ended up with a pushd/popd) | 22:05:11 |
| Lily Foster | So yeah the npmRoot PR needs to a rebase and then I'll merge and deal with further changes in follow-ups, to unblock other stuff | 22:06:33 |
| hexa | just the upper pushd hunk needs a massage | 22:11:08 |
| Lily Foster | Yep, I've already done it locally. Was gonna get permission from winter before just pushing to the branch myself though | 22:11:34 |
| 22 May 2023 |
 raitobezarius | is there a way with buildNpmPackage to just "install" the node_modules and be done? | 11:45:23 |
| raitobezarius | is dontNpmInstall the option I am looking for? I tried it and I'm not sure I am holding it correctly | 11:45:40 |
| Lily Foster | Is the default install hook over-copying or failing or something? | 11:48:19 |
| raitobezarius | > Executing npmInstallHook
> npm ERR! Invalid package, must have name and version
| 11:58:22 |
| raitobezarius | this is an invalid Node.js package per se | 11:58:26 |
| raitobezarius | because they reuse it in some Makefile later on | 11:58:31 |
| raitobezarius | And they really only need the node deps | 11:58:35 |
| Lily Foster | If the project has custom install logic, just override installPhase and do what you need to do | 12:05:52 |
| Lily Foster | Or if you want the default stdenv installPhase then dontNpmInstall does what you want | 12:07:42 |
| raitobezarius | Okay, thank you! | 12:09:20 |
| Lily Foster | You may need to prune node_modules first yourself though so you don't end up with dev deps in the output | 12:10:07 |
| Lily Foster | If the makefile doesn't | 12:10:17 |
| raitobezarius | is there a way to extract a nodejs from buildNpmPackage function? | 12:11:02 |
| raitobezarius | or the result of the buildNpmPackage? | 12:11:06 |
| Lily Foster | Like what nodejs that buildNpmPackage is using? | 12:16:48 |
| Lily Foster | No, but we maybe should add it as a passthrough or something | 12:17:17 |
| Lily Foster | In reply to @raitobezarius:matrix.org
or the result of the buildNpmPackage? Like the output, or am I misunderstanding? | 12:17:33 |
| raitobezarius | In reply to @lily:lily.flowers
Like what nodejs that buildNpmPackage is using? yes this | 12:17:45 |
 dotlambda | In reply to @lily:lily.flowers
dotlambda: If it helps for fixing the above package, would it be okay if I separate out the change to add an npmWorkspace argument from that bitwarden-cli PR and make my own separate PR? (or alternatively if you have time to respond to the review comments, we can keep the work in that PR) Definitely! Thank you very much | 20:05:25 |
| 24 May 2023 |
| Lily Foster | In reply to @raitobezarius:matrix.org
yes this You're just wanting something like this on buildNpmPackage, right?
diff --git a/pkgs/build-support/node/build-npm-package/default.nix b/pkgs/build-support/node/build-npm-package/default.nix
index 1c3fb6a74ef..f4cb7d763ed 100644
--- a/pkgs/build-support/node/build-npm-package/default.nix
+++ b/pkgs/build-support/node/build-npm-package/default.nix
@@ -53,7 +53,7 @@ stdenv.mkDerivation (args // {
# Stripping takes way too long with the amount of files required by a typical Node.js project.
dontStrip = args.dontStrip or true;
- passthru = { inherit npmDeps; } // (args.passthru or { });
+ passthru = { inherit nodejs npmDeps; } // (args.passthru or { });
meta = (args.meta or { }) // { platforms = args.meta.platforms or nodejs.meta.platforms; };
})
| 00:21:33 |
| Lily Foster | Wait are you wanting (buildNpmPackage { ... }).nodejs to be available or something just like buildNpmPackage.nodejs? | 00:22:23 |
| Lily Foster | In reply to @robert:funklause.de
Definitely! Thank you very much Opened as https://github.com/NixOS/nixpkgs/pull/233804 and tested against your second commit in your bitwarden-cli PR :) | 11:18:28 |
| Lily Foster | Oh I just noticed your comment about npm prune --workspace being non-ideal. That seems bizarre to me, and remove the --workspace on the prune command didn't seem to reduce output size 🤔 | 11:23:52 |
| Lily Foster | * Oh I just noticed your comment about npm prune --workspace being non-ideal. That seems bizarre to me, and removing the --workspace on the prune command didn't seem to reduce output size 🤔 | 11:23:59 |
