Don't rely on SCALA_VERSION in phases
#1559
Conversation
aszady
force-pushed
the
phases
branch
3 times, most recently
from
7ff0262
to
5396eb4
Compare
scala/scala_toolchain.bzl
Outdated
| @@ -173,6 +176,12 @@ scala_toolchain = rule( | |||
| 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.string( | |||
There was a problem hiding this comment.
I'm thinking maybe this should be an implicit/private attribute pointing to scala_version setting (which would be exposed via toolchain provider as in this PR).
For example "_scala_version": attr.label(default = "@io_bazel_rules_scala_config//:scala_version")
I'm thinking so because actual mapping of toolchain to scala version would be done on toolchain type ie
toolchain(toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type", target_setting = "@io_bazel_rules_scala_config//:3_3_1")
And there could be errors of scala version mismatch
scala_toolchain(name = "toolchain_def", scala_version = "3.3.1", ...)
toolchain(toolchain = "toolchain_def", target_setting = "@io_bazel_rules_scala_config//:2_12_x", ....)
Or at least there should be validation that version specified matches current setting value.
What do you think? Do I miss something?
Side note: looking at potential toolchain declarations I'm thinking maybe generated config_setting could have some prefix for example scala_version_3_3_1 so target setting would look like @io_bazel_rules_scala_config//:scala_version_3_3_1)
There was a problem hiding this comment.
That's a good observation! Updated.
Now this PR depends on #1558 (don't know if there is any way to indicate that).
aszady
force-pushed
the
phases
branch
3 times, most recently
from
ecb918e
to
9b2f52b
Compare
Use a new field of toolchain, `scala_version`, instead. Its value is based on the current value of Scala version build setting. Co-authored-by: mkuta <mkuta@virtuslab.com>
Description
Before enabling cross-build we need to review (and most likely replace) all uses of
SCALA_VERSION. This PR reviews usage ofSCALA_VERSIONwhen a resolved toolchain is available.This shouldn't change any behavior.
Motivation
Originally #1290.
Partitioned from #1552.