| 21 May 2023 |
raitobezarius | Lily Foster: the PR is good to go IMHO | 14:36:54 |
Lily Foster | Confirmed with Winter and merged it 🎉 | 15:17:28 |
Lily Foster | I tested the heck out of it but will still watch the next hydra eval for regressions just in case | 15:17:57 |
raitobezarius | yeah you have some stabilization days ahead of you if needed :) | 15:25:46 |
raitobezarius | I have been dreaming those last days about starting the yarn backend for buildNpmPackage | 15:25:55 |
raitobezarius | hopefully when the branch off is done and things becomes calmer on QEMU, I will do that :) | 15:26:10 |
raitobezarius | * hopefully when the ~branch off~ release is done and things becomes calmer on QEMU, I will do that :) | 15:26:19 |
Lily Foster | In reply to @raitobezarius:matrix.org yeah you have some stabilization days ahead of you if needed :) Don't we all | 15:26:38 |
raitobezarius | yes :D | 15:27:00 |
Lily Foster | In reply to @raitobezarius:matrix.org I have been dreaming those last days about starting the yarn backend for buildNpmPackage I've a few open yarn PRs to help that and there's a draft buildYarnPackage too we'll pick up soon | 15:27:05 |
hexa | and https://github.com/NixOS/nixpkgs/pull/230991? 😲 | 19:50:04 |
hexa | should we merge botamusique and navidrome with the npmCOnfigHook reschedule hack? | 19:50:34 |
hexa | * should we merge botamusique and navidrome with the npmConfigHook reschedule hack? | 19:50:39 |
Lily Foster | In reply to @hexa:lossy.network should we merge botamusique and navidrome with the npmConfigHook reschedule hack? Given it unblocks those packages, I was starting to think we should just go ahead and merge the npmRoot PR and do further review items in follow-up PRs (and backport post branchoff) | 20:43:35 |
Lily Foster | I would like npmRoot, I was just still wondering if we wanted it on the other hooks so that it worked with buildNpmPackage and wondering if we wanted it also on fetchNpmDeps. But that doesn't need to be a blocker | 20:44:37 |
hexa | Did the npmRoot change work for botamusique? Because it caused problems with navidrome. | 20:44:41 |
Lily Foster | In reply to @hexa:lossy.network Did the npmRoot change work for botamusique? Because it caused problems with navidrome. Oh really? I hadn't tested yet | 20:44:55 |
hexa | https://github.com/NixOS/nixpkgs/pull/230991#issuecomment-1552133508 | 20:45:17 |
Lily Foster | In reply to @hexa:lossy.network https://github.com/NixOS/nixpkgs/pull/230991#issuecomment-1552133508 Weird 🤔 | 20:46:07 |
Lily Foster | I'll investigate in like ~1-2 hours (sorry for delays, day ended up with more non-computer things to do than I thought) | 20:46:45 |
hexa | No worries, this is a hobby after all | 20:47:25 |
Lily Foster | In reply to @hexa:lossy.network https://github.com/NixOS/nixpkgs/pull/230991#issuecomment-1552133508 I cannot replicate this by cherry-picking your commit from #229953 on top of #230991 and adding this diff:
diff --git a/pkgs/servers/misc/navidrome/default.nix b/pkgs/servers/misc/navidrome/default.nix
index 7c121e82210..309601e059f 100644
--- a/pkgs/servers/misc/navidrome/default.nix
+++ b/pkgs/servers/misc/navidrome/default.nix
@@ -28,22 +28,14 @@ buildGoModule rec {
vendorHash = "sha256-C8w/qCts8VqNDTQVXtykjmSbo5uDrvS9NOu3SHpAlDE=";
+ npmRoot = "ui";
+
npmDeps = fetchNpmDeps {
inherit src;
sourceRoot = "source/ui";
hash = "sha256-qxwTiXLmZnTnmTSBmWPjeFCP7qzvTFN0xXp5lFkWFog=";
};
- prePatch = ''
- postPatchHooks=("''${postPatchHooks[@]/npmConfigHook}")
- '';
-
- postPatch = ''
- pushd ui
- npmConfigHook
- popd
- '';
-
nativeBuildInputs = [
makeWrapper
nodejs
| 21:31:44 |
Lily Foster | Instead I just get a successfully built /nix/store/0qjbbmqfvgn9x4g9rgyvbqsfzj9bda91-navidrome-0.49.3 | 21:32:36 |
Lily Foster | Also the navidrome update script fails because building .go-modules in general on the derivation fails due to buildGoModule propagating nativeBuildInputs to the fetcher (???) which obviously adds the npm hook which when run, expects to be building npm stuff, not fetching go modules | 21:38:51 |
Lily Foster | I would say this is a bug in buildGoModule and not npmConfigHook because I imagine the same would happen with, e.g., the rust hooks | 21:39:12 |
Lily Foster | * I would say this is a bug in buildGoModule and not npmConfigHook because I imagine the same would happen with, e.g., the cargo/rust hooks | 21:39:16 |
hexa | hm ok | 21:39:38 |
Lily Foster | Now for what to do about it, idk | 21:39:46 |
Lily Foster | 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? | 21:40:20 |
Lily Foster | That seems... counterintuitive. Stuff like buildRustPackage don't do that | 21:41:11 |