Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add partial support for scoped packages #28

Merged
merged 1 commit into from
Nov 3, 2020

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Nov 1, 2020

npm supports scoped packages, whose name matches the following scheme:

@somescope/somepackagename

These are used all over the modern NPM ecosystem, most notably by the Babel transpiler.

Unfortunately, Servant is not expressive enough so the code is a bit messy. Also, the GET /:scope/:name route gets shadowed by GET /:name/:version. It is still an improvement, since running npm install in a project with a lock file only seems to care about the tarball route, which works both scoped and unscoped.

Partial fix for #27

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 1, 2020

See that all routes work except for the shadowed one:

$ curl -I http://localhost:3030/@babel/helper-create-regexp-features-plugin/-/helper-create-regexp-features-plugin-7.12.1.tgz
HTTP/1.1 200 OK
Date: Sun, 01 Nov 2020 06:20:07 GMT
Server: Warp/3.3.13
Content-Type: application/octet-stream

$ curl -I http://localhost:3030/yaeti/-/yaeti-0.0.6.tgz
HTTP/1.1 200 OK
Date: Sun, 01 Nov 2020 06:20:27 GMT
Server: Warp/3.3.13
Content-Type: application/octet-stream

$ curl -I http://localhost:3030/@babel/helper-create-regexp-features-plugin/7.12.1
HTTP/1.1 200 OK
Date: Sun, 01 Nov 2020 06:20:48 GMT
Server: Warp/3.3.13
Content-Type: application/json;charset=utf-8

$ curl -I http://localhost:3030/yaeti/0.0.6
HTTP/1.1 200 OK
Date: Sun, 01 Nov 2020 06:20:59 GMT
Server: Warp/3.3.13
Content-Type: application/json;charset=utf-8

$ curl -I http://localhost:3030/yaeti
HTTP/1.1 200 OK
Date: Sun, 01 Nov 2020 06:21:03 GMT
Server: Warp/3.3.13
Content-Type: application/json;charset=utf-8

$ curl -I http://localhost:3030/@babel/helper-create-regexp-features-plugin
HTTP/1.1 500 Internal Server Error
Date: Sun, 01 Nov 2020 06:21:11 GMT
Server: Warp/3.3.13
Content-Type: text/plain; charset=utf-8

But in my experience, npm only uses the tarball route.

@nmattia
Copy link
Collaborator

nmattia commented Nov 1, 2020

Hey! Thanks for the PR. I've never run into problems with scoped packages, can you share an example where this breaks?

EDIT: this in particular uses tons of scoped packages IIRC: https://github.com/deckgo/deckdeckgo/blob/91a6bd3f9ad6383f5adb7d3d034a4159fa953831/infra/default.nix#L62

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 1, 2020

I was getting ton of 404s from npm install. Currently I have this mess trying to fix node-gyp from #11, make parcel run and it does not even succeed:

{ pkgs ? import <nixpkgs> {} }: # nixos unstable for npm 7

let
  src = pkgs.fetchFromGitHub {
    owner = "jtojnar";
    repo = "pengu";
    rev = "464198bf06a96e698a37d2f21047337530d22a0f";
    sha256 = "lOdV1iN9cADHg51+GjVGK1h2CxD+tv7K8HZ53ii4gU4=";
  };
  nodejs = pkgs.nodejs_latest;
  napalm = import /home/jtojnar/Projects/napalm { pkgs = pkgs // { inherit nodejs; }; }; # this branch
in
  # Inspired by https://github.com/nmattia/napalm/pull/11
  napalm.buildPackage src {
    npmCommands = [
      # Just download and unpack all the npm packages,
      # we need napalm to patch shebangs before we can run install scripts.
      "npm install --loglevel verbose --ignore-scripts"
    ];

    nativeBuildInputs = with pkgs; [
      autoPatchelfHook

      nodejs.passthru.python

      # These tend to be needed when
      autoconf
      automake
      gettext
      gnum4
      gnused
      nasm
      pkg-config
    ];

    # Add hooks to configure phase.
    # https://github.com/nmattia/napalm/pull/26
    configurePhase = ''
      runHook preConfigure

      export HOME=$(mktemp -d)

      runHook postConfigure
    '';

    postConfigure = ''
      # Add node headers for node-gyp.
      mkdir -p $HOME/.node-gyp/${nodejs.version}
      echo 9 > $HOME/.node-gyp/${nodejs.version}/installVersion
      ln -sv ${nodejs}/include $HOME/.node-gyp/${nodejs.version}/include

      # Do not try to find npm in napalm-registry –
      # it is not there and checking will slow down the build.
      npm config set update-notifier false

      # Same for security auditing, it does not make sense in the sandbox.
      npm config set audit false
    '';

    postBuild = ''
      # Fix all the binaries with the .so from buildInputs. Magic!
      # autoPatchelf node_modules

      # Let’s install again, this time running scripts.
      npm install --loglevel verbose
    '';

    installPhase = ''
      runHook preInstall

      mkdir $out
      cp -r node_modules $out/

      runHook postInstall
    '';
  }

I use npm workspaces but I would not expect it to have any effect on napalm – as I understand it, npm will just install the workspace dependencies transparently using the registry, just like any other dependency.

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 1, 2020

Here is more minimal and completely hermetic example:

let
  nixpkgs = builtins.fetchTarball {
    url = "https://github.com/NixOS/nixpkgs/archive/1dc37370c489b610f8b91d7fdd40633163ffbafd.tar.gz"; # nixos-unstable
    sha256 = "1qvfxf83rya7shffvmy364p79isxmzcq4dxa0lkm5b3ybicnd8f4";
  };
  pkgs = import nixpkgs { };
  src = pkgs.fetchFromGitHub {
    owner = "jtojnar";
    repo = "pengu";
    rev = "464198bf06a96e698a37d2f21047337530d22a0f"; # master
    sha256 = "lOdV1iN9cADHg51+GjVGK1h2CxD+tv7K8HZ53ii4gU4=";
  };
  nodejs = pkgs.nodejs_latest;
  napalm = import (pkgs.fetchFromGitHub {
    owner = "nmattia";
    repo = "napalm";
    rev = "3c456911f9a0ae9804334344473173bd8ac314c5"; # master
    sha256 = "Ev31Fi24Zw93oJZUeFwBvR6ZUlbYodk+Am6ca6aMKGM=";
  }) { pkgs = pkgs // { inherit nodejs; }; };
in
  napalm.buildPackage src {
    postConfigure = ''
      # Do not try to find npm in napalm-registry –
      # it is not there and checking will slow down the build.
      npm config set update-notifier false

      # Same for security auditing, it does not make sense in the sandbox.
      npm config set audit false
    '';
  }

It produces the following error without this patch:

npm verb stack Error: 404 Not Found - GET http://localhost:41357/@types/q/-/q-1.5.4.tgz
npm verb stack     at /nix/store/ngga5g4siw4q9i0bqjxckg8aaq35rzgs-nodejs-15.0.1/lib/node_modules/npm/node_modules/npm-registry-fetch/check-response.js:123:15
npm verb stack     at runMicrotasks (<anonymous>)
npm verb stack     at processTicksAndRejections (node:internal/process/task_queues:93:5)
npm verb statusCode 404
npm verb pkgid @types/q@http://localhost:41357/@types/q/-/q-1.5.4.tgz
npm verb cwd /build/source
npm verb Linux 5.4.0-52-generic
npm verb argv "/nix/store/ngga5g4siw4q9i0bqjxckg8aaq35rzgs-nodejs-15.0.1/bin/node" "/nix/store/ngga5g4siw4q9i0bqjxckg8aaq35rzgs-nodejs-15.0.1/bin/npm" "install" "--loglevel" "verbose"
npm verb node v15.0.1
npm verb npm  v7.0.3
npm ERR! code E404
npm ERR! 404 Not Found - GET http://localhost:41357/@types/q/-/q-1.5.4.tgz
npm ERR! 404 
npm ERR! 404  '@types/q@http://localhost:41357/@types/q/-/q-1.5.4.tgz' is not in the npm registry.
npm ERR! 404 You should bug the author to publish it (or use the name yourself!)
npm ERR! 404 
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

With this patch, I get a bit further, having issues with node-gyp.

Edit: With the following added, it finally builds successfully.

    npmCommands = [
      # Just download and unpack all the npm packages,
      # we need napalm to patch shebangs before we can run install scripts.
      "npm install --loglevel verbose --ignore-scripts"
    ];

    postBuild = ''
      # Patch shebangs so that scripts can run.
      for f in node_modules/.bin/node-gyp-build node_modules/.bin/parcel; do
          patchShebangs "$(readlink -f "$f")"
      done

      # Let’s install again, this time running scripts.
      npm install --loglevel verbose
    '';

npm supports scoped packages, whose name matches the following scheme:

    @somescope/somepackagename

These are used all over the modern NPM ecosystem,
most notably by the Babel transpiler.

Unfortunately, Servant is not expressive enough so the code is a bit messy.
Also, the `GET /:scope/:name` route gets shadowed by `GET /:name/:version`.
It is still an improvement, since running `npm install` in a project
with a lock file only seems to care about the tarball route, which works both
scoped and unscoped.

https://docs.npmjs.com/cli/v6/using-npm/scope
@nmattia
Copy link
Collaborator

nmattia commented Nov 3, 2020

I see, thanks a lot for this!

@nmattia nmattia merged commit 684378a into nix-community:master Nov 3, 2020
@jtojnar jtojnar deleted the support-scopes branch November 3, 2020 13:46
@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 3, 2020

It is still curious why would it work for you. Do you have some idea?

@nmattia
Copy link
Collaborator

nmattia commented Nov 3, 2020

Really unclear, maybe I dreamt it. @peterpeterparker we do use scoped packages in the DDG starter correct?

@peterpeterparker
Copy link

@nmattia indeed all our components are published as org scoped packages

npm i @deckdeckgo/core 
npm i @deckdeckgo/deck-utils
npm i @deckdeckgo/slide-title
etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants