Skip to content

Toolchainize //scala/scalafmt:scalafmt_toolchain #1678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ load("//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(
fetch_sources = True,
scalafmt = True,
testing = True,
)

Expand All @@ -71,12 +72,6 @@ load("//scala_proto:scala_proto.bzl", "scala_proto_repositories")

scala_proto_repositories()

load("//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_default_config", "scalafmt_repositories")

scalafmt_default_config()

scalafmt_repositories()

# needed for the cross repo proto test
local_repository(
name = "proto_cross_repo_boundary",
Expand Down
8 changes: 4 additions & 4 deletions scala/scalafmt/BUILD
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
load("//scala/scalafmt/toolchain:setup_scalafmt_toolchain.bzl", "setup_scalafmt_toolchains")
load("//scala/scalafmt/toolchain:toolchain.bzl", "export_scalafmt_deps")
load("//scala/scalafmt:phase_scalafmt_ext.bzl", "scalafmt_singleton")
load("//scala:scala.bzl", "scala_binary")
Expand Down Expand Up @@ -37,12 +36,13 @@ scalafmt_singleton(
visibility = ["//visibility:public"],
)

setup_scalafmt_toolchains()

# Alias for backward compatibility:
alias(
name = "scalafmt_toolchain",
actual = "scalafmt_toolchain" + version_suffix(SCALA_VERSION),
actual = (
"@io_bazel_rules_scala_toolchains//scalafmt:scalafmt_toolchain" +
version_suffix(SCALA_VERSION)
),
)

export_scalafmt_deps(
Expand Down
8 changes: 4 additions & 4 deletions scala/scalafmt/phase_scalafmt_ext.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ ext_scalafmt = {
),
"_fmt": attr.label(
cfg = "exec",
default = "//scala/scalafmt",
default = Label("//scala/scalafmt"),
executable = True,
),
"_java_host_runtime": attr.label(
Expand All @@ -30,19 +30,19 @@ ext_scalafmt = {
),
"_runner": attr.label(
allow_single_file = True,
default = "//scala/scalafmt:runner",
default = Label("//scala/scalafmt:runner"),
),
"_testrunner": attr.label(
allow_single_file = True,
default = "//scala/scalafmt:testrunner",
default = Label("//scala/scalafmt:testrunner"),
),
},
"outputs": {
"scalafmt_runner": "%{name}.format",
"scalafmt_testrunner": "%{name}.format-test",
},
"phase_providers": [
"//scala/scalafmt:phase_scalafmt",
Label("//scala/scalafmt:phase_scalafmt"),
],
}

Expand Down
20 changes: 6 additions & 14 deletions scala/scalafmt/scalafmt_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ load(
"version_suffix",
_default_maven_server_urls = "default_maven_server_urls",
)
load(
"//scala_proto/default:repositories.bzl",
"SCALAPB_COMPILE_ARTIFACT_IDS",
)
load("//third_party/repositories:repositories.bzl", "repositories")
load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_VERSIONS")

Expand Down Expand Up @@ -32,22 +36,16 @@ def scalafmt_default_config(path = ".scalafmt.conf", **kwargs):
_SCALAFMT_DEPS = [
"com_geirsson_metaconfig_core",
"com_geirsson_metaconfig_typesafe_config",
"com_google_protobuf_protobuf_java",
"com_lihaoyi_fansi",
"com_lihaoyi_fastparse",
"com_lihaoyi_sourcecode",
"com_typesafe_config",
"org_scala_lang_modules_scala_collection_compat",
"org_scala_lang_scalap",
"org_scalameta_common",
"org_scalameta_parsers",
"org_scalameta_scalafmt_core",
"org_scalameta_scalameta",
"org_scalameta_trees",
"org_typelevel_paiges_core",
"scala_proto_rules_scalapb_lenses",
"scala_proto_rules_scalapb_runtime",
]
] + SCALAPB_COMPILE_ARTIFACT_IDS

