Skip to content

Commit

Permalink
refactor: improve worker correctness and add observability (#264)
Browse files Browse the repository at this point in the history
* refactor: write debug info to worker log

* fix state bugs

* fix ci

* add fstree debugging

* fix ts_project

* add tests and fix sourcemap roots

* add new tests and remove old e2e/worker_*

* cleanup

* fix fstree tests

* handle tsconfig changes

* fix ci

* load system wide libraries

* fix tests

* remove protobuf repos

* address changes

* fix outputs

* expand symlinks and notify at last

* fix tests

* fix ci

* remove extra debug
  • Loading branch information
thesayyn authored Jan 25, 2023
1 parent 7c478da commit ad69d93
Show file tree
Hide file tree
Showing 75 changed files with 1,734 additions and 1,735 deletions.
1 change: 1 addition & 0 deletions .bazelignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
e2e/
bazel-output-base
1 change: 0 additions & 1 deletion .github/workflows/ci.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
# Debug where options came from
build --announce_rc
# This directory is configured in GitHub actions to be persisted between runs.
build --disk_cache=~/.cache/bazel
build --repository_cache=~/.cache/bazel-repo
# Don't rely on test logs being easily accessible from the test runner,
# though it makes the log noisier.
Expand Down
89 changes: 48 additions & 41 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ jobs:
folder:
- '.'
- 'e2e/bzlmod'
- 'e2e/worker_workspace'
- 'e2e/worker'
- 'e2e/workspace'
exclude:
Expand All @@ -73,11 +72,6 @@ jobs:
# Don't test bzlmod with Bazel 5 (not supported)
- bazelversion: 5.3.2
folder: e2e/bzlmod
# TODO: e2e/worker needs a bit of to be configured for RBE
- config: rbe
folder: e2e/worker
- config: rbe
folder: e2e/worker_workspace
# Steps represent a sequence of tasks that will be executed as part of the job
steps:
# Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
Expand Down Expand Up @@ -129,40 +123,53 @@ jobs:
working-directory: ${{ matrix.folder }}
run: bazel --bazelrc=$GITHUB_WORKSPACE/.github/workflows/ci.bazelrc --bazelrc=.bazelrc test --config=${{ matrix.config }} //...

# Some tests need buildozer to manipulate BUILD files.
- uses: actions/setup-go@v3
if: ${{ hashFiles(format('{0}/test.sh', matrix.folder)) != '' }}
with:
go-version: '1.17.0'

- run: go install github.com/bazelbuild/buildtools/buildozer@latest
if: ${{ hashFiles(format('{0}/test.sh', matrix.folder)) != '' }}

- uses: pnpm/action-setup@v2
with:
version: '7.14.2'

test-worker:
runs-on: ubuntu-latest
needs:
- matrix-prep-bazelversion
strategy:
fail-fast: false
matrix:
bazelversion: ${{ fromJSON(needs.matrix-prep-bazelversion.outputs.bazelversions) }}
steps:
- uses: actions/checkout@v3

- name: Configure Bazel version
working-directory: e2e/test
# Overwrite the .bazelversion instead of using USE_BAZEL_VERSION so that Bazelisk
# still bootstraps Aspect CLI from configuration in .bazeliskrc. Aspect CLI will
# then use .bazelversion to determine which Bazel version to use
run: echo "${{ matrix.bazelversion }}" > .bazelversion

- uses: pnpm/action-setup@v2
with:
version: '7.14.2'

- name: Setup bats
uses: mig4/setup-bats@v1
with:
bats-version: '1.8.2'

- name: Setup bats helpers
uses: brokenpip3/[email protected]
with:
support-path: /usr/lib/bats/bats-support
support-version: '0.3.0'
assert-path: /usr/lib/bats/bats-assert
assert-version: '2.1.0'

- name: bats -r .
working-directory: e2e/test
run: |
echo "import $GITHUB_WORKSPACE/.github/workflows/ci.bazelrc" > .bazelrc
bats -r .
- name: bats -r . --noexperimental_allow_unresolved_symlinks
working-directory: e2e/test
run: |
echo "import $GITHUB_WORKSPACE/.github/workflows/ci.bazelrc" > .bazelrc
echo "build --noexperimental_allow_unresolved_symlinks" >> .bazelrc
bats -r .
- name: run ./test.sh
working-directory: ${{ matrix.folder }}
# hashFiles returns an empty string if test.sh is absent
if: ${{ hashFiles(format('{0}/test.sh', matrix.folder)) != '' }}
run: |
cp -f $GITHUB_WORKSPACE/.github/workflows/ci.bazelrc .bazelrc.test
echo "build --config=${{ matrix.config }}" >> .bazelrc.test
echo "build --disk_cache=/tmp/cache" >> .bazelrc.test
bazel clean
./test.sh
./test.sh
- name: run ./test.sh with --noexperimental_allow_unresolved_symlinks
working-directory: ${{ matrix.folder }}
# hashFiles returns an empty string if test.sh is absent
if: ${{ hashFiles(format('{0}/test.sh', matrix.folder)) != '' }}
run: |
cp -f $GITHUB_WORKSPACE/.github/workflows/ci.bazelrc .bazelrc.test
echo "build --config=${{ matrix.config }}" >> .bazelrc.test
echo "build --disk_cache=/tmp/cache" >> .bazelrc.test
echo "build --noexperimental_allow_unresolved_symlinks" >> ".bazelrc.test"
bazel clean
./test.sh
./test.sh
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
bazel-*
.bazelrc.user
.bazelrc.test
node_modules/
4 changes: 2 additions & 2 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ aspect_bazel_lib_dependencies(override_local_config_platform = True)
load("@rules_nodejs//nodejs:repositories.bzl", "nodejs_register_toolchains")

nodejs_register_toolchains(
name = "node16",
node_version = "16.9.0",
name = "node18",
node_version = "18.12.0",
)

# For running our own unit tests
Expand Down
28 changes: 28 additions & 0 deletions docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,34 @@ You can often create a minimal repro of your problem outside of Bazel.
This is a good way to bisect whether your issue is purely with TypeScript, or there's something
Bazel-specific going on.

# Not reproducible ts_project worker bugs
Not reproducible ts_project, a.k.a. state, bugs has been a challenge for anyone to diagnose and possibly, fix in ts_project.
Not knowing what state the worker has been at when it falsely failed, or what went wrong along the way is hard to know.
For this, we introduced support for `--worker_verbose` flag which prints a bunch of helpful logs to worker log file.

If you find yourself getting yelled at by ts_project falsely on occasion, drop `build --worker_verbose` to the `.bazelrc` file.
In addition to `--worker_verbose`, set `extendedDiagnostics` and `traceResolution` to true in the `tsconfig.json` file to log
additional information about how tsc reacts to events fed by worker protocol.

```
{
"compilerOptions": {
"extendedDiagnostics": true,
"traceResolution": true
}
}
```

Next time, the ts_project yields false negative diagnostics messages, collect the logs files output_base and file a bug with the log files.

To collect log files run the command below at the workspace directory and attach the logs.tar file to issued file.
```
tar -cf logs.tar $(ls $(bazel info output_base)/bazel-workers/worker-*-TsProject.log)
```

This will help us understand what went wrong in your case, and hopefully implement a permanent fix for it.


# Getting unstuck

The basic methodology for diagnosing problems is:
Expand Down
6 changes: 1 addition & 5 deletions e2e/bzlmod/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,4 @@ rules_ts_ext = use_extension(

rules_ts_ext.deps()

use_repo(rules_ts_ext, "npm_typescript")

use_repo(rules_ts_ext, "npm_google_protobuf")

use_repo(rules_ts_ext, "npm_at_bazel_worker")
use_repo(rules_ts_ext, "npm_typescript")
214 changes: 214 additions & 0 deletions e2e/test/common.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
bats_load_library "bats-support"
bats_load_library "bats-assert"

function workspace() {
local rules_ts_path="$(realpath $BATS_TEST_DIRNAME/../../)"
local -i is_npm_translate_lock=0
local -i no_convenience_symlinks=0
while (( $# > 0 )); do
case "$1" in
-t|--npm-translate-lock) is_npm_translate_lock=1; shift ;;
-t|--noconvenience-symlinks) no_convenience_symlinks=1; shift ;;
*) break ;;
esac
done

cat > WORKSPACE <<EOF
local_repository(
name = "aspect_rules_ts",
path = "$rules_ts_path",
)
load("@aspect_rules_ts//ts:repositories.bzl", "LATEST_VERSION", "rules_ts_dependencies")
rules_ts_dependencies(ts_version = LATEST_VERSION)
# Fetch and register node, if you haven't already
load("@rules_nodejs//nodejs:repositories.bzl", "DEFAULT_NODE_VERSION", "nodejs_register_toolchains")
nodejs_register_toolchains(
name = "node",
node_version = DEFAULT_NODE_VERSION,
)
load("@aspect_bazel_lib//lib:repositories.bzl", "register_copy_directory_toolchains", "register_copy_to_directory_toolchains")
register_copy_directory_toolchains()
register_copy_to_directory_toolchains()
EOF


[[ -e "$BATS_TEST_DIRNAME/.bazelversion" ]] && cp "$BATS_TEST_DIRNAME/.bazelversion" .bazelversion

cat > .bazelrc << EOF
try-import $BATS_TEST_DIRNAME/.bazelrc
startup --max_idle_secs=10
build --worker_verbose
EOF


if (( no_convenience_symlinks )); then
echo "build --noexperimental_convenience_symlinks" >> .bazelrc
fi

if (( is_npm_translate_lock )); then
cat >> WORKSPACE <<EOF
load("@aspect_rules_js//npm:npm_import.bzl", "npm_translate_lock")
npm_translate_lock(
name = "npm",
pnpm_lock = "//:pnpm-lock.yaml",
)
load("@npm//:repositories.bzl", "npm_repositories")
npm_repositories()
EOF
fi
}

function tsconfig() {
local path="."
local no_implicit_any="false"
local isolated_modules="false"
local source_map="false"
local declaration="false"
local target="ES2020"
local module_resolution="node"
local composite="false"
local trace_resolution="false"
local extended_diagnostics="false"
while (( $# > 0 )); do
case "$1" in
--path) shift; path="$1"; shift ;;
--no-implicit-any) no_implicit_any="true"; shift ;;
--isolated-modules) isolated_modules="true"; shift ;;
--source-map) source_map="true"; shift ;;
--declaration) declaration="true"; shift ;;
--composite) composite="true"; shift ;;
--target) shift; target="$1"; shift ;;
--module-resolution) shift; module_resolution="$1"; shift ;;
--trace-resolution) trace_resolution="true"; shift ;;
--extended-diagnostics) extended_diagnostics="true"; shift ;;
*) break ;;
esac
done
cat > "$path/tsconfig.json" <<EOF
{
"compilerOptions": {
"noImplicitAny": $no_implicit_any,
"isolatedModules": $isolated_modules,
"sourceMap": $source_map,
"declaration": $declaration,
"target": "$target",
"moduleResolution": "$module_resolution",
"traceResolution": $trace_resolution,
"extendedDiagnostics": $extended_diagnostics,
"composite": $composite
}
}
EOF
}


function ts_project() {
local path="."
local name="foo"
local -a deps=()
local -a srcs=()
local -a args=()
local tsconfig="tsconfig.json"
local npm_link_all_packages="#"
local source_map=""
local declaration=""
local composite=""
while (( $# > 0 )); do
case "$1" in
--path) shift; path="$1"; shift ;;
--tsconfig) shift; tsconfig="$1"; shift ;;
-n|--name) shift; name="$1"; shift ;;
-l|--npm-link-all-packages) npm_link_all_packages=""; shift ;;
-d|--dep) shift; deps+=( "\"$1"\" ); shift ;;
-s|--src) shift; srcs+=( "\"$1\"" ); shift ;;
--arg) shift; args+=( "\"$1\"" "\"$2\"" ); shift; shift ;;
--source-map) shift; source_map="source_map = True," ;;
--declaration) shift; declaration="declaration = True," ;;
--composite) shift; composite="composite = True," ;;
--) shift; break ;;
*) break ;;
esac
done
local deps_joined=$(IFS=, ; echo "${deps[*]}")
local srcs_joined=$(IFS=, ; echo "${srcs[*]}")
local args_joined=$(IFS=, ; echo "${args[*]}")
cat > "$path/BUILD.bazel" <<EOF
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")
${npm_link_all_packages}load("@npm//:defs.bzl", "npm_link_all_packages")
${npm_link_all_packages}npm_link_all_packages(name = "node_modules")
ts_project(
name = "${name}",
visibility = ["//visibility:public"],
srcs = [${srcs_joined}],
tsconfig = "${tsconfig}",
deps = [${deps_joined}],
args = [${args_joined}],
$source_map
$declaration
$composite
)
EOF
}

function js_library() {
local path="."
local name="foo"
local npm_link_all_packages="#"
local -a srcs=()
while (( $# > 0 )); do
case "$1" in
--path) shift; path="$1"; shift ;;
-n|--name) shift; name="$1"; shift ;;
-s|--src) shift; srcs+=( "\"$1\"" ); shift ;;
-l|--npm-link-all-packages) npm_link_all_packages=""; shift ;;
--) shift; break ;;
*) break ;;
esac
done
local -a srcs_joined=$(IFS=, ; echo "${srcs[*]}")
cat > "$path/BUILD.bazel" <<EOF
load("@aspect_rules_js//js:defs.bzl", "js_library")
${npm_link_all_packages}load("@npm//:defs.bzl", "npm_link_all_packages")
${npm_link_all_packages}npm_link_all_packages(name = "node_modules")
js_library(
name = "${name}",
visibility = ["//visibility:public"],
srcs = [${srcs_joined}]
)
EOF
}

function npm_package() {
local path="."
local name="foo"
local -a srcs=()
while (( $# > 0 )); do
case "$1" in
--path) shift; path="$1"; shift ;;
-n|--name) shift; name="$1"; shift ;;
-s|--src) shift; srcs+=( "\"$1\"" ); shift ;;
--) shift; break ;;
*) break ;;
esac
done
local -a srcs_joined=$(IFS=, ; echo "${srcs[*]}")
cat >> "$path/BUILD.bazel" <<EOF
load("@aspect_rules_js//npm:defs.bzl", "npm_package")
npm_package(
name = "${name}",
visibility = ["//visibility:public"],
srcs = [${srcs_joined}]
)
EOF
}
Loading

0 comments on commit ad69d93

Please sign in to comment.