From 1282132d4449305e69e795ec4506ce3762b14edf Mon Sep 17 00:00:00 2001 From: simuons Date: Wed, 6 Mar 2024 09:54:58 +0200 Subject: [PATCH 1/5] Experiment: move toolchain attrs to build settings Rationale: thse settings are orhogonal to scala version. If we will intorduce multiple toolchains for different scala versions then we would need to repeat same attrs for each scala version Toolchains are meant for platform specific tools/flags in our case it loosely maps to scala version Build settings/flags are meant to change behaviour of the rules like strict_deps, use_args_file etc --- scala/scala_toolchain.bzl | 147 +++++++++++++++++++------------------- scala/settings/BUILD | 92 +++++++++++++++++++++++- 2 files changed, 166 insertions(+), 73 deletions(-) diff --git a/scala/scala_toolchain.bzl b/scala/scala_toolchain.bzl index 2eadbc41b..5274f84d5 100644 --- a/scala/scala_toolchain.bzl +++ b/scala/scala_toolchain.bzl @@ -5,6 +5,7 @@ load( "SCALA_MAJOR_VERSION", ) load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") +load("@bazel_skylib//lib:dicts.bzl", _dicts = "dicts") def _compute_strict_deps_mode(input_strict_deps_mode, dependency_mode): if dependency_mode == "direct": @@ -39,19 +40,66 @@ def _partition_patterns(patterns): ] return includes, excludes +_deprecated_attrs = { + "dependency_mode": attr.string(values = ["direct", "plus-one", "transitive"]), + "strict_deps_mode": attr.string(values = ["off", "warn", "error", "default"]), + "unused_dependency_checker_mode": attr.string(values = ["off", "warn", "error"]), + "compiler_deps_mode": attr.string(values = ["off", "warn", "error"]), + "dependency_tracking_method": attr.string(values = ["ast-plus", "ast", "high-level", "default"]), + "dependency_tracking_strict_deps_patterns": attr.string_list( + doc = "List of target prefixes included for strict deps analysis. Exclude patterns with '-'", + ), + "dependency_tracking_unused_deps_patterns": attr.string_list( + doc = "List of target prefixes included for unused deps analysis. Exclude patterns with '-'", + ), + "enable_diagnostics_report": attr.bool( + doc = "Enable the output of structured diagnostics through the BEP", + ), + "enable_stats_file": attr.bool( + doc = "Enable writing of statsfile", + ), + "enable_semanticdb": attr.bool( + doc = "Enable SemanticDb", + ), + "semanticdb_bundle_in_jar": attr.bool( + doc = "Option to bundle the semanticdb files inside the output jar file", + ), + "use_argument_file_in_runner": attr.bool( + doc = "Changes java binaries scripts (including tests) to use argument files and not classpath jars to improve performance, requires java > 8", + ), +} + +def _attr_to_flag(name): + return "//scala/settings:%s" % name + +_flag_attrs = { + "_" + k: attr.label(default = _attr_to_flag(k)) + for k in _deprecated_attrs.keys() +} + +def _resolve_flags(ctx): + def compute(name): + attr = getattr(ctx.attr, name) + return attr if attr else getattr(ctx.attr, "_" + name)[BuildSettingInfo].value + + return struct(**{k: compute(k) for k in _deprecated_attrs.keys()}) + def _scala_toolchain_impl(ctx): - dependency_mode = ctx.attr.dependency_mode + flags = _resolve_flags(ctx) + + dependency_mode = flags.dependency_mode + strict_deps_mode = _compute_strict_deps_mode( - ctx.attr.strict_deps_mode, + flags.strict_deps_mode, dependency_mode, ) - compiler_deps_mode = ctx.attr.compiler_deps_mode + compiler_deps_mode = flags.compiler_deps_mode - unused_dependency_checker_mode = ctx.attr.unused_dependency_checker_mode + unused_dependency_checker_mode = flags.unused_dependency_checker_mode dependency_tracking_method = _compute_dependency_tracking_method( dependency_mode, - ctx.attr.dependency_tracking_method, + flags.dependency_tracking_method, ) # Final quality checks to possibly detect buggy code above @@ -70,13 +118,10 @@ def _scala_toolchain_impl(ctx): if "ast-plus" == dependency_tracking_method and not ENABLE_COMPILER_DEPENDENCY_TRACKING: fail("To use 'ast-plus' dependency tracking, you must set 'enable_compiler_dependency_tracking' to True in scala_config") - enable_stats_file = ctx.attr.enable_stats_file - enable_diagnostics_report = ctx.attr.enable_diagnostics_report - - all_strict_deps_patterns = ctx.attr.dependency_tracking_strict_deps_patterns + all_strict_deps_patterns = flags.dependency_tracking_strict_deps_patterns strict_deps_includes, strict_deps_excludes = _partition_patterns(all_strict_deps_patterns) - all_unused_deps_patterns = ctx.attr.dependency_tracking_unused_deps_patterns + all_unused_deps_patterns = flags.dependency_tracking_unused_deps_patterns unused_deps_includes, unused_deps_excludes = _partition_patterns(all_unused_deps_patterns) toolchain = platform_common.ToolchainInfo( @@ -93,12 +138,12 @@ def _scala_toolchain_impl(ctx): unused_deps_exclude_patterns = unused_deps_excludes, scalac_jvm_flags = ctx.attr.scalac_jvm_flags, scala_test_jvm_flags = ctx.attr.scala_test_jvm_flags, - enable_diagnostics_report = enable_diagnostics_report, + enable_diagnostics_report = flags.enable_diagnostics_report, jacocorunner = ctx.attr.jacocorunner, - enable_stats_file = enable_stats_file, - enable_semanticdb = ctx.attr.enable_semanticdb, - semanticdb_bundle_in_jar = ctx.attr.semanticdb_bundle_in_jar, - use_argument_file_in_runner = ctx.attr.use_argument_file_in_runner, + enable_stats_file = flags.enable_stats_file, + enable_semanticdb = flags.enable_semanticdb, + semanticdb_bundle_in_jar = flags.semanticdb_bundle_in_jar, + use_argument_file_in_runner = flags.use_argument_file_in_runner, scala_version = ctx.attr._scala_version[BuildSettingInfo].value, ) return [toolchain] @@ -120,63 +165,21 @@ def _default_dep_providers(): _scala_toolchain = rule( _scala_toolchain_impl, - attrs = { - "scalacopts": attr.string_list(), - "dep_providers": attr.label_list( - default = _default_dep_providers(), - providers = [_DepsInfo], - ), - "dependency_mode": attr.string( - default = "direct", - values = ["direct", "plus-one", "transitive"], - ), - "strict_deps_mode": attr.string( - default = "default", - values = ["off", "warn", "error", "default"], - ), - "unused_dependency_checker_mode": attr.string( - default = "off", - values = ["off", "warn", "error"], - ), - "compiler_deps_mode": attr.string( - default = "off", - values = ["off", "warn", "error"], - ), - "dependency_tracking_method": attr.string( - default = "default", - values = ["ast-plus", "ast", "high-level", "default"], - ), - "dependency_tracking_strict_deps_patterns": attr.string_list( - doc = "List of target prefixes included for strict deps analysis. Exclude patterns with '-'", - default = [""], - ), - "dependency_tracking_unused_deps_patterns": attr.string_list( - doc = "List of target prefixes included for unused deps analysis. Exclude patterns with '-'", - default = [""], - ), - "scalac_jvm_flags": attr.string_list(), - "scala_test_jvm_flags": attr.string_list(), - "enable_diagnostics_report": attr.bool( - doc = "Enable the output of structured diagnostics through the BEP", - ), - "jacocorunner": attr.label( - default = Label("@bazel_tools//tools/jdk:JacocoCoverage"), - ), - "enable_stats_file": attr.bool( - default = True, - doc = "Enable writing of statsfile", - ), - "enable_semanticdb": attr.bool( - default = False, - doc = "Enable SemanticDb", - ), - "semanticdb_bundle_in_jar": attr.bool(default = False, doc = "Option to bundle the semanticdb files inside the output jar file"), - "use_argument_file_in_runner": attr.bool( - default = False, - doc = "Changes java binaries scripts (including tests) to use argument files and not classpath jars to improve performance, requires java > 8", - ), - "_scala_version": attr.label(default = "@io_bazel_rules_scala_config//:scala_version"), - }, + attrs = _dicts.add( + _deprecated_attrs, + _flag_attrs, + { + "dep_providers": attr.label_list( + default = _default_dep_providers(), + providers = [_DepsInfo], + ), + "jacocorunner": attr.label(default = Label("@bazel_tools//tools/jdk:JacocoCoverage")), + "scalacopts": attr.string_list(), + "scalac_jvm_flags": attr.string_list(), + "scala_test_jvm_flags": attr.string_list(), + "_scala_version": attr.label(default = "@io_bazel_rules_scala_config//:scala_version"), + }, + ), fragments = ["java"], ) diff --git a/scala/settings/BUILD b/scala/settings/BUILD index 919410bea..defbc2514 100644 --- a/scala/settings/BUILD +++ b/scala/settings/BUILD @@ -1,7 +1,97 @@ +package(default_visibility = ["//visibility:public"]) + load("//scala/settings:stamp_settings.bzl", "stamp_scala_import") stamp_scala_import( name = "stamp_scala_import", build_setting_default = True, - visibility = ["//visibility:public"], +) + +load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "string_flag", "string_list_flag") + +string_flag( + name = "dependency_mode", + build_setting_default = "direct", + values = [ + "direct", + "plus-one", + "transitive", + ], +) + +string_flag( + name = "strict_deps_mode", + build_setting_default = "default", + values = [ + "off", + "warn", + "error", + "default", + ], +) + +string_flag( + name = "unused_dependency_checker_mode", + build_setting_default = "off", + values = [ + "off", + "warn", + "error", + ], +) + +string_flag( + name = "compiler_deps_mode", + build_setting_default = "off", + values = [ + "off", + "warn", + "error", + ], +) + +string_flag( + name = "dependency_tracking_method", + build_setting_default = "default", + values = [ + "ast-plus", + "ast", + "high-level", + "default", + ], +) + +string_list_flag( + name = "dependency_tracking_strict_deps_patterns", + build_setting_default = [], +) + +string_list_flag( + name = "dependency_tracking_unused_deps_patterns", + build_setting_default = [], +) + +bool_flag( + name = "enable_diagnostics_report", + build_setting_default = False, +) + +bool_flag( + name = "enable_stats_file", + build_setting_default = True, +) + +bool_flag( + name = "enable_semanticdb", + build_setting_default = False, +) + +bool_flag( + name = "semanticdb_bundle_in_jar", + build_setting_default = False, +) + +bool_flag( + name = "use_argument_file_in_runner", + build_setting_default = False, ) From be4046f08df0970bfb50906df167f3924a364cba Mon Sep 17 00:00:00 2001 From: simuons Date: Fri, 8 Mar 2024 13:49:27 +0200 Subject: [PATCH 2/5] Intorduce flag that switches where toolchains reads config from Unfortunately bazel rules doesn't know if attr was set by user or not https://github.com/bazelbuild/bazel/issues/14434 --- scala/scala_toolchain.bzl | 87 +++++++++++++++++++++++---------------- scala/settings/BUILD | 5 +++ 2 files changed, 56 insertions(+), 36 deletions(-) diff --git a/scala/scala_toolchain.bzl b/scala/scala_toolchain.bzl index 5274f84d5..e870b644e 100644 --- a/scala/scala_toolchain.bzl +++ b/scala/scala_toolchain.bzl @@ -40,66 +40,80 @@ def _partition_patterns(patterns): ] return includes, excludes -_deprecated_attrs = { - "dependency_mode": attr.string(values = ["direct", "plus-one", "transitive"]), - "strict_deps_mode": attr.string(values = ["off", "warn", "error", "default"]), - "unused_dependency_checker_mode": attr.string(values = ["off", "warn", "error"]), - "compiler_deps_mode": attr.string(values = ["off", "warn", "error"]), - "dependency_tracking_method": attr.string(values = ["ast-plus", "ast", "high-level", "default"]), +_config_attrs = { + "dependency_mode": attr.string( + default = "direct", + values = ["direct", "plus-one", "transitive"], + ), + "strict_deps_mode": attr.string( + default = "default", + values = ["off", "warn", "error", "default"], + ), + "unused_dependency_checker_mode": attr.string( + default = "off", + values = ["off", "warn", "error"], + ), + "compiler_deps_mode": attr.string( + default = "off", + values = ["off", "warn", "error"], + ), + "dependency_tracking_method": attr.string( + default = "default", + values = ["ast-plus", "ast", "high-level", "default"], + ), "dependency_tracking_strict_deps_patterns": attr.string_list( doc = "List of target prefixes included for strict deps analysis. Exclude patterns with '-'", + default = [""], ), "dependency_tracking_unused_deps_patterns": attr.string_list( doc = "List of target prefixes included for unused deps analysis. Exclude patterns with '-'", + default = [""], ), "enable_diagnostics_report": attr.bool( doc = "Enable the output of structured diagnostics through the BEP", ), "enable_stats_file": attr.bool( + default = True, doc = "Enable writing of statsfile", ), "enable_semanticdb": attr.bool( + default = False, doc = "Enable SemanticDb", ), - "semanticdb_bundle_in_jar": attr.bool( - doc = "Option to bundle the semanticdb files inside the output jar file", - ), + "semanticdb_bundle_in_jar": attr.bool(default = False, doc = "Option to bundle the semanticdb files inside the output jar file"), "use_argument_file_in_runner": attr.bool( + default = False, doc = "Changes java binaries scripts (including tests) to use argument files and not classpath jars to improve performance, requires java > 8", ), } -def _attr_to_flag(name): - return "//scala/settings:%s" % name - -_flag_attrs = { - "_" + k: attr.label(default = _attr_to_flag(k)) - for k in _deprecated_attrs.keys() +_config_flags = { + "_" + k: attr.label(default = "//scala/settings:%s" % k) + for k in _config_attrs.keys() } -def _resolve_flags(ctx): - def compute(name): - attr = getattr(ctx.attr, name) - return attr if attr else getattr(ctx.attr, "_" + name)[BuildSettingInfo].value - - return struct(**{k: compute(k) for k in _deprecated_attrs.keys()}) +def _config(ctx): + if ctx.attr._scala_toolchain_flags[BuildSettingInfo].value: + return struct(**{k: getattr(ctx.attr, "_" + k)[BuildSettingInfo].value for k in _config_attrs.keys()}) + else: + return struct(**{k: getattr(ctx.attr, k) for k in _config_attrs.keys()}) def _scala_toolchain_impl(ctx): - flags = _resolve_flags(ctx) + config = _config(ctx) - dependency_mode = flags.dependency_mode + dependency_mode = config.dependency_mode strict_deps_mode = _compute_strict_deps_mode( - flags.strict_deps_mode, + config.strict_deps_mode, dependency_mode, ) - compiler_deps_mode = flags.compiler_deps_mode + compiler_deps_mode = config.compiler_deps_mode - unused_dependency_checker_mode = flags.unused_dependency_checker_mode + unused_dependency_checker_mode = config.unused_dependency_checker_mode dependency_tracking_method = _compute_dependency_tracking_method( dependency_mode, - flags.dependency_tracking_method, + config.dependency_tracking_method, ) # Final quality checks to possibly detect buggy code above @@ -118,10 +132,10 @@ def _scala_toolchain_impl(ctx): if "ast-plus" == dependency_tracking_method and not ENABLE_COMPILER_DEPENDENCY_TRACKING: fail("To use 'ast-plus' dependency tracking, you must set 'enable_compiler_dependency_tracking' to True in scala_config") - all_strict_deps_patterns = flags.dependency_tracking_strict_deps_patterns + all_strict_deps_patterns = config.dependency_tracking_strict_deps_patterns strict_deps_includes, strict_deps_excludes = _partition_patterns(all_strict_deps_patterns) - all_unused_deps_patterns = flags.dependency_tracking_unused_deps_patterns + all_unused_deps_patterns = config.dependency_tracking_unused_deps_patterns unused_deps_includes, unused_deps_excludes = _partition_patterns(all_unused_deps_patterns) toolchain = platform_common.ToolchainInfo( @@ -138,12 +152,12 @@ def _scala_toolchain_impl(ctx): unused_deps_exclude_patterns = unused_deps_excludes, scalac_jvm_flags = ctx.attr.scalac_jvm_flags, scala_test_jvm_flags = ctx.attr.scala_test_jvm_flags, - enable_diagnostics_report = flags.enable_diagnostics_report, + enable_diagnostics_report = config.enable_diagnostics_report, jacocorunner = ctx.attr.jacocorunner, - enable_stats_file = flags.enable_stats_file, - enable_semanticdb = flags.enable_semanticdb, - semanticdb_bundle_in_jar = flags.semanticdb_bundle_in_jar, - use_argument_file_in_runner = flags.use_argument_file_in_runner, + enable_stats_file = config.enable_stats_file, + enable_semanticdb = config.enable_semanticdb, + semanticdb_bundle_in_jar = config.semanticdb_bundle_in_jar, + use_argument_file_in_runner = config.use_argument_file_in_runner, scala_version = ctx.attr._scala_version[BuildSettingInfo].value, ) return [toolchain] @@ -166,8 +180,8 @@ def _default_dep_providers(): _scala_toolchain = rule( _scala_toolchain_impl, attrs = _dicts.add( - _deprecated_attrs, - _flag_attrs, + _config_attrs, + _config_flags, { "dep_providers": attr.label_list( default = _default_dep_providers(), @@ -178,6 +192,7 @@ _scala_toolchain = rule( "scalac_jvm_flags": attr.string_list(), "scala_test_jvm_flags": attr.string_list(), "_scala_version": attr.label(default = "@io_bazel_rules_scala_config//:scala_version"), + "_scala_toolchain_flags": attr.label(default = "//scala/settings:scala_toolchain_flags"), }, ), fragments = ["java"], diff --git a/scala/settings/BUILD b/scala/settings/BUILD index defbc2514..766a80b43 100644 --- a/scala/settings/BUILD +++ b/scala/settings/BUILD @@ -9,6 +9,11 @@ stamp_scala_import( load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "string_flag", "string_list_flag") +bool_flag( + name = "scala_toolchain_flags", + build_setting_default = False, +) + string_flag( name = "dependency_mode", build_setting_default = "direct", From 6da427337d87e508fb925eac9469b8f3d14ad99b Mon Sep 17 00:00:00 2001 From: simuons Date: Thu, 25 Apr 2024 15:44:59 +0300 Subject: [PATCH 3/5] Move jvm_flags to build settings --- scala/scala_toolchain.bzl | 4 ++-- scala/settings/BUILD | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/scala/scala_toolchain.bzl b/scala/scala_toolchain.bzl index e870b644e..1733356a2 100644 --- a/scala/scala_toolchain.bzl +++ b/scala/scala_toolchain.bzl @@ -85,6 +85,8 @@ _config_attrs = { default = False, doc = "Changes java binaries scripts (including tests) to use argument files and not classpath jars to improve performance, requires java > 8", ), + "scalac_jvm_flags": attr.string_list(), + "scala_test_jvm_flags": attr.string_list(), } _config_flags = { @@ -189,8 +191,6 @@ _scala_toolchain = rule( ), "jacocorunner": attr.label(default = Label("@bazel_tools//tools/jdk:JacocoCoverage")), "scalacopts": attr.string_list(), - "scalac_jvm_flags": attr.string_list(), - "scala_test_jvm_flags": attr.string_list(), "_scala_version": attr.label(default = "@io_bazel_rules_scala_config//:scala_version"), "_scala_toolchain_flags": attr.label(default = "//scala/settings:scala_toolchain_flags"), }, diff --git a/scala/settings/BUILD b/scala/settings/BUILD index 766a80b43..9bc9e1b77 100644 --- a/scala/settings/BUILD +++ b/scala/settings/BUILD @@ -100,3 +100,13 @@ bool_flag( name = "use_argument_file_in_runner", build_setting_default = False, ) + +string_list_flag( + name = "scalac_jvm_flags", + build_setting_default = [], +) + +string_list_flag( + name = "scala_test_jvm_flags", + build_setting_default = [], +) From 57ea1eab6b500a5cb8386105583d02e90139ec17 Mon Sep 17 00:00:00 2001 From: simuons Date: Thu, 25 Apr 2024 15:47:45 +0300 Subject: [PATCH 4/5] lint --- scala/settings/BUILD | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/scala/settings/BUILD b/scala/settings/BUILD index 9bc9e1b77..75aec7c7f 100644 --- a/scala/settings/BUILD +++ b/scala/settings/BUILD @@ -1,14 +1,13 @@ -package(default_visibility = ["//visibility:public"]) - load("//scala/settings:stamp_settings.bzl", "stamp_scala_import") +load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "string_flag", "string_list_flag") + +package(default_visibility = ["//visibility:public"]) stamp_scala_import( name = "stamp_scala_import", build_setting_default = True, ) -load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "string_flag", "string_list_flag") - bool_flag( name = "scala_toolchain_flags", build_setting_default = False, From 5558c3959d47f8a08b876e3f0bc8728e0cc267a1 Mon Sep 17 00:00:00 2001 From: simuons Date: Wed, 12 Feb 2025 10:46:04 +0200 Subject: [PATCH 5/5] Empty commit