-
Notifications
You must be signed in to change notification settings - Fork 278
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
Support for selecting scala version #544
Changes from all commits
d27e73f
abae2b2
f670436
50deaf3
b9f1b22
2aa315c
27d3af0
0e5c5dd
72aba58
25132c9
d8edfb6
cf37e0f
5b2a053
2dc260f
94d0ab1
a47af87
52bdef1
a125428
dd28058
e6aff25
e600177
927e5f8
22f216f
87dad40
d45943d
990cb9d
57449bb
adeff15
2bd3825
e1846f5
5d67d4d
0dc4361
a66d27a
8b48a1e
1648bd4
e1e7cfb
b819162
b6c5806
ed646b4
bc36a86
ae407ac
f5e5ff6
5f26b2b
8a7b120
7b36b2d
dc9eb43
a72372c
260fd7f
829705d
8abd79f
2252748
b43be39
2db2ada
f0c262c
78cb80e
6c05916
50cbd54
e1b12ba
4774178
4661265
a834ba5
e518edc
16767e3
25db5f6
20ee544
c9dffbf
b131900
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
load( | ||
"@io_bazel_rules_scala//scala:providers.bzl", | ||
_declare_scalac_provider = "declare_scalac_provider", | ||
) | ||
load("//scala:scala_toolchain.bzl", "scala_toolchain") | ||
|
||
toolchain_type( | ||
|
@@ -23,3 +27,22 @@ java_import( | |
jars = ["@bazel_tools//tools/jdk:TestRunner_deploy.jar"], | ||
visibility = ["//visibility:public"], | ||
) | ||
|
||
_declare_scalac_provider( | ||
name = "scala_default", | ||
default_classpath = [ | ||
"@io_bazel_rules_scala_scala_library", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these look weird... this looks like just a string, not a target.... I would expect There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add the full paths There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. honestly I disagree. This is the style promoted (though not enforced) by *_import_external. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ittaiz what does a raw |
||
"@io_bazel_rules_scala_scala_reflect", | ||
], | ||
default_macro_classpath = [ | ||
"@io_bazel_rules_scala_scala_library", | ||
"@io_bazel_rules_scala_scala_reflect", | ||
], | ||
default_repl_classpath = [ | ||
"@io_bazel_rules_scala_scala_library", | ||
"@io_bazel_rules_scala_scala_reflect", | ||
"@io_bazel_rules_scala_scala_compiler", | ||
], | ||
scalac = "@io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac", | ||
visibility = ["//visibility:public"], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,10 @@ | |
# limitations under the License. | ||
"""Rules for supporting the Scala language.""" | ||
load("@io_bazel_rules_scala//scala:scala_toolchain.bzl", "scala_toolchain") | ||
load("@io_bazel_rules_scala//scala:providers.bzl", "create_scala_provider") | ||
load( | ||
"@io_bazel_rules_scala//scala:providers.bzl", | ||
"create_scala_provider", | ||
_ScalacProvider = "ScalacProvider") | ||
load( | ||
":common.bzl", | ||
"add_labels_of_jars_to", | ||
|
@@ -132,7 +135,7 @@ def compile_scala(ctx, target_label, output, manifest, statsfile, sources, | |
cjars, all_srcjars, transitive_compile_jars, plugins, | ||
resource_strip_prefix, resources, resource_jars, labels, | ||
in_scalacopts, print_compile_time, expect_java_output, | ||
scalac_jvm_flags): | ||
scalac_jvm_flags, scalac_provider): | ||
# look for any plugins: | ||
plugins = _collect_plugin_paths(plugins) | ||
dependency_analyzer_plugin_jars = [] | ||
|
@@ -223,15 +226,20 @@ StatsfileOutput: {statsfile_output} | |
ctx.actions.write( | ||
output = argfile, content = scalac_args + optional_scalac_args) | ||
|
||
scalac_inputs, _, scalac_input_manifests = ctx.resolve_command( | ||
tools = [scalac_provider.scalac]) | ||
|
||
outs = [output, statsfile] | ||
ins = (compiler_classpath_jars.to_list() + all_srcjars.to_list() + | ||
list(sources) + plugins_list + dependency_analyzer_plugin_jars + | ||
classpath_resources + resources + resource_jars + [manifest, argfile]) | ||
ins = ( | ||
compiler_classpath_jars.to_list() + all_srcjars.to_list() + list(sources) | ||
+ plugins_list + dependency_analyzer_plugin_jars + classpath_resources + | ||
resources + resource_jars + [manifest, argfile] + scalac_inputs) | ||
|
||
ctx.actions.run( | ||
inputs = ins, | ||
outputs = outs, | ||
executable = ctx.executable._scalac, | ||
executable = scalac_provider.scalac.files_to_run.executable, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you take this from the tuple output ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't work, command is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @laurentlb is this the expected behavior? Sounds strange we can get the inputs and manifests from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ping @laurentlb @c-parsons |
||
input_manifests = scalac_input_manifests, | ||
mnemonic = "Scalac", | ||
progress_message = "scala %s" % target_label, | ||
execution_requirements = {"supports-workers": "1"}, | ||
|
@@ -263,7 +271,8 @@ def try_to_compile_java_jar(ctx, scala_output, all_srcjars, java_srcs, | |
providers_of_dependencies = collect_java_providers_of(ctx.attr.deps) | ||
providers_of_dependencies += collect_java_providers_of( | ||
implicit_junit_deps_needed_for_java_compilation) | ||
providers_of_dependencies += collect_java_providers_of([ctx.attr._scalalib]) | ||
providers_of_dependencies += collect_java_providers_of( | ||
ctx.attr._scala_provider[_ScalacProvider].default_classpath) | ||
scala_sources_java_provider = _interim_java_provider_for_java_compilation( | ||
scala_output) | ||
providers_of_dependencies += [scala_sources_java_provider] | ||
|
@@ -323,13 +332,13 @@ def _compile_or_empty(ctx, manifest, jars, srcjars, buildijar, | |
sources = [ | ||
f for f in ctx.files.srcs if f.basename.endswith(_scala_extension) | ||
] + java_srcs | ||
compile_scala(ctx, ctx.label, ctx.outputs.jar, manifest, | ||
ctx.outputs.statsfile, sources, jars, all_srcjars, | ||
transitive_compile_jars, ctx.attr.plugins, | ||
ctx.attr.resource_strip_prefix, ctx.files.resources, | ||
ctx.files.resource_jars, jars2labels, ctx.attr.scalacopts, | ||
ctx.attr.print_compile_time, ctx.attr.expect_java_output, | ||
ctx.attr.scalac_jvm_flags) | ||
compile_scala( | ||
ctx, ctx.label, ctx.outputs.jar, manifest, ctx.outputs.statsfile, | ||
sources, jars, all_srcjars, transitive_compile_jars, ctx.attr.plugins, | ||
ctx.attr.resource_strip_prefix, ctx.files.resources, | ||
ctx.files.resource_jars, jars2labels, ctx.attr.scalacopts, | ||
ctx.attr.print_compile_time, ctx.attr.expect_java_output, | ||
ctx.attr.scalac_jvm_flags, ctx.attr._scala_provider[_ScalacProvider]) | ||
|
||
# build ijar if needed | ||
if buildijar: | ||
|
@@ -474,15 +483,16 @@ def is_dependency_analyzer_off(ctx): | |
# Extract very common code out from dependency analysis into single place | ||
# automatically adds dependency on scala-library and scala-reflect | ||
# collects jars from deps, runtime jars from runtime_deps, and | ||
def _collect_jars_from_common_ctx(ctx, extra_deps = [], | ||
def _collect_jars_from_common_ctx(ctx, | ||
base_classpath, | ||
extra_deps = [], | ||
extra_runtime_deps = []): | ||
|
||
dependency_analyzer_is_off = is_dependency_analyzer_off(ctx) | ||
|
||
# Get jars from deps | ||
auto_deps = [ctx.attr._scalalib, ctx.attr._scalareflect] | ||
deps_jars = collect_jars(ctx.attr.deps + auto_deps + extra_deps, | ||
deps_jars = collect_jars(ctx.attr.deps + extra_deps + base_classpath, | ||
dependency_analyzer_is_off) | ||
|
||
(cjars, transitive_rjars, jars2labels, | ||
transitive_compile_jars) = (deps_jars.compile_jars, | ||
deps_jars.transitive_runtime_jars, | ||
|
@@ -499,13 +509,14 @@ def _collect_jars_from_common_ctx(ctx, extra_deps = [], | |
jars2labels = jars2labels, | ||
transitive_compile_jars = transitive_compile_jars) | ||
|
||
def _lib(ctx, non_macro_lib): | ||
def _lib(ctx, base_classpath, non_macro_lib): | ||
# Build up information from dependency-like attributes | ||
|
||
# This will be used to pick up srcjars from non-scala library | ||
# targets (like thrift code generation) | ||
srcjars = collect_srcjars(ctx.attr.deps) | ||
jars = _collect_jars_from_common_ctx(ctx) | ||
|
||
jars = _collect_jars_from_common_ctx(ctx, base_classpath) | ||
(cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) | ||
|
||
write_manifest(ctx) | ||
|
@@ -551,10 +562,13 @@ def _lib(ctx, non_macro_lib): | |
) | ||
|
||
def scala_library_impl(ctx): | ||
return _lib(ctx, True) | ||
scalac_provider = ctx.attr._scala_provider[_ScalacProvider] | ||
return _lib(ctx, scalac_provider.default_classpath, True) | ||
|
||
def scala_macro_library_impl(ctx): | ||
return _lib(ctx, False) # don't build the ijar for macros | ||
scalac_provider = ctx.attr._scala_provider[_ScalacProvider] | ||
return _lib(ctx, scalac_provider.default_macro_classpath, | ||
False) # don't build the ijar for macros | ||
|
||
# Common code shared by all scala binary implementations. | ||
def _scala_binary_common(ctx, | ||
|
@@ -600,7 +614,8 @@ def _scala_binary_common(ctx, | |
runfiles = runfiles) | ||
|
||
def scala_binary_impl(ctx): | ||
jars = _collect_jars_from_common_ctx(ctx) | ||
scalac_provider = ctx.attr._scala_provider[_ScalacProvider] | ||
jars = _collect_jars_from_common_ctx(ctx, scalac_provider.default_classpath) | ||
(cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) | ||
|
||
wrapper = _write_java_wrapper(ctx, "", "") | ||
|
@@ -616,9 +631,12 @@ def scala_binary_impl(ctx): | |
return out | ||
|
||
def scala_repl_impl(ctx): | ||
scalac_provider = ctx.attr._scala_provider[_ScalacProvider] | ||
# need scala-compiler for MainGenericRunner below | ||
jars = _collect_jars_from_common_ctx( | ||
ctx, extra_runtime_deps = [ctx.attr._scalacompiler]) | ||
ctx, | ||
scalac_provider.default_repl_classpath, | ||
) | ||
(cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) | ||
|
||
args = " ".join(ctx.attr.scalacopts) | ||
|
@@ -666,8 +684,11 @@ def _scala_test_flags(ctx): | |
def scala_test_impl(ctx): | ||
if len(ctx.attr.suites) != 0: | ||
print("suites attribute is deprecated. All scalatest test suites are run") | ||
|
||
scalac_provider = ctx.attr._scala_provider[_ScalacProvider] | ||
jars = _collect_jars_from_common_ctx( | ||
ctx, | ||
scalac_provider.default_classpath, | ||
extra_runtime_deps = [ | ||
ctx.attr._scalatest_reporter, ctx.attr._scalatest_runner | ||
], | ||
|
@@ -731,8 +752,10 @@ def scala_junit_test_impl(ctx): | |
fail( | ||
"Setting at least one of the attributes ('prefixes','suffixes') is required" | ||
) | ||
scalac_provider = ctx.attr._scala_provider[_ScalacProvider] | ||
jars = _collect_jars_from_common_ctx( | ||
ctx, | ||
scalac_provider.default_classpath, | ||
extra_deps = [ | ||
ctx.attr._junit, ctx.attr._hamcrest, ctx.attr.suite_label, | ||
ctx.attr._bazel_test_runner | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the shas need to become a map of scala version -> artifact -> sha.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that these maven_jars are just for tests and as they always use
default_scala_version()
(which is hardcoded) we shouldn't need a map here. The shas only need to be changed if we changedefault_scala_version()
. Or should we add a map just to make the fact that they need to be changed more explicit?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, we should refactor these into a bzl function
load_test_jars()
or something to make it clearer. Don't need to do that in this PR.