Update node version (#79)

- Fix dependency referrer null pointer
- s/module/mod/g for variable names
- include yallist in webpack example (needed to avoid this bug)
- update .travis.yml to current version 0.24.0
- Upgrade node to 8.15.1
- Upgrade yarn to 1.15.2
- Fix a bug in generated node_binary rules (for labels with special characters)
- Remove polymer-cli example from CI unit tests (not working ATM)

Author: pcj

.travis.yml

@@ -1,35 +1,31 @@
+# trusty beta image has jdk8, gcc4.8.4
 dist: trusty
 sudo: required
+# xcode8 has jdk8
 osx_image: xcode8
+# Not technically required but suppresses 'Ruby' in Job status message.
 language: java
 
 os:
   - linux
   - osx
 
 env:
-  - V=0.20.0
-  - V=0.21.0
-  - V=0.22.0
-  - V=0.23.2
-  - V=0.24.0
+  - BAZEL=0.20.0
+  - BAZEL=0.24.0
 
 before_install:
-  - OS=linux
-  - ARCH=x86_64
-  - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then OS=darwin; fi
-  - GH_BASE="https://github.com/bazelbuild/bazel/releases/download/$V"
-  - GH_ARTIFACT="bazel-$V-installer-$OS-$ARCH.sh"
-  - CI_BASE="http://ci.bazel.io/job/Bazel/JAVA_VERSION=1.8,PLATFORM_NAME=$OS-$ARCH/lastSuccessfulBuild/artifact/output/ci"
-  - CI_ARTIFACT="bazel--installer.sh"
-  - URL="$GH_BASE/$GH_ARTIFACT"
-  - if [[ "$V" == "HEAD" ]]; then CI_ARTIFACT="`wget -qO- $CI_BASE | grep -o 'bazel-[-_a-zA-Z0-9\.]*-installer.sh' | uniq`"; fi
-  - if [[ "$V" == "HEAD" ]]; then URL="$CI_BASE/$CI_ARTIFACT"; fi
-  - echo $URL
-  - wget -O install.sh $URL
-  - chmod +x install.sh
-  - ./install.sh --user
-  - rm -f install.sh
+  - |
+    if [ "${TRAVIS_OS_NAME}" = "osx" ]; then
+      OS=darwin
+    else
+      sysctl kernel.unprivileged_userns_clone=1
+      OS=linux
+    fi
+    wget -O install.sh "https://github.com/bazelbuild/bazel/releases/download/${BAZEL}/bazel-${BAZEL}-installer-${OS}-x86_64.sh"
+    chmod +x install.sh
+    ./install.sh --user
+    rm -f install.sh
 
 script:
   - make test_all

Makefile

@@ -25,4 +25,5 @@ test_polymer-cli:
 test_mocha:
 	(cd tests/mocha && bazel test //...)
 
-test_all: test_helloworld test_lyrics test_express test_namespace test_typescript test_webpack test_polymer-cli test_mocha test_rollup
+# polymer cli not included as it appears to still be a WIP
+test_all: test_helloworld test_lyrics test_express test_namespace test_typescript test_webpack test_mocha test_rollup

README.md

@@ -30,7 +30,7 @@
 ## node_repositories
 
 WORKSPACE rule that downloads and configures node based on your
-operating system.  Includes `node` (7.10.1) and `yarn` (1.0.1).
+operating system.  Includes `node` (8.15.1) and `yarn` (1.0.1).
 
 ```python
 RULES_NODE_COMMIT = '...' # Update to current HEAD

node/internal/node_repositories.bzl

@@ -88,23 +88,23 @@ _node_repository = repository_rule(
   _node_repository_impl,
   attrs = {
     "node_version": attr.string(
-      default = "7.10.1",
+      default = "8.15.1",
     ),
     "linux_sha256": attr.string(
-      default = "7b0e9d1af945671a0365a64ee58a2b0d72b3632a1cebe6b5bd75094b93627bf3",
+      default = "5643b54c583eebaa40c1623b16cba4e3955ff5dfdd44036f6bafd761160c993d",
     ),
     "darwin_sha256": attr.string(
-      default = "d67d2eb9456aab925416ad58aa18b9680e66a4bcc243a89b22e646f7fffc4ff9",
+      default = "aacdc9d5d8bbeaf47c398815147e052aac53cf19319f4c140c1798a82d419e65",
     ),
     "windows_sha256": attr.string(
-      default = "617590f06f9a0266ceecb3fd17120fc2fbf8669980974f339a83f3b56ed05f7b",
+      default = "f636fa578dc079bacc6c4bef13284ddb893c99f7640b96701c2690bd9c1431f5",
     ),
   },
 )
 
 
-def node_repositories(yarn_version="v1.0.1",
-                      yarn_sha256="6b00b5e0a7074a512d39d2d91ba6262dde911d452617939ca4be4a700dd77cf1",
+def node_repositories(yarn_version="v1.15.2",
+                      yarn_sha256="c4feca9ba5d6bf1e820e8828609d3de733edf0e4722d17ed7ce493ed39f61abd",
                       **kwargs):
 
     http_archive(
@@ -117,13 +117,13 @@ def node_repositories(yarn_version="v1.0.1",
       build_file_content = YARN_BUILD_FILE_CONTENT,
     )
 
-    http_archive(
-      name = "yarnpkg_lockfile",
-      url = "https://registry.yarnpkg.com/@yarnpkg/lockfile/-/lockfile-1.0.0.tgz",
-      sha256 = "472add7ad141c75811f93dca421e2b7456045504afacec814b0565f092156250",
-      strip_prefix="package",
-      build_file_content =  YARN_LOCKFILE_BUILD_FILE_CONTENT,
-    )
+    # http_archive(
+    #   name = "yarnpkg_lockfile",
+    #   url = "https://registry.yarnpkg.com/@yarnpkg/lockfile/-/lockfile-1.1.1.tgz",
+    #   sha256 = "472add7ad141c75811f93dca421e2b7456045504afacec814b0565f092156251",
+    #   strip_prefix="package",
+    #   build_file_content =  YARN_LOCKFILE_BUILD_FILE_CONTENT,
+    # )
 
     _node_repository(
       name = "node",

node/internal/parse_yarn_lock.js

@@ -39,7 +39,7 @@ function main() {
   const modules = moduleDirs.map(dir => parseNodeModulePackageJson(dir));
 
   // Iterate all the modules and merge the information from yarn into the module
-  modules.forEach(module => mergePackageJsonWithYarnEntry(entries, module));
+  modules.forEach(mod => mergePackageJsonWithYarnEntry(entries, mod));
 
   // Didn't realize that the nodejs module ecosystem can contain
   // circular references, but apparently it can.
@@ -51,15 +51,15 @@ function main() {
   print("package(default_visibility = ['//visibility:public'])");
   print("load('@org_pubref_rules_node//node:rules.bzl', 'node_module', 'node_binary')");
 
-  modules.forEach(module => printNodeModule(module));
+  modules.forEach(mod => printNodeModule(mod));
 
   printNodeModuleAll(modules);
 
   // Create an executable rule all executable entryies in the modules
-  modules.forEach(module => {
-    if (module.executables) {
-      for (const [name, path] of module.executables.entries()) {
-        printNodeBinary(module, name, path);
+  modules.forEach(mod => {
+    if (mod.executables) {
+      for (const [name, path] of mod.executables.entries()) {
+        printNodeBinary(mod, name, path);
       }
     }
   });
@@ -94,17 +94,17 @@ function findMatchingYarnEntryByNameAndVersion(entries, module) {
  * Actually, this is pretty simple as the yarn entry is simply
  * attached to the module.
  */
-function mergePackageJsonWithYarnEntry(entries, module) {
-  const entry = findMatchingYarnEntryByNameAndVersion(entries, module);
+function mergePackageJsonWithYarnEntry(entries, mod) {
+  const entry = findMatchingYarnEntryByNameAndVersion(entries, mod);
   if (!entry) {
-    throw new Error("No matching node_module found for " + module.name);
+    throw new Error("No matching node_module found for this module", mod);
   }
 
   // Use the bazelified name as the module name
-  module.original_name = module.name
-  module.name = entry.name
+  mod.original_name = mod.name
+  mod.name = entry.name
   // Store everything else here
-  module.yarn = entry;
+  mod.yarn = entry;
 }
 
 /**
@@ -117,30 +117,31 @@ function mergePackageJsonWithYarnEntry(entries, module) {
 function breakCircularDependencies(modules) {
 
   const byName = new Map();
-  modules.forEach(module => byName.set(module.name, module));
+  modules.forEach(mod => byName.set(mod.name, mod));
 
   // Make a list of nodes 
   const nodes = Array.from(byName.keys());
   // An Array<Array<number>> array for the edges
   const edges = [];
-  // And a mapping for backreferences mapped by name
-  const backrefs = new Map();
 
   // Build the adjacencyList
   nodes.forEach((node, index) => {
     const list = [];
     edges[index] = list;
-    const entry = byName.get(node);
+    const entry = byName.get(node); // get the module by name
     // Make a set of deps rather than using the entry.dependencies
     // mapping.
     entry.deps = new Set();
 
     if (entry.dependencies) {
-      
+
       Object.keys(entry.dependencies).forEach(name => {
 
         // Save this in the deps set
         const dependency = byName.get(name);
+        if (!dependency) {
+          throw new ReferenceError(`# For module ${entry.name} ${entry.version}: for some reason the module for transitive dependency ${name} was not included.  Please add it manually to your list of top-level dependencies (rule yarn_modules.deps attribute) .`);
+        }
         entry.deps.add(dependency);
 
         // Populate the adjacency list
@@ -246,8 +247,7 @@ function parseName(key, entry) {
   const at = key.indexOf("@", 1);
   entry.name = key.slice(0, at);
 
-  const label = entry.name.replace('@', 'at-');
-  entry.label = label;
+  entry.label = entry.name.replace('@', 'at-');
 }
 
 
@@ -283,26 +283,26 @@ function printJson(entry) {
 /**
  * Given a module, print a skylark `node_module` rule.
  */
-function printNodeModule(module) {
-  const deps = module.deps;
+function printNodeModule(mod) {
+  const deps = mod.deps;
 
   print(``);
-  printJson(module);
+  printJson(mod);
   print(`node_module(`);
-  print(`    name = "${module.yarn ? module.yarn.label : module.name}",`);
+  print(`    name = "${mod.yarn ? mod.yarn.label : mod.name}",`);
 
   // SCC pseudomodule wont have 'yarn' property
-  if (module.yarn) {
-    const url = module.yarn.url || module.url;
-    const sha1 = module.yarn.sha1;
-    const executables = module.executables;
-
-    print(`    module_name = "${module.name}",`);
-    print(`    version = "${module.version}",`);
-    print(`    package_json = "node_modules/${module.name}/package.json",`);
+  if (mod.yarn) {
+    const url = mod.yarn.url || mod.url;
+    const sha1 = mod.yarn.sha1;
+    const executables = mod.executables;
+
+    print(`    module_name = "${mod.name}",`);
+    print(`    version = "${mod.version}",`);
+    print(`    package_json = "node_modules/${mod.name}/package.json",`);
     // Exclude filenames with spaces: Bazel can't cope with them (we just have to hope they aren't needed later...)
-    print(`    srcs = glob(["node_modules/${module.name}/**/*"], exclude = [
-		"node_modules/${module.name}/package.json",
+    print(`    srcs = glob(["node_modules/${mod.name}/**/*"], exclude = [
+		"node_modules/${mod.name}/package.json",
 		"**/* *",
 	]),`);
     if (url) {
@@ -355,7 +355,9 @@ function printNodeModuleAll(modules) {
  * property, print a skylark `node_binary` rule.
  */
 function printNodeBinary(module, key, path) {
-  const name = module.name === key ? key : `${module.name}_${key}`;
+  let name = module.name === key ? key : `${module.name}_${key}`;
+  // escape special characters like '@' and '/'
+  name = name.replace('@', '').replace('/', '_')
   print(``);
   print(`node_binary(`);
   print(`    name = "${name}_bin",`);
@@ -370,23 +372,23 @@ function printNodeBinary(module, key, path) {
  * package json and return it as an object.
  */
 function parseNodeModulePackageJson(name) {
-  const module = require(`../node_modules/${name}/package.json`);
+  const mod = require(`../node_modules/${name}/package.json`);
 
   // Take this opportunity to cleanup the module.bin entries
   // into a new Map called 'executables'
-  const executables = module.executables = new Map();
-  
-  if (Array.isArray(module.bin)) {
+  const executables = mod.executables = new Map();
+
+  if (Array.isArray(mod.bin)) {
     // should not happen, but ignore it if present
-  } else if (typeof module.bin === 'string') {
-    executables.set(name, stripBinPrefix(module.bin));
-  } else if (typeof module.bin === 'object') {
-    for (let key in module.bin) {
-      executables.set(key, stripBinPrefix(module.bin[key]));
+  } else if (typeof mod.bin === 'string') {
+    executables.set(name, stripBinPrefix(mod.bin));
+  } else if (typeof mod.bin === 'object') {
+    for (let key in mod.bin) {
+      executables.set(key, stripBinPrefix(mod.bin[key]));
     }
   }
 
-  return module;  
+  return mod;  
 }
 
 /**

tests/helloworld/gen/BUILD

@@ -31,6 +31,7 @@ genrule(
 # If the test passes, we probably emitted hello world
 node_test(
     name = "helloworld_test",
+    size = "small",
     main = "helloworld.js",
     visibility = ["//visibility:public"],
 )

tests/polymer-cli/README.md

@@ -15,3 +15,8 @@ $ bazel build @yarn_modules//:polymer-cli_polymer_bin -- --help
 # Should be able to invoke polymer-cli as standalone script 
 $ ./bazel-bin/external/yarn_modules/polymer-cli_polymer_bin --help
 ```
+
+> NOTE: currently this example is not working.  When invoked,
+> polymer-cli_polymer_bin fails due to a missing module dependency `cycle`.  It
+> is currently unclear why 'cycle' is being included but not linked in the
+> dependency tree.

tests/typescript/ts_module.bzl

@@ -134,7 +134,7 @@ _ts_module = rule(
     implementation = _ts_module_impl,
     attrs = {
         "srcs": attr.label_list(
-            allow_files = FileType([".ts", ".tsx"]),
+            allow_files = [".ts", ".tsx"],
             mandatory = True,
         ),
         "deps": attr.label_list(

tests/webpack/BUILD

@@ -37,4 +37,3 @@ sh_test(
                                  # file output implicitly invokes
                                  # webback to generate it
 )
-

tests/webpack/README.md

@@ -12,7 +12,7 @@ breaks this strongly connected component into a separate pseudo
 
 ```sh
 # Should be able to run webpack directly
-$ bazel build @yarn_modules//:webback_bin -- --help
+$ bazel run @yarn_modules//:webpack_bin -- --help
 
 # Should be able to invoke webpack as standalone script 
 $ ./bazel-bin/external/yarn_modules/webpack_bin --help
@@ -22,4 +22,4 @@ $ bazel build :webpack_compile
 
 # Should be able to invoke another (contrived) genrule
 $ bazel build :compile
-```
+```