Skip to content
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

Merged
merged 67 commits into from
Jul 17, 2018
Merged
Show file tree
Hide file tree
Changes from 61 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
d27e73f
WIP compile using worker supplied from provider
johan-stripe Jun 13, 2018
abae2b2
Add scalareflect + scalacompiler to [ScalaWorker]
johan-stripe Jun 14, 2018
f670436
All rules using [ScalaWorker]
johan-stripe Jun 14, 2018
50deaf3
Create [ScalaWorker] using new_scala_repository
johan-stripe Jun 14, 2018
b9f1b22
Use filegroup for scalac worker files
johan-stripe Jun 14, 2018
2aa315c
Specify scala version and label in new_scala_repository
johan-stripe Jun 14, 2018
27d3af0
Merge branch 'master' into johan/cross-build
johan-stripe Jun 14, 2018
0e5c5dd
Run formatter
johan-stripe Jun 14, 2018
72aba58
Run scalatest for both 2.11/2.12
johan-stripe Jun 16, 2018
25132c9
Update scalatest to 3.0.5 + remove defaults from ScalaWorker
johan-stripe Jun 18, 2018
d8edfb6
Symlink scalac_worker sources to repo_rule workspace
johan-stripe Jun 18, 2018
cf37e0f
Resolve scalatest_reporter version in rule
johan-stripe Jun 19, 2018
5b2a053
Use new_scala_repository for default 2.11/2.12
johan-stripe Jun 19, 2018
2dc260f
scala_repositories takes scala versions as parameters
johan-stripe Jun 19, 2018
94d0ab1
improve naming
johan-stripe Jun 19, 2018
a47af87
Use single scala version
johan-stripe Jun 21, 2018
52bdef1
Fix runfiles not being generated
johan-stripe Jun 21, 2018
a125428
Support multiple scala versions for tut and scrooge
johan-stripe Jun 22, 2018
dd28058
Format .bzl and BUILD files
johan-stripe Jun 22, 2018
e6aff25
Tests passing with 2.11
johan-stripe Jun 22, 2018
e600177
Make scala_proto work with 2.12
johan-stripe Jun 23, 2018
927e5f8
Support specs2 for both 2.11/2.12
jhnj Jun 26, 2018
22f216f
Don't pass scalatest dependencies through provider
jhnj Jun 26, 2018
87dad40
Pass _scalac through _implicit_deps
jhnj Jun 26, 2018
d45943d
Remove temp test directory
jhnj Jun 26, 2018
990cb9d
Remove debugging println
jhnj Jun 26, 2018
57449bb
Cleanup
jhnj Jun 27, 2018
adeff15
Run formatter
jhnj Jun 27, 2018
2bd3825
Use inputs from resolve_command
jhnj Jun 27, 2018
e1846f5
Combine scalatest+scalactic to single target
jhnj Jun 27, 2018
5d67d4d
Add utils
jhnj Jun 27, 2018
0dc4361
Increase scala_mvn_artifact usage, add shas
jhnj Jun 27, 2018
a66d27a
Add default_major_scala_version()
jhnj Jun 28, 2018
8b48a1e
Add scala_jar_shas dict
jhnj Jun 28, 2018
1648bd4
Fix aspect test
jhnj Jun 28, 2018
e1e7cfb
Merge branch 'master' into johan/single-scala-version-selectable
jhnj Jun 30, 2018
b819162
Use scala_maven_import_external for all imports
jhnj Jul 2, 2018
b6c5806
Add util_core to scrooge, fix aspect test
jhnj Jul 2, 2018
ed646b4
Supply shas to scala_library,compiler and reflect
jhnj Jul 2, 2018
bc36a86
namespace scala_default
jhnj Jul 3, 2018
ae407ac
Scala default version to 2.11.12
jhnj Jul 3, 2018
f5e5ff6
Lookup scala_extra_jar_shas[major_version] once, renaming
jhnj Jul 3, 2018
5f26b2b
tut server_urls as parameter
jhnj Jul 3, 2018
8a7b120
Pass ScalacProvider as paramater
jhnj Jul 3, 2018
7b36b2d
Replace pass classpaths instead of jars
jhnj Jul 3, 2018
dc9eb43
linting
jhnj Jul 3, 2018
a72372c
Lookup scala_jar_shas[major_version] only once
jhnj Jul 3, 2018
260fd7f
Merge branch 'master' into johan/single-scala-version-selectable
jhnj Jul 3, 2018
829705d
Run version specific tests in their own workspace
jhnj Jul 4, 2018
8abd79f
remove more non version-specific tests
jhnj Jul 4, 2018
2252748
Remove version from scala labels fix tests
jhnj Jul 5, 2018
b43be39
Use default_runtime_classpath for macros, fix aspect test
jhnj Jul 5, 2018
2db2ada
Run more tests in test_version.sh
jhnj Jul 5, 2018
f0c262c
Update naming, default_compile_classpath -> base_compile_classpath
jhnj Jul 5, 2018
78cb80e
Use base_classpath instead of base_compile.. + base_runtime...
jhnj Jul 5, 2018
6c05916
Add scala_reflect to default_classpath
jhnj Jul 5, 2018
50cbd54
scala_version and shas as on tuple paramter
jhnj Jul 5, 2018
e1b12ba
Reduce amount of version specific junit tests
jhnj Jul 5, 2018
4774178
Run version specific tests
jhnj Jul 5, 2018
4661265
Run linter
jhnj Jul 5, 2018
a834ba5
Remove dynamic creation of scala_default ScalacProvider
jhnj Jul 11, 2018
e518edc
PR comments, clearer naming
jhnj Jul 16, 2018
16767e3
Merge branch 'master' into johan/single-scala-version-selectable
jhnj Jul 16, 2018
25db5f6
Use ctx.attr._scala_provider instead of ..attr.scalalib, run linter
jhnj Jul 16, 2018
20ee544
Address comments, clean up WORKSPACE.template
jhnj Jul 16, 2018
c9dffbf
Remove unused code
jhnj Jul 16, 2018
b131900
Add section about different scala versions to the readme
jhnj Jul 16, 2018
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
17 changes: 13 additions & 4 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,35 @@ load("//specs2:specs2_junit.bzl", "specs2_junit_repositories")

specs2_junit_repositories()

load("//scala:scala_cross_version.bzl", "scala_mvn_artifact")
load("//scala:scala_cross_version.bzl", "scala_mvn_artifact", "default_scala_major_version")

# test adding a scala jar:
maven_jar(
name = "com_twitter__scalding_date",
artifact = scala_mvn_artifact("com.twitter:scalding-date:0.17.0"),
artifact = scala_mvn_artifact(
"com.twitter:scalding-date:0.17.0",
default_scala_major_version(),
),
sha1 = "420fb0c4f737a24b851c4316ee0362095710caa5",
Copy link
Member

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.

Copy link
Contributor Author

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 change default_scala_version(). Or should we add a map just to make the fact that they need to be changed more explicit?

Copy link
Member

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.

)

# For testing that we don't include sources jars to the classpath
maven_jar(
name = "org_typelevel__cats_core",
artifact = scala_mvn_artifact("org.typelevel:cats-core:0.9.0"),
artifact = scala_mvn_artifact(
"org.typelevel:cats-core:0.9.0",
default_scala_major_version(),
),
sha1 = "b2f8629c6ec834d8b6321288c9fe77823f1e1314",
)

# test of a plugin
maven_jar(
name = "org_psywerx_hairyfotr__linter",
artifact = scala_mvn_artifact("org.psywerx.hairyfotr:linter:0.1.13"),
artifact = scala_mvn_artifact(
"org.psywerx.hairyfotr:linter:0.1.13",
default_scala_major_version(),
),
sha1 = "e5b3e2753d0817b622c32aedcb888bcf39e275b4",
)

Expand Down
23 changes: 23 additions & 0 deletions scala/BUILD
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(
Expand All @@ -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",
Copy link
Member

Choose a reason for hiding this comment

The 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 @foo//:bar

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add the full paths

Copy link
Member

Choose a reason for hiding this comment

The 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.
I don't feel strongly enough about it to object but just wanted to add these 2c

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ittaiz what does a raw @foo correspond to? Is it the same as @foo//:foo?

"@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:scalac_worker",
visibility = ["//visibility:public"],
)
56 changes: 39 additions & 17 deletions scala/private/rule_impls.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -131,7 +134,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 = []
Expand Down Expand Up @@ -222,15 +225,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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you take this from the tuple output (command) of the resolve_command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't work, command is ["/bin/bash", "-c", ""] not an executable

Copy link
Member

Choose a reason for hiding this comment

The 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 resolve_command but not the executable. (@jhnj this comment isn't blocking the merge)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input_manifests = scalac_input_manifests,
mnemonic = "Scalac",
progress_message = "scala %s" % target_label,
execution_requirements = {"supports-workers": "1"},
Expand Down Expand Up @@ -327,7 +335,7 @@ def _compile_or_empty(ctx, manifest, jars, srcjars, buildijar,
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.scalac_jvm_flags, ctx.attr._scalac[_ScalacProvider])

# build ijar if needed
if buildijar:
Expand Down Expand Up @@ -472,15 +480,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,
Expand All @@ -497,13 +506,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)
Expand Down Expand Up @@ -549,10 +559,13 @@ def _lib(ctx, non_macro_lib):
)

def scala_library_impl(ctx):
return _lib(ctx, True)
scalac_provider = ctx.attr._scalac[_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._scalac[_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,
Expand Down Expand Up @@ -598,7 +611,8 @@ def _scala_binary_common(ctx,
runfiles = runfiles)

def scala_binary_impl(ctx):
jars = _collect_jars_from_common_ctx(ctx)
scalac_provider = ctx.attr._scalac[_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, "", "")
Expand All @@ -614,9 +628,12 @@ def scala_binary_impl(ctx):
return out

def scala_repl_impl(ctx):
scalac_provider = ctx.attr._scalac[_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)
Expand Down Expand Up @@ -664,8 +681,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._scalac[_ScalacProvider]
jars = _collect_jars_from_common_ctx(
ctx,
scalac_provider.default_classpath,
extra_runtime_deps = [
ctx.attr._scalatest_reporter, ctx.attr._scalatest_runner
],
Expand Down Expand Up @@ -727,8 +747,10 @@ def scala_junit_test_impl(ctx):
fail(
"Setting at least one of the attributes ('prefixes','suffixes') is required"
)
scalac_provider = ctx.attr._scalac[_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
Expand Down
32 changes: 32 additions & 0 deletions scala/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,35 @@ def create_scala_provider(ijar, class_jar, compile_jars,
transitive_runtime_jars = transitive_runtime_jars,
transitive_exports = [] #needed by intellij plugin
)

ScalacProvider = provider(
doc = "ScalaProvider",
fields = [
"scalac",
"default_classpath",
"default_macro_classpath",
"default_repl_classpath",
])

def _declare_scalac_provider(ctx):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an interesting approach.

return [
ScalacProvider(
scalac = ctx.attr.scalac,
default_classpath = ctx.attr.default_classpath,
default_repl_classpath = ctx.attr.default_repl_classpath,
default_macro_classpath = ctx.attr.default_macro_classpath,
)
]

declare_scalac_provider = rule(
implementation = _declare_scalac_provider,
attrs = {
"scalac": attr.label(
executable = True,
cfg = "host",
allow_files = True,
mandatory = True),
"default_classpath": attr.label_list(allow_files = True),
"default_repl_classpath": attr.label_list(allow_files = True),
"default_macro_classpath": attr.label_list(allow_files = True),
})
Loading