Build go module with pnpm deps

[martinetd]

Describe the bug

I tried to build rmfakecloud (a go app, with since the latest version a pnpm web ui component) in #358324 in a single derivation but that didn't go so well.

Steps To Reproduce

Steps to reproduce the behavior:

1. checkout https://github.com/martinetd/nixpkgs/commits/rmfakecloud_gopnpm/
2. fail to build rmfakecloud: nix -L build -f . rmfakecloud

...
Error: 'pnpmDeps' must be set when using pnpmConfigHook
...

Additional context

It fails because the pnpmDeps / pnpmRoot attributes defined in buildGoModule arguments aren't present in the final derivation.

I could build by patching go module like this:

diff --git a/pkgs/build-support/go/module.nix b/pkgs/build-support/go/module.nix
index 3a3ae71de508..3e063f5f7126 100644
--- a/pkgs/build-support/go/module.nix
+++ b/pkgs/build-support/go/module.nix
@@ -81,7 +81,7 @@ in
 
     nativeBuildInputs = (finalAttrs.nativeBuildInputs or [ ]) ++ [ go git cacert ];
 
-    inherit (finalAttrs) src modRoot;
+    inherit (finalAttrs) src modRoot pnpmDeps pnpmRoot;
     inherit (go) GOOS GOARCH;
     inherit GO111MODULE GOTOOLCHAIN;
 

But it's rather unsightly 😅

Perhaps there is a way to inherit all attributes from the argument?

Notify maintainers

buildGoModules itself has no maintainer... @NixOS/golang ?
For the pnpm side, @Scrumplex @gepbird

Thanks!

[TomaSajt]

If you really must, you can set env.pnpmDeps and env.pnpmRoot, since IIRC env.* is always propagated. (or do some hacking around with adding pnpmConfigHook only to the main package and not the FOD).
This is very much just a hack. I agree that this should be more composable.

A part of #353526 might address this because it changes how the builder args are automatically inherited. It needs more work and maybe it needs to be split up into multiple PRs...

[martinetd]

Thanks for the quick reply!

Yeah, changing buildGoModule to work with hooks like pnpm does is probably the most composable approach long term, I hadn't realized this already was in the pipes (just had a quick look at the PR and that does look like it was a lot of work, and as you said there probably is some more left -- thank you)

Short term, there really isn't any urgent need to move away from the current "build ui in a separate derivation" apart from elegance (and removing an intermediate step), so as far as I'm concerned it might make sense to just wait for this rework to finish on its own time.

I've also just tried env. and it seems to work, I agree it's a hack but it might be better than the current approach if it allows the update script to run on rmfakecloud (that was my motivation for bringing the UI in the same nix file, but I didn't actually get to try if it even works yet...)
I'll try to find some time to find how it's actually supposed to work on a simpler package first 😁

[winterqt]

Closing, discussion should be moved to the PR. Thanks for contributing!

[martinetd]

Ah, actually I was misunderstanding how buildGoModule works; the problem isn't so much that pnpmDeps/Root aren't inherited in the goModules it's that nativeBuildInputs all are inherited..
I'd "just" need some way to specify that pnpm_9.configHook is in nativeBuildInputs for the final package but not for goModules

With the current env workaround the build actually runs the pnpm hook twice (once when vendoring the module and once for the actual package build)

@TomaSajt do you know why nativeBuildInputs have to be propagated to the goModules? I'm not familiar with go but if it's just downloading deps and not actually building anything I don't see why it would require any special input (and in your new version I don't see it being propagated)

(and, no, I won't post that in the PR as it has nothing to do with that PR... It's a side effect that it happens to probably fix it)

[martinetd]

(alternatively making pnpm config hook not error but just ignore executing if pnpmDeps isn't set, but that would be a downgrade of UX for "normal" folks trying to build a new package with pnpm...? sorry for back-to-back comment, I'll stop spamming for a bit)