From 24aa924241e15c68624349adb248e38648dd4cb4 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Sat, 20 Apr 2019 10:43:50 -0700 Subject: [PATCH] Switch to a single node_modules symlink in skylark --- internal/npm_install/generate_build_file.js | 93 ++++++++++++--------- internal/npm_install/npm_install.bzl | 9 +- 2 files changed, 59 insertions(+), 43 deletions(-) diff --git a/internal/npm_install/generate_build_file.js b/internal/npm_install/generate_build_file.js index 283eb1f06c..d5e412aa5f 100644 --- a/internal/npm_install/generate_build_file.js +++ b/internal/npm_install/generate_build_file.js @@ -51,10 +51,9 @@ package(default_visibility = ["//visibility:public"]) ` const args = process.argv.slice(2); -const USER_DIR = args[0] ? args[0] : process.cwd(); -const WORKSPACE = args[1]; +const WORKSPACE = args[0]; +const LOCK_FILE_LABEL = args[1]; const INCLUDED_FILES = args[2] ? args[2].split(',') : []; -const LOCK_FILE_LABEL = args[3]; if (require.main === module) { main(); @@ -82,14 +81,6 @@ function writeFileSync(p, content) { fs.fdatasyncSync(fd); } -/** - * Given a relative path, returns the absolute path to the user's - * package.json directory. - */ -function userDir(p) { - return path.posix.join(USER_DIR, p); -} - /** * Main entrypoint. * Write BUILD and .bzl files. @@ -104,25 +95,15 @@ function main() { pkgs.forEach(pkg => pkgsMap.set(pkg._dir, pkg)); pkgs.forEach(pkg => flattenDependencies(pkg, pkg, pkgsMap)); + // handle Bazel files in npm packages + pkgs.forEach(pkg => renameBazelFiles(pkg)); + // generate Bazel workspaces install files const bazelWorkspaces = {}; pkgs.forEach(pkg => processBazelWorkspaces(pkg, bazelWorkspaces)); generateBazelWorkspaces(bazelWorkspaces) generateInstallBazelDependencies(Object.keys(bazelWorkspaces)); - // symlink all files (except BUILD & WORKSPACES files) before generating BUILD file - listFiles(userDir('node_modules')).forEach(f => { - const basename = path.basename(f); - if (/^WORKSPACE$/i.test(basename) || /^BUILD$/i.test(basename) || - /^BUILD\.bazel$/i.test(basename)) { - return; - } - const src = path.posix.join(USER_DIR, 'node_modules', f); - const dest = path.posix.join('node_modules', f); - mkdirp(path.dirname(dest)); - fs.symlinkSync(src, dest); - }); - // generate BUILD files generateRootBuildFile(pkgs) pkgs.filter(pkg => !pkg._isNested).forEach(pkg => generatePackageBuildFiles(pkg)); @@ -134,6 +115,28 @@ module.exports = { printPackage }; +/** + * Adds a `_` prefix to WORKSPACE & BUILD files in an npm package to make room for generated files. + */ +function renameBazelFiles(pkg) { + pkg._files = pkg._files.map(file => { + if (/^node_modules[/\\]/.test(file)) { + // don't rename files in nested node_modules + return file; + } + const basename = path.basename(file); + if (/^WORKSPACE$/i.test(basename) || /^BUILD$/i.test(basename) || + /^BUILD\.bazel$/i.test(basename)) { + const newFile = path.posix.join(path.dirname(file), `_${basename}`); + const srcPath = path.posix.join('node_modules', pkg._dir, file); + const dstPath = path.posix.join('node_modules', pkg._dir, newFile); + fs.renameSync(srcPath, dstPath); + return newFile; + } + return file; + }); +} + /** * Generates the root BUILD file. */ @@ -307,7 +310,7 @@ def _maybe(repo_rule, name, **kwargs): `; // Copy all files for this workspace to a folder under _workspaces - // to restore the Bazel files which have bee renamed from the npm package + // to restore the Bazel files which have be renamed from the npm package const workspaceSourcePath = path.posix.join('_workspaces', bwName); mkdirp(workspaceSourcePath); bwDetails.pkg._files.forEach(file => { @@ -315,8 +318,16 @@ def _maybe(repo_rule, name, **kwargs): // don't copy over nested node_modules return; } - const src = path.posix.join(USER_DIR, 'node_modules', bwDetails.pkg._dir, file); - let dest = path.posix.join(workspaceSourcePath, file); + let destFile = file; + const basename = path.basename(file); + // Bazel files would have been renamed earlier with a _ prefix so + // we restore them on the copy + if (/^_WORKSPACE$/i.test(basename) || /^_BUILD$/i.test(basename) || + /^_BUILD\.bazel$/i.test(basename)) { + destFile = path.posix.join(path.dirname(file), basename.substr(1)); + } + const src = path.posix.join('node_modules', bwDetails.pkg._dir, file); + const dest = path.posix.join(workspaceSourcePath, destFile); mkdirp(path.dirname(dest)); fs.copyFileSync(src, dest); console.error(src, dest); @@ -443,7 +454,8 @@ function listFiles(rootDir, subDir = '') { */ function hasRootBuildFile(pkg) { for (const file of pkg._files) { - if (/^BUILD$/i.test(file) || /^BUILD\.bazel$/i.test(file)) { + // Bazel files would have been renamed earlier with a _ prefix + if (/^_BUILD$/i.test(file) || /^_BUILD\.bazel$/i.test(file)) { return true; } } @@ -454,24 +466,23 @@ function hasRootBuildFile(pkg) { * Finds and returns an array of all packages under a given path. */ function findPackages(p = 'node_modules') { - const fp = userDir(p); - if (!isDirectory(fp)) { + if (!isDirectory(p)) { return []; } const result = []; - const listing = fs.readdirSync(fp); + const listing = fs.readdirSync(p); const packages = listing.filter(f => !f.startsWith('@')) .map(f => path.posix.join(p, f)) - .filter(f => isDirectory(userDir(f))); + .filter(f => isDirectory(f)); packages.forEach( f => result.push(parsePackage(f), ...findPackages(path.posix.join(f, 'node_modules')))); const scopes = listing.filter(f => f.startsWith('@')) .map(f => path.posix.join(p, f)) - .filter(f => isDirectory(userDir(f))); + .filter(f => isDirectory(f)); scopes.forEach(f => result.push(...findPackages(f))); return result; @@ -482,16 +493,15 @@ function findPackages(p = 'node_modules') { */ function findScopes() { const p = 'node_modules'; - const fp = userDir(p); - if (!isDirectory(fp)) { + if (!isDirectory(p)) { return []; } - const listing = fs.readdirSync(fp); + const listing = fs.readdirSync(p); const scopes = listing.filter(f => f.startsWith('@')) .map(f => path.posix.join(p, f)) - .filter(f => isDirectory(userDir(f))) + .filter(f => isDirectory(f)) .map(f => f.replace(/^node_modules\//, '')); return scopes; @@ -504,8 +514,7 @@ function findScopes() { */ function parsePackage(p) { // Parse the package.json file of this package - const fp = userDir(p); - const packageJson = path.posix.join(fp, 'package.json'); + const packageJson = path.posix.join(p, 'package.json'); const pkg = isFile(packageJson) ? JSON.parse(fs.readFileSync(packageJson, {encoding: 'utf8'})) : {version: '0.0.0'}; @@ -520,7 +529,7 @@ function parsePackage(p) { pkg._isNested = p.match(/\/node_modules\//); // List all the files in the npm package for later use - pkg._files = listFiles(fp); + pkg._files = listFiles(p); // Initialize _dependencies to an empty array // which is later filled with the flattened dependency list @@ -675,8 +684,10 @@ function filterFiles(files, exts = []) { files = files.filter(f => !f.match(/[^\x21-\x7E]/)); files = files.filter(f => { const basename = path.basename(f); + // Bazel files of non-nested node_modules would have been renamed earlier with a _ prefix if (/^WORKSPACE$/i.test(basename) || /^BUILD$/i.test(basename) || - /^BUILD\.bazel$/i.test(basename)) { + /^BUILD\.bazel$/i.test(basename) || /^_WORKSPACE$/i.test(basename) || + /^_BUILD$/i.test(basename) || /^_BUILD\.bazel$/i.test(basename)) { return false; } else { return true; diff --git a/internal/npm_install/npm_install.bzl b/internal/npm_install/npm_install.bzl index 4d11a97144..a0d177b9be 100644 --- a/internal/npm_install/npm_install.bzl +++ b/internal/npm_install/npm_install.bzl @@ -81,10 +81,9 @@ def _create_build_files(repository_ctx, node, lock_file): result = repository_ctx.execute([ node, "generate_build_file.js", - repository_ctx.path(repository_ctx.attr.package_json).dirname, repository_ctx.attr.name, - ",".join(repository_ctx.attr.included_files), str(lock_file), + ",".join(repository_ctx.attr.included_files), ]) if result.return_code: fail("generate_build_file.js failed: \nSTDOUT:\n%s\nSTDERR:\n%s" % (result.stdout, result.stderr)) @@ -102,6 +101,10 @@ def _add_scripts(repository_ctx): {}, ) +def _symlink_node_modules(repository_ctx): + package_json_dir = repository_ctx.path(repository_ctx.attr.package_json).dirname + repository_ctx.symlink(repository_ctx.path(str(package_json_dir) + "/node_modules"), repository_ctx.path("node_modules")) + def _npm_install_impl(repository_ctx): """Core implementation of npm_install.""" @@ -173,6 +176,7 @@ cd "{root}" && "{npm}" {npm_args} if result.return_code: fail("remove_npm_absolute_paths failed: %s (%s)" % (result.stdout, result.stderr)) + _symlink_node_modules(repository_ctx) _create_build_files(repository_ctx, node, repository_ctx.attr.package_lock_json) npm_install = repository_rule( @@ -238,6 +242,7 @@ def _yarn_install_impl(repository_ctx): if result.return_code: fail("yarn_install failed: %s (%s)" % (result.stdout, result.stderr)) + _symlink_node_modules(repository_ctx) _create_build_files(repository_ctx, node, repository_ctx.attr.yarn_lock) yarn_install = repository_rule(