Restructure scala provider, add JavaProvider (bazel-contrib#227)

* Restructure scala provider, add JavaProvider

The key change is that the scala provider has been completely
restructured to match the structure of the JavaProvider. It no longer
tracks exports separately but instead has the following fields:

* outputs = contains all the outputs generated by the current rule
(nothing or ijar and class_jar); however, no rules here actually use it.

* compile_jars = contains all the jars dependent rules should compile
with. Thus, ijar and exports.compile_jars

* transitive_runtime_jars = contains all the jars needed to handle this
target at runtime. Thus, class_jar plus (deps + exports +
runtime_deps).transitive_runtime_jars

The created java provider (used by dependent native java rules) uses
just compile_jars and transitive_runtime_jars.

In general, this change was seamless. The major exception is if you were
specifically relying on only exports being re-exported by specifically
gen_scrooge_srcjar. Beforehand, any jars produced by direct dependencies
were not being exported to dependents but now they are. Note that, if
you were using scrooge_scala_library instead, nothing should change.

Other changes:
* Use depset instead of set. (set docs already say set should be
considered deprecated and just points to depset anyway)

* Tests amended to exploit that JavaProvider is properly constructed.
Note that things are still a little strange until Bazel releases
incorporate some fixes to JavaProvider and native provider rules.
Generally, only deps for java_library and java_binary work at the
moment.

* Using JavaProvider, requires Bazel 0.5.2

Remove scala_exports_to_java
Prefer using JavaProvider over scala provider
  (can probably avoid using scala provider completely?)

* update minimum bazel version to 0.5.2

Author: sdtwigg

.travis.yml

@@ -12,7 +12,7 @@ os:
 
 env:
   - V=HEAD
-  - V=0.4.5
+  - V=0.5.2
 
 before_install:
   - |

jmh/jmh.bzl

@@ -66,8 +66,7 @@ def _scala_construct_runtime_classpath(deps):
   java_targets = [d.java for d in deps if hasattr(d, "java")]
   files = []
   for scala in scala_targets:
-    files += list(scala.transitive_runtime_deps)
-    files += list(scala.transitive_runtime_exports)
+    files += list(scala.transitive_runtime_jars)
   for java in java_targets:
     files += list(java.transitive_runtime_deps)
   return files

scala/scala.bzl

@@ -149,8 +149,7 @@ def _collect_plugin_paths(plugins):
     return paths
 
 
-def _compile(ctx, _jars, dep_srcjars, buildijar):
-    jars = _jars
+def _compile(ctx, cjars, dep_srcjars, buildijar):
     ijar_output_path = ""
     ijar_cmd_path = ""
     if buildijar:
@@ -165,7 +164,7 @@ def _compile(ctx, _jars, dep_srcjars, buildijar):
     plugins = _collect_plugin_paths(ctx.attr.plugins)
     plugin_arg = ",".join(list(plugins))
 
-    compiler_classpath = ":".join([j.path for j in jars])
+    compiler_classpath = ":".join([j.path for j in cjars])
 
     scalac_args = """
 Classpath: {cp}
@@ -222,7 +221,7 @@ SourceJars: {srcjars}
     # _jdk added manually since _java doesn't currently setup runfiles
     # _scalac, as a java_binary, should already have it in its runfiles; however,
     # adding does ensure _java not orphaned if _scalac ever was not a java_binary
-    ins = (list(jars) +
+    ins = (list(cjars) +
            list(dep_srcjars) +
            list(srcjars) +
            list(sources) +
@@ -355,32 +354,20 @@ def collect_srcjars(targets):
 
 def _collect_jars(targets):
     """Compute the runtime and compile-time dependencies from the given targets"""  # noqa
-    compile_jars = set()
-    runtime_jars = set()
+    compile_jars = depset()
+    runtime_jars = depset()
     for target in targets:
-        found = False
-        if hasattr(target, "scala"):
-            if hasattr(target.scala.outputs, "ijar"):
-                compile_jars += [target.scala.outputs.ijar]
-            compile_jars += target.scala.transitive_compile_exports
-            runtime_jars += target.scala.transitive_runtime_deps
-            runtime_jars += target.scala.transitive_runtime_exports
-            found = True
-        if hasattr(target, "java"):
-            # see JavaSkylarkApiProvider.java,
-            # this is just the compile-time deps
-            # this should be improved in bazel 0.1.5 to get outputs.ijar
-            # compile_jars += [output.ijar for output in target.java.outputs.jars]
-            compile_jars += target.java.transitive_deps
-            runtime_jars += target.java.transitive_runtime_deps
-            found = True
-        if not found:
+        if java_common.provider in target:
+            java_provider = target[java_common.provider]
+            compile_jars += java_provider.compile_jars
+            runtime_jars += java_provider.transitive_runtime_jars
+        else:
             # support http_file pointed at a jar. http_jar uses ijar,
             # which breaks scala macros
-            runtime_jars += target.files
             compile_jars += target.files
+            runtime_jars += target.files
 
-    return struct(compiletime = compile_jars, runtime = runtime_jars)
+    return struct(compile_jars = compile_jars, transitive_runtime_jars = runtime_jars)
 
 # Extract very common code out from dependency analysis into single place
 # automatically adds dependency on scala-library and scala-reflect
@@ -389,42 +376,67 @@ def _collect_jars_from_common_ctx(ctx, extra_deps = [], extra_runtime_deps = [])
     # Get jars from deps
     auto_deps = [ctx.attr._scalalib, ctx.attr._scalareflect]
     deps_jars = _collect_jars(ctx.attr.deps + auto_deps + extra_deps)
-    (cjars, rjars) = (deps_jars.compiletime, deps_jars.runtime)
-    rjars += _collect_jars(ctx.attr.runtime_deps + extra_runtime_deps).runtime
-    return struct(compiletime = cjars, runtime = rjars)
+    (cjars, transitive_rjars) = (deps_jars.compile_jars, deps_jars.transitive_runtime_jars)
+    transitive_rjars += _collect_jars(
+        ctx.attr.runtime_deps + extra_runtime_deps).transitive_runtime_jars
+    return struct(compile_jars = cjars, transitive_runtime_jars = transitive_rjars)
 
 def _lib(ctx, 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)
-    (cjars, rjars) = (jars.compiletime, jars.runtime)
+    (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars)
 
     write_manifest(ctx)
     outputs = _compile_or_empty(ctx, cjars, srcjars, non_macro_lib)
 
-    rjars += [ctx.outputs.jar]
+    transitive_rjars += [ctx.outputs.jar]
 
-    _build_deployable(ctx, rjars)
-    rule_outputs = struct(ijar=outputs.ijar, class_jar=outputs.class_jar, deploy_jar=ctx.outputs.deploy_jar)
+    _build_deployable(ctx, transitive_rjars)
 
-    texp = _collect_jars(ctx.attr.exports)
-    scalaattr = struct(outputs=rule_outputs,
-                       transitive_runtime_deps=rjars,
-                       transitive_compile_exports=texp.compiletime,
-                       transitive_runtime_exports=texp.runtime
-                       )
+    # Now, need to setup providers for dependents
+    # Notice that transitive_rjars just carries over from dependency analysis
+    # but cjars 'resets' between cjars and next_cjars
+    next_cjars = depset([outputs.ijar])  # use ijar, if available, for future compiles
 
-    # Note that rjars already transitive so don't really
-    # need to use transitive_files with _get_all_runfiles
+    # Using transitive_files since transitive_rjars a depset and avoiding linearization
     runfiles = ctx.runfiles(
-        files=list(rjars),
-        collect_data=True)
+        transitive_files = transitive_rjars,
+        collect_data = True,
+    )
+
+    # Add information from exports (is key that AFTER all build actions/runfiles analysis)
+    # Since after, will not show up in deploy_jar or old jars runfiles
+    # Notice that compile_jars is intentionally transitive for exports
+    exports_jars = _collect_jars(ctx.attr.exports)
+    next_cjars += exports_jars.compile_jars
+    transitive_rjars += exports_jars.transitive_runtime_jars
+
+    rule_outputs = struct(
+        ijar = outputs.ijar,
+        class_jar = outputs.class_jar,
+        deploy_jar = ctx.outputs.deploy_jar,
+    )
+    # Note that, internally, rules only care about compile_jars and transitive_runtime_jars
+    # in a similar manner as the java_library and JavaProvider
+    scalaattr = struct(
+        outputs = rule_outputs,
+        compile_jars = next_cjars,
+        transitive_runtime_jars = transitive_rjars,
+    )
+    java_provider = java_common.create_provider(
+        compile_time_jars = scalaattr.compile_jars,
+        runtime_jars = scalaattr.transitive_runtime_jars,
+    )
 
     return struct(
-        files=set([ctx.outputs.jar]),  # Here is the default output
-        scala=scalaattr,
-        runfiles=runfiles,
+        files = depset([ctx.outputs.jar]),  # Here is the default output
+        scala = scalaattr,
+        providers = [java_provider],
+        runfiles = runfiles,
         # This is a free monoid given to the graph for the purpose of
         # extensibility. This is necessary when one wants to create
         # new targets which want to leverage a scala_library. For example,
@@ -470,39 +482,39 @@ def _scala_binary_common(ctx, cjars, rjars):
       class_jar=outputs.class_jar,
       deploy_jar=ctx.outputs.deploy_jar,
   )
-  scalaattr = struct(outputs = rule_outputs,
-                     transitive_runtime_deps = rjars,
-                     transitive_compile_exports = set(),
-                     transitive_runtime_exports = set()
-                     )
+  scalaattr = struct(
+      outputs = rule_outputs,
+      compile_jars = depset([outputs.class_jar]),
+      transitive_runtime_jars = rjars,
+  )
   return struct(
       files=set([ctx.outputs.executable]),
       scala = scalaattr,
       runfiles=runfiles)
 
 def _scala_binary_impl(ctx):
   jars = _collect_jars_from_common_ctx(ctx)
-  (cjars, rjars) = (jars.compiletime, jars.runtime)
-  rjars += [ctx.outputs.jar]
+  (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars)
+  transitive_rjars += [ctx.outputs.jar]
 
   _write_launcher(
       ctx = ctx,
-      rjars = rjars,
+      rjars = transitive_rjars,
       main_class = ctx.attr.main_class,
       jvm_flags = ctx.attr.jvm_flags,
   )
-  return _scala_binary_common(ctx, cjars, rjars)
+  return _scala_binary_common(ctx, cjars, transitive_rjars)
 
 def _scala_repl_impl(ctx):
   # need scala-compiler for MainGenericRunner below
   jars = _collect_jars_from_common_ctx(ctx, extra_runtime_deps = [ctx.attr._scalacompiler])
-  (cjars, rjars) = (jars.compiletime, jars.runtime)
-  rjars += [ctx.outputs.jar]
+  (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars)
+  transitive_rjars += [ctx.outputs.jar]
 
   args = " ".join(ctx.attr.scalacopts)
   _write_launcher(
       ctx = ctx,
-      rjars = rjars,
+      rjars = transitive_rjars,
       main_class = "scala.tools.nsc.MainGenericRunner",
       jvm_flags = ["-Dscala.usejavacp=true"] + ctx.attr.jvm_flags,
       args = args,
@@ -522,7 +534,7 @@ trap finish EXIT
 """,
   )
 
