From 5de1910620a9214b90eaac6a70f7b0ceda48aa32 Mon Sep 17 00:00:00 2001 From: Peter van Zetten Date: Tue, 31 Oct 2017 15:28:28 +0000 Subject: [PATCH 1/6] Add tests for polymer-cli --- Makefile | 5 ++++- tests/polymer-cli/BUILD | 9 +++++++++ tests/polymer-cli/README.md | 17 +++++++++++++++++ tests/polymer-cli/WORKSPACE | 17 +++++++++++++++++ tests/polymer-cli/polymer-cli_bin_test.sh | 7 +++++++ 5 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 tests/polymer-cli/BUILD create mode 100644 tests/polymer-cli/README.md create mode 100644 tests/polymer-cli/WORKSPACE create mode 100755 tests/polymer-cli/polymer-cli_bin_test.sh diff --git a/Makefile b/Makefile index 6dd9581..f3c4341 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,10 @@ test_typescript: test_webpack: (cd tests/webpack && bazel test //...) +test_polymer-cli: + (cd tests/polymer-cli && bazel test //...) + test_mocha: (cd tests/mocha && bazel test //...) -test_all: test_helloworld test_lyrics test_express test_namespace test_typescript test_webpack test_mocha +test_all: test_helloworld test_lyrics test_express test_namespace test_typescript test_webpack test_polymer-cli test_mocha diff --git a/tests/polymer-cli/BUILD b/tests/polymer-cli/BUILD new file mode 100644 index 0000000..7a61a36 --- /dev/null +++ b/tests/polymer-cli/BUILD @@ -0,0 +1,9 @@ +package(default_visibility = ["//visibility:public"]) + +sh_test( + name = "polymer-cli_bin_test", + size = "small", + srcs = ["polymer-cli_bin_test.sh"], + data = ["@yarn_modules//:polymer-cli_polymer_bin"], +) + diff --git a/tests/polymer-cli/README.md b/tests/polymer-cli/README.md new file mode 100644 index 0000000..98929b5 --- /dev/null +++ b/tests/polymer-cli/README.md @@ -0,0 +1,17 @@ +# Polymer CLI Example + +This folder demonstrates that the generated `BUILD` file for +`@yarn_modules` contains a `node_binary` rule for the polymer-cli module +executable. It tests that the target is callable and that the help +message is output. + +Polymer CLI is a fairly complex dependency that contains @-scoped dependencies +and complex cyclic dependencies. + +```sh +# Should be able to run polymer-cli directly +$ 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 +``` diff --git a/tests/polymer-cli/WORKSPACE b/tests/polymer-cli/WORKSPACE new file mode 100644 index 0000000..59cc69e --- /dev/null +++ b/tests/polymer-cli/WORKSPACE @@ -0,0 +1,17 @@ +workspace(name = "org_pubref_rules_node_polymer_test") + +local_repository( + name = "org_pubref_rules_node", + path = "../..", +) + +load("@org_pubref_rules_node//node:rules.bzl", "node_repositories", "yarn_modules") + +node_repositories() + +yarn_modules( + name = "yarn_modules", + deps = { + "polymer-cli": "1.5.7", + }, +) diff --git a/tests/polymer-cli/polymer-cli_bin_test.sh b/tests/polymer-cli/polymer-cli_bin_test.sh new file mode 100755 index 0000000..5743aa8 --- /dev/null +++ b/tests/polymer-cli/polymer-cli_bin_test.sh @@ -0,0 +1,7 @@ +set -e + +if (./external/yarn_modules/polymer-cli_polymer_bin --help &) | grep -qF '/__\/ /__\/ \/__\ The multi-tool for Polymer projects'; then + echo "PASS" +else + exit 1 +fi From d6e42d50faf78284c87e8eb7b638b75330b27566 Mon Sep 17 00:00:00 2001 From: Peter van Zetten Date: Tue, 31 Oct 2017 15:46:51 +0000 Subject: [PATCH 2/6] Omit filenames with spaces from node_modules --- node/internal/parse_yarn_lock.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/node/internal/parse_yarn_lock.js b/node/internal/parse_yarn_lock.js index f2b82ec..8092037 100644 --- a/node/internal/parse_yarn_lock.js +++ b/node/internal/parse_yarn_lock.js @@ -288,7 +288,11 @@ function printNodeModule(module) { print(` version = "${module.version}",`); print(` package_json = "node_modules/${module.name}/package.json",`); - print(` srcs = glob(["node_modules/${module.name}/**/*"], exclude = ["node_modules/${module.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", + "**/* *", + ]),`); if (url) { print(` url = "${url}",`); } From e7ba80eab96eea2a1f0651153aa3eae455d7eef8 Mon Sep 17 00:00:00 2001 From: Peter van Zetten Date: Tue, 31 Oct 2017 16:04:32 +0000 Subject: [PATCH 3/6] Collect @foo/bar modules from node_modules --- node/internal/parse_yarn_lock.js | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/node/internal/parse_yarn_lock.js b/node/internal/parse_yarn_lock.js index 8092037..f46126f 100644 --- a/node/internal/parse_yarn_lock.js +++ b/node/internal/parse_yarn_lock.js @@ -26,19 +26,19 @@ function main() { // const entries = Object.keys(yarn.object).map(key => makeYarnEntry(key, yarn.object[key])); - // For all top-level folders in the node_modules directory that - // contain a package.json file... - const getModulesIn = p => fs.readdirSync(p) - .filter(f => - fs.statSync(path.join(p, f)).isDirectory() && - fs.existsSync(path.join(p, f, 'package.json')) && - fs.statSync(path.join(p, f, 'package.json')).isFile()); - - // ... parse ithem - const modules = getModulesIn('node_modules').map(dir => parseNodeModulePackageJson(dir)); - - // Iterate all the modules and merge the information from yarn into - // the module + // Scan the node_modules directory and find all top-level ('foo') or scoped (@bar/baz) + // modules, i.e. folders which contain a package.json file... + const getModulesIn = p => fs.readdirSync(p).filter(f => isPackage(p, undefined, f)); + const findScopes = p => fs.readdirSync(p).filter(f => f.startsWith("@") && fs.statSync(path.join(p, f)).isDirectory()); + const getModulesInScope = (p, s) => fs.readdirSync(path.join(p, s)).filter(f => isPackage(p, s, f)); + + // ...then parse the package.json files to collect the metadata + let topLevelModuleDirs = getModulesIn('node_modules'); + let scopeModuleDirs = findScopes('node_modules').map(scope => getModulesInScope('node_modules', scope).map(m => scope+'/'+m)).reduce((a, b) => a.concat(b), []); + let moduleDirs = topLevelModuleDirs.concat(scopeModuleDirs); + 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)); // Didn't realize that the nodejs module ecosystem can contain @@ -68,6 +68,12 @@ function main() { print("# EOF"); } +function isPackage(p, s, f) { + let dir = s ? path.join(p, s, f) : path.join(p, f); + return fs.statSync(dir).isDirectory() && + fs.existsSync(path.join(dir, 'package.json')) && + fs.statSync(path.join(dir, 'package.json')).isFile() +} /** * Given a list of yarn entries and a target module, find an exact From e51015e8347cf47db96f2a168d4afed70db83324 Mon Sep 17 00:00:00 2001 From: Peter van Zetten Date: Tue, 31 Oct 2017 15:08:30 +0000 Subject: [PATCH 4/6] Set node_modules dir correctly for @foo/bar modules --- .gitignore | 1 + node/internal/parse_yarn_lock.js | 11 ++++++----- 2 files changed, 7 insertions(+), 5 deletions(-) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..790296b --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +/tests/*/bazel-* diff --git a/node/internal/parse_yarn_lock.js b/node/internal/parse_yarn_lock.js index f46126f..caa4fe1 100644 --- a/node/internal/parse_yarn_lock.js +++ b/node/internal/parse_yarn_lock.js @@ -284,14 +284,15 @@ function printNodeModule(module) { print(``); printJson(module); print(`node_module(`); - print(` name = "${module.name}",`); + print(` name = "${module.yarn ? module.yarn.label : module.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",`); // Exclude filenames with spaces: Bazel can't cope with them (we just have to hope they aren't needed later...) @@ -313,12 +314,12 @@ function printNodeModule(module) { } print(` },`); } - } + if (deps && deps.size) { print(` deps = [`); deps.forEach(dep => { - print(` ":${dep.name}",`); + print(` ":${dep.yarn ? dep.yarn.label : dep.name}",`); }); print(` ],`); } @@ -337,7 +338,7 @@ function printNodeModuleAll(modules) { print(` name = "_all_",`); print(` deps = [`); modules.forEach(module => { - print(` ":${module.name}",`); + print(` ":${module.yarn ? module.yarn.label : module.name}",`); }); print(` ],`); print(`)`); From 2c7dc599519c47b31587cb359dda4c5a7e54c4b7 Mon Sep 17 00:00:00 2001 From: Peter van Zetten Date: Tue, 31 Oct 2017 16:07:21 +0000 Subject: [PATCH 5/6] Remove only circular dependencies from a cycle, not all dependencies --- node/internal/parse_yarn_lock.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/node/internal/parse_yarn_lock.js b/node/internal/parse_yarn_lock.js index caa4fe1..a99b903 100644 --- a/node/internal/parse_yarn_lock.js +++ b/node/internal/parse_yarn_lock.js @@ -208,9 +208,10 @@ function breakCircularDependencies(modules) { } }); - // Each entry in the cluster must have no other outgoing - // dependencies - entry.deps = new Set(); + // Each entry in the cluster must have no connections to other + // dependencies in the cluster, or on the cluster pseudo-dep + pseudo.deps.forEach(circ => entry.deps.delete(circ)); + entry.deps.delete(pseudo); }); // Store this new pseudo-module in the modules list From a57ab239015b53cdf369f3cd74b8746154d6bfd7 Mon Sep 17 00:00:00 2001 From: Peter van Zetten Date: Wed, 1 Nov 2017 11:29:12 +0000 Subject: [PATCH 6/6] Add named-workspace-relative runfiles lookup to node_binary --- node/internal/node_binary.bzl | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/node/internal/node_binary.bzl b/node/internal/node_binary.bzl index 391bacf..17feee9 100644 --- a/node/internal/node_binary.bzl +++ b/node/internal/node_binary.bzl @@ -28,7 +28,7 @@ def create_launcher(ctx, output_dir, node, manifest): # The package path is the path package_path = [] if ctx.label.workspace_root: - package_path.append(ctx.label.workspace_root) + package_path.append(ctx.label.workspace_root.replace("external/", "", 1)) if ctx.label.package: package_path.append(ctx.label.package) @@ -52,25 +52,31 @@ def create_launcher(ctx, output_dir, node, manifest): # Namespace where node and node_modules assets have been # built. 'TARGET_NAME="%s"' % ctx.attr.target, + # Workspace name where this rule is defined + 'WORKSPACE_NAME="%s"' % ctx.workspace_name, # Based on the way the script has been invoked, $PACKAGE_PATH # can exist in various locations. We need to discover this # and assign the value to $TARGET_PATH. 'TARGET_PATH=""' , 'ENTRYPOINT="%s"' % entrypoint, - + 'if [[ -e "${0}_files/${NODE_EXE}" ]]; then', ' TARGET_PATH="${0}_files/"', #' echo "Matched [standalone script] or [bazel test] context [${TARGET_PATH}]"', '', - 'elif [[ -e "${0}_files/${NODE_EXE}" ]]; then', - ' TARGET_PATH="${0}_files/"', - #' echo "Matched [standalone script] or [bazel test] context [${TARGET_PATH}]"', - '', 'elif [[ -e "${0}.runfiles/__main__/${PACKAGE_PATH}/${TARGET_NAME}/${NODE_EXE}" ]]; then', - ' TARGET_PATH="${0}.runfiles/__main__/${PACKAGE_PATH}/${TARGET_NAME}/"', + ' TARGET_PATH="${0}.runfiles/__main__/${PACKAGE_PATH}/${TARGET_NAME}/"', #' echo "Matched [bazel run] context [${TARGET_PATH}]"', '', + 'elif [[ -e "${0}.runfiles/${WORKSPACE_NAME}/${PACKAGE_PATH}/${TARGET_NAME}/${NODE_EXE}" ]]; then', + ' TARGET_PATH="${0}.runfiles/${WORKSPACE_NAME}/${PACKAGE_PATH}/${TARGET_NAME}/"', + #' echo "Matched [bazel build] or [bazel run] context in a named workspace [${TARGET_PATH}]"', + '', + 'elif [[ -e "${0}.runfiles/${PACKAGE_PATH}/${TARGET_NAME}/${NODE_EXE}" ]]; then', + ' TARGET_PATH="${0}.runfiles/${PACKAGE_PATH}/${TARGET_NAME}/"', + #' echo "Matched [bazel build] context [${TARGET_PATH}]"', + '', 'elif [[ -e "${PACKAGE_PATH}/${TARGET_NAME}/${NODE_EXE}" ]]; then', ' TARGET_PATH="${PACKAGE_PATH}/${TARGET_NAME}/"', #' echo "Matched [bazel test TESTRULE] context [${TARGET_PATH}]"',