nix-community / npmlock2nix

nixify npm based packages [maintainer=@andir]
Apache License 2.0
130 stars 42 forks source link

Fail to build electron quickstart #162

Closed tobiasBora closed 1 year ago

tobiasBora commented 2 years ago

I tried to package the most basic electron project using this tool, but it fails :

$ cd /tmp
$ git clone https://github.com/tobiasBora/quickstart-electron # This is just the quickstart tutorial in a git repo
$ cd quickstart-electron
$ nix-shell -p niv
$ niv init
$ niv add nix-community/npmlock2nix
$ cat nix/default.nix # Edit this file to have this content:
# nix/default.nix
let
  sources = import ./sources.nix;
  pkgs = import <nixpkgs> {};
in
  import sources.nixpkgs {
    overlays = [
      (self: super: {
        npmlock2nix = pkgs.callPackage sources.npmlock2nix { };
      })
    ];
  }
$ cat shell.nix # Edit this file to have this content
let
  pkgs = import ./nix/default.nix;
in
pkgs.npmlock2nix.shell {
  src = ./.;
}
$ nix-shell
[...]
> process-nextick-args@2.0.1 preinstall /build/node_modules/process-nextick-args
> /build/node_modules/.hooks/preinstall
7m            ......] - install:yallist: info lifecycle yallist@4.0.0~install: y
> electron@19.0.9 postinstall /build/node_modules/electron
> node install.js

RequestError: getaddrinfo ENOTFOUND github.com
    at ClientRequest.<anonymous> (/build/node_modules/got/source/request-as-event-emitter.js:178:14)
    at Object.onceWrapper (events.js:520:26)
    at ClientRequest.emit (events.js:412:35)
    at ClientRequest.origin.emit (/build/node_modules/@szmarczak/http-timer/source/index.js:37:11)
    at TLSSocket.socketErrorListener (_http_client.js:475:9)
    at TLSSocket.emit (events.js:400:28)
    at emitErrorNT (internal/streams/destroy.js:106:8)
    at emitErrorCloseNT (internal/streams/destroy.js:74:3)
    at processTicksAndRejections (internal/process/task_queues.js:82:21)
npm WARN my-electron-app@1.0.0 No repository field.9.0.9~postinstall:[0m

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! electron@19.0.9 postinstall: `node install.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the electron@19.0.9 postinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /build/.npm/_logs/2022-07-25T11_35_17_790Z-debug.log

error: builder for '/nix/store/6x7ynmm42ijjy4bmd15a77kwbvc2cfy0-my-electron-app-1.0.0.drv' failed with exit code 1;
       last 10 log lines:
       > npm ERR! errno 1
       > npm ERR! electron@19.0.9 postinstall: `node install.js`
       > npm ERR! Exit status 1
       > npm ERR!
       > npm ERR! Failed at the electron@19.0.9 postinstall script.
       > npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
       > 
       > npm ERR! A complete log of this run can be found in:
       > npm ERR!     /build/.npm/_logs/2022-07-25T11_35_17_790Z-debug.log
       > 
       For full logs, run 'nix log /nix/store/6x7ynmm42ijjy4bmd15a77kwbvc2cfy0-my-electron-app-1.0.0.drv'.
xFA25E commented 1 year ago

This is probably because there is a dependency on a package from github in your package-lock.json or some package fetches a dependency from github during a build. Find what package fails to build and try to add its dependency manualy to npmlock2nix build environment.

Maybe you should provide full logs.

Also, the lock file of the project you are trying to build uses version 2. AFAIK, npmlock2nix does not support it.

Ekleog commented 1 year ago

I'm having the same issue with electron 14 (the version used by cordova), and my package-lock.json does not have any instance of github.com that is not a sponsoring link.

My guess would have been that electron's install.js attempts to make a request to github, but it turns out I can't find this file anywhere, be it in the electron source code or in the leftover of a nix-build -K shell.nix.

So… I have no idea how to debug this further :/

andir commented 1 year ago

The problem with all this node shit (yes mostly it is just shit ;-)) is that it often tries to download stuff as part of their installation javascript code. NodeJS is able to run arbitrary code during package installation and for tools that link to native binaries or libaries that often means they try to download a binary or compile sometthing from source. With Nix that is not what we want and also not possible with the standbox (try darwin instead, that might work...).

The solution to the quickstart is rather simple once you know what to look for. I've just done it locally: image

I used the initially provided example and changed the shell.nix to this:

let
  pkgs = import ./nix/default.nix;
in
pkgs.npmlock2nix.shell {
  src = ./.;
  # https://github.com/electron/electron/blob/3a94634ae559e5e9cc41e26b56af95a3f916a7f9/npm/install.js#L12
  node_modules_attrs.ELECTRON_SKIP_BINARY_DOWNLOAD = 1;
  # https://github.com/electron/electron/blob/3a94634ae559e5e9cc41e26b56af95a3f916a7f9/docs/api/environment-variables.md?plain=1#L178
  node_modules_attrs.ELECTRON_OVERRIDE_DIST_PATH = "${pkgs.electron}/bin";
  ELECTRON_OVERRIDE_DIST_PATH = "${pkgs.electron}/bin";
}

now not everything in there is required. You can build the shell environment entirely without the ELECTRON_OVERRIDE_DIST_PATH being set as long as the ``` environment variable is set.

During runtime (within the nix-shell environment) you wan the ELECTRON_OVERRIDE_DIST_PATH variable to be set in order for the node stuff to find the right electron executable. You can probably also just supply ELECTRON_OVERRIDE_DIST_PATH instead of ever thinking about ELECTRON_SKIP_BINARY_DOWNLOAD.

Generally for these kinds of (stupid) downloaders we've got the preInstallSymlinks in npmlock2nix because those will put symlinks to binaries into the node packages unpacked dir before they run their downloader. Most of the downloaders check if the file already exists and then skip the download. However, I prefer if packages would stop doing this or at least provide a clear opt-out. Electron provides an opt-out but it is not obvious if you have never deal with this before.

Let me know if there is any further issue with this and I'll reopen it.

Ekleog commented 1 year ago

Thank you for your answer! You're right that it works :D (and also that it's shitty, I've spent like 6 hours today already fighting cordova trying to get an example to work without node_modules, and there are still issues)

Thinking of what could be done to improve the UX of this case (though now thanks to you the answer is documented here at least!), I'm wondering…

It seemed to me that nixpkgs had a collection of overrides to apply to derivations for some languages at least? But I can't find something for nodejs again that'd be supported by npmlock2nix.

Do you think it'd be possible to upstream such hacks into nixpkgs, or at least npmlock2nix, so that not everyone needs to know about it? (I also have an env var for a 7zip downloader, that could eg. be propagated to shells with this package – for future reference it is USE_SYSTEM_7ZA = "true";)

tobiasBora commented 1 year ago

Thanks for your answer. So as I understand these variables are in no way standardized, and some packages may not even provide a way to use nix's input.That's a really poor design… but not your choice. Thanks a lot!

However I agree with @Ekleog that it would be much better to automatically detect these edge cases… Or at least print out a warning pointing to the documentation. Even better would be a script that patches all binaries after they are downloaded (e.g. with LD_PRELOAD to override the functions that download or write files)… but I guess it would need sandbox to be disabled in that case as it would access the internet at build time.

andir commented 1 year ago

I think something that I could get on-board with would be a way to opt-in to some of these well-known quirks. What I don't want to introduce is an overlay mechanism such as is in poetry2nix. They tend to accumulate lots of hacks. Often that is very handy to get going but requires ongoing maintenance and usually those adding them aren't around by the time they are failing.

If we were to do detection of these cases I'd do it statically based on well-known misbehaving dependencies. I know this wont scale but we could at least provide some mechanism for high-profile dependencies (such as electron). The whole thing doesn't appeal me much because of the sad story of providing a UX to consumers of nix derivations. We are essentially restricted to builtins.throw and builtins.trace (via the various nixpkgs wrappers/formatters).

The experience I'd love to provide is something like this:

$ nix-build
[npmlock2nix]: The build will likely fail because it requires network access during installation. This isn't a Nix or a npmlock2nix bug but an issue with the libraries:

 * electron: Electron tries to download an electron binary from the internet. There are environment variables you can set to provide an electron binary and avoid the download failure. Please see the upstream bug report /  feature request to handle these situation more gracefully in hermetic builds (such as Nix builds): https://github.com/......
 * esbuild: Esbuild is a go binary that is downloaded during installation. You can provide a binary using the environment variables .....

To silence these warnings set the `silence_hints = true` in `node_modules` (respectively `node_modules_attrs` for the `.shell` and `.build` functions).

      [..]

       > npm ERR! errno 1
       > npm ERR! electron@19.0.9 postinstall: `node install.js`
       > npm ERR! Exit status 1
       > npm ERR!
       > npm ERR! Failed at the electron@19.0.9 postinstall script.
       > npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
       > 
       > npm ERR! A complete log of this run can be found in:
       > npm ERR!     /build/.npm/_logs/2022-07-25T11_35_17_790Z-debug.log
       > 

The problem I'd see with anything like this is that users usually don't read. They just look at the last few lines and then move on / try random things. I can't blame them.

Since I don't want to make npmlock2nix create actual nix file (philosopically I think they aren't a good or acceptable solution) it is unlikely that we will create a CLI tool that detects these things for you.

The best way forward would be if the NPM world gets its shit together and stops doing these random downloads and formalizes these (optional) requirements in the lockfile. That way we could provide our own binaries without having to rely on environment variables within each of these dependencies.

Knowing how stuck most node people are in their Docker world (not realizing that Docker isn't providing hermetic builds but simply an artifact format/builder) I don't see that happen soon.

Even better would be a script that patches all binaries after they are downloaded (e.g. with LD_PRELOAD to override the functions that download or write files)…

It isn't standardized how things are downloadded. Some might be using fetch, others use another api or execute a shell script that then uses wget/curl/... . Even in those cases they likely want TLS so we'd have to intercept TLS to some degree. Decode that actual URL they want to retrieve and provide some kind of transparent replacement data stream?!? Sure you can do that. I've thought about using HTTP(s) proxies for all of what npmlock2nix is doing right now but that is a lot more runtime interaction than currently. The major benefit of npmlock2nix was/is the simplicityof the approach. We don't replace npm. We change the inputs to NPM such that a developer might sitll be able to use the project without any of the Nix bits. I strongly believe that you don't want to create projects that only work with Nix. For us Nix folks that is fine or even desireable but your average developer will curse you for this. They want their first class vscode/vim/npm/... integration (and they aren't wrong, life is too short to suffer through Nix if you don't have to).

That all being said I am very happy to discuss sustainable ways to provide the tooling to work with these projects without having to collect tons of quirks and keeping them updated. That might be a quick way forward but it isn't a long-term solution to the problem. :/

tobiasBora commented 1 year ago

Thanks for your answer. I agree there is no good solution as soon as npm allows users to download pre-built binaries. When I was talking about LD_PRELOAD I was mostly talking about the primitives to write it down on disk, like fopen/fputc/fclose… or alike (if it's even possible to overwrite them?), so that we don't need to support too many libraries. But even curl-like functions may be good enough to overwrite if it's not possible to overwrite the write functions as the number of such libraries is quite small compared to the whole list of npm packages. Moreover, this may benefit to other systems like java and more, so we can potentially merge the effort of multiple nix communities here.

Another alternative would be to define directly a FUSE file system that patches the executables right after they are written on disk. The main issue (beside the fact that it uses fuse that is maybe not extremely portable across OS) I can see here is that the hash may be less stable (who knows what npm install scripts can download :-P) or we may need to rely on disabling sandbox (to enable internet) if the hash is not stable enough to use fixed-output derivations (that is the only exception to disabling internet).

Finally the last idea I have (and the one I prefer) would be to use fake-chroot/proot-like methods to put the loader in (a fake) /lib/… while downloading the dependencies… The advantage over the methods given above is that it does not patch the downloaded files on-the-go and therefore you don't need to know when a file needs to be patched, which decreases a lot the complexity of the approach. It may also be more portable than FUSE (but potentially slower and less stable… even if Termux is still able to use a fork it quite reliably), but it would suffer from the same hash-variation issues if npm's download are not reliable enough.

EDIT I started a discussion here: https://discourse.nixos.org/t/use-proot-or-alike-to-build-complex-npm-java-libraries-that-try-to-download-arbitrary-executables/23379

nixos-discourse commented 1 year ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/use-proot-or-alike-to-build-complex-npm-java-libraries-that-try-to-download-arbitrary-executables/23379/1