dream2nix icon indicating copy to clipboard operation
dream2nix copied to clipboard

fix: nodejs: strict-builder: patch shebangs

Open tgunnoe opened this issue 2 years ago • 6 comments
trafficstars

For the scenario where the installed node_modules were used for building (nestjs in my situation), the shebangs were not patched for build time in the strict-builder. That lead to no such file or directory in the sandbox. reference: https://matrix.to/#/!PrIEtcffTLLzUWvJOS:matrix.org/$zslZWYi4HF0xramEQum6xf808-VQbbJIkN4kbgGFqt0?via=nixos.org&via=matrix.org&via=nixos.dev

Also, the bins weren't runnable permissions.

This didn't seem like an issue within the granular builder, but maybe you all would know better than I if a similar fix is needed there as well.

@hsjobeki @DavHau

tgunnoe avatar Apr 26 '23 16:04 tgunnoe

@hsjobeki I wonder why the we even produce node_modules that contain broken shebangs. Usually patchShebang is executed automatically in the fixup phase of each individual derivation, so there shouldn't be any corrupt shebangs.

DavHau avatar Apr 26 '23 16:04 DavHau

Strange. I‘d need to investigate this. questions:

  • (if needed manually) Should „patchShebangs“ be run only on node_modules/.bin
  • Do we really need „+x“ applied to every file in the whole node_modules?
  • Should patch shebangs be in node-modules-tree.nix (where the node_modules are created.

hsjobeki avatar Apr 26 '23 17:04 hsjobeki

(if needed manually) Should „patchShebangs“ be run only on node_modules/.bin

It wasn't straightforward to only patch what's in .binwith patchShebangs. I suppose it needs to follow symlinks or something similar. However, I assumed finally that probably everything in node_modules needs to be patched for their scope, but I dont know how much of a performance loss that would entail.

Do we really need „+x“ applied to every file in the whole node_modules?

No, there can be a cleaner operation for the runnables. Any suggestions?

Should patch shebangs be in node-modules-tree.nix (where the node_modules are created.

I can do it there instead. Sorry, still not completely familiar with this builder

tgunnoe avatar Apr 26 '23 17:04 tgunnoe

Thanks for your efforts. I appreciate it. If you could move this fix to node-modules-tree.nix and check if your use case still works. I think we can merge this and add a todo comment for me to come back later.

hsjobeki avatar Apr 26 '23 17:04 hsjobeki

I wonder why the we even produce node_modules that contain broken shebangs

Oh, I suppose since the tree is built fully with python, there's not a fixup phase that the derivation goes into at that time. As far as I understand, anyways. I don't know what that would entail to run the nix path substitution from within the python builds

If you could move this fix to node-modules-tree.nix and check if your use case still works.

Sounds good! no problem

tgunnoe avatar Apr 26 '23 17:04 tgunnoe

Oh, I suppose since the tree is built fully with python

Oh yeah, I forgot about the node-modules-tree.nix

If you could move this fix to node-modules-tree.nix and check if your use case still works.

Note that patchShebangs can only substitute paths in shebangs for known buildInputs. The runCommandLocal which is used to execute the python builder probably needs nodejs as a build input and any other things that are expected to appear in shebangs.

DavHau avatar Apr 27 '23 10:04 DavHau