_SCALAFMT_DEPS_2_11 = [
"com_lihaoyi_pprint",
Expand Down Expand Up @@ -79,8 +77,7 @@ def scalafmt_artifact_ids(scala_version):

def scalafmt_repositories(
maven_servers = _default_maven_server_urls(),
overriden_artifacts = {},
bzlmod_enabled = False):
overriden_artifacts = {}):
for scala_version in SCALA_VERSIONS:
repositories(
scala_version = scala_version,
Expand All @@ -89,11 +86,6 @@ def scalafmt_repositories(
overriden_artifacts = overriden_artifacts,
)

if not bzlmod_enabled:
_register_scalafmt_toolchains()

def _register_scalafmt_toolchains():
for scala_version in SCALA_VERSIONS:
native.register_toolchains(str(Label(
"//scala/scalafmt:scalafmt_toolchain" +
version_suffix(scala_version),
Expand Down
10 changes: 6 additions & 4 deletions scala/scalafmt/toolchain/setup_scalafmt_toolchain.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
load("//scala/scalafmt/toolchain:toolchain.bzl", "scalafmt_toolchain")
load(
"//scala/scalafmt/toolchain:toolchain.bzl",
"SCALAFMT_TOOLCHAIN_TYPE",
"scalafmt_toolchain",
)
load("//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_artifact_ids")
load("//scala:providers.bzl", "declare_deps_provider")
load("//scala:scala_cross_version.bzl", "version_suffix")
Expand Down Expand Up @@ -28,9 +32,7 @@ def setup_scalafmt_toolchain(
version_suffix(scala_version),
],
toolchain = ":%s_impl" % name,
toolchain_type = Label(
"//scala/scalafmt/toolchain:scalafmt_toolchain_type",
),
toolchain_type = SCALAFMT_TOOLCHAIN_TYPE,
visibility = visibility,
)

Expand Down
18 changes: 7 additions & 11 deletions scala/scalafmt/toolchain/toolchain.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
load("@io_bazel_rules_scala//scala:providers.bzl", _DepsInfo = "DepsInfo")
load("//scala/private/toolchain_deps:toolchain_deps.bzl", "expose_toolchain_deps")

SCALAFMT_TOOLCHAIN_TYPE = Label(
"//scala/scalafmt/toolchain:scalafmt_toolchain_type",
)

def _scalafmt_toolchain_impl(ctx):
toolchain = platform_common.ToolchainInfo(
dep_providers = ctx.attr.dep_providers,
Expand All @@ -10,20 +14,12 @@ def _scalafmt_toolchain_impl(ctx):
scalafmt_toolchain = rule(
_scalafmt_toolchain_impl,
attrs = {
"dep_providers": attr.label_list(
default = [
"@io_bazel_rules_scala//scala/scalafmt:scalafmt_classpath_provider",
],
providers = [_DepsInfo],
),
"dep_providers": attr.label_list(providers = [_DepsInfo]),
},
)

def _export_scalafmt_deps_impl(ctx):
return expose_toolchain_deps(
ctx,
"@io_bazel_rules_scala//scala/scalafmt/toolchain:scalafmt_toolchain_type",
)
return expose_toolchain_deps(ctx, SCALAFMT_TOOLCHAIN_TYPE)

export_scalafmt_deps = rule(
_export_scalafmt_deps_impl,
Expand All @@ -32,6 +28,6 @@ export_scalafmt_deps = rule(
mandatory = True,
),
},
toolchains = ["@io_bazel_rules_scala//scala/scalafmt/toolchain:scalafmt_toolchain_type"],
toolchains = [SCALAFMT_TOOLCHAIN_TYPE],
incompatible_use_toolchain_transition = True,
)
24 changes: 22 additions & 2 deletions scala/toolchains.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@

load("//junit:junit.bzl", "junit_artifact_ids")
load("//scala/private:macros/scala_repositories.bzl", "scala_repositories")
load("//scala:toolchains_repo.bzl", "scala_toolchains_repo")
load(
"//scala/scalafmt:scalafmt_repositories.bzl",
"scalafmt_artifact_ids",
"scalafmt_default_config",
)
load("//scala:scala_cross_version.bzl", "default_maven_server_urls")
load("//scala:toolchains_repo.bzl", "scala_toolchains_repo")
load("//scalatest:scalatest.bzl", "scalatest_artifact_ids")
load("//specs2:specs2.bzl", "specs2_artifact_ids")
load("//specs2:specs2_junit.bzl", "specs2_junit_artifact_ids")
Expand All @@ -21,7 +26,9 @@ def scala_toolchains(
scalatest = False,
junit = False,
specs2 = False,
testing = False):
testing = False,
scalafmt = False,
scalafmt_default_config_path = ".scalafmt.conf"):
"""Instantiates @io_bazel_rules_scala_toolchains and all its dependencies.

Provides a unified interface to configuring rules_scala both directly in a
Expand Down Expand Up @@ -66,6 +73,9 @@ def scala_toolchains(
specs2: whether to instantiate the Specs2 JUnit toolchain
testing: whether to instantiate the Scalatest, JUnit, and Specs2 JUnit
toolchains combined
scalafmt: whether to instantiate the Scalafmt toolchain
scalafmt_default_config_path: the relative path to the default Scalafmt
config file within the repository
"""
scala_repositories(
maven_servers = maven_servers,
Expand All @@ -78,6 +88,9 @@ def scala_toolchains(
scala_compiler_srcjars = scala_compiler_srcjars,
)

if scalafmt:
scalafmt_default_config(scalafmt_default_config_path)

if testing:
scalatest = True
junit = True
Expand Down Expand Up @@ -106,6 +119,12 @@ def scala_toolchains(
for scala_version in SCALA_VERSIONS:
version_specific_artifact_ids = {}

if scalafmt:
version_specific_artifact_ids.update({
id: fetch_sources
for id in scalafmt_artifact_ids(scala_version)
})

all_artifacts = (
artifact_ids_to_fetch_sources | version_specific_artifact_ids
)
Expand All @@ -125,6 +144,7 @@ def scala_toolchains(
junit = junit,
specs2 = specs2,
testing = testing,
scalafmt = scalafmt,
)

def scala_register_toolchains():
Expand Down
13 changes: 13 additions & 0 deletions scala/toolchains_repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ def _scala_toolchains_repo_impl(repository_ctx):
format_args.update(testing_build_args)
toolchains["testing"] = _TESTING_TOOLCHAIN_BUILD

if repo_attr.scalafmt:
toolchains["scalafmt"] = _SCALAFMT_TOOLCHAIN_BUILD

if len(toolchains) == 0:
fail("no toolchains specified")

Expand All @@ -69,6 +72,7 @@ _scala_toolchains_repo = repository_rule(
"junit": attr.bool(),
"specs2": attr.bool(),
"testing": attr.bool(),
"scalafmt": attr.bool(),
},
)

Expand Down Expand Up @@ -143,3 +147,12 @@ load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_VERSIONS")
for scala_version in SCALA_VERSIONS
]
"""

_SCALAFMT_TOOLCHAIN_BUILD = """
load(
"@@{rules_scala_repo}//scala/scalafmt/toolchain:setup_scalafmt_toolchain.bzl",
"setup_scalafmt_toolchains",
)

setup_scalafmt_toolchains()
"""
4 changes: 2 additions & 2 deletions test/shell/test_scala_library.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ test_scala_library_expect_failure_on_missing_direct_internal_deps() {
}

test_scala_library_expect_failure_on_missing_direct_external_deps_jar() {
dependenecy_target='@com_google_guava_guava_21_0//:com_google_guava_guava_21_0'
dependenecy_target='@[a-z_.~+-]*com_google_guava_guava_21_0//:com_google_guava_guava_21_0'
test_target='test_expect_failure/missing_direct_deps/external_deps:transitive_external_dependency_user'

test_scala_library_expect_failure_on_missing_direct_deps $dependenecy_target $test_target
}

test_scala_library_expect_failure_on_missing_direct_external_deps_file_group() {
dependenecy_target='@com_google_guava_guava_21_0_with_file//:com_google_guava_guava_21_0_with_file'
dependenecy_target='@[a-z_.~+-]*com_google_guava_guava_21_0_with_file//:com_google_guava_guava_21_0_with_file'
test_target='test_expect_failure/missing_direct_deps/external_deps:transitive_external_dependency_user_file_group'

test_scala_library_expect_failure_on_missing_direct_deps $dependenecy_target $test_target
Expand Down
2 changes: 1 addition & 1 deletion test/shell/test_strict_dependency.sh
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ test_strict_deps_filter_excluded_target() {

test_strict_deps_filter_included_target() {
local test_target="//test_expect_failure/missing_direct_deps/filtering:b"
local expected_message="buildozer 'add deps @com_google_guava_guava_21_0//:com_google_guava_guava_21_0' ${test_target}"
local expected_message="buildozer 'add deps @[a-z_.~+-]*com_google_guava_guava_21_0//:com_google_guava_guava_21_0' ${test_target}"

test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message \
"${expected_message}" ${test_target} \
Expand Down
2 changes: 1 addition & 1 deletion test/shell/test_unused_dependency.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ test_unused_deps_filter_excluded_target() {

test_unused_deps_filter_included_target() {
local test_target="//test_expect_failure/unused_dependency_checker/filtering:b"
local expected_message="buildozer 'remove deps @com_google_guava_guava_21_0//:com_google_guava_guava_21_0' ${test_target}"
local expected_message="buildozer 'remove deps @[a-z_.~+-]*com_google_guava_guava_21_0//:com_google_guava_guava_21_0' ${test_target}"

test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message \
"${expected_message}" ${test_target} \
Expand Down
5 changes: 1 addition & 4 deletions test_cross_build/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,8 @@ scala_config(
load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(
scalafmt = True,
scalatest = True,
)

register_toolchains("@io_bazel_rules_scala_toolchains//...:all")

load("@io_bazel_rules_scala//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_repositories")

scalafmt_repositories()
12 changes: 10 additions & 2 deletions test_reproducibility.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,18 @@ non_deploy_jar_md5_sum() {
find bazel-bin/test -name "*.jar" ! -name "*_deploy.jar" ! -path 'bazel-bin/test/jmh/*' | xargs -n 1 -P 5 $(md5_util) | sort
}


test_build_is_identical() {
local test_coverage_packages=()

for package_dir in test/coverage_*; do
test_coverage_packages+=("//${package_dir}/...")
done

bazel clean #ensure we are starting from square one
bazel build test/...
# Also build instrumented jars.
bazel build --collect_code_coverage -- //test/coverage/...
bazel build --collect_code_coverage -- "${test_coverage_packages[@]}"
non_deploy_jar_md5_sum > hash1
bazel clean
sleep 10 # to make sure that if timestamps slip in we get different ones
Expand All @@ -37,7 +44,8 @@ test_build_is_identical() {
fi

bazel build --disk_cache $random_dir test/...
bazel build --disk_cache $random_dir --collect_code_coverage -- //test/coverage/...
bazel build --disk_cache $random_dir --collect_code_coverage -- \
"${test_coverage_packages[@]}"
non_deploy_jar_md5_sum > hash2
diff hash1 hash2
}
Expand Down
4 changes: 2 additions & 2 deletions third_party/dependency_analyzer/src/test/analyzer_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def tests():
],
jvm_flags = common_jvm_flags + [
"-Dguava.jar.location=$(rootpath @com_google_guava_guava_21_0_with_file//jar)",
"-Dapache.commons.jar.location=$(location @org_apache_commons_commons_lang_3_5_without_file//:linkable_org_apache_commons_commons_lang_3_5_without_file)",
"-Dapache.commons.jar.location=$(rootpath @org_apache_commons_commons_lang_3_5_without_file//:linkable_org_apache_commons_commons_lang_3_5_without_file)",
],
unused_dependency_checker_mode = "off",
deps = scala_std_dependencies + [
Expand All @@ -76,7 +76,7 @@ def tests():
"io/bazel/rulesscala/dependencyanalyzer/UnusedDependencyCheckerTest.scala",
],
jvm_flags = common_jvm_flags + [
"-Dapache.commons.jar.location=$(location @org_apache_commons_commons_lang_3_5_without_file//:linkable_org_apache_commons_commons_lang_3_5_without_file)",
"-Dapache.commons.jar.location=$(rootpath @org_apache_commons_commons_lang_3_5_without_file//:linkable_org_apache_commons_commons_lang_3_5_without_file)",
],
unused_dependency_checker_mode = "off",
deps = scala_std_dependencies + [
Expand Down