From 6f4fc17a2399735a48ca122fdb4e2a282c3f7d70 Mon Sep 17 00:00:00 2001 From: Tiago Costa Date: Wed, 16 Dec 2020 23:18:58 +0000 Subject: [PATCH] feat: create symlink for build files present on node modules installed with relative paths (#2330) --- WORKSPACE | 6 +++ internal/npm_install/generate_build_file.ts | 48 +++++++++++++++++-- internal/npm_install/index.js | 19 +++++++- internal/npm_install/npm_install.bzl | 42 +++++++++++++++- internal/npm_install/test/BUILD.bazel | 1 + internal/npm_install/test/common.spec.js | 4 ++ tools/fine_grained_deps_npm/package-lock.json | 3 ++ tools/fine_grained_deps_npm/package.json | 1 + tools/fine_grained_deps_yarn/package.json | 1 + tools/fine_grained_deps_yarn/yarn.lock | 4 ++ .../npm_packages/local_module/npm/BUILD.bazel | 35 ++++++++++++++ tools/npm_packages/local_module/npm/index.js | 1 + .../local_module/npm/package.json | 4 ++ .../local_module/yarn/BUILD.bazel | 35 ++++++++++++++ tools/npm_packages/local_module/yarn/index.js | 1 + .../local_module/yarn/package.json | 4 ++ 16 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 tools/npm_packages/local_module/npm/BUILD.bazel create mode 100644 tools/npm_packages/local_module/npm/index.js create mode 100644 tools/npm_packages/local_module/npm/package.json create mode 100644 tools/npm_packages/local_module/yarn/BUILD.bazel create mode 100644 tools/npm_packages/local_module/yarn/index.js create mode 100644 tools/npm_packages/local_module/yarn/package.json diff --git a/WORKSPACE b/WORKSPACE index 8dd222bd2b..d720cf8c7f 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -182,6 +182,9 @@ local_repository( yarn_install( name = "fine_grained_deps_yarn", data = [ + "//:tools/npm_packages/local_module/yarn/BUILD.bazel", + "//:tools/npm_packages/local_module/yarn/index.js", + "//:tools/npm_packages/local_module/yarn/package.json", "//internal/npm_install/test:postinstall.js", ], environment = { @@ -202,6 +205,9 @@ yarn_install( npm_install( name = "fine_grained_deps_npm", data = [ + "//:tools/npm_packages/local_module/npm/BUILD.bazel", + "//:tools/npm_packages/local_module/npm/index.js", + "//:tools/npm_packages/local_module/npm/package.json", "//internal/npm_install/test:postinstall.js", ], environment = { diff --git a/internal/npm_install/generate_build_file.ts b/internal/npm_install/generate_build_file.ts index 704dae4c29..e566da15bf 100644 --- a/internal/npm_install/generate_build_file.ts +++ b/internal/npm_install/generate_build_file.ts @@ -93,6 +93,15 @@ function writeFileSync(p: string, content: string) { fs.writeFileSync(p, content); } +/** + * Creates a file symlink, first ensuring that the directory to + * create it into exists. + */ +function createFileSymlinkSync(target: string, p: string) { + mkdirp(path.dirname(p)); + fs.symlinkSync(target, p, 'file'); +} + /** * Main entrypoint. */ @@ -194,11 +203,37 @@ js_library( * Generates all BUILD & bzl files for a package. */ function generatePackageBuildFiles(pkg: Dep) { - // If a BUILD file was shipped with the package, append its contents to the end of - // what we generate for the package. + // If a BUILD file was shipped with the package we should symlink the generated BUILD file + // instead of append its contents to the end of the one we were going to generate. + // https://github.com/bazelbuild/rules_nodejs/issues/2131 let buildFilePath: string|undefined; if (pkg._files.includes('BUILD')) buildFilePath = 'BUILD'; if (pkg._files.includes('BUILD.bazel')) buildFilePath = 'BUILD.bazel'; + + // Recreate the pkg dir inside the node_modules folder + const nodeModulesPkgDir = `node_modules/${pkg._dir}`; + // Check if the current package dep dir is a symlink (which happens when we + // install a node_module with link:) + const isPkgDirASymlink = + fs.existsSync(nodeModulesPkgDir) && fs.lstatSync(nodeModulesPkgDir).isSymbolicLink(); + // Check if the current package is also written inside the workspace + // NOTE: It's a corner case but fs.realpathSync(.) will not be the root of + // the workspace if symlink_node_modules = False as yarn & npm are run in the root of the + // external repository and anything linked with a relative path would have to be copied + // over via that data attribute + const isPkgInsideWorkspace = fs.realpathSync(nodeModulesPkgDir).includes(fs.realpathSync(`.`)); + // Mark build file as one to symlink instead of generate as the package dir is a symlink, we + // have a BUILD file and the pkg is written inside the workspace + const symlinkBuildFile = isPkgDirASymlink && buildFilePath && isPkgInsideWorkspace; + + // Log if a BUILD file was expected but was not found + if (!symlinkBuildFile && isPkgDirASymlink) { + console.log(`[yarn_install/npm_install]: package ${ + nodeModulesPkgDir} is local symlink and as such a BUILD file for it is expected but none was found. Please add one at ${ + fs.realpathSync(nodeModulesPkgDir)}`) + } + + // The following won't be used in a symlink build file case let buildFile = printPackage(pkg); if (buildFilePath) { buildFile = buildFile + '\n' + @@ -256,7 +291,14 @@ exports_files(["index.bzl"]) } } - writeFileSync(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile); + if (!symlinkBuildFile) { + writeFileSync( + path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile); + } else { + const realPathBuildFileForPkg = + fs.realpathSync(path.posix.join(nodeModulesPkgDir, buildFilePath)); + createFileSymlinkSync(realPathBuildFileForPkg, path.posix.join(pkg._dir, buildFilePath)); + } } /** diff --git a/internal/npm_install/index.js b/internal/npm_install/index.js index 1283aa49cf..2ad3d0e659 100644 --- a/internal/npm_install/index.js +++ b/internal/npm_install/index.js @@ -39,6 +39,10 @@ function writeFileSync(p, content) { mkdirp(path.dirname(p)); fs.writeFileSync(p, content); } +function createFileSymlinkSync(target, p) { + mkdirp(path.dirname(p)); + fs.symlinkSync(target, p, 'file'); +} function main() { const deps = getDirectDependencySet(PKG_JSON_FILE_PATH); const pkgs = findPackages('node_modules', deps); @@ -113,6 +117,13 @@ function generatePackageBuildFiles(pkg) { buildFilePath = 'BUILD'; if (pkg._files.includes('BUILD.bazel')) buildFilePath = 'BUILD.bazel'; + const nodeModulesPkgDir = `node_modules/${pkg._dir}`; + const isPkgDirASymlink = fs.existsSync(nodeModulesPkgDir) && fs.lstatSync(nodeModulesPkgDir).isSymbolicLink(); + const isPkgInsideWorkspace = fs.realpathSync(nodeModulesPkgDir).includes(fs.realpathSync(`.`)); + const symlinkBuildFile = isPkgDirASymlink && buildFilePath && isPkgInsideWorkspace; + if (!symlinkBuildFile && isPkgDirASymlink) { + console.log(`[yarn_install/npm_install]: package ${nodeModulesPkgDir} is local symlink and as such a BUILD file for it is expected but none was found. Please add one at ${fs.realpathSync(nodeModulesPkgDir)}`); + } let buildFile = printPackage(pkg); if (buildFilePath) { buildFile = buildFile + '\n' + @@ -154,7 +165,13 @@ exports_files(["index.bzl"]) `; } } - writeFileSync(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile); + if (!symlinkBuildFile) { + writeFileSync(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile); + } + else { + const realPathBuildFileForPkg = fs.realpathSync(path.posix.join(nodeModulesPkgDir, buildFilePath)); + createFileSymlinkSync(realPathBuildFileForPkg, path.posix.join(pkg._dir, buildFilePath)); + } } function generateBazelWorkspaces(pkgs) { const workspaces = {}; diff --git a/internal/npm_install/npm_install.bzl b/internal/npm_install/npm_install.bzl index a9266f52fd..323cecfe67 100644 --- a/internal/npm_install/npm_install.bzl +++ b/internal/npm_install/npm_install.bzl @@ -312,7 +312,26 @@ See npm CLI docs https://docs.npmjs.com/cli/install.html for complete list of su This rule will set the environment variable `BAZEL_NPM_INSTALL` to '1' (unless it set to another value in the environment attribute). Scripts may use to this to -check if yarn is being run by the `npm_install` repository rule.""", +check if yarn is being run by the `npm_install` repository rule. + + +**LOCAL MODULES WITH THE NEED TO BE USED BOTH INSIDE AND OUTSIDE BAZEL** + +When using a monorepo it's common to have modules that we want to use locally and +publish to an external package repository. This can be achieved using a `js_library` rule +with a `package_name` attribute defined inside the local package `BUILD` file. However, +if the project relies on the local package dependency with `file:`, this could introduce a +race condition with the `npm_install` rule. + +In order to overcome it, a link will be created to the package `BUILD` file from the +npm external Bazel repository, which require us to complete a last step which is writing +the expected targets on that same `BUILD` file to be later used by the `npm_install` +rule, which are: ``, ``, +``, `` and the last +one just ``. + +If you doubt what those targets should look like, check the +generated `BUILD` file for a given node module.""", implementation = _npm_install_impl, ) @@ -475,6 +494,25 @@ to yarn so that the local cache is contained within the external repository. This rule will set the environment variable `BAZEL_YARN_INSTALL` to '1' (unless it set to another value in the environment attribute). Scripts may use to this to -check if yarn is being run by the `yarn_install` repository rule.""", +check if yarn is being run by the `yarn_install` repository rule. + + +**LOCAL MODULES WITH THE NEED TO BE USED BOTH INSIDE AND OUTSIDE BAZEL** + +When using a monorepo it's common to have modules that we want to use locally and +publish to an external package repository. This can be achieved using a `js_library` rule +with a `package_name` attribute defined inside the local package `BUILD` file. However, +if the project relies on the local package dependency with `link:`, this could introduce a +race condition with the `yarn_install` rule. + +In order to overcome it, a link will be created to the package `BUILD` file from the +npm external Bazel repository, which require us to complete a last step which is writing +the expected targets on that same `BUILD` file to be later used by the `yarn_install` +rule, which are: ``, ``, +``, `` and the last +one just ``. + +If you doubt what those targets should look like, check the +generated `BUILD` file for a given node module.""", implementation = _yarn_install_impl, ) diff --git a/internal/npm_install/test/BUILD.bazel b/internal/npm_install/test/BUILD.bazel index b6f257cdd4..b71b8a33c9 100644 --- a/internal/npm_install/test/BUILD.bazel +++ b/internal/npm_install/test/BUILD.bazel @@ -133,6 +133,7 @@ sh_test( "@fine_grained_deps_%s//jasmine" % pkgmgr, "@fine_grained_deps_%s//jasmine-core" % pkgmgr, "@fine_grained_deps_%s//ajv" % pkgmgr, + "@fine_grained_deps_%s//local-module" % pkgmgr, "@fine_grained_deps_%s//typescript" % pkgmgr, "@fine_grained_deps_%s//rxjs" % pkgmgr, # Note, test-b depends on test-a@0.0.1 which should be diff --git a/internal/npm_install/test/common.spec.js b/internal/npm_install/test/common.spec.js index a82d6832ff..b731df71f7 100644 --- a/internal/npm_install/test/common.spec.js +++ b/internal/npm_install/test/common.spec.js @@ -15,6 +15,10 @@ describe('dependencies', () => { require('ajv/lib/$data'); }); + it(`should resolve local-module`, () => { + require('local-module'); + }); + it(`should resolve rxjs/src/tsconfig.json`, () => { // the BUILD.bazel file in rxjs/src should have been // deleted by fine grained deps and rxjs/src/tsconfig.json diff --git a/tools/fine_grained_deps_npm/package-lock.json b/tools/fine_grained_deps_npm/package-lock.json index 65038156e4..e5c85d553c 100644 --- a/tools/fine_grained_deps_npm/package-lock.json +++ b/tools/fine_grained_deps_npm/package-lock.json @@ -1288,6 +1288,9 @@ "graceful-fs": "^4.1.9" } }, + "local-module": { + "version": "file:tools/npm_packages/local_module/npm" + }, "lodash.debounce": { "version": "4.0.8", "resolved": "https://registry.npmjs.org/lodash.debounce/-/lodash.debounce-4.0.8.tgz", diff --git a/tools/fine_grained_deps_npm/package.json b/tools/fine_grained_deps_npm/package.json index 756fce271b..77d8e903e1 100644 --- a/tools/fine_grained_deps_npm/package.json +++ b/tools/fine_grained_deps_npm/package.json @@ -12,6 +12,7 @@ "chokidar": "2.0.4", "http-server": "github:alexeagle/http-server#97205e945b69091606ed83aa0c8489e9ce65d282", "klaw": "1.3.1", + "local-module": "file:tools/npm_packages/local_module/npm", "rxjs": "6.5.0" }, "scripts": { diff --git a/tools/fine_grained_deps_yarn/package.json b/tools/fine_grained_deps_yarn/package.json index 756fce271b..54be5a1e9e 100644 --- a/tools/fine_grained_deps_yarn/package.json +++ b/tools/fine_grained_deps_yarn/package.json @@ -12,6 +12,7 @@ "chokidar": "2.0.4", "http-server": "github:alexeagle/http-server#97205e945b69091606ed83aa0c8489e9ce65d282", "klaw": "1.3.1", + "local-module": "link:tools/npm_packages/local_module/yarn", "rxjs": "6.5.0" }, "scripts": { diff --git a/tools/fine_grained_deps_yarn/yarn.lock b/tools/fine_grained_deps_yarn/yarn.lock index babc1065b2..03752e0a23 100644 --- a/tools/fine_grained_deps_yarn/yarn.lock +++ b/tools/fine_grained_deps_yarn/yarn.lock @@ -751,6 +751,10 @@ klaw@1.3.1: optionalDependencies: graceful-fs "^4.1.9" +"local-module@link:tools/npm_packages/local_module/yarn": + version "0.0.0" + uid "" + lodash.debounce@^4.0.8: version "4.0.8" resolved "https://registry.yarnpkg.com/lodash.debounce/-/lodash.debounce-4.0.8.tgz#82d79bff30a67c4005ffd5e2515300ad9ca4d7af" diff --git a/tools/npm_packages/local_module/npm/BUILD.bazel b/tools/npm_packages/local_module/npm/BUILD.bazel new file mode 100644 index 0000000000..7ede7d5b33 --- /dev/null +++ b/tools/npm_packages/local_module/npm/BUILD.bazel @@ -0,0 +1,35 @@ +load("@build_bazel_rules_nodejs//:index.bzl", "js_library") + +package(default_visibility = ["//visibility:public"]) + +SRCS = [ + "index.js", + "package.json", +] + +filegroup( + name = "local-module__files", + srcs = ["@fine_grained_deps_npm//:node_modules/local-module/%s" % file for file in SRCS], +) + +js_library( + name = "local-module", + srcs = [":local-module__files"], + deps = [ + ":local-module__contents", + ], +) + +js_library( + name = "local-module__contents", + srcs = [ + ":local-module__files", + ":local-module__nested_node_modules", + ], + visibility = ["//:__subpackages__"], +) + +filegroup( + name = "local-module__nested_node_modules", + visibility = ["//:__subpackages__"], +) diff --git a/tools/npm_packages/local_module/npm/index.js b/tools/npm_packages/local_module/npm/index.js new file mode 100644 index 0000000000..e8e15eefe1 --- /dev/null +++ b/tools/npm_packages/local_module/npm/index.js @@ -0,0 +1 @@ +module.exports = 'local_module'; diff --git a/tools/npm_packages/local_module/npm/package.json b/tools/npm_packages/local_module/npm/package.json new file mode 100644 index 0000000000..870f1ba6e6 --- /dev/null +++ b/tools/npm_packages/local_module/npm/package.json @@ -0,0 +1,4 @@ +{ + "version": "0.0.1", + "main": "index.js" +} diff --git a/tools/npm_packages/local_module/yarn/BUILD.bazel b/tools/npm_packages/local_module/yarn/BUILD.bazel new file mode 100644 index 0000000000..a899ea4269 --- /dev/null +++ b/tools/npm_packages/local_module/yarn/BUILD.bazel @@ -0,0 +1,35 @@ +load("@build_bazel_rules_nodejs//:index.bzl", "js_library") + +package(default_visibility = ["//visibility:public"]) + +SRCS = [ + "index.js", + "package.json", +] + +filegroup( + name = "local-module__files", + srcs = ["@fine_grained_deps_yarn//:node_modules/local-module/%s" % file for file in SRCS], +) + +js_library( + name = "local-module", + srcs = [":local-module__files"], + deps = [ + ":local-module__contents", + ], +) + +js_library( + name = "local-module__contents", + srcs = [ + ":local-module__files", + ":local-module__nested_node_modules", + ], + visibility = ["//:__subpackages__"], +) + +filegroup( + name = "local-module__nested_node_modules", + visibility = ["//:__subpackages__"], +) diff --git a/tools/npm_packages/local_module/yarn/index.js b/tools/npm_packages/local_module/yarn/index.js new file mode 100644 index 0000000000..e8e15eefe1 --- /dev/null +++ b/tools/npm_packages/local_module/yarn/index.js @@ -0,0 +1 @@ +module.exports = 'local_module'; diff --git a/tools/npm_packages/local_module/yarn/package.json b/tools/npm_packages/local_module/yarn/package.json new file mode 100644 index 0000000000..870f1ba6e6 --- /dev/null +++ b/tools/npm_packages/local_module/yarn/package.json @@ -0,0 +1,4 @@ +{ + "version": "0.0.1", + "main": "index.js" +}