Toolchainize twitter_scrooge (#1693)

mbland · web-flow · commit 21f70db35b86 · 2025-02-13T11:29:31.000+02:00
* Toolchainize twitter_scrooge This is the last of the toolchain to receive the "toolchainization" treatment prior to Bzlmodification, and moves `twitter_scrooge()` to `twitter_scrooge/toolchain/toolchain.bzl` for `rules_java` 8 compatibility. Part of #1482 and #1652. * Use twitter_scrooge args dict, fix propagation As requested by @simuons in #1693, `scala_toolchains` now receives `twitter_scrooge` options as a `dict`. Since all of these options are for alternative toolchain dependencies, I've called this new `dict` argument `twitter_scrooge_deps`. However, after looking closely at how the options propagated, I realized they never made it to the toolchain generated by `scala_toolchains`. So this change also passes these options all the way through to the generated `BUILD` file and through the `setup_scala_toolchain()` call. * Retry ./test_reproducibility.sh on macOS Seemingly spurious failure during `git checkout`: - https://buildkite.com/organizations/bazel/pipelines/rules-scala-scala/builds/5350/jobs/0194f660-94eb-464a-a81d-9d1c576c2968/log ```txt cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala $ cd /usr/local/var/bazelbuild ⚠️ Warning: Checkout failed! getting/updating git mirror: setting remote URL: exit status 71 (Attempt 1/3 Retrying in 2s) $ cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala $ cd /usr/local/var/bazelbuild ⚠️ Warning: Checkout failed! getting/updating git mirror: setting remote URL: exit status 71 (Attempt 2/3 Retrying in 2s) $ cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala $ cd /usr/local/var/bazelbuild ⚠️ Warning: Checkout failed! getting/updating git mirror: setting remote URL: exit status 71 (Attempt 3/3) $ cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala 🚨 Error: getting/updating git mirror: setting remote URL: exit status 71 ```
diff --git a/WORKSPACE b/WORKSPACE
@@ -54,6 +54,7 @@ scala_toolchains(
     scala_proto = True,
     scalafmt = True,
     testing = True,
+    twitter_scrooge = True,
 )
 
 register_toolchains(
@@ -62,10 +63,6 @@ register_toolchains(
     "@io_bazel_rules_scala_toolchains//...:all",
 )
 
-load("//twitter_scrooge:twitter_scrooge.bzl", "twitter_scrooge")
-
-twitter_scrooge()
-
 # needed for the cross repo proto test
 local_repository(
     name = "proto_cross_repo_boundary",
diff --git a/scala/toolchains.bzl b/scala/toolchains.bzl
@@ -15,8 +15,16 @@ load("//scalatest:scalatest.bzl", "scalatest_artifact_ids")
 load("//specs2:specs2.bzl", "specs2_artifact_ids")
 load("//specs2:specs2_junit.bzl", "specs2_junit_artifact_ids")
 load("//third_party/repositories:repositories.bzl", "repositories")
+load(
+    "//twitter_scrooge/toolchain:toolchain.bzl",
+    "twitter_scrooge_artifact_ids",
+    _TWITTER_SCROOGE_DEPS = "TOOLCHAIN_DEPS",
+)
 load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_VERSIONS")
 
+def _get_unknown_entries(entries, allowed_entries):
+    return [e for e in entries if e not in allowed_entries]
+
 def scala_toolchains(
         maven_servers = default_maven_server_urls(),
         overridden_artifacts = {},
@@ -33,7 +41,9 @@ def scala_toolchains(
         scalafmt_default_config_path = ".scalafmt.conf",
         scala_proto = False,
         scala_proto_enable_all_options = False,
-        jmh = False):
+        jmh = False,
+        twitter_scrooge = False,
+        twitter_scrooge_deps = {}):
     """Instantiates @io_bazel_rules_scala_toolchains and all its dependencies.
 
     Provides a unified interface to configuring rules_scala both directly in a
@@ -86,7 +96,23 @@ def scala_toolchains(
             toolchain with all options enabled; `scala_proto` must also be
             `True` for this to take effect
         jmh: whether to instantiate the jmh toolchain
+        twitter_scrooge: whether to instantiate the twitter_scrooge toolchain
+        twitter_scrooge_deps: dictionary of string to Label containing overrides
+            for twitter_scrooge toolchain dependency providers with keys:
+                libthrift
+                scrooge_core
+                scrooge_generator
+                util_core
+                util_logging
     """
+    unknown_ts_deps = _get_unknown_entries(
+        twitter_scrooge_deps,
+        _TWITTER_SCROOGE_DEPS,
+    )
+
+    if unknown_ts_deps:
+        fail("unknown twitter_scrooge_deps:", ", ".join(unknown_ts_deps))
+
     scala_repositories(
         maven_servers = maven_servers,
         # Note the internal macro parameter misspells "overriden".
@@ -130,6 +156,11 @@ def scala_toolchains(
             id: False
             for id in jmh_artifact_ids()
         })
+    if twitter_scrooge:
+        artifact_ids_to_fetch_sources.update({
+            id: False
+            for id in twitter_scrooge_artifact_ids(**twitter_scrooge_deps)
+        })
 
     for scala_version in SCALA_VERSIONS:
         version_specific_artifact_ids = {}
@@ -168,6 +199,8 @@ def scala_toolchains(
         scala_proto = scala_proto,
         scala_proto_enable_all_options = scala_proto_enable_all_options,
         jmh = jmh,
+        twitter_scrooge = twitter_scrooge,
+        twitter_scrooge_deps = twitter_scrooge_deps,
     )
 
 def scala_register_toolchains():
diff --git a/scala/toolchains_repo.bzl b/scala/toolchains_repo.bzl
@@ -35,6 +35,20 @@ def _generate_testing_toolchain_build_file_args(repo_attr):
         "specs2_junit": framework_deps.get("specs2_junit"),
     }
 
+_TWITTER_SCROOGE_ARGS = [
+    "libthrift",
+    "scrooge_core",
+    "scrooge_generator",
+    "util_core",
+    "util_logging",
+]
+
+def _stringify_template_args(args, arg_names):
+    return {
+        arg: ("\"%s\"" % value if type(value) == "string" else value)
+        for arg, value in {name: args.get(name) for name in arg_names}.items()
+    }
+
 def _scala_toolchains_repo_impl(repository_ctx):
     repo_attr = repository_ctx.attr
     format_args = {
@@ -49,6 +63,12 @@ def _scala_toolchains_repo_impl(repository_ctx):
         toolchains["scala_proto"] = _SCALA_PROTO_TOOLCHAIN_BUILD
     if repo_attr.jmh:
         toolchains["jmh"] = _JMH_TOOLCHAIN_BUILD
+    if repo_attr.twitter_scrooge:
+        toolchains["twitter_scrooge"] = _TWITTER_SCROOGE_TOOLCHAIN_BUILD
+        format_args.update(_stringify_template_args(
+            repo_attr.twitter_scrooge_deps,
+            _TWITTER_SCROOGE_ARGS,
+        ))
 
     testing_build_args = _generate_testing_toolchain_build_file_args(repo_attr)
     if testing_build_args != None:
@@ -81,6 +101,9 @@ _scala_toolchains_repo = repository_rule(
         "scala_proto": attr.bool(),
         "scala_proto_enable_all_options": attr.bool(),
         "jmh": attr.bool(),
+        "twitter_scrooge": attr.bool(),
+        # attr.string_keyed_label_dict isn't available in Bazel 6
+        "twitter_scrooge_deps": attr.string_dict(),
     },
 )
 
@@ -210,3 +233,19 @@ load("@@{rules_scala_repo}//jmh/toolchain:toolchain.bzl", "setup_jmh_toolchain")
 
 setup_jmh_toolchain(name = "jmh_toolchain")
 """
+
+_TWITTER_SCROOGE_TOOLCHAIN_BUILD = """
+load(
+    "@@{rules_scala_repo}//twitter_scrooge/toolchain:toolchain.bzl",
+    "setup_scrooge_toolchain",
+)
+
+setup_scrooge_toolchain(
+    name = "scrooge_toolchain",
+    libthrift = {libthrift},
+    scrooge_core = {scrooge_core},
+    scrooge_generator = {scrooge_generator},
+    util_core = {util_core},
+    util_logging = {util_logging},
+)
+"""
diff --git a/test_version.sh b/test_version.sh
@@ -46,7 +46,7 @@ run_in_test_repo() {
 
   if [[ -n "$TWITTER_SCROOGE_VERSION" ]]; then
     local version_param="version = \"$TWITTER_SCROOGE_VERSION\""
-    scrooge_ws="scrooge_repositories($version_param)"
+    scrooge_ws="$version_param"
   fi
 
   sed -e "s%\${twitter_scrooge_repositories}%${scrooge_ws}\n%" \
diff --git a/test_version/WORKSPACE.template b/test_version/WORKSPACE.template
@@ -54,6 +54,10 @@ scala_config(enable_compiler_dependency_tracking = True)
 
 load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")
 
+load(":scrooge_repositories.bzl", "scrooge_repositories")
+
+scrooge_repositories(${twitter_scrooge_repositories})
+
 scala_toolchains(
     fetch_sources = True,
     scala_proto = True,
@@ -63,11 +67,6 @@ scala_toolchains(
 
 register_toolchains(
     "@io_bazel_rules_scala//scala:unused_dependency_checker_error_toolchain",
-    "@io_bazel_rules_scala//testing:testing_toolchain",
+    "@twitter_scrooge_test_toolchain//...:all",
     "@io_bazel_rules_scala_toolchains//...:all",
 )
-
-load(":scrooge_repositories.bzl", "scrooge_repositories")
-${twitter_scrooge_repositories}
-load("@io_bazel_rules_scala//twitter_scrooge:twitter_scrooge.bzl", "twitter_scrooge")
-twitter_scrooge()
diff --git a/test_version/version_specific_tests_dir/scrooge_repositories.bzl b/test_version/version_specific_tests_dir/scrooge_repositories.bzl
@@ -1,15 +1,24 @@
+load(
+    "@io_bazel_rules_scala//scala:scala_cross_version.bzl",
+    "default_maven_server_urls",
+)
 load(
     "@io_bazel_rules_scala//scala:scala_maven_import_external.bzl",
     _scala_maven_import_external = "scala_maven_import_external",
 )
 load(
-    "@io_bazel_rules_scala//scala:scala_cross_version.bzl",
-    "default_maven_server_urls",
+    "@io_bazel_rules_scala//scala:toolchains_repo.bzl",
+    "scala_toolchains_repo",
+)
+load(
+    "@io_bazel_rules_scala//twitter_scrooge/toolchain:toolchain.bzl",
+    "twitter_scrooge",
 )
 
 def _import_external(id, artifact, sha256, deps = [], runtime_deps = []):
     _scala_maven_import_external(
         name = id,
+        generated_rule_name = id,
         artifact = artifact,
         artifact_sha256 = sha256,
         licenses = ["notice"],
@@ -20,8 +29,11 @@ def _import_external(id, artifact, sha256, deps = [], runtime_deps = []):
         fetch_sources = False,
     )
 
-def scrooge_repositories(version):
+def scrooge_repositories(version = None):
+    use_custom_toolchain_deps = False
+
     if version == "18.6.0":
+        use_custom_toolchain_deps = True
         _import_external(
             id = "io_bazel_rules_scala_scrooge_core",
             artifact = "com.twitter:scrooge-core_2.11:18.6.0",
@@ -48,7 +60,8 @@ def scrooge_repositories(version):
             sha256 = "73ddd61cedabd4dab82b30e6c52c1be6c692b063b8ba310d716ead9e3b4e9267",
         )
 
-    if version == "21.2.0":
+    elif version == "21.2.0":
+        use_custom_toolchain_deps = True
         _import_external(
             id = "io_bazel_rules_scala_scrooge_core",
             artifact = "com.twitter:scrooge-core_2.11:21.2.0",
@@ -74,3 +87,20 @@ def scrooge_repositories(version):
             artifact = "com.twitter:util-logging_2.11:21.2.0",
             sha256 = "f3b62465963fbf0fe9860036e6255337996bb48a1a3f21a29503a2750d34f319",
         )
+
+    toolchain_deps = {} if use_custom_toolchain_deps == False else {
+        dep: "@io_bazel_rules_scala_%s" % dep
+        for dep in [
+            "scrooge_core",
+            "scrooge_generator",
+            "util_core",
+            "util_logging",
+        ]
+    }
+
+    twitter_scrooge(register_toolchains = False, **toolchain_deps)
+    scala_toolchains_repo(
+        name = "twitter_scrooge_test_toolchain",
+        twitter_scrooge = True,
+        twitter_scrooge_deps = toolchain_deps,
+    )
diff --git a/test_version/version_specific_tests_dir/twitter_scrooge/BUILD b/test_version/version_specific_tests_dir/twitter_scrooge/BUILD
@@ -8,7 +8,7 @@ dependencies_to_test = [
 deps_with_external_binds = [
     (
         dep_name,
-        "//external:io_bazel_rules_scala/dependency/thrift/{}".format(dep_name),
+        "@io_bazel_rules_scala_{}".format(dep_name),
     )
     for dep_name in dependencies_to_test
 ]
diff --git a/twitter_scrooge/BUILD b/twitter_scrooge/BUILD
@@ -1,85 +1,14 @@
-load("//twitter_scrooge/toolchain:toolchain.bzl", "export_scrooge_deps", "scrooge_toolchain")
-load("//scala:providers.bzl", "declare_deps_provider")
-
-scrooge_toolchain(
-    name = "scrooge_toolchain_impl",
-    visibility = ["//visibility:public"],
-)
-
-toolchain(
-    name = "scrooge_toolchain",
-    toolchain = ":scrooge_toolchain_impl",
-    toolchain_type = "@io_bazel_rules_scala//twitter_scrooge/toolchain:scrooge_toolchain_type",
-    visibility = ["//visibility:public"],
-)
-
-declare_deps_provider(
-    name = "aspect_compile_classpath_provider",
-    deps_id = "aspect_compile_classpath",
-    visibility = ["//visibility:public"],
-    deps = [
-        "//external:io_bazel_rules_scala/dependency/thrift/javax_annotation_api",
-        "//external:io_bazel_rules_scala/dependency/thrift/libthrift",
-        "//external:io_bazel_rules_scala/dependency/thrift/scrooge_core",
-        "//external:io_bazel_rules_scala/dependency/thrift/util_core",
-        "//scala/private/toolchain_deps:scala_library_classpath",
-    ],
-)
-
-declare_deps_provider(
-    name = "compile_classpath_provider",
-    deps_id = "compile_classpath",
-    visibility = ["//visibility:public"],
-    deps = [
-        "//external:io_bazel_rules_scala/dependency/thrift/libthrift",
-        "//external:io_bazel_rules_scala/dependency/thrift/scrooge_core",
-        "//scala/private/toolchain_deps:scala_library_classpath",
-    ],
-)
-
-declare_deps_provider(
-    name = "scrooge_generator_classpath_provider",
-    deps_id = "scrooge_generator_classpath",
-    visibility = ["//visibility:public"],
-    deps = [
-        "//external:io_bazel_rules_scala/dependency/thrift/scrooge_generator",
-    ],
-)
-
-declare_deps_provider(
-    name = "compiler_classpath_provider",
-    deps_id = "compiler_classpath",
-    visibility = ["//visibility:public"],
-    deps = [
-        "//external:io_bazel_rules_scala/dependency/thrift/mustache",
-        "//external:io_bazel_rules_scala/dependency/thrift/scopt",
-        "//external:io_bazel_rules_scala/dependency/thrift/scrooge_generator",
-        "//external:io_bazel_rules_scala/dependency/thrift/util_core",
-        "//external:io_bazel_rules_scala/dependency/thrift/util_logging",
-        "//scala/private/toolchain_deps:parser_combinators",
-    ],
-)
-
-export_scrooge_deps(
-    name = "compile_classpath",
-    deps_id = "compile_classpath",
-    visibility = ["//visibility:public"],
-)
-
-export_scrooge_deps(
-    name = "aspect_compile_classpath",
-    deps_id = "aspect_compile_classpath",
-    visibility = ["//visibility:public"],
-)
-
-export_scrooge_deps(
-    name = "scrooge_generator_classpath",
-    deps_id = "scrooge_generator_classpath",
-    visibility = ["//visibility:public"],
-)
-
-export_scrooge_deps(
-    name = "compiler_classpath",
-    deps_id = "compiler_classpath",
-    visibility = ["//visibility:public"],
-)
+load(
+    "//twitter_scrooge/toolchain:toolchain.bzl",
+    "DEP_PROVIDERS",
+    "export_scrooge_deps",
+)
+
+[
+    export_scrooge_deps(
+        name = dep,
+        deps_id = dep,
+        visibility = ["//visibility:public"],
+    )
+    for dep in DEP_PROVIDERS
+]
diff --git a/twitter_scrooge/toolchain/toolchain.bzl b/twitter_scrooge/toolchain/toolchain.bzl
diff --git a/twitter_scrooge/twitter_scrooge.bzl b/twitter_scrooge/twitter_scrooge.bzl

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ dependencies_to_test = [`
`8`	`8`	`deps_with_external_binds = [`
`9`	`9`	`(`
`10`	`10`	`dep_name,`
`11`		`- "//external:io_bazel_rules_scala/dependency/thrift/{}".format(dep_name),`
	`11`	`+ "@io_bazel_rules_scala_{}".format(dep_name),`
`12`	`12`	`)`
`13`	`13`	`for dep_name in dependencies_to_test`
`14`	`14`	`]`