build(bazel): fix race conditions in windows aio build

Disables the rules_nodejs linker and adds a custom esm module resolver
This commit is contained in:
Derek Cormier 2022-06-27 14:41:05 -07:00 committed by Joey Perrott
parent 7c8ca113c1
commit 07126ae40f
20 changed files with 461 additions and 19 deletions

View file

@ -24,8 +24,12 @@ http_archive(
http_archive(
name = "build_bazel_rules_nodejs",
sha256 = "c78216f5be5d451a42275b0b7dc809fb9347e2b04a68f68bad620a2b01f5c774",
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/5.5.2/rules_nodejs-5.5.2.tar.gz"],
patch_args = ["-p1"],
patches = [
"//:tools/bazel-repo-patches/rules_nodejs__esm_no_linker_fix.patch",
],
sha256 = "ee3280a7f58aa5c1caa45cb9e08cbb8f4d74300848c508374daf37314d5390d6",
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/5.5.1/rules_nodejs-5.5.1.tar.gz"],
)
load("@build_bazel_rules_nodejs//:repositories.bzl", "build_bazel_rules_nodejs_dependencies")

View file

@ -1,5 +1,6 @@
load("@aio_npm//@angular-devkit/architect-cli:index.bzl", "architect", "architect_test")
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "npm_package_bin")
load("@build_bazel_rules_nodejs//:index.bzl", "npm_package_bin")
load("//aio/tools:defaults.bzl", "nodejs_binary")
load("@aspect_bazel_lib//lib:copy_to_directory.bzl", "copy_to_directory")
# The write_source_files macro is used to write bazel outputs to the source tree and test that they are up to date.

View file

@ -1,4 +1,5 @@
load("@build_bazel_rules_nodejs//:index.bzl", "js_library", "nodejs_binary")
load("@build_bazel_rules_nodejs//:index.bzl", "js_library")
load("//aio/tools:defaults.bzl", "nodejs_binary")
package(default_visibility = ["//visibility:public"])

0
aio/tools/BUILD.bazel Normal file
View file

25
aio/tools/defaults.bzl Normal file
View file

@ -0,0 +1,25 @@
load("@build_bazel_rules_nodejs//:index.bzl", _nodejs_binary = "nodejs_binary")
def nodejs_binary(data = [], env = {}, templated_args = [], **kwargs):
data = data + [
"//aio/tools/esm-loader",
"//aio/tools/esm-loader:esm-loader.mjs",
]
env = dict(env, **{"NODE_MODULES_WORKSPACE_NAME": "aio_npm"})
templated_args = templated_args + [
# Disable the linker and rely on patched resolution which works better on Windows
# and is less prone to race conditions when targets build concurrently.
"--nobazel_run_linker",
# Provide a custom esm loader to resolve third-party depenencies. Unlike for cjs
# modules, rules_nodejs doesn't patch imports when the linker is disabled.
"--node_options=--loader=./$(rootpath //aio/tools/esm-loader:esm-loader.mjs)",
]
_nodejs_binary(
data = data,
env = env,
templated_args = templated_args,
**kwargs
)

View file

@ -0,0 +1,13 @@
package(default_visibility = ["//aio:__subpackages__"])
exports_files([
"esm-loader.mjs",
])
filegroup(
name = "esm-loader",
srcs = [
"esm-loader.mjs",
"//third_party/github.com/lukeed/resolve.exports",
],
)

View file

@ -0,0 +1,56 @@
import fs from 'fs';
import path from 'path';
import {pathToFileURL} from 'url';
import {resolve as resolveExports} from '../../../third_party/github.com/lukeed/resolve.exports/index.mjs';
/*
Custom module loader (see https://nodejs.org/api/cli.html#--experimental-loadermodule) to support
loading third-party packages in esm modules when the rules_nodejs linker is disabled. Resolves
third-party imports from the node_modules folder in the bazel workspace defined by
process.env.NODE_MODULES_WORKSPACE_NAME, and uses default resolution for all other imports.
This is required because rules_nodejs only patches requires in cjs modules when the linker
is disabled, not imports in mjs modules.
*/
export async function resolve(specifier, context, defaultResolve) {
if (!isNodeOrNpmPackageImport(specifier)) {
return defaultResolve(specifier, context, defaultResolve);
}
const nodeModules = path.resolve('external', process.env.NODE_MODULES_WORKSPACE_NAME, 'node_modules');
const packageImport = parsePackageImport(specifier);
const pathToNodeModule = path.join(nodeModules, packageImport.packageName);
const isInternalNodePackage = !fs.existsSync(pathToNodeModule);
if (isInternalNodePackage) {
return defaultResolve(specifier, context, defaultResolve);
}
const packageJson = JSON.parse(fs.readFileSync(path.join(pathToNodeModule, 'package.json'), 'utf-8'));
const localPackagePath = resolvePackageLocalFilepath(packageImport, packageJson);
const resolvedFilePath = path.join(pathToNodeModule, localPackagePath);
return {url: pathToFileURL(resolvedFilePath).href};
}
function isNodeOrNpmPackageImport(specifier) {
return !specifier.startsWith('./') && !specifier.startsWith('../') && !specifier.startsWith('node:') && !specifier.startsWith('file:');
}
function parsePackageImport(specifier) {
const [, packageName, pathInPackage = ''] = /^((?:@[^/]+\/)?[^/]+)(?:\/(.+))?$/.exec(specifier) ?? [];
if (!packageName) {
throw new Error(`Could not parse package name import statement '${specifier}'`);
}
return {packageName, pathInPackage, specifier};
}
function resolvePackageLocalFilepath(packageImport, packageJson) {
if (packageJson.exports) {
return resolveExports(packageJson, packageImport.specifier);
}
return packageImport.pathInPackage || packageJson.module || packageJson.main || 'index.js';
}

View file

@ -1,4 +1,4 @@
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
load("//aio/tools:defaults.bzl", "nodejs_binary")
package(default_visibility = ["//visibility:public"])
@ -7,13 +7,11 @@ nodejs_binary(
data = [
"exampleZipper.mjs",
"//aio/tools/transforms/examples-package",
"//aio/tools/transforms/helpers",
"@aio_npm//archiver",
"@aio_npm//canonical-path",
"@aio_npm//fs-extra",
"@aio_npm//globby",
],
entry_point = "generateZip.mjs",
# --preserve-symlinks-main is not enabled by default (see https://github.com/bazelbuild/rules_nodejs/pull/2176)
# However it seems to be required for mjs entry points to resolve node_modules within the sandbox.
templated_args = ["--node_options=--preserve-symlinks-main"],
)

View file

@ -5,7 +5,7 @@ import fs from 'fs-extra';
import {globbySync} from 'globby';
import {fileURLToPath} from 'url';
import regionExtractor from 'aio/tools/examples-package/services/region-parser.js';
import regionExtractor from '../transforms/examples-package/services/region-parser.js';
const __dirname = path.dirname(fileURLToPath(import.meta.url));
const EXAMPLE_CONFIG_NAME = 'example-config.json';

View file

@ -1,4 +1,4 @@
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
load("//aio/tools:defaults.bzl", "nodejs_binary")
load("@aio_npm//@bazel/jasmine:index.bzl", "jasmine_node_test")
package(default_visibility = ["//visibility:public"])

View file

@ -1,4 +1,4 @@
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
load("//aio/tools:defaults.bzl", "nodejs_binary")
package(default_visibility = ["//visibility:public"])
@ -7,13 +7,11 @@ nodejs_binary(
data = [
"builder.mjs",
"//aio/tools/transforms/examples-package",
"//aio/tools/transforms/helpers",
"@aio_npm//canonical-path",
"@aio_npm//fs-extra",
"@aio_npm//globby",
"@aio_npm//jsdom",
],
entry_point = "generateStackblitz.mjs",
# --preserve-symlinks-main is not enabled by default (see https://github.com/bazelbuild/rules_nodejs/pull/2176)
# However it seems to be required for mjs entry points to resolve node_modules within the sandbox.
templated_args = ["--node_options=--preserve-symlinks-main"],
)

View file

@ -4,7 +4,7 @@ import fs from 'fs-extra';
import {globbySync} from 'globby';
import jsdom from 'jsdom';
import regionExtractor from 'aio/tools/examples-package/services/region-parser.js';
import regionExtractor from '../transforms/examples-package/services/region-parser.js';
export class StackblitzBuilder {
constructor(examplePath, destPath) {

View file

@ -3,7 +3,6 @@ load("@aio_npm//@bazel/jasmine:index.bzl", "jasmine_node_test")
js_library(
name = "examples-package",
package_name = "aio/tools/examples-package",
srcs = glob(
["**/*.js"],
exclude = ["**/*.spec.js"],

View file

@ -4,7 +4,7 @@ const inlineC = require('./region-matchers/inline-c');
const inlineCOnly = require('./region-matchers/inline-c-only');
const inlineHash = require('./region-matchers/inline-hash');
const DEFAULT_PLASTER = '. . .';
const {mapObject} = require('helpers/utils');
const {mapObject} = require('../../helpers/utils');
const removeEslintComments = require('./removeEslintComments');
module.exports = function regionParser() {

View file

@ -3,12 +3,11 @@ load("@aio_npm//@bazel/jasmine:index.bzl", "jasmine_node_test")
js_library(
name = "helpers",
package_name = "helpers",
srcs = [
"test-package.js",
"utils.js",
],
visibility = ["//aio/tools/transforms:__subpackages__"],
visibility = ["//aio/tools:__subpackages__"],
deps = [
"//:package_json",
"@aio_npm//canonical-path",

View file

@ -0,0 +1,16 @@
licenses(["notice"])
# Downloaded from: https://github.com/lukeed/resolve.exports/blob/v1.1.0/license
# Timestamp: 07/05/2022
exports_files(["license"])
filegroup(
name = "resolve.exports",
srcs = [
# https://github.com/lukeed/resolve.exports/blob/v1.1.0/src/index.js
"index.mjs",
],
visibility = [
"//aio/tools/esm-loader:__pkg__",
],
)

View file

@ -0,0 +1 @@
- `index.js` has been renamed to `index.mjs` so that it's `export` declarations are read correctly by esm scripts

View file

@ -0,0 +1,146 @@
/**
* @param {object} exports
* @param {Set<string>} keys
*/
function loop(exports, keys) {
if (typeof exports === 'string') {
return exports;
}
if (exports) {
let idx, tmp;
if (Array.isArray(exports)) {
for (idx=0; idx < exports.length; idx++) {
if (tmp = loop(exports[idx], keys)) return tmp;
}
} else {
for (idx in exports) {
if (keys.has(idx)) {
return loop(exports[idx], keys);
}
}
}
}
}
/**
* @param {string} name The package name
* @param {string} entry The target entry, eg "."
* @param {number} [condition] Unmatched condition?
*/
function bail(name, entry, condition) {
throw new Error(
condition
? `No known conditions for "${entry}" entry in "${name}" package`
: `Missing "${entry}" export in "${name}" package`
);
}
/**
* @param {string} name the package name
* @param {string} entry the target path/import
*/
function toName(name, entry) {
return entry === name ? '.'
: entry[0] === '.' ? entry
: entry.replace(new RegExp('^' + name + '\/'), './');
}
/**
* @param {object} pkg package.json contents
* @param {string} [entry] entry name or import path
* @param {object} [options]
* @param {boolean} [options.browser]
* @param {boolean} [options.require]
* @param {string[]} [options.conditions]
* @param {boolean} [options.unsafe]
*/
export function resolve(pkg, entry='.', options={}) {
let { name, exports } = pkg;
if (exports) {
let { browser, require, unsafe, conditions=[] } = options;
let target = toName(name, entry);
if (target[0] !== '.') target = './' + target;
if (typeof exports === 'string') {
return target === '.' ? exports : bail(name, target);
}
let allows = new Set(['default', ...conditions]);
unsafe || allows.add(require ? 'require' : 'import');
unsafe || allows.add(browser ? 'browser' : 'node');
let key, tmp, isSingle=false;
for (key in exports) {
isSingle = key[0] !== '.';
break;
}
if (isSingle) {
return target === '.'
? loop(exports, allows) || bail(name, target, 1)
: bail(name, target);
}
if (tmp = exports[target]) {
return loop(tmp, allows) || bail(name, target, 1);
}
for (key in exports) {
tmp = key[key.length - 1];
if (tmp === '/' && target.startsWith(key)) {
return (tmp = loop(exports[key], allows))
? (tmp + target.substring(key.length))
: bail(name, target, 1);
}
if (tmp === '*' && target.startsWith(key.slice(0, -1))) {
// do not trigger if no *content* to inject
if (target.substring(key.length - 1).length > 0) {
return (tmp = loop(exports[key], allows))
? tmp.replace('*', target.substring(key.length - 1))
: bail(name, target, 1);
}
}
}
return bail(name, target);
}
}
/**
* @param {object} pkg
* @param {object} [options]
* @param {string|boolean} [options.browser]
* @param {string[]} [options.fields]
*/
export function legacy(pkg, options={}) {
let i=0, value,
browser = options.browser,
fields = options.fields || ['module', 'main'];
if (browser && !fields.includes('browser')) {
fields.unshift('browser');
}
for (; i < fields.length; i++) {
if (value = pkg[fields[i]]) {
if (typeof value == 'string') {
//
} else if (typeof value == 'object' && fields[i] == 'browser') {
if (typeof browser == 'string') {
value = value[browser=toName(pkg.name, browser)];
if (value == null) return browser;
}
} else {
continue;
}
return typeof value == 'string'
? ('./' + value.replace(/^\.?\//, ''))
: value;
}
}
}

View file

@ -0,0 +1,21 @@
The MIT License (MIT)
Copyright (c) Luke Edwards <luke.edwards05@gmail.com> (lukeed.com)
Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.

View file

@ -0,0 +1,164 @@
diff --git a/internal/node/launcher.sh b/internal/node/launcher.sh
index 9d1e8df4..db31cb45 100644
--- a/internal/node/launcher.sh
+++ b/internal/node/launcher.sh
@@ -331,26 +331,24 @@ if [ "$PATCH_REQUIRE" = true ]; then
* ) require_patch_script="${PWD}/${require_patch_script}" ;;
esac
LAUNCHER_NODE_OPTIONS+=( "--require" "$require_patch_script" )
- # Change the entry point to be the loader.cjs script so we run code before node
- MAIN=$(rlocation "TEMPLATED_loader_script")
-else
- # Entry point is the user-supplied script
- MAIN="${PWD}/"TEMPLATED_entry_point_execroot_path
- # TODO: after we link-all-bins we should not need this extra lookup
- if [[ ! -e "$MAIN" ]]; then
- if [ "$FROM_EXECROOT" = true ]; then
- MAIN="$EXECROOT/"TEMPLATED_entry_point_execroot_path
- else
- MAIN=TEMPLATED_entry_point_manifest_path
- fi
- fi
- # Always set up source-map-support using our vendored copy, just like the require_patch_script
- register_source_map_support=$(rlocation build_bazel_rules_nodejs/third_party/github.com/source-map-support/register.js)
- LAUNCHER_NODE_OPTIONS+=( "--require" "${register_source_map_support}" )
- if [[ -n "TEMPLATED_entry_point_main" ]]; then
- MAIN="${MAIN}/"TEMPLATED_entry_point_main
+fi
+
+# Entry point is the user-supplied script
+MAIN="${PWD}/"TEMPLATED_entry_point_execroot_path
+# TODO: after we link-all-bins we should not need this extra lookup
+if [[ ! -e "$MAIN" ]]; then
+ if [ "$FROM_EXECROOT" = true ]; then
+ MAIN="$EXECROOT/"TEMPLATED_entry_point_execroot_path
+ else
+ MAIN=TEMPLATED_entry_point_manifest_path
fi
fi
+# Always set up source-map-support using our vendored copy, just like the require_patch_script
+register_source_map_support=$(rlocation build_bazel_rules_nodejs/third_party/github.com/source-map-support/register.js)
+LAUNCHER_NODE_OPTIONS+=( "--require" "${register_source_map_support}" )
+if [[ -n "TEMPLATED_entry_point_main" ]]; then
+ MAIN="${MAIN}/"TEMPLATED_entry_point_main
+fi
if [ "${SILENT_ON_SUCCESS:-}" = true ]; then
if [[ -z "${STDOUT_CAPTURE}" ]]; then
@@ -484,3 +482,4 @@ if [[ -n "${EXIT_CODE_CAPTURE}" ]]; then
else
exit ${RESULT}
fi
+
diff --git a/internal/node/loader.cjs b/internal/node/loader.cjs
deleted file mode 100644
index dd7bc0b4..00000000
--- a/internal/node/loader.cjs
+++ /dev/null
@@ -1,39 +0,0 @@
-/**
- * @license
- * Copyright 2017 The Bazel Authors. All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- *
- * You may obtain a copy of the License at
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-/**
- * @fileoverview NodeJS module loader for bazel.
- */
-'use strict';
-
-// Ensure that node is added to the path for any subprocess calls
-process.env.PATH = [require('path').dirname(process.execPath), process.env.PATH].join(
- /^win/i.test(process.platform) ? ';' : ':');
-
-if (require.main === module) {
- // Set the actual entry point in the arguments list.
- // argv[0] == node, argv[1] == entry point.
- // NB: 'TEMPLATED_entry_point_path' & 'TEMPLATED_entry_point' below are replaced during the build process.
- var entryPointPath = 'TEMPLATED_entry_point_path';
- var entryPointMain = 'TEMPLATED_entry_point_main';
- var mainScript = process.argv[1] = entryPointMain ? `${entryPointPath}/${entryPointMain}` : entryPointPath;
- try {
- module.constructor._load(mainScript, this, /*isMain=*/true);
- } catch (e) {
- console.error(e.stack || e);
- process.exit(1);
- }
-}
diff --git a/internal/node/node.bzl b/internal/node/node.bzl
index b4debf8f..d8608c2b 100644
--- a/internal/node/node.bzl
+++ b/internal/node/node.bzl
@@ -117,21 +117,6 @@ def _get_entry_point_file(ctx):
return ctx.attr.entry_point[DirectoryFilePathInfo].directory
fail("entry_point must either be a file, or provide DirectoryFilePathInfo")
-def _write_loader_script(ctx):
- substitutions = {}
- substitutions["TEMPLATED_entry_point_path"] = _ts_to_js(_to_manifest_path(ctx, _get_entry_point_file(ctx)))
- if DirectoryFilePathInfo in ctx.attr.entry_point:
- substitutions["TEMPLATED_entry_point_main"] = ctx.attr.entry_point[DirectoryFilePathInfo].path
- else:
- substitutions["TEMPLATED_entry_point_main"] = ""
-
- ctx.actions.expand_template(
- template = ctx.file._loader_template,
- output = ctx.outputs.loader_script,
- substitutions = substitutions,
- is_executable = True,
- )
-
# Avoid using non-normalized paths (workspace/../other_workspace/path)
def _to_manifest_path(ctx, file):
if file.short_path.startswith("../"):
@@ -208,8 +193,6 @@ def _nodejs_binary_impl(ctx, data = [], runfiles = [], expanded_args = []):
node_modules_root = "build_bazel_rules_nodejs/node_modules"
_write_require_patch_script(ctx, data, node_modules_root)
- _write_loader_script(ctx)
-
# Provide the target name as an environment variable avaiable to all actions for the
# runfiles helpers to use.
env_vars = "export BAZEL_TARGET=%s\n" % ctx.label
@@ -276,7 +259,6 @@ fi
runfiles = runfiles[:]
runfiles.extend(node_tool_files)
runfiles.extend(ctx.files._bash_runfile_helper)
- runfiles.append(ctx.outputs.loader_script)
runfiles.append(ctx.outputs.require_patch_script)
# First replace any instances of "$(rlocation " with "$$(rlocation " to preserve
@@ -331,7 +313,6 @@ if (process.cwd() !== __dirname) {
"TEMPLATED_expected_exit_code": str(expected_exit_code),
"TEMPLATED_lcov_merger_script": _to_manifest_path(ctx, ctx.file._lcov_merger_script),
"TEMPLATED_link_modules_script": _to_manifest_path(ctx, ctx.file._link_modules_script),
- "TEMPLATED_loader_script": _to_manifest_path(ctx, ctx.outputs.loader_script),
"TEMPLATED_modules_manifest": _to_manifest_path(ctx, node_modules_manifest),
"TEMPLATED_node_patches_script": _to_manifest_path(ctx, ctx.file._node_patches_script),
"TEMPLATED_require_patch_script": _to_manifest_path(ctx, ctx.outputs.require_patch_script),
@@ -378,7 +359,6 @@ if (process.cwd() !== __dirname) {
runfiles = ctx.runfiles(
transitive_files = depset(runfiles),
files = node_tool_files + [
- ctx.outputs.loader_script,
ctx.outputs.require_patch_script,
] + ctx.files._source_map_support_files +
@@ -642,7 +622,6 @@ Predefined genrule variables are not supported in this context.
_NODEJS_EXECUTABLE_OUTPUTS = {
"launcher_sh": "%{name}.sh",
- "loader_script": "%{name}_loader.cjs",
"require_patch_script": "%{name}_require_patch.cjs",
}