Skip to content
This repository has been archived by the owner on May 20, 2021. It is now read-only.

Add support for local and GitHub packages #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chrismwendt
Copy link

I needed support local and GitHub packages, which look like this in package.json:

  "foo": "./lib/foo",
  "bar-repo": "github:user/bar-repo"

This was hacked together pretty quickly as a proof of concept. Ultimately I can't use yarn2nix because some of my dependencies attempt to install packages at runtime (e.g. protobuf via the pbjs command) and that fails because node_modules is symlinked into /nix/store somewhere, which is read-only.

I figured I would put up this PR in case someone wants this functionality and is willing to fix it up a bit.

let url;
let sha1;
let file_name;
if (/codeload.github.com.*tar.gz\//.test(dep["resolved"])) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is brittle because not all GitHub URLs look like this.

let file_name;
if (/codeload.github.com.*tar.gz\//.test(dep["resolved"])) {
url = dep["resolved"];
sha1 = cp.execSync("curl -sS " + url + " | shasum | cut -d \" \" -f 1").toString().trim();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably be done in Node.js without shelling out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the whole point of yarn2nix is that it doesn't need network access to build the nix file. Because it's pure, it can be loaded recursively.

I found that patching the yarn.lock to add the #sha1 to the github urls works fine (but is an operation that has to be done by hand)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command converts a git commit into a tarball SHA, which is pure barring the same URL responding with different tarballs at different times. Do you foresee any issues with this?

Maybe we could automate the process of appending #sha1 and call the script hashify-urls.js or something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

barring the same URL responding with different tarballs at different times

I think this does happen — this is part of why nix and nixpkgs have fetchTarball which unpacks it after fetching, so that different but equivalent tarballs have the same nar hash.

@@ -68,6 +70,13 @@ pkgs.lib.fix (self: rec {
chmod +w ./yarn.lock

yarn config --offline set yarn-offline-mirror ${offlineCache}
yarn config --offline set yarn-offline-mirror-pruning false
yarn config set yarn-offline-mirror-pruning false
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that yarn was attempting to unlink some packages in the offline mirror and I thought it was because it was trying to prune unused packages. However, this config setting didn't resolve the problem, so maybe there's another, yet-to-be understood reason.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's the yarn 1.2.0 update. Do you know what version was used?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was using yarn 1.2.1. Do you see anything fishy in https://github.com/yarnpkg/yarn/releases ?

yarn config set yarn-offline-mirror-pruning false

ls -l
/usr/local/bin/tree -l ${offlineCache}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were for debugging purposes.

@zimbatm
Copy link
Member

zimbatm commented Oct 14, 2017

wouldn't it be possible to force the protobuf package to do the compilation at install time?

@chrismwendt
Copy link
Author

protobufjs checks for and installs additional packages when you run the CLI (command-line) program, not during installation of protobufjs itself. Here's where it's installing the packages:

https://github.com/dcodeIO/protobuf.js/blob/2ebb1b781812e77de914cd260e7ab69612ffd99e/cli/util.js#L128-L157

Here's where the CLI dependencies are defined:

https://github.com/dcodeIO/protobuf.js/blob/952c7d1b478cc7c6de82475a17a1387992e8651f/package.json#L98-L109

protobufjs/protobuf.js#648 seems to be where all this started. The goal was presumably to avoid burdening users of the library with additional CLI dependencies. The chosen solution was to install them upon first use of the CLI program. That seems pretty hacky compared to the standard approach of creating another package protobufjs-cli (cf. babel-core and babel-cli).

Here's a thread talking about how to address this problem: protobufjs/protobuf.js#716

@zimbatm zimbatm mentioned this pull request Oct 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants