Skip to content

Commit

Permalink
bazel: upgrade to rules_nodejs 5.4.2
Browse files Browse the repository at this point in the history
Upgrading to Bazel's rules_nodejs 5.x exposed a flaw in our previous
Bazel integration: because rules_nodejs explicitly doesn't support
yarn's "workspaces" feature [1] (in which common dependencies are
hoisted to the lowest common parent directory), any NPM dependencies
with different major versions between db-console and cluster-ui would
get flattened to a single version. This left one of those packages using
an unsupported (and un-requested) version of a dependency. Removing
the yarn workspace layout and using separate Bazel repositories for
each JS project ensured each project received the correct dependencies,
but revealed incompatibilities with the requested versions. Upgrade
rules_nodejs to the latest released version, remove yarn workspaces from
the pkg/ui/ tree, and fix all revealed compatibility issues.

[1] bazel-contrib/rules_nodejs#266
  • Loading branch information
sjbarag committed May 26, 2022
1 parent f6c92a5 commit 5b6c271
Show file tree
Hide file tree
Showing 30 changed files with 37,238 additions and 18,813 deletions.
8 changes: 4 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,9 @@ pkg/ui/yarn.installed: pkg/ui/package.json pkg/ui/yarn.lock | bin/.submodules-in
@# Also some linux distributions (that are used as development env) don't support some of
@# optional dependencies (i.e. cypress) so it is important to make these deps optional.
$(NODE_RUN) -C pkg/ui yarn install --ignore-optional --offline
@# We remove this broken dependency again in pkg/ui/webpack.config.js.
@# We remove this broken dependency again in pkg/ui/workspaces/db-console/webpack.config.js.
@# See the comment there for details.
rm -rf pkg/ui/node_modules/@types/node
rm -rf pkg/ui/workspaces/db-console/node_modules/@types/node
touch $@

vendor/modules.txt: | bin/.submodules-initialized
Expand Down Expand Up @@ -1275,8 +1275,8 @@ GW_SOURCES := $(GW_PROTOS:%.proto=%.pb.gw.go)
GO_PROTOS := $(sort $(shell $(FIND_RELEVANT) -type f -name '*.proto' -print))
GO_SOURCES := $(GO_PROTOS:%.proto=%.pb.go)

PBJS := $(NODE_RUN) pkg/ui/node_modules/.bin/pbjs
PBTS := $(NODE_RUN) pkg/ui/node_modules/.bin/pbts
PBJS := $(NODE_RUN) pkg/ui/workspaces/db-console/src/js/node_modules/.bin/pbjs
PBTS := $(NODE_RUN) pkg/ui/workspaces/db-console/src/js/node_modules/.bin/pbts

# Unlike the protobuf compiler for Go and C++, the protobuf compiler for
# JavaScript only needs the entrypoint protobufs to be listed. It automatically
Expand Down
98 changes: 82 additions & 16 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
workspace(
name = "com_github_cockroachdb_cockroach",
managed_directories = {
"@npm": ["pkg/ui/node_modules"],
"@yarn_vendor": ["pkg/ui/yarn-vendor"],
"@npm_protos": ["pkg/ui/workspaces/db-console/src/js/node_modules"],
"@npm_cluster_ui": ["pkg/ui/workspaces/cluster_ui/node_modules"],
"@npm_db_console": ["pkg/ui/workspaces/db-console/node_modules"],
},
)

Expand All @@ -23,12 +26,16 @@ http_archive(
)

# Like the above, but for nodeJS.
http_archive(
name = "rules_nodejs",
sha256 = "26766278d815a6e2c43d2f6c9c72fde3fec8729e84138ffa4dabee47edc7702a",
urls = ["https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_nodejs-core-5.4.2.tar.gz"],
)

http_archive(
name = "build_bazel_rules_nodejs",
sha256 = "cfc289523cf1594598215901154a6c2515e8bf3671fd708264a6f6aefe02bf39",
urls = [
"https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_nodejs-4.4.6.tar.gz",
],
sha256 = "e328cb2c9401be495fa7d79c306f5ee3040e8a03b2ebb79b022e15ca03770096",
urls = [ "https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_nodejs-5.4.2.tar.gz" ],
)

# Load gazelle. This lets us auto-generate BUILD.bazel files throughout the
Expand Down Expand Up @@ -195,8 +202,16 @@ go_register_toolchains(nogo = "@com_github_cockroachdb_cockroach//:crdb_nogo")
# begin rules_nodejs dependencies #
###################################

# Install rules_nodejs dependencies

# bazel_skylib handled above.
# rules_nodejs handled above.
load("@build_bazel_rules_nodejs//:repositories.bzl", "build_bazel_rules_nodejs_dependencies")
build_bazel_rules_nodejs_dependencies()

# Configure nodeJS.
load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "yarn_install")
load("@rules_nodejs//nodejs:yarn_repositories.bzl", "yarn_repositories")

node_repositories(
node_repositories = {
Expand All @@ -210,8 +225,11 @@ node_repositories(
"https://storage.googleapis.com/public-bazel-artifacts/js/node/v{version}/{filename}",
],
node_version = "16.13.0",
package_json = ["//pkg/ui:package.json"],
yarn_repositories = {
)

yarn_repositories(
name = "yarn",
yarn_releases = {
"1.22.11": ("yarn-v1.22.11.tar.gz", "yarn-v1.22.11", "2c320de14a6014f62d29c34fec78fdbb0bc71c9ccba48ed0668de452c1f5fe6c"),
},
yarn_urls = [
Expand All @@ -220,21 +238,69 @@ node_repositories(
yarn_version = "1.22.11",
)

# install external dependencies for pkg/ui package
load("//build/bazelutil:seed_yarn_cache.bzl", "seed_yarn_cache")
seed_yarn_cache(name = "yarn_cache")

# Install external dependencies for NPM packages in pkg/ui/ as separate bazel
# repositories, to avoid version conflicts between those packages.
# Unfortunately Bazel's rules_nodejs does not support yarn workspaces, so
# packages have isolated dependencies and must be installed as isolated
# Bazel repositories.
yarn_install(
name = "npm",
name = "npm_protos",
args = [
"--offline",
"--ignore-optional",
],
data = [
"//pkg/ui:.yarnrc",
"@yarn_cache//:.seed",
],
package_path = "/",
package_json = "//pkg/ui/workspaces/db-console/src/js:package.json",
strict_visibility = False,
yarn_lock = "//pkg/ui/workspaces/db-console/src/js:yarn.lock",
)

yarn_install(
name = "npm_db_console",
args = [
"--offline",
"--ignore-optional",
],
data = [
"//pkg/ui:.yarnrc",
"@yarn_cache//:.seed",
],
package_json = "//pkg/ui:package.json",
package_json = "//pkg/ui/workspaces/db-console:package.json",
yarn_lock = "//pkg/ui/workspaces/db-console:yarn.lock",
strict_visibility = False,
yarn_lock = "//pkg/ui:yarn.lock",
links = {
"@cockroachlabs/crdb-protobuf-client": "//pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client",
"@cockroachlabs/crdb-protobuf-client-ccl": "//pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl",
"@cockroachlabs/cluster-ui": "//pkg/ui/workspaces/cluster-ui:cluster-ui",
},
patch_args = [ "-p0", "--remove-empty-files", "--silent" ],
post_install_patches = [
"//pkg/ui:patches/aria-query/remove-filenames-with-spaces.db-console.patch",
],
symlink_node_modules = True,
)

yarn_install(
name = "npm_cluster_ui",
args = [
"--verbose",
"--offline",
"--ignore-optional",
],
data = [
"//pkg/ui:.yarnrc",
"@yarn_cache//:.seed",
],
package_json = "//pkg/ui/workspaces/cluster-ui:package.json",
strict_visibility = False,
yarn_lock = "//pkg/ui/workspaces/cluster-ui:yarn.lock",
patch_args = [ "-p0", "--remove-empty-files", "--silent" ],
post_install_patches = [
"//pkg/ui:patches/aria-query/remove-filenames-with-spaces.cluster-ui.patch",
],
symlink_node_modules = True,
)

#################################
Expand Down
16 changes: 16 additions & 0 deletions build/bazelutil/check_yarn_vendor_submodule.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/usr/bin/env bash
set -eu

if [ "$#" -ne 1 ]; then
echo "Usage: $0 PATH"
echo "Example: check_yarn_vendor_submodule.sh /path/to/cockroachdb/cockroach/pkg/ui/yarn-vendor"
exit 1
fi

yarnVendorDir=$1
if [ -z "$(ls -A $yarnVendorDir)" ]; then
echo "No packages available from yarn-vendor submodule." >&2
echo "You may need to run 'git submodule update --init'." >&2
exit 2
fi

3 changes: 2 additions & 1 deletion build/bazelutil/distdir_files.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,8 @@ DISTDIR_FILES = {
"https://storage.googleapis.com/public-bazel-artifacts/bazel/protobuf-e809d75ecb5770fdc531081eef306b3e672bcdd2.tar.gz": "071ccf561d127d5702910340cf038cb869aa239683544e1cca68a78ea865099e",
"https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_java-981f06c3d2bd10225e85209904090eb7b5fb26bd.tar.gz": "f5a3e477e579231fca27bf202bb0e8fbe4fc6339d63b38ccb87c2760b533d1c3",
"https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_license-0.0.1.tar.gz": "4865059254da674e3d18ab242e21c17f7e3e8c6b1f1421fffa4c5070f82e98b5",
"https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_nodejs-4.4.6.tar.gz": "cfc289523cf1594598215901154a6c2515e8bf3671fd708264a6f6aefe02bf39",
"https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_nodejs-5.4.2.tar.gz": "e328cb2c9401be495fa7d79c306f5ee3040e8a03b2ebb79b022e15ca03770096",
"https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_nodejs-core-5.4.2.tar.gz": "26766278d815a6e2c43d2f6c9c72fde3fec8729e84138ffa4dabee47edc7702a",
"https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_pkg-0.7.0.tar.gz": "8a298e832762eda1830597d64fe7db58178aa84cd5926d76d5b744d6558941c2",
"https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_proto-b0cc14be5da05168b01db282fe93bdf17aa2b9f4.tar.gz": "88b0a90433866b44bb4450d4c30bc5738b8c4f9c9ba14e9661deb123f56a833d",
"https://storage.googleapis.com/public-bazel-artifacts/bazel/rules_python-0.1.0.tar.gz": "b6d46438523a3ec0f3cead544190ee13223a52f6a6765a29eae7b7cc24cc83a0",
Expand Down
85 changes: 85 additions & 0 deletions build/bazelutil/seed_yarn_cache.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "yarn_install")

_doc = """
Ensures the current user's global yarn cache (typically in ~/.cache/yarn)
contains all vendored yarn dependencies, so that later yarn_install calls can
be --offline without having access to the yarn-vendor submodule.
build_bazel_rules_nodejs's yarn_install can only get access to the
pkg/ui/yarn-vendor submodule by explicitly listing each .tgz file in that
submodule as a dependency, which causes every file to be copied individually
(in serial) and makes the yarn_install task unusably slow (> 50 minutes on a
24-core Xeon machine with 32GB of RAM). To ensure yarn_install only uses
packages from the yarn-vendor submodule this rule helps to leverage the fact
that yarn_install has access to the user's "global" yarn cache, which is
typically used to accelerate `yarn install` by not repeatedly fetching packages
from remote locations.
Specifically, this rule performs one `yarn install` invocation for each package
from within the Bazel workspace (not the sandbox!) where access to
pkg/ui/yarn-vendor is guaranteed, and stores the resulting modules in a location
that intentionally won't be read. It relies on the fact that performing those
installations has a side-effect of adding each installed dependency to the
global yarn cache.
"""

def _seed_yarn_cache_impl(rctx):
workspace_root = str(
rctx.path(
Label("//:WORKSPACE")
).dirname
)

rctx.report_progress("Checking for yarn-vendor submodule...")
res = rctx.execute(
[
rctx.path(Label("//build/bazelutil:check_yarn_vendor_submodule.sh")),
workspace_root + "/pkg/ui"
],
)
if res.return_code != 0:
fail("Unable to seed yarn cache: " + res.stderr)

paths = {
"protos": "pkg/ui/workspaces/db-console/src/js",
"cluster_ui": "pkg/ui/workspaces/cluster-ui",
"db_console": "pkg/ui/workspaces/db-console",
}
yarn_dir = str(rctx.path(Label("@yarn//:bin/yarn")).dirname)
for key, path in paths.items():
rctx.report_progress("Seeding yarn cache for {}...".format(path))
# Execute a script that uses the bazel-installed yarn (via $PATH) to install
# dependencies while running at the root of the *non-sandboxed* node
# package (argv[1]), but putting those dependencies in a repo-specific
# location (argv[2]) to avoid corrupting the local node_modules
# directories.
res = rctx.execute(
[
rctx.path(Label("//build/bazelutil:seed_yarn_cache.sh")),
workspace_root + "/" + path,
rctx.path("node_modules." + key),
],
environment = {
"PATH": "{}:{}".format(yarn_dir, rctx.os.environ["PATH"])
},
)

if res.return_code != 0:
fail("Unable to seed yarn cache: " + res.stderr)

rctx.file(
".seed",
content = "",
executable = False,
)
rctx.file(
"BUILD.bazel",
content = """exports_files([".seed"])""",
executable = False,
)

seed_yarn_cache = repository_rule(
implementation = _seed_yarn_cache_impl,
doc = _doc,
local = True,
)
20 changes: 20 additions & 0 deletions build/bazelutil/seed_yarn_cache.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash
set -euo pipefail

if [ "$#" -ne 2 ]; then
echo "Usage: $0 PKG_PATH MODULES_DIR"
echo "Example: seed_yarn_cache.sh /path/to/cockroachdb/cockroach/pkg/ui/workspaces/cluster-ui /path/to/repo/node_modules.cluster_ui"
exit 1
fi

rootDir=$1
moduleDir=$2

yarn \
--cwd $rootDir \
--modules-folder $moduleDir \
--offline \
--ignore-optional \
--no-progress \
--mutex network \
install
13 changes: 1 addition & 12 deletions pkg/cmd/dev/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,6 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error {
args = append(args, additionalBazelArgs...)

if cross == "" {
// run "yarn --check-files" before any bazel target that includes UI to ensure that node_modules dir is consistent
// see related issue: https://github.com/cockroachdb/cockroach/issues/70867
for _, arg := range args {
if arg == "--config=with_ui" {
logCommand("bazel", "run", "@nodejs//:yarn", "--", "--check-files", "--cwd", "pkg/ui", "--offline")
if err := d.exec.CommandContextInheritingStdStreams(ctx, "bazel", "run", "@nodejs//:yarn", "--", "--check-files", "--cwd", "pkg/ui", "--offline"); err != nil {
return err
}
break
}
}
logCommand("bazel", args...)
if err := d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...); err != nil {
return err
Expand Down Expand Up @@ -174,7 +163,7 @@ func (d *dev) crossBuild(
script.WriteString(fmt.Sprintf("bazel %s\n", shellescape.QuoteCommand(bazelArgs)))
for _, arg := range bazelArgs {
if arg == "--config=with_ui" {
script.WriteString("bazel run @nodejs//:yarn -- --check-files --cwd pkg/ui --offline\n")
script.WriteString("bazel run @yarn//:yarn -- --check-files --cwd pkg/ui --offline\n")
break
}
}
Expand Down
1 change: 0 additions & 1 deletion pkg/cmd/dev/testdata/datadriven/dev-build
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ cp sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkou
exec
dev build -- --verbose_failures --sandbox_debug
----
bazel run @nodejs//:yarn -- --check-files --cwd pkg/ui --offline
bazel build //pkg/cmd/cockroach:cockroach --config=with_ui --verbose_failures --sandbox_debug
bazel info workspace --color=no
mkdir crdb-checkout/bin
Expand Down
24 changes: 12 additions & 12 deletions pkg/cmd/dev/testdata/datadriven/ui
Original file line number Diff line number Diff line change
Expand Up @@ -3,40 +3,40 @@ dev ui watch
----
bazel build //pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client //pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl
bazel info workspace --color=no
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=ccl --env.target=http://localhost:8080 --port 3000
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=ccl --env.target=http://localhost:8080 --port 3000

exec
dev ui watch --oss
----
bazel build //pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client
bazel info workspace --color=no
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=oss --env.target=http://localhost:8080 --port 3000
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=oss --env.target=http://localhost:8080 --port 3000

exec
dev ui watch --secure
----
bazel build //pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client //pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl
bazel info workspace --color=no
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=ccl --env.target=http://localhost:8080 --port 3000 --https
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=ccl --env.target=http://localhost:8080 --port 3000 --https

exec
dev ui watch --db http://example.crdb.io:4848
----
bazel build //pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client //pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl
bazel info workspace --color=no
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=ccl --env.target=http://example.crdb.io:4848 --port 3000
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=ccl --env.target=http://example.crdb.io:4848 --port 3000

exec
dev ui watch --port 12345
----
bazel build //pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client //pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl
bazel info workspace --color=no
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=ccl --env.target=http://localhost:8080 --port 12345
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui build:watch
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console webpack-dev-server --config webpack.app.js --mode development --env.WEBPACK_SERVE --env.dist=ccl --env.target=http://localhost:8080 --port 12345

exec
dev ui lint
Expand Down Expand Up @@ -71,8 +71,8 @@ cp sandbox/pkg/ui/workspaces/db-console/ccl/src/js/protos.d.ts crdb-checkout/pkg
rm -rf crdb-checkout/pkg/ui/workspaces/cluster-ui/dist
cp -r sandbox/pkg/ui/workspaces/cluster-ui/dist crdb-checkout/pkg/ui/workspaces/cluster-ui/dist
bazel info workspace --color=no
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console karma:watch
bazel run @nodejs//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui jest --watch
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/db-console karma:watch
bazel run @yarn//:yarn -- --silent --cwd crdb-checkout/pkg/ui/workspaces/cluster-ui jest --watch

exec
dev ui clean
Expand Down
Loading

0 comments on commit 5b6c271

Please sign in to comment.