build: enable bazel node modules symlinking [resubmit w/ fixes] (#17776)

Apparently we accidentally skipped a few post-install patches because
we added the edit marker file for the first patch being applied to a file. This
causes subsequent patches targeting the same file to be skipped accidentally

Author: devversion

.circleci/config.yml

@@ -13,8 +13,10 @@ var_2: &docker-firefox-image circleci/node:12.9.1-browsers
 # **Note**: When updating the beginning of the cache key, also update the cache key to match
 # the new cache key prefix. This allows us to take advantage of CircleCI's fallback caching.
 # Read more here: https://circleci.com/docs/2.0/caching/#restoring-cache.
-var_3: &cache_key v4-ng-mat-{{ checksum "WORKSPACE" }}-{{ checksum "yarn.lock" }}
-var_4: &cache_fallback_key v4-ng-mat-
+var_3: &cache_key v5-ng-mat-{{ checksum "tools/bazel/postinstall-patches.js" }}-{{ checksum "WORKSPACE" }}-{{ checksum "yarn.lock" }}
+# We want to invalidate the cache if the postinstall patches change. In order to apply new
+# patches, a clean version of the node modules is needed.
+var_4: &cache_fallback_key v5-ng-mat-{{ checksum "tools/bazel/postinstall-patches.js" }}-
 
 # Settings common to each job
 var_5: &job_defaults

WORKSPACE

@@ -1,4 +1,7 @@
-workspace(name = "angular_material")
+workspace(
+    name = "angular_material",
+    managed_directories = {"@npm": ["node_modules"]},
+)
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
@@ -42,20 +45,10 @@ node_repositories(
 
 yarn_install(
     name = "npm",
-    # Ensure that all resources are available when the "postinstall" or "preinstall" scripts
-    # are executed in the Bazel sandbox.
-    data = [
-        "//:angular-tsconfig.json",
-        "//:tools/bazel/flat_module_factory_resolution.patch",
-        "//:tools/bazel/manifest_externs_hermeticity.patch",
-        "//:tools/bazel/postinstall-patches.js",
-        "//:tools/npm/check-npm.js",
-    ],
+    # We add the postinstall patches file here so that Yarn will rerun whenever
+    # the patches script changes.
+    data = ["//:tools/bazel/postinstall-patches.js"],
     package_json = "//:package.json",
-    # Temporarily disable node_modules symlinking until the fix for
-    # https://github.com/bazelbuild/bazel/issues/8487 makes it into a
-    # future Bazel release
-    symlink_node_modules = False,
     yarn_lock = "//:yarn.lock",
 )
 

package.json

@@ -13,7 +13,7 @@
     "yarn": ">= 1.19.1"
   },
   "scripts": {
-    "postinstall": "node --preserve-symlinks --preserve-symlinks-main tools/bazel/postinstall-patches.js && ngcc --properties main --create-ivy-entry-points",
+    "postinstall": "node tools/bazel/postinstall-patches.js && ngcc --properties main --create-ivy-entry-points",
     "build": "node ./scripts/build-packages-dist.js",
     "bazel:buildifier": "find . -type f \\( -name \"*.bzl\" -or -name WORKSPACE -or -name BUILD -or -name BUILD.bazel \\) ! -path \"*/node_modules/*\" | xargs buildifier -v --warnings=attr-cfg,attr-license,attr-non-empty,attr-output-default,attr-single-file,constant-glob,ctx-args,depset-iteration,depset-union,dict-concatenation,duplicated-name,filetype,git-repository,http-archive,integer-division,load,load-on-top,native-build,native-package,output-group,package-name,package-on-top,redefined-variable,repository-name,same-origin-load,string-iteration,unused-variable,unsorted-dict-items,out-of-order-load",
     "bazel:format-lint": "yarn -s bazel:buildifier --lint=warn --mode=check",

tools/bazel/postinstall-patches.js

@@ -8,18 +8,24 @@ const shelljs = require('shelljs');
 const path = require('path');
 const fs = require('fs');
 
+/**
+ * Version of the post install patch. Needs to be incremented when patches
+ * have been added or removed.
+ */
+const PATCH_VERSION = 1;
+
 /** Path to the project directory. */
 const projectDir = path.join(__dirname, '../..');
 
+/**
+ * Object that maps a given file path to a list of patches that need to be
+ * applied.
+ */
+const PATCHES_PER_FILE = {};
+
 shelljs.set('-e');
 shelljs.cd(projectDir);
 
-// Do not apply postinstall patches when running "postinstall" outside. The
-// "generate_build_file.js" file indicates that we run in Bazel managed node modules.
-if (!shelljs.test('-e', 'generate_build_file.js')) {
-  return;
-}
-
 // Workaround for https://github.com/angular/angular/issues/18810.
 shelljs.exec('ngc -p angular-tsconfig.json');
 
@@ -69,7 +75,7 @@ searchAndReplace(
   const hasFlatModuleBundle = fs.existsSync(filePath.replace('.d.ts', '.metadata.json'));
   if ((filePath.includes('node_modules/') || !hasFlatModuleBundle) && $1`,
     'node_modules/@angular/compiler-cli/src/transformers/compiler_host.js');
-shelljs.cat(path.join(__dirname, './flat_module_factory_resolution.patch')).exec('patch -p0');
+applyPatch(path.join(__dirname, './flat_module_factory_resolution.patch'));
 // The three replacements below ensure that metadata files can be read by NGC and
 // that metadata files are collected as Bazel action inputs.
 searchAndReplace(
@@ -90,7 +96,7 @@ searchAndReplace(
     'node_modules/@angular/bazel/src/ng_module.bzl');
 
 // Workaround for: https://github.com/bazelbuild/rules_nodejs/issues/1208.
-shelljs.cat(path.join(__dirname, './manifest_externs_hermeticity.patch')).exec('patch -p0');
+applyPatch(path.join(__dirname, './manifest_externs_hermeticity.patch'));
 
 // Workaround for using Ngcc with "--create-ivy-entry-points". This is a special
 // issue for our repository since we want to run Ivy by default in the module resolution,
@@ -136,18 +142,67 @@ shelljs.rm('-rf', [
   'node_modules/rxjs/Subscription.*',
 ]);
 
+// Apply all collected patches on a per-file basis. This is necessary because
+// multiple edits might apply to the same file, and we only want to mark a given
+// file as patched once all edits have been made.
+Object.keys(PATCHES_PER_FILE).forEach(filePath => {
+  if (hasFileBeenPatched(filePath)) {
+    console.info('File ' + filePath + ' is already patched. Skipping..');
+    return;
+  }
+
+  let content = fs.readFileSync(filePath, 'utf8');
+  const patchFunctions = PATCHES_PER_FILE[filePath];
+
+  console.info(`Patching file ${filePath} with ${patchFunctions.length} edits..`);
+  patchFunctions.forEach(patchFn => content = patchFn(content));
+
+  fs.writeFileSync(filePath, content, 'utf8');
+  writePatchMarker(filePath);
+});
+
 /**
- * Reads the specified file and replaces matches of the search expression
- * with the given replacement. Throws if no changes were made.
+ * Applies the given patch if not done already. Throws if the patch does
+ * not apply cleanly.
+ */
+function applyPatch(patchFile) {
+  const patchMarkerFileName = `${path.basename(patchFile)}.patch_marker`;
+  const patchMarkerPath = path.join(projectDir, 'node_modules/', patchMarkerFileName);
+
+  if (hasFileBeenPatched(patchMarkerPath)) {
+    return;
+  }
+
+  writePatchMarker(patchMarkerPath);
+  shelljs.cat(patchFile).exec('patch -p0');
+}
+
+/**
+ * Schedules an edit where the specified file is read and its content replaced based on
+ * the given search expression and corresponding replacement. Throws if no changes were made
+ * and the patch has not been applied.
  */
 function searchAndReplace(search, replacement, relativeFilePath) {
   const filePath = path.join(projectDir, relativeFilePath);
-  const originalContent = fs.readFileSync(filePath, 'utf8');
-  const newFileContent = originalContent.replace(search, replacement);
+  const fileEdits = PATCHES_PER_FILE[filePath] || (PATCHES_PER_FILE[filePath] = []);
+
+  fileEdits.push(originalContent => {
+    const newFileContent = originalContent.replace(search, replacement);
+    if (originalContent === newFileContent) {
+      throw Error(`Could not perform replacement in: ${filePath}.`);
+    }
+    return newFileContent;
+  });
+}
 
-  if (originalContent === newFileContent) {
-    throw Error(`Could not perform replacement in: ${filePath}.`);
-  }
+/** Marks the specified file as patched. */
+function writePatchMarker(filePath) {
+  new shelljs.ShellString(PATCH_VERSION).to(`${filePath}.patch_marker`);
+}
 
-  fs.writeFileSync(filePath, newFileContent, 'utf8');
+/** Checks if the given file has been patched. */
+function hasFileBeenPatched(filePath) {
+  const markerFilePath = `${filePath}.patch_marker`;
+  return shelljs.test('-e', markerFilePath) &&
+      shelljs.cat(markerFilePath).toString().trim() === `${PATCH_VERSION}`;
 }