Angular upgrade v17 to v18 #7099
Open
cdavalos7 wants to merge 16 commits into
Open
Conversation
arcra
approved these changes
Apr 27, 2026
arcra
left a comment
arcra
left a comment
Member
There was a problem hiding this comment.
From the PR description:
The patch file was also rewritten for v18 because the new tooling has a code-removal step that deletes parts of the app and leaves the page blank; the patch turns it off.
This existed in the previous version too, right? In the PR description it sounds like a new change. Can we just say that we update the patch for v18?
Updated the patch name in WORKSPACE; removed the old entry from whitespace_hygiene_test.py.
I don't see anything related to whitespace_hygiene_test.py, is this out of date? Or did I miss something?
| data = [ | ||
| "//patches:@angular+build-tooling+0.0.0-2113cd7f66a089ac0208ea84eee672b2529f4f6c.patch", | ||
| "//patches:@angular+build-tooling+0.0.0-db91da4e742cd081bfba01db2edc4e816018419b.patch", | ||
| "//patches:@bazel+concatjs+5.8.1.patch", |
Member
There was a problem hiding this comment.
I remember on the previous upgrade (#7085 ), there was a comment about the upgrade to concatjs 5.8.1.
I was under the impression we'd need to do this for this next migration. Was this not the case? This might be a bit related with upgrading our bazel set-up with new dependencies, potentially using bazel modules, that Pablo might be working on at some point, so perhaps this is the reason?
Contributor
Author
There was a problem hiding this comment.
Current versions @bazel/concatjs, @bazel/esbuild, @bazel/typescript: still 5.8.1 are still compatible with Angular 18.
I plan to wait until Angular dependencies are not compatible anymore with @bazel(-angular) dependencies to update those packages. If we reach 21 without replacing @bazel/concatjs with rules_nodejs I can create a follow-up task to do this major change.
rules_nodejs 6.x is a major change, it removes @bazel/concatjs, @bazel/esbuild, @bazel/typescript entirely and replaces them with the new rules_js / aspect_rules_js system. It rewrites how Bazel loads npm packages and runs TypeScript and esbuild.
cdavalos7
changed the title
Contributor
Author
|
Updated PR comment, thank you for pointing out @arcra. Yes, there is a change in the file |
## Motivation for features / changes Unblock internal LSC intended to remove workarounds for Sass and bad practices. Internal issue: b/508363660. This PR removes the wrappers for Sass build rules. Sass rules must directly declare their dependencies. Per Sass build rules guidelines, wrapper macros are discouraged, and tf_external_sass_libray was relying on a loophole (passing a sass_library to another sass_library's srcs) that is being closed. ## Technical description of changes - Replace `tf_sass_binary`,`tf_sass_library`, and `tf_external_sass_libray` wrappers with direct calls to `sass_binary`, `sass_library`, and `npm_sass_library` from `@io_bazel_rules_sass`. - Updated hash version rules_sass from 1.55.0 to 1.69.5 - Replaces the angular_material_sass_deps target with a direct npm_sass_library call. This produces a SassInfo provider consumed via deps, complying with the rule that sass libraries must declare deps directly rather than passing one library to another's srcs. - Added `include_paths = ["external/npm/node_modules"]` to each sass_binary so dart-sass can resolve `@use '@angular/material'`. The old wrapper set this automatically, now it has to be set per target. ## Detailed steps to verify changes work correctly (as executed by you) - BUILD - Standalone running - No console errors
## Summary This PR updates TensorBoard’s local Bazel/protobuf build stack to stay compatible with the TensorFlow 2.21 ecosystem changes that this branch needs: - Bazel 6.5.0 -> 7.7.0 - protobuf build/runtime alignment to 6.31.1 - related Bazel repo-loading, packaging, and test fixes needed for local source builds and CI The goal of this PR is to keep the smallest working set of changes needed to make the branch build, test, and package correctly with the upgraded toolchain. ## What changed ### Bazel / workspace updates - update `.bazelversion` and Bazel version guards to 7.7.0 - update workspace dependencies and repository wiring needed for Bazel 7 - keep existing repo setup model (`WORKSPACE`) rather than switching this PR to bzlmod ### Protobuf 6.31.1 alignment - update the Bazel-side protobuf dependency to 6.31.1 - add the small compatibility shims/patches needed for protobuf’s Bazel build in this repo - regenerate/update checked-in generated artifacts where required ### rules_closure / Soy compatibility - keep TensorBoard’s existing Closure/Soy toolchain working with protobuf 6 - vendor `third_party/safe_html_types` as a build-time Java dependency override for the existing Soy toolchain - explicitly prevent `rules_closure_dependencies(...)` from re-introducing conflicting older transitive copies ### Packaging and test fixes - keep `tensorboard.compat` package exports working correctly in Bazel runfiles and wheel/runtime smoke tests - keep `test_pip_package` and local Bazel test flows working under the upgraded toolchain - reduce PR-specific drift where possible by reverting summary modules back toward upstream import structure ## Vendored code This PR vendors `third_party/safe_html_types`, which is the main true vendored code in this branch. Why it is needed: - TensorBoard still uses the Closure/Soy Java toolchain during Bazel builds - the older transitive safe-html-types classes pulled by that toolchain were not working for this protobuf 6.31.1 upgrade path - this vendored copy is used as a build-time dependency override so the branch can stay on the upgraded Bazel/protobuf stack Why this is safe: - it is build-time Java tooling support, not TensorBoard Python runtime logic - it is not intended to change TensorBoard user-facing behavior - it is scoped to keeping the existing Soy/Closure path working with protobuf 6 We investigated avoiding vendoring here, but the straightforward `http_archive` alternatives we tried did not work for this branch’s current dependency combination. ## Review-driven cleanup in this PR In response to review, this PR now also: - adds clearer comments for patches and Bazel helper/shim files - documents the vendored `safe_html_types` dependency more explicitly - keeps optional cleanup/refactor work out of scope where possible - simplifies some summary-module changes back toward upstream shape ## Validation Validated through the local/container workflow used for this branch, including: - Bazel repo loading / fetch - focused and full Bazel test runs - pip package build - `test_pip_package` smoke validation - TensorBoard import/runtime verification against TensorFlow 2.21 ## Follow-up work The following are better handled in follow-up PRs rather than expanding this one further: - CI/setup-python cleanup and container-specific Python setup simplification - any future move to bzlmod - further dependency-management cleanup in `third_party/repo.bzl` - splitting this large change into smaller independently reviewable PRs once the full working baseline is settled
## Summary This PR simplifies the Python setup used by the main Bazel `build` job in CI. Previously, the self-hosted/container-based build job relied on `actions/setup-python`, and the workflow had to manually copy Python headers into the hosted-toolcache layout so Bazel's `@system_python` repository could find `Python.h` correctly. This PR removes that workaround by using container-managed Python 3.10 directly inside the build job. The goal is to keep the same Bazel build/test/package behavior while making the CI setup simpler and easier to maintain. ## What changed - removed `actions/setup-python` from the main `build` job - installed Python 3.10, `python3.10-dev`, and `python3.10-venv` directly in the container - created a local virtualenv for the job and used that interpreter for pip/Bazel-related steps - removed the manual Python header-copy workaround - removed the temporary debug steps that were only needed to diagnose the hosted-toolcache/header mismatch - kept the rest of the build/test/package flow unchanged ## Validation This change has been validated against the same CI behaviors as before: - `bazel fetch //tensorboard/...` - `bazel build //tensorboard/...` - `bazel test //tensorboard/...` - `bazel test //tensorboard/... --test_tag_filters="support_notf"` - `bazel build //tensorboard/pip_package:test_pip_package` - `./bazel-bin/tensorboard/pip_package/test_pip_package --tf-version tf-nightly` - `./bazel-bin/tensorboard/pip_package/test_pip_package --tf-version notf`
## Summary This PR does a small round of Bazel 7 cleanup and dependency-wiring simplification on top of the already-working Bazel 7 / protobuf 6 baseline. The goal is not to change dependency versions or CI behavior, but to reduce migration-specific scaffolding where it is no longer needed, keep the remaining shims clearly documented, and preserve a clean, working build/test state. ## What changed ### Repository helper cleanup - simplified `third_party/repo.bzl` - removed unused `tb_http_archive` support for `build_file` / `link_files` - kept `tb_http_archive` focused on the behaviors still used in this repo: - mirrored URLs - optional patch application ### WORKSPACE cleanup - switched mirror-only repositories back to plain `http_archive` where the TensorBoard-specific helper was no longer adding value - kept `tb_http_archive(...)` only for repositories that still need: - patch application - repo mapping - clarified comments around the remaining Bazel/protobuf compatibility repos ### Compatibility shim review - verified that `third_party/compatibility_proxy` is still required by the current `rules_java` / protobuf loading path - kept it in place rather than removing a live dependency - verified that `third_party/protobuf_pip_deps` and `third_party/protobuf_pip_deps_setuptools` are still required by protobuf 6.31.1’s Bazel macros and Python packaging flow - improved documentation explaining why those repos still exist ## Why this PR is useful This PR keeps the Bazel 7 migration easier to maintain by: - removing dead helper code - narrowing custom repository logic to the cases that still need it - documenting the remaining compatibility shims with concrete rationale - avoiding behavior changes to the working build graph ## Validation Validated with: - `bazel fetch //tensorboard/...` - `bazel build //tensorboard/... --nobuild` - `bazel build //tensorboard/...` - `bazel test //tensorboard/plugins/debugger_v2:debugger_v2_plugin_test` - full local Bazel test runs after the cleanup changes
## Summary Fixes an arbitrary file read issue in the TensorBoard Projector plugin by restricting asset paths to the directory that contains `projector_config.pbtxt`. Previously, user-controlled fields such as `metadata_path`, `tensor_path`, `bookmarks_path`, and `sprite.image_path` could resolve to absolute paths or traversal paths outside the intended logdir/config directory. That allowed a malicious config to make TensorBoard read and return arbitrary local files from the host. ## What Changed - Hardened projector asset path resolution to: - expand and normalize candidate paths - resolve them against the directory containing `projector_config.pbtxt` - reject any path that escapes that directory boundary - Returned a clean `400` response when a requested asset path is invalid - Applied this validation consistently across: - metadata loading - tensor loading - bookmarks loading - sprite image loading - Updated config augmentation logic to safely skip invalid external tensor paths instead of trying to read them ## Security Impact This closes a path traversal / arbitrary local file read vector in the Projector plugin for deployments where an attacker can write or influence `projector_config.pbtxt` contents under a scanned logdir. ## Tests Added projector integration coverage for: - `metadata_path` using traversal outside the logdir - `tensor_path` using an absolute path outside the logdir - `bookmarks_path` using an absolute path outside the logdir - `sprite.image_path` using traversal outside the logdir ## Validation Verified: - `python -m py_compile tensorboard/plugins/projector/projector_plugin.py tensorboard/plugins/projector/projector_plugin_test.py` - `bazel test //tensorboard/plugins/projector:projector_plugin_test` - Full build and test suite ## Risk / Compatibility Low risk for valid configurations. This change may reject projector configs that previously referenced assets outside the config directory, but that behavior is now considered unsafe and is intentionally blocked.
# Conflicts: # WORKSPACE
…rd into feature/upgrade17-18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation for features / changes
This PR is the third step in a planned major Angular upgrade cycle, where each major version will be delivered in a separate PR, incrementally progressing until the project reaches Angular 21. This specific PR upgrades TensorBoard from Angular 17 to Angular 18
Technical description of changes
Detailed steps to verify changes work correctly (as executed by you)