From 12443ea7395c9ddc8e36314a8cc92253e2d07d26 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 6 Jul 2021 15:39:58 +0200 Subject: [PATCH] build: remove `skydoc` and `rules_sass` from repository (#42760) Skydoc is no longer used as `@angular/bazel` is no longer a public API. The Sass rules were only used in a single place in the repo where Sass is not really needed and has just been added by accident most likely. We want to remove the Sass dependency in preparation for Rules NodeJS v4.x where the Sass rules currently still use an older version of `@bazel/worker` that is incompatible. PR Close #42760 --- .circleci/config.yml | 4 +- WORKSPACE | 15 ------ integration/bazel/src/hello-world/BUILD.bazel | 4 +- .../benchmarks/src/class_bindings/BUILD.bazel | 10 +--- .../{styles.scss => styles.css} | 0 packages/bazel/package.bzl | 51 ------------------- packages/bazel/src/BUILD.bazel | 7 +-- packages/bazel/src/ng_module.bzl | 10 +--- packages/bazel/src/ng_package/ng_package.bzl | 12 ++--- 9 files changed, 15 insertions(+), 98 deletions(-) rename modules/benchmarks/src/class_bindings/{styles.scss => styles.css} (100%) delete mode 100644 packages/bazel/package.bzl diff --git a/.circleci/config.yml b/.circleci/config.yml index 2997e4006e..91e3214cad 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -24,14 +24,14 @@ version: 2.1 # **NOTE 2 **: If you change the cache key prefix, also sync the cache_key_fallback to match. # **NOTE 3 **: Keep the static part of the cache key as prefix to enable correct fallbacks. # See https://circleci.com/docs/2.0/caching/#restoring-cache for how prefixes work in CircleCI. -var_3: &cache_key v4-angular-node-14-{{ checksum "month.txt" }}-{{ checksum ".bazelversion" }}-{{ checksum "yarn.lock" }}-{{ checksum "WORKSPACE" }}-{{ checksum "packages/bazel/package.bzl" }}-{{ checksum "aio/yarn.lock" }} +var_3: &cache_key v4-angular-node-14-{{ checksum "month.txt" }}-{{ checksum ".bazelversion" }}-{{ checksum "yarn.lock" }}-{{ checksum "WORKSPACE" }}-{{ checksum "aio/yarn.lock" }} # We invalidate the cache if the Bazel version changes because otherwise the `bazelisk` cache # folder will contain all previously used versions and ultimately cause the cache restoring to # be slower due to its growing size. var_4: &cache_key_fallback v4-angular-node-14-{{ checksum "month.txt" }}-{{ checksum ".bazelversion" }} # Windows needs its own cache key because binaries in node_modules are different. -var_3_win: &cache_key_win v4-angular-win-node-14-{{ checksum "month.txt" }}-{{ checksum ".bazelversion" }}-{{ checksum "yarn.lock" }}-{{ checksum "WORKSPACE" }}-{{ checksum "packages/bazel/package.bzl" }}-{{ checksum "aio/yarn.lock" }} +var_3_win: &cache_key_win v4-angular-win-node-14-{{ checksum "month.txt" }}-{{ checksum ".bazelversion" }}-{{ checksum "yarn.lock" }}-{{ checksum "WORKSPACE" }}-{{ checksum "aio/yarn.lock" }} var_4_win: &cache_key_win_fallback v4-angular-win-node-14-{{ checksum "month.txt" }}-{{ checksum ".bazelversion" }} # Cache key for the `components-repo-unit-tests` job. **Note** when updating the SHA in the diff --git a/WORKSPACE b/WORKSPACE index 429b8ab86a..6c77e2d931 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -34,11 +34,6 @@ yarn_install( yarn_lock = "//:yarn.lock", ) -# Load angular dependencies -load("//packages/bazel:package.bzl", "rules_angular_dev_dependencies") - -rules_angular_dev_dependencies() - # Load protractor dependencies load("@npm//@bazel/protractor:package.bzl", "npm_bazel_protractor_dependencies") @@ -52,13 +47,3 @@ web_test_repositories() load("//dev-infra/bazel/browsers:browser_repositories.bzl", "browser_repositories") browser_repositories() - -# Setup the rules_sass toolchain -load("@io_bazel_rules_sass//:defs.bzl", "sass_repositories") - -sass_repositories() - -# Setup the skydoc toolchain -load("@io_bazel_skydoc//skylark:skylark.bzl", "skydoc_repositories") - -skydoc_repositories() diff --git a/integration/bazel/src/hello-world/BUILD.bazel b/integration/bazel/src/hello-world/BUILD.bazel index b8f506142f..45c2f809ab 100644 --- a/integration/bazel/src/hello-world/BUILD.bazel +++ b/integration/bazel/src/hello-world/BUILD.bazel @@ -1,10 +1,10 @@ -package(default_visibility = ["//visibility:public"]) - load("@npm//@bazel/concatjs:index.bzl", "karma_web_test_suite") load("@io_bazel_rules_sass//:defs.bzl", "sass_binary") load("@npm//@angular/bazel:index.bzl", "ng_package") load("//tools:ng_ts_library.bzl", "ng_ts_library") +package(default_visibility = ["//visibility:public"]) + sass_binary( name = "hello-world-styles", src = "hello-world.component.scss", diff --git a/modules/benchmarks/src/class_bindings/BUILD.bazel b/modules/benchmarks/src/class_bindings/BUILD.bazel index 9832ec19ce..8d727306bd 100644 --- a/modules/benchmarks/src/class_bindings/BUILD.bazel +++ b/modules/benchmarks/src/class_bindings/BUILD.bazel @@ -1,12 +1,6 @@ -package(default_visibility = ["//modules/benchmarks:__subpackages__"]) - -load("@io_bazel_rules_sass//:defs.bzl", "sass_binary") load("//dev-infra/benchmark/component_benchmark:component_benchmark.bzl", "component_benchmark") -sass_binary( - name = "class_bindings_styles", - src = ":styles.scss", -) +package(default_visibility = ["//modules/benchmarks:__subpackages__"]) component_benchmark( name = "benchmark", @@ -28,5 +22,5 @@ component_benchmark( exclude = ["**/*.perf-spec.ts"], ), prefix = "", - styles = [":class_bindings_styles"], + styles = ["styles.css"], ) diff --git a/modules/benchmarks/src/class_bindings/styles.scss b/modules/benchmarks/src/class_bindings/styles.css similarity index 100% rename from modules/benchmarks/src/class_bindings/styles.scss rename to modules/benchmarks/src/class_bindings/styles.css diff --git a/packages/bazel/package.bzl b/packages/bazel/package.bzl deleted file mode 100644 index 64bad9c349..0000000000 --- a/packages/bazel/package.bzl +++ /dev/null @@ -1,51 +0,0 @@ -# Copyright Google LLC All Rights Reserved. -# -# Use of this source code is governed by an MIT-style license that can be -# found in the LICENSE file at https://angular.io/license - -"""Package file which defines dependencies of Angular rules in skylark -""" - -load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") - -def rules_angular_dependencies(): - print("""DEPRECATION WARNING: - rules_angular_dependencies is no longer needed, and will be removed in a future release. - We assume you will fetch rules_nodejs in your WORKSPACE file, and no other dependencies remain here. - Simply remove any calls to this function and the corresponding call to - load("@angular//:package.bzl", "rules_angular_dependencies") - """) - -def rules_angular_dev_dependencies(): - """ - Fetch dependencies needed for local development, but not needed by users. - - These are in this file to keep version information in one place, and make the WORKSPACE - shorter. - """ - - ############################################# - # Dependencies for generating documentation # - ############################################# - _maybe( - http_archive, - name = "io_bazel_rules_sass", - sha256 = "596ab3616d370135e0ecc710e103422e0aa3719f1c970303a0886b70c81ee819", - strip_prefix = "rules_sass-1.32.2", - urls = [ - "https://github.com/bazelbuild/rules_sass/archive/1.32.2.zip", - "https://mirror.bazel.build/github.com/bazelbuild/rules_sass/archive/1.32.2.zip", - ], - ) - - http_archive( - name = "io_bazel_skydoc", - sha256 = "f88058b43112e9bdc7fdb0abbdc17c5653268708c01194a159641119195e45c6", - strip_prefix = "skydoc-a9550cb3ca3939cbabe3b589c57b6f531937fa99", - # TODO: switch to upstream when https://github.com/bazelbuild/skydoc/pull/103 is merged - url = "https://github.com/alexeagle/skydoc/archive/a9550cb3ca3939cbabe3b589c57b6f531937fa99.zip", - ) - -def _maybe(repo_rule, name, **kwargs): - if name not in native.existing_rules(): - repo_rule(name = name, **kwargs) diff --git a/packages/bazel/src/BUILD.bazel b/packages/bazel/src/BUILD.bazel index 4b5587150a..5a5ec0b009 100644 --- a/packages/bazel/src/BUILD.bazel +++ b/packages/bazel/src/BUILD.bazel @@ -1,3 +1,5 @@ +load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary") + # BEGIN-DEV-ONLY package(default_visibility = ["//packages/bazel:__subpackages__"]) @@ -6,11 +8,6 @@ filegroup( srcs = glob(["*"]), ) -# For generating skydoc -exports_files(glob(["*.bzl"])) - -load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary") - nodejs_binary( name = "modify_tsconfig", data = [ diff --git a/packages/bazel/src/ng_module.bzl b/packages/bazel/src/ng_module.bzl index c8551e83a3..c5a295f7be 100644 --- a/packages/bazel/src/ng_module.bzl +++ b/packages/bazel/src/ng_module.bzl @@ -693,19 +693,11 @@ def _ng_module_impl(ctx): return ts_providers_dict_to_struct(ts_providers) -local_deps_aspects = [node_modules_aspect, _collect_summaries_aspect] - -# Workaround skydoc bug which assumes DEPS_ASPECTS is a str type -[local_deps_aspects.append(a) for a in DEPS_ASPECTS] - NG_MODULE_ATTRIBUTES = { "srcs": attr.label_list(allow_files = [".ts"]), - - # Note: DEPS_ASPECTS is already a list, we add the cast to workaround - # https://github.com/bazelbuild/skydoc/issues/21 "deps": attr.label_list( doc = "Targets that are imported by this target", - aspects = local_deps_aspects, + aspects = [node_modules_aspect, _collect_summaries_aspect] + DEPS_ASPECTS, ), "assets": attr.label_list( doc = ".html and .css files needed by the Angular compiler", diff --git a/packages/bazel/src/ng_package/ng_package.bzl b/packages/bazel/src/ng_package/ng_package.bzl index 310043dc35..011cebcbd5 100644 --- a/packages/bazel/src/ng_package/ng_package.bzl +++ b/packages/bazel/src/ng_package/ng_package.bzl @@ -125,10 +125,6 @@ WELL_KNOWN_GLOBALS = {p: _global_name(p) for p in [ "rxjs/operators", ]} -# skydoc fails with type(depset()) so using "depset" here instead -# TODO(gregmagolan): clean this up -_DEPSET_TYPE = "depset" - def _compute_node_modules_root(ctx): """Computes the node_modules root from the node_modules and deps attributes. @@ -282,7 +278,11 @@ def _flatten_paths(directory): # Optionally can filter out files that do not belong to a specified package path. def _filter_out_generated_files(files, extension, package_path = None): result = [] - files_list = files.to_list() if type(files) == _DEPSET_TYPE else files + + # If `files` is a depset, convert it to a list. Note that we do not compare the type + # of files against a literal as per best practices within Bazel Starlark. + # https://docs.bazel.build/versions/main/skylark/lib/globals.html#type. + files_list = files.to_list() if type(files) == type(depset()) else files for file in files_list: # If the "package_path" parameter has been specified, filter out files # that do not start with the specified package path. @@ -298,7 +298,7 @@ def _filter_out_generated_files(files, extension, package_path = None): return depset(result) def _filter_js_inputs(all_inputs): - all_inputs_list = all_inputs.to_list() if type(all_inputs) == _DEPSET_TYPE else all_inputs + all_inputs_list = all_inputs.to_list() if type(all_inputs) == type(depset()) else all_inputs return [ f for f in all_inputs_list