From 31001b8418101c68717cfbb1c14edd8ea2003f5c Mon Sep 17 00:00:00 2001 From: simuons Date: Wed, 6 Mar 2024 09:54:58 +0200 Subject: [PATCH 1/4] 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 a09e03f73..cfcd80409 100644 --- a/scala/scala_toolchain.bzl +++ b/scala/scala_toolchain.bzl @@ -8,6 +8,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": @@ -42,19 +43,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 @@ -73,13 +121,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( @@ -96,12 +141,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,62 +165,20 @@ 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 23b3b973c7b4087cf6e123f4a80ea90f0ef61dfa Mon Sep 17 00:00:00 2001 From: simuons Date: Fri, 8 Mar 2024 13:49:27 +0200 Subject: [PATCH 2/4] 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 cfcd80409..f7e8166b9 100644 --- a/scala/scala_toolchain.bzl +++ b/scala/scala_toolchain.bzl @@ -43,66 +43,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 @@ -121,10 +135,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( @@ -141,12 +155,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 558a4466798dac2549f8627d29f9bae37a999c2c Mon Sep 17 00:00:00 2001 From: simuons Date: Thu, 25 Apr 2024 15:44:59 +0300 Subject: [PATCH 3/4] 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 f7e8166b9..666dfb52f 100644 --- a/scala/scala_toolchain.bzl +++ b/scala/scala_toolchain.bzl @@ -88,6 +88,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 4eb9d5a0343962b37f6bac3727619c796108d6dc Mon Sep 17 00:00:00 2001 From: simuons Date: Thu, 25 Apr 2024 15:47:45 +0300 Subject: [PATCH 4/4] 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,