-  return _scala_binary_common(ctx, cjars, rjars)
+  return _scala_binary_common(ctx, cjars, transitive_rjars)
 
 def _scala_test_impl(ctx):
     if len(ctx.attr.suites) != 0:
@@ -532,14 +544,14 @@ def _scala_test_impl(ctx):
     jars = _collect_jars_from_common_ctx(ctx,
         extra_runtime_deps = [ctx.attr._scalatest_reporter, ctx.attr._scalatest_runner],
     )
-    (cjars, rjars) = (jars.compiletime, jars.runtime)
+    (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars)
     # _scalatest is an http_jar, so its compile jar is run through ijar
     # however, contains macros, so need to handle separately
-    scalatest_jars = _collect_jars([ctx.attr._scalatest]).runtime
+    scalatest_jars = _collect_jars([ctx.attr._scalatest]).transitive_runtime_jars
     cjars += scalatest_jars
-    rjars += scalatest_jars
+    transitive_rjars += scalatest_jars
 
-    rjars += [ctx.outputs.jar]
+    transitive_rjars += [ctx.outputs.jar]
 
     args = " ".join([
         "-R \"{path}\"".format(path=ctx.outputs.jar.short_path),
@@ -549,12 +561,12 @@ def _scala_test_impl(ctx):
     # main_class almost has to be "org.scalatest.tools.Runner" due to args....
     _write_launcher(
         ctx = ctx,
-        rjars = rjars,
+        rjars = transitive_rjars,
         main_class = ctx.attr.main_class,
         jvm_flags = ctx.attr.jvm_flags,
         args = args,
     )
-    return _scala_binary_common(ctx, cjars, rjars)
+    return _scala_binary_common(ctx, cjars, transitive_rjars)
 
 def _gen_test_suite_flags_based_on_prefixes_and_suffixes(ctx, archive):
     return struct(testSuiteFlag = "-Dbazel.test_suite=io.bazel.rulesscala.test_discovery.DiscoveredTestSuite",
@@ -569,20 +581,20 @@ def _scala_junit_test_impl(ctx):
     jars = _collect_jars_from_common_ctx(ctx,
         extra_deps = [ctx.attr._junit, ctx.attr._hamcrest, ctx.attr._suite, ctx.attr._bazel_test_runner],
     )
-    (cjars, rjars) = (jars.compiletime, jars.runtime)
+    (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars)
 
-    rjars += [ctx.outputs.jar]
+    transitive_rjars += [ctx.outputs.jar]
 
     test_suite = _gen_test_suite_flags_based_on_prefixes_and_suffixes(ctx, ctx.outputs.jar)
     launcherJvmFlags = ["-ea", test_suite.archiveFlag, test_suite.prefixesFlag, test_suite.suffixesFlag, test_suite.printFlag, test_suite.testSuiteFlag]
     _write_launcher(
         ctx = ctx,
-        rjars = rjars,
+        rjars = transitive_rjars,
         main_class = "com.google.testing.junit.runner.BazelTestRunner",
         jvm_flags = launcherJvmFlags + ctx.attr.jvm_flags,
     )
 
-    return _scala_binary_common(ctx, cjars, rjars)
+    return _scala_binary_common(ctx, cjars, transitive_rjars)
 
 _launcher_template = {
   "_java_stub_template": attr.label(default=Label("@java_stub_template//file")),
@@ -786,18 +798,6 @@ def scala_repositories():
 
   native.bind(name = "io_bazel_rules_scala/dependency/scalatest/scalatest", actual = "@scalatest//jar")
 
-def scala_export_to_java(name, exports, runtime_deps):
-  jars = []
-  for target in exports:
-    jars.append("{}_deploy.jar".format(target))
-
-  native.java_import(
-    name = name,
-    # these are the outputs of the scala_library targets
-    jars = jars,
-    runtime_deps = ["//external:io_bazel_rules_scala/dependency/scala/scala_library"] + runtime_deps
-  )
-
 def _sanitize_string_for_usage(s):
     res_array = []
     for c in s:

test/BUILD

@@ -5,7 +5,6 @@ load("//scala:scala.bzl",
     "scala_library",
     "scala_test",
     "scala_macro_library",
-    "scala_export_to_java",
     "scala_repl",
     "scala_test_suite",
     "scala_library_suite",
@@ -19,41 +18,28 @@ java_binary(
     name = "JavaBinary",
     srcs = ["JavaBinary.java"],
     main_class = "scala.test.JavaBinary",
-    deps = [":lib_import"],
-)
-
-# TODO(bazel-team): Allow java rules to depend directly on scala_library.
-# For now, we have to use a java_import proxy.
-java_import(
-    name = "lib_import",
-    # these are the outputs of the scala_library targets
-    jars = [
-        ":HelloLib_deploy.jar",
-        "OtherLib_deploy.jar",
-        "Exported_deploy.jar",
-        "Runtime_deploy.jar",
+    deps = [
+        ":HelloLib",
+        ":OtherLib",
+        ":Exported",
+        ":Runtime",
     ],
     runtime_deps = [
-        "OtherJavaLib",
+        ":OtherJavaLib",
         "@scala//:scala-library",
-    ],
-)
-
-scala_export_to_java(
-    name = "lib_import_2",
-    exports = [":HelloLib",
-               "OtherLib",
-               "Exported",
-               "Runtime",
-              ],
-    runtime_deps = ["OtherJavaLib"],
+    ]
 )
 
 java_binary(
     name = "JavaBinary2",
     srcs = ["JavaBinary.java"],
     main_class = "scala.test.JavaBinary",
-    deps = [":lib_import_2"],
+    deps = [
+        ":HelloLib",
+        ":OtherLib",
+        ":Exported",
+        ":Runtime",
+    ],
 )
 
 scala_binary(