nix-community / napalm

Support for building npm packages in Nix and lightweight npm registry [maintainer=?]
MIT License
104 stars 17 forks source link

Simplify npmOverrideScript #41

Closed jtojnar closed 2 years ago

jtojnar commented 2 years ago

Previously, the script was created in buildPhase by Nix interpolating the code into a heredoc string and being written to a file by cat there. But the script contained a comment that included backticks, which were interpreted by bash as command substitution inside the heredoc because, unlike dollar signs, they were not escaped. This resulted in an execution of generic builder setup script, as well as an attempt to execute the $stdenv directory. Warning from the latter is how this mess was discovered.

Since no variable other than $stdenv was used, and that one is also readily available as a Nix variable ${pkgs.stdenv}, this commit delegates the creation of the script to the appropriate trivial builder from Nixpkgs, allowing us to get rid of the nasty escaping.

cc @Mazurel who introduced this in #38 (the inappropriate execution comes specifically from 915bcc39b18c39fd4cd8765a396053db9947e0c7).

Mazurel commented 2 years ago

Yeah, my bad, good thing you have figured that out !