From 1a8175dfa7234cb7e93ebc09138bdbc99034a3c2 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Fri, 14 Jun 2019 18:06:45 -0700 Subject: [PATCH] feat: implicit hide-bazel-files @bazel/hide-bazel-files updated to automatically run its postinstall step and @bazel/bazel now has a dependency on the tool so that users get the fix automatically and when a better fix is provided by Bazel it can be removed seamlessly. --- README.md | 25 -------------------- e2e/symlinked_node_modules_yarn/package.json | 1 - internal/npm_install/generate_build_file.js | 2 +- internal/npm_install/npm_install.bzl | 14 +++-------- package.json | 1 - packages/bazel/package.json | 4 ++++ packages/create/index.js | 2 -- packages/hide-bazel-files/README.md | 5 +--- packages/hide-bazel-files/index.js | 11 ++++++++- packages/hide-bazel-files/package.json | 5 +++- 10 files changed, 23 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index f42824ffad..06497f4809 100644 --- a/README.md +++ b/README.md @@ -271,31 +271,6 @@ As of Bazel 0.26 this feature is still experimental, so also add this line to th common --experimental_allow_incremental_repository_updates ``` -#### @bazel/hide-bazel-files - -We recommend adding the `@bazel/hide-bazel-files` utility as a postinstall step to any `package.json` files that -are being used by `yarn_install` or `npm_install`. This utility hides Bazel files that may be shipped with npm -packages you are using by renaming them with a `_` prefix. - -Bazel files such as `BUILD` or `BUILD.bazel` in node_modules will cause build failures when using Bazel-managed dependencies. If you see an error such as - -``` -ERROR: /private/var/tmp/_bazel_greg/37b273501bbecefcf5ce4f3afcd7c47a/external/npm/BUILD.bazel:9:1: Label '@npm//:node_modules/rxjs/src/AsyncSubject.ts' crosses boundary of subpackage '@npm//node_modules/rxjs/src' (perhaps you meant to put the colon here: '@npm//node_modules/rxjs/src:AsyncSubject.ts'?) -``` - -then chances are there is an npm package in your dependencies that contains a `BUILD` file. To resolve this, add `@bazel/hide-bazel-files` to your `devDependencies` and `hide-bazel-files` to your `postinstall` script like so: - -``` -"devDependencies": { - "@bazel/hide-bazel-files": "0.0.0-PLACEHOLDER" -}, -"scripts": { - "postinstall": "hide-bazel-files" -} -``` - -Note: The commonly used npm package rxjs contains `BUILD` files from version 5.5.5 to 6.4.0 inclusive. These have now been removed in version 6.5.0. If you are using an rxjs version in that range and that is the only npm package in your dependencies that contains `BUILD` files then you can try upgrading to rxjs 6.4.0 instead of using `hide-bazel-files`. - #### yarn_install vs. npm_install `yarn_install` is the preferred rule for setting up Bazel-managed dependencies for a number of reasons: diff --git a/e2e/symlinked_node_modules_yarn/package.json b/e2e/symlinked_node_modules_yarn/package.json index dfe7507599..f22a381145 100644 --- a/e2e/symlinked_node_modules_yarn/package.json +++ b/e2e/symlinked_node_modules_yarn/package.json @@ -7,7 +7,6 @@ "@bazel/hide-bazel-files": "file:../../dist/npm_bazel_hide-bazel-files" }, "scripts": { - "postinstall": "hide-bazel-files", "test": "bazel test ... && rm -rf node_modules && bazel test ... && rm -rf node_modules && yarn install && bazel test ..." } } diff --git a/internal/npm_install/generate_build_file.js b/internal/npm_install/generate_build_file.js index 74498d1efc..14fe2079cd 100644 --- a/internal/npm_install/generate_build_file.js +++ b/internal/npm_install/generate_build_file.js @@ -134,7 +134,7 @@ function hideBazelFiles(pkg) { if (ERROR_ON_BAZEL_FILES) { console.error(`npm package '${pkg._dir}' from @${WORKSPACE} ${RULE_TYPE} rule has a Bazel BUILD file '${file}'. Use the @bazel/hide-bazel-files utility to hide these files. -See https://github.com/gregmagolan/rules_nodejs/blob/master/packages/hide-bazel-files/README.md +See https://github.com/bazelbuild/rules_nodejs/blob/master/packages/hide-bazel-files/README.md for installation instructions.`); process.exit(1); } else { diff --git a/internal/npm_install/npm_install.bzl b/internal/npm_install/npm_install.bzl index d500057214..2bf4e32f33 100644 --- a/internal/npm_install/npm_install.bzl +++ b/internal/npm_install/npm_install.bzl @@ -48,17 +48,9 @@ COMMON_ATTRIBUTES = dict(dict(), **{ See https://github.com/bazelbuild/rules_nodejs/issues/802 for more details. - The recommended solution to the above is to add `@bazel/hide-bazel-files` to your `devDependencies` - and `hide-bazel-files` to your `postinstall` script like so: - - ``` - "devDependencies": { - "@bazel/hide-bazel-files": "latest" - }, - "scripts": { - "postinstall": "hide-bazel-files" - } - ``` + The recommended solution is to use the @bazel/hide-bazel-files utility to hide these files. + See https://github.com/bazelbuild/rules_nodejs/blob/master/packages/hide-bazel-files/README.md + for installation instructions. The alternate solution is to set `always_hide_bazel_files` to True which tell this rule to hide Bazel files even when `symlink_node_modules` is True. This means diff --git a/package.json b/package.json index 8e70cb06b4..faecdb9219 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,6 @@ "zone.js": "0.8.29" }, "scripts": { - "postinstall": "hide-bazel-files", "build_packages_all": "./scripts/build_packages_all.sh", "build_packages": "./scripts/build_packages.sh", "build_release": "./scripts/build_release.sh", diff --git a/packages/bazel/package.json b/packages/bazel/package.json index 3d3efe9d01..03ba69d0f9 100644 --- a/packages/bazel/package.json +++ b/packages/bazel/package.json @@ -15,6 +15,10 @@ "bugs": { "url": "https://github.com/bazelbuild/bazel/issues" }, + "//": "@bazel/hide-bazel-files is required to fix https://github.com/bazelbuild/rules_nodejs/issues/802. See https://github.com/bazelbuild/rules_nodejs/blob/master/packages/hide-bazel-files/README.md for more information.", + "dependencies": { + "@bazel/hide-bazel-files": "latest" + }, "optionalDependencies": { "@bazel/bazel-linux_x64": "0.26.0-rc10", "@bazel/bazel-darwin_x64": "0.26.0-rc10", diff --git a/packages/create/index.js b/packages/create/index.js index eba9dd12ae..350f8a414b 100644 --- a/packages/create/index.js +++ b/packages/create/index.js @@ -99,7 +99,6 @@ function main(argv, error = console.error, log = console.log) { '@bazel/bazel': 'latest', '@bazel/ibazel': 'latest', '@bazel/buildifier': 'latest', - '@bazel/hide-bazel-files': 'latest', }; let rootBuildContent = '# Add rules here to build your software\n' + '# See https://docs.bazel.build/versions/master/build-ref.html#BUILD_files\n\n'; @@ -183,7 +182,6 @@ ts_setup_workspace()`; private: true, devDependencies, scripts: { - 'postinstall': 'hide-bazel-files', 'build': 'bazel build //...', 'test': 'bazel test //...', } diff --git a/packages/hide-bazel-files/README.md b/packages/hide-bazel-files/README.md index b6c6973f29..48bffcb612 100644 --- a/packages/hide-bazel-files/README.md +++ b/packages/hide-bazel-files/README.md @@ -10,15 +10,12 @@ If you see an error such as ERROR: /private/var/tmp/_bazel_greg/37b273501bbecefcf5ce4f3afcd7c47a/external/npm/BUILD.bazel:9:1: Label '@npm//:node_modules/rxjs/src/AsyncSubject.ts' crosses boundary of subpackage '@npm//node_modules/rxjs/src' (perhaps you meant to put the colon here: '@npm//node_modules/rxjs/src:AsyncSubject.ts'?) ``` -then chances are there is an npm package in your dependencies that contains a `BUILD` file. To resolve this, add `@bazel/hide-bazel-files` to your `devDependencies` and `hide-bazel-files` to your `postinstall` script like so: +then chances are there is an npm package in your dependencies that contains a `BUILD` file. To resolve this, add `@bazel/hide-bazel-files` to your `devDependencies`. The `@bazel/hide-bazel-files` npm package automatically runs a postinstall step that renames all Bazel build files in your node_modules. ``` "devDependencies": { "@bazel/hide-bazel-files": "0.0.0-PLACEHOLDER" }, -"scripts": { - "postinstall": "hide-bazel-files" -} ``` Note: The commonly used npm package rxjs contains `BUILD` files from version 5.5.5 to 6.4.0 inclusive. These have now been removed in version 6.5.0. If you are using an rxjs version in that range and that is the only npm package in your dependencies that contains `BUILD` files then you can try upgrading to rxjs 6.4.0 instead of using `hide-bazel-files`. diff --git a/packages/hide-bazel-files/index.js b/packages/hide-bazel-files/index.js index a9694e85ef..39afb09a86 100755 --- a/packages/hide-bazel-files/index.js +++ b/packages/hide-bazel-files/index.js @@ -4,6 +4,10 @@ const fs = require('fs'); const path = require('path'); function findBazelFiles(dir) { + if (!fs.existsSync(dir)) { + // Fail-safe + return []; + } return fs.readdirSync(dir).reduce((files, file) => { const fullPath = path.posix.join(dir, file); const isSymbolicLink = fs.lstatSync(fullPath).isSymbolicLink(); @@ -42,7 +46,12 @@ function findBazelFiles(dir) { function main() { // Rename all bazel files found by prefixing them with `_` - for (f of findBazelFiles('node_modules')) { + const cwd = process.cwd(); + const rootNodeModules = + /\/node_modules\/@bazel\/hide-bazel-files$/.test(cwd.replace(/\\/g, '/')) ? + path.dirname(path.dirname(cwd)) : + path.posix.join(cwd, 'node_modules'); + for (f of findBazelFiles(rootNodeModules)) { const d = path.posix.join(path.dirname(f), `_${path.basename(f)}`); fs.renameSync(f, d); } diff --git a/packages/hide-bazel-files/package.json b/packages/hide-bazel-files/package.json index 06e32d8524..db19f63be4 100644 --- a/packages/hide-bazel-files/package.json +++ b/packages/hide-bazel-files/package.json @@ -9,5 +9,8 @@ "bazel", "javascript" ], - "description": "A tool that hides all Bazel files in node_modules by prefixing them with an underscore" + "description": "A tool that hides all Bazel files in node_modules by prefixing them with an underscore", + "scripts": { + "postinstall": "node ./index.js" + } }