From fbefffbf8bb031e84be8180c725391039f175c2c Mon Sep 17 00:00:00 2001 From: Paul Cody Johnston Date: Sat, 13 Apr 2019 20:47:25 -0600 Subject: [PATCH] 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) --- .travis.yml | 36 +++++------- Makefile | 3 +- README.md | 2 +- node/internal/node_repositories.bzl | 26 ++++----- node/internal/parse_yarn_lock.js | 90 +++++++++++++++-------------- tests/helloworld/gen/BUILD | 1 + tests/polymer-cli/README.md | 5 ++ tests/typescript/ts_module.bzl | 2 +- tests/webpack/BUILD | 1 - tests/webpack/README.md | 4 +- tests/webpack/WORKSPACE | 1 + 11 files changed, 88 insertions(+), 83 deletions(-) diff --git a/.travis.yml b/.travis.yml index 842ecc9..02b1a28 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,9 @@ +# 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: @@ -8,28 +11,21 @@ os: - 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 diff --git a/Makefile b/Makefile index 37c0277..ff98ff1 100644 --- a/Makefile +++ b/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 \ No newline at end of file diff --git a/README.md b/README.md index 8327655..bcc44b2 100644 --- a/README.md +++ b/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 diff --git a/node/internal/node_repositories.bzl b/node/internal/node_repositories.bzl index b2aa2c4..72dcccf 100644 --- a/node/internal/node_repositories.bzl +++ b/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", diff --git a/node/internal/parse_yarn_lock.js b/node/internal/parse_yarn_lock.js index d4bb33a..4b07104 100644 --- a/node/internal/parse_yarn_lock.js +++ b/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 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; } /** diff --git a/tests/helloworld/gen/BUILD b/tests/helloworld/gen/BUILD index a31bbc2..2cbb7cf 100644 --- a/tests/helloworld/gen/BUILD +++ b/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"], ) diff --git a/tests/polymer-cli/README.md b/tests/polymer-cli/README.md index 98929b5..a1d7a09 100644 --- a/tests/polymer-cli/README.md +++ b/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. \ No newline at end of file diff --git a/tests/typescript/ts_module.bzl b/tests/typescript/ts_module.bzl index 92a118f..2f3bccd 100644 --- a/tests/typescript/ts_module.bzl +++ b/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( diff --git a/tests/webpack/BUILD b/tests/webpack/BUILD index 99fa7c7..5cfa71f 100644 --- a/tests/webpack/BUILD +++ b/tests/webpack/BUILD @@ -37,4 +37,3 @@ sh_test( # file output implicitly invokes # webback to generate it ) - diff --git a/tests/webpack/README.md b/tests/webpack/README.md index 60ebc12..26420ae 100644 --- a/tests/webpack/README.md +++ b/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 -``` +``` \ No newline at end of file diff --git a/tests/webpack/WORKSPACE b/tests/webpack/WORKSPACE index dc4eb9c..b7e5af8 100644 --- a/tests/webpack/WORKSPACE +++ b/tests/webpack/WORKSPACE @@ -11,6 +11,7 @@ yarn_modules( name = "yarn_modules", deps = { "webpack": "3.4.0", + "yallist": "2.1.2", }, resolutions = { "acorn": "5.1.2",