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

yarn2nix error handling improvements #39992

Closed
wants to merge 4 commits into from

Conversation

nicknovitski
Copy link
Contributor

Motivation for this change

package.json files can specify local paths as dependencies, and yarn creates corresponding entries in the yarn.lock file for them. Currently yarn2nix errors when processing those entries, but still exits with a zero status. This means that yarn2nix.mkYarnPackage writes an incomplete and unparseable nix file to the store and then tries to read it, with an error like error: syntax error, unexpected $end, at /nix/store/76ml5lgvrc41b5c4y8jsy1iw8z50cw8s-yarn.nix:3081:6.

One way to reproduce:

$ git clone git@github.com:yarnpkg/yarn.git
# ...
$ cd yarn 
$ nix run -f channel:nixos-unstable yarn2nix -c yarn2nix
# ...
    {
      name = "eslint-plugin-relay-0.0.21.tgz";
      path = fetchurl {
        name = "eslint-plugin-relay-0.0.21.tgz";
        url  = "https://registry.yarnpkg.com/eslint-plugin-relay/-/eslint-plugin-relay-0.0.21.tgz";
        sha1 = "fbda01bd783aa7ed9eb0effe410049911e1449ef";
      };
    }
(node:62298) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'split' of undefined
$ echo $?
0

I don't know what the policy around the yarn2nix version number is; sorry if I was wrong to bump it.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@nicknovitski
Copy link
Contributor Author

Actually, I can't even tell: should I be contributing this to the yarn2nix repository? When do changes there get reflected in nixpkgs?

@Mic92
Copy link
Member

Mic92 commented May 6, 2018

cc @manveru

@manveru
Copy link
Contributor

manveru commented May 7, 2018

We don't really have a policy for that yet. But I think it's better to make a PR for both? @zimbatm @moretea

@moretea
Copy link
Contributor

moretea commented May 7, 2018

@NickHu if you could add that those are not supported, I'll merge the PR.

@NickHu
Copy link
Contributor

NickHu commented May 7, 2018

@nicknovitski guessing @moretea meant to tag you

@nicknovitski
Copy link
Contributor Author

Okay, I'm giving it a shot, borrowing liberally from nix-community/yarn2nix#54.

@nicknovitski
Copy link
Contributor Author

Better to do this on the yarn2nix repo itself.

@nicknovitski nicknovitski deleted the yarn2nix-errors branch July 14, 2018 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants