From 697f23a8c595902c3683c8c505fda9f50847a78a Mon Sep 17 00:00:00 2001 From: Stephen Twigg Date: Sun, 2 Apr 2017 20:24:01 -0700 Subject: [PATCH 01/24] 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. --- jmh/jmh.bzl | 3 +- scala/scala.bzl | 153 ++++++++++++++++------------ test/BUILD | 39 +++---- twitter_scrooge/twitter_scrooge.bzl | 35 ++++--- 4 files changed, 117 insertions(+), 113 deletions(-) diff --git a/jmh/jmh.bzl b/jmh/jmh.bzl index 577d8cf99..7fe0f49eb 100644 --- a/jmh/jmh.bzl +++ b/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 diff --git a/scala/scala.bzl b/scala/scala.bzl index 2c579afb8..f391712fc 100644 --- a/scala/scala.bzl +++ b/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} @@ -223,7 +222,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) + @@ -356,32 +355,24 @@ 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: + compile_jars += target.scala.compile_jars + runtime_jars += target.scala.transitive_runtime_jars + elif java_common.provider in target: + java_provider = target[java_common.provider] + # TODO(twigg): Use compile_jars when released (narrowly missed 0.5.0 :( ) + compile_jars += target.java.transitive_deps # 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 @@ -390,42 +381,68 @@ 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, + ) + # TODO(twigg): Linearization is concerning here. + java_provider = java_common.create_provider( + compile_time_jars = scalaattr.compile_jars.to_list(), + runtime_jars = scalaattr.transitive_runtime_jars.to_list(), + ) 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, @@ -471,11 +488,11 @@ 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, @@ -483,27 +500,27 @@ def _scala_binary_common(ctx, cjars, rjars): 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, @@ -523,7 +540,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: @@ -533,14 +550,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), @@ -550,12 +567,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", @@ -570,20 +587,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")), @@ -785,6 +802,8 @@ def scala_repositories(): native.bind(name = "io_bazel_rules_scala/dependency/scalatest/scalatest", actual = "@scalatest//jar") +# With providers changes, this is completely deprecatable once bazel release includes +# fixes to java rules using JavaProvider def scala_export_to_java(name, exports, runtime_deps): jars = [] for target in exports: diff --git a/test/BUILD b/test/BUILD index fe0091a97..78ab8438b 100644 --- a/test/BUILD +++ b/test/BUILD @@ -19,41 +19,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( diff --git a/twitter_scrooge/twitter_scrooge.bzl b/twitter_scrooge/twitter_scrooge.bzl index dd216ff8a..cb253a1d3 100644 --- a/twitter_scrooge/twitter_scrooge.bzl +++ b/twitter_scrooge/twitter_scrooge.bzl @@ -153,13 +153,13 @@ def _gen_scrooge_srcjar_impl(ctx): arguments=["--jvm_flag=%s" % flag for flag in ctx.attr.jvm_flags] + ["@" + argfile.path], ) - jars = _collect_scalaattr(ctx.attr.deps) + deps_jars = _collect_scalaattr(ctx.attr.deps) - scalaattr = struct(outputs = None, - transitive_runtime_deps = jars.transitive_runtime_deps, - transitive_compile_exports = jars.transitive_compile_exports, - transitive_runtime_exports = jars.transitive_runtime_exports, - ) + scalaattr = struct( + outputs = None, + compile_jars = deps_jars.compile_jars, + transitive_runtime_jars = deps_jars.transitive_runtime_jars, + ) transitive_srcjars = collect_srcjars(ctx.attr.deps) + collect_extra_srcjars(ctx.attr.deps) @@ -177,22 +177,19 @@ def _gen_scrooge_srcjar_impl(ctx): )], ) +# TODO(twigg): Use one in scala.bzl ... def _collect_scalaattr(targets): - transitive_runtime_deps = set() - transitive_compile_exports = set() - transitive_runtime_exports = set() + compile_jars = depset() + transitive_runtime_jars = depset() + for target in targets: if hasattr(target, "scala"): - transitive_runtime_deps += target.scala.transitive_runtime_deps - transitive_compile_exports += target.scala.transitive_compile_exports - if hasattr(target.scala.outputs, "ijar"): - transitive_compile_exports += [target.scala.outputs.ijar] - transitive_runtime_exports += target.scala.transitive_runtime_exports + compile_jars += target.scala.compile_jars + transitive_runtime_jars += target.scala.transitive_runtime_jars return struct( - transitive_runtime_deps = transitive_runtime_deps, - transitive_compile_exports = transitive_compile_exports, - transitive_runtime_exports = transitive_runtime_exports, + compile_jars = compile_jars, + transitive_runtime_jars = transitive_runtime_jars, ) scrooge_scala_srcjar = rule( @@ -204,7 +201,7 @@ scrooge_scala_srcjar = rule( # is saying that we have a jar with a bunch # of thrifts that we want to depend on. Seems like # that should be a concern of thrift_library? we have - # it here through becuase we need to show that it is + # it here through because we need to show that it is # "covered," as well as needing the thrifts to # do the code gen. "remote_jars": attr.label_list(), @@ -229,6 +226,8 @@ def scrooge_scala_library(name, deps=[], remote_jars=[], jvm_flags=[], visibilit visibility = visibility, ) + # deps from macro invocation would come via srcjar + # however, retained to make dependency analysis via aspects easier scala_library( name = name, deps = deps + remote_jars + [ From 49fae27973d9d5d6af9cda7aac24e582ecfca781 Mon Sep 17 00:00:00 2001 From: Stephen Twigg Date: Mon, 19 Jun 2017 17:10:11 -0700 Subject: [PATCH 02/24] 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?) --- scala/scala.bzl | 27 ++++----------------------- test/BUILD | 1 - twitter_scrooge/twitter_scrooge.bzl | 13 +++++++------ 3 files changed, 11 insertions(+), 30 deletions(-) diff --git a/scala/scala.bzl b/scala/scala.bzl index f391712fc..8154f6d4a 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -358,13 +358,9 @@ def _collect_jars(targets): compile_jars = depset() runtime_jars = depset() for target in targets: - if hasattr(target, "scala"): - compile_jars += target.scala.compile_jars - runtime_jars += target.scala.transitive_runtime_jars - elif java_common.provider in target: + if java_common.provider in target: java_provider = target[java_common.provider] - # TODO(twigg): Use compile_jars when released (narrowly missed 0.5.0 :( ) - compile_jars += target.java.transitive_deps # java_provider.compile_jars + 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, @@ -432,10 +428,9 @@ def _lib(ctx, non_macro_lib): compile_jars = next_cjars, transitive_runtime_jars = transitive_rjars, ) - # TODO(twigg): Linearization is concerning here. java_provider = java_common.create_provider( - compile_time_jars = scalaattr.compile_jars.to_list(), - runtime_jars = scalaattr.transitive_runtime_jars.to_list(), + compile_time_jars = scalaattr.compile_jars, + runtime_jars = scalaattr.transitive_runtime_jars, ) return struct( @@ -802,20 +797,6 @@ def scala_repositories(): native.bind(name = "io_bazel_rules_scala/dependency/scalatest/scalatest", actual = "@scalatest//jar") -# With providers changes, this is completely deprecatable once bazel release includes -# fixes to java rules using JavaProvider -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: diff --git a/test/BUILD b/test/BUILD index 78ab8438b..b1f03b243 100644 --- a/test/BUILD +++ b/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", diff --git a/twitter_scrooge/twitter_scrooge.bzl b/twitter_scrooge/twitter_scrooge.bzl index cb253a1d3..dbf1e2049 100644 --- a/twitter_scrooge/twitter_scrooge.bzl +++ b/twitter_scrooge/twitter_scrooge.bzl @@ -153,7 +153,7 @@ def _gen_scrooge_srcjar_impl(ctx): arguments=["--jvm_flag=%s" % flag for flag in ctx.attr.jvm_flags] + ["@" + argfile.path], ) - deps_jars = _collect_scalaattr(ctx.attr.deps) + deps_jars = _collect_jars(ctx.attr.deps) scalaattr = struct( outputs = None, @@ -177,15 +177,16 @@ def _gen_scrooge_srcjar_impl(ctx): )], ) -# TODO(twigg): Use one in scala.bzl ... -def _collect_scalaattr(targets): +# TODO(twigg): Use the one in scala.bzl? +def _collect_jars(targets): compile_jars = depset() transitive_runtime_jars = depset() for target in targets: - if hasattr(target, "scala"): - compile_jars += target.scala.compile_jars - transitive_runtime_jars += target.scala.transitive_runtime_jars + if java_common.provider in target: + java_provider = target[java_common.provider] + compile_jars += java_provider.compile_jars + transitive_runtime_jars += java_provider.transitive_runtime_jars return struct( compile_jars = compile_jars, From 031ddee14950ae20f132502090a9327f43a3d950 Mon Sep 17 00:00:00 2001 From: natans Date: Wed, 28 Jun 2017 18:30:15 +0300 Subject: [PATCH 03/24] using classpath shrinker plugin to warn about indirect dependency usage --- WORKSPACE | 4 + scala/scala.bzl | 38 +++++++--- .../rulesscala/scalac/CompileOptions.java | 7 ++ .../rulesscala/scalac/ScalacProcessor.java | 5 +- test_expect_logs/missing_direct_deps/A.scala | 8 ++ test_expect_logs/missing_direct_deps/B.scala | 7 ++ test_expect_logs/missing_direct_deps/BUILD | 30 ++++++++ test_expect_logs/missing_direct_deps/C.scala | 7 ++ test_run.sh | 76 +++++++++++-------- 9 files changed, 139 insertions(+), 43 deletions(-) create mode 100644 test_expect_logs/missing_direct_deps/A.scala create mode 100644 test_expect_logs/missing_direct_deps/B.scala create mode 100644 test_expect_logs/missing_direct_deps/BUILD create mode 100644 test_expect_logs/missing_direct_deps/C.scala diff --git a/WORKSPACE b/WORKSPACE index 833b8e577..4204edd68 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -29,3 +29,7 @@ maven_jar( sha1 = "e5b3e2753d0817b622c32aedcb888bcf39e275b4") +http_jar( + name = "classpath_shrinker", + url = "file:///Users/natans/work/classpath-shrinker/plugin/target/scala-2.11/classpath-shrinker_2.11-0.1.1.jar", +) \ No newline at end of file diff --git a/scala/scala.bzl b/scala/scala.bzl index 8154f6d4a..955bc1320 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -149,7 +149,7 @@ def _collect_plugin_paths(plugins): return paths -def _compile(ctx, cjars, dep_srcjars, buildijar): +def _compile(ctx, cjars, dep_srcjars, buildijar, rjars=[], labels = {}): ijar_output_path = "" ijar_cmd_path = "" if buildijar: @@ -164,7 +164,11 @@ def _compile(ctx, cjars, dep_srcjars, buildijar): plugins = _collect_plugin_paths(ctx.attr.plugins) plugin_arg = ",".join(list(plugins)) - compiler_classpath = ":".join([j.path for j in cjars]) + all_jars = cjars + rjars + compiler_classpath = ":".join([j.path for j in all_jars]) + direct_jars = ":".join([j.path for j in cjars]) + indirect_jars = ":".join([j.path for j in rjars]) + indirect_targets = ":".join([labels[j.path] for j in rjars]) scalac_args = """ Classpath: {cp} @@ -186,6 +190,9 @@ ResourceSrcs: {resource_src} ResourceStripPrefix: {resource_strip_prefix} ScalacOpts: {scala_opts} SourceJars: {srcjars} +DirectJars: {direct_jars} +IndirectJars: {indirect_jars} +IndirectTargets: {indirect_targets} """.format( out=ctx.outputs.jar.path, manifest=ctx.outputs.manifest.path, @@ -209,6 +216,9 @@ SourceJars: {srcjars} ), resource_strip_prefix=ctx.attr.resource_strip_prefix, resource_jars=",".join([f.path for f in ctx.files.resource_jars]), + direct_jars=direct_jars, + indirect_jars=indirect_jars, + indirect_targets=indirect_targets ) argfile = ctx.new_file( ctx.outputs.jar, @@ -222,7 +232,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(cjars) + + ins = (list(all_jars) + list(dep_srcjars) + list(srcjars) + list(sources) + @@ -256,14 +266,14 @@ SourceJars: {srcjars} ) -def _compile_or_empty(ctx, jars, srcjars, buildijar): +def _compile_or_empty(ctx, jars, srcjars, buildijar, transitive_jars, jars2labels): # We assume that if a srcjar is present, it is not empty if len(ctx.files.srcs) + len(srcjars) == 0: _build_nosrc_jar(ctx, buildijar) # no need to build ijar when empty return struct(ijar=ctx.outputs.jar, class_jar=ctx.outputs.jar) else: - _compile(ctx, jars, srcjars, buildijar) + _compile(ctx, jars, srcjars, buildijar, transitive_jars, jars2labels) ijar = None if buildijar: ijar = ctx.outputs.ijar @@ -357,18 +367,21 @@ def _collect_jars(targets): """Compute the runtime and compile-time dependencies from the given targets""" # noqa compile_jars = depset() runtime_jars = depset() + jars2labels = {} for target in targets: 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 + jars2labels.update(dict([(jar.path, target.label) + for jar in (java_provider.compile_jars + java_provider.transitive_runtime_jars)])) else: # support http_file pointed at a jar. http_jar uses ijar, # which breaks scala macros compile_jars += target.files runtime_jars += target.files - return struct(compile_jars = compile_jars, transitive_runtime_jars = runtime_jars) + return struct(compile_jars = compile_jars, transitive_runtime_jars = runtime_jars, jars2labels=jars2labels) # Extract very common code out from dependency analysis into single place # automatically adds dependency on scala-library and scala-reflect @@ -377,10 +390,13 @@ 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, 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) + (cjars, transitive_rjars, jars2labels) = (deps_jars.compile_jars, deps_jars.transitive_runtime_jars, deps_jars.jars2labels) + + runtime_dep_jars = _collect_jars(ctx.attr.runtime_deps + extra_runtime_deps) + transitive_rjars += runtime_dep_jars.transitive_runtime_jars + + jars2labels.update(runtime_dep_jars.jars2labels) + return struct(compile_jars = cjars, transitive_runtime_jars = transitive_rjars, jars2labels=jars2labels) def _lib(ctx, non_macro_lib): # Build up information from dependency-like attributes @@ -392,7 +408,7 @@ def _lib(ctx, non_macro_lib): (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) write_manifest(ctx) - outputs = _compile_or_empty(ctx, cjars, srcjars, non_macro_lib) + outputs = _compile_or_empty(ctx, cjars, srcjars, non_macro_lib, transitive_rjars, jars.jars2labels) transitive_rjars += [ctx.outputs.jar] diff --git a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java index de8d66f8d..10f74ac1d 100644 --- a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java +++ b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java @@ -23,6 +23,9 @@ public class CompileOptions { final public Map resourceFiles; final public String resourceStripPrefix; final public String[] resourceJars; + final public String[] directJars; + final public String[] indirectJars; + final public String[] indirectTargets; public CompileOptions(List args) { Map argMap = buildArgMap(args); @@ -54,6 +57,10 @@ public CompileOptions(List args) { resourceFiles = getResources(argMap); resourceStripPrefix = getOrEmpty(argMap, "ResourceStripPrefix"); resourceJars = getCommaList(argMap, "ResourceJars"); + + directJars = getCommaList(argMap, "DirectJars"); + indirectJars = getCommaList(argMap, "IndirectJars"); + indirectTargets = getCommaList(argMap, "IndirectTargets"); } private static Map getResources(Map args) { diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index e1a368b73..a4b816696 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -192,7 +192,10 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource "-classpath", ops.classpath, "-d", - tmpPath.toString() + tmpPath.toString(), + "-P:classpath-shrinker:direct-jars:" + String.join(":", ops.directJars), + "-P:classpath-shrinker:indirect-jars:" + String.join(":", ops.indirectJars), + "-P:classpath-shrinker:indirect-targets:" + String.join(":", ops.indirectTargets), }; String[] compilerArgs = GenericWorker.merge( diff --git a/test_expect_logs/missing_direct_deps/A.scala b/test_expect_logs/missing_direct_deps/A.scala new file mode 100644 index 000000000..58b66b63f --- /dev/null +++ b/test_expect_logs/missing_direct_deps/A.scala @@ -0,0 +1,8 @@ +package test_expect_logs.missing_direct_deps + +object A { + def foo = { + B.foo + C.foo + } +} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/B.scala b/test_expect_logs/missing_direct_deps/B.scala new file mode 100644 index 000000000..7884bc65f --- /dev/null +++ b/test_expect_logs/missing_direct_deps/B.scala @@ -0,0 +1,7 @@ +package test_expect_logs.missing_direct_deps + +object B { + def foo = { + C.foo + } +} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/BUILD b/test_expect_logs/missing_direct_deps/BUILD new file mode 100644 index 000000000..a9c4a8074 --- /dev/null +++ b/test_expect_logs/missing_direct_deps/BUILD @@ -0,0 +1,30 @@ +package(default_visibility = ["//visibility:public"]) +load("//scala:scala.bzl", "scala_library", "scala_test") + +scala_library( + name="transitive_dependency_user", + srcs=[ + "A.scala", + ], + deps = ["direct_dependency"], + plugins = ["@classpath_shrinker//jar"], +) + +scala_library( + name="direct_dependency", + srcs=[ + "B.scala", + ], + deps = ["transitive_dependency"], + plugins = ["@classpath_shrinker//jar"], + +) + +scala_library( + name="transitive_dependency", + srcs=[ + "C.scala", + ], + plugins = ["@classpath_shrinker//jar"], + +) \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/C.scala b/test_expect_logs/missing_direct_deps/C.scala new file mode 100644 index 000000000..ae9d5f28b --- /dev/null +++ b/test_expect_logs/missing_direct_deps/C.scala @@ -0,0 +1,7 @@ +package test_expect_logs.missing_direct_deps + +object C { + def foo = { + println("in C") + } +} \ No newline at end of file diff --git a/test_run.sh b/test_run.sh index fb64abf53..2396dd7da 100755 --- a/test_run.sh +++ b/test_run.sh @@ -71,6 +71,19 @@ test_scala_library_suite() { exit 0 } +test_scala_library_expect_warning_on_missing_direct_deps() { + expected_message="target 'transitive_dependency' should be added to deps" + command='bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' + output=$($command 2>&1) + echo "$output" + echo $output | grep "$expected_message" + if [ $? -ne 0 ]; then + echo "'bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' should have logged \"$expected_message\"." + exit 1 + fi + exit 0 +} + test_scala_junit_test_can_fail() { set +e @@ -339,34 +352,35 @@ else runner="run_test_ci" fi -$runner bazel build test/... -$runner bazel test test/... -$runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges -$runner bazel run test:JavaBinary -$runner bazel run test:JavaBinary2 -$runner bazel run test:MixJavaScalaLibBinary -$runner bazel run test:MixJavaScalaSrcjarLibBinary -$runner bazel run test:ScalaBinary -$runner bazel run test:ScalaLibBinary -$runner test_disappearing_class -$runner find -L ./bazel-testlogs -iname "*.xml" -$runner xmllint_test -$runner test_build_is_identical -$runner test_transitive_deps -$runner test_scala_library_suite -$runner test_repl -$runner bazel run test:JavaOnlySources -$runner test_benchmark_jmh -$runner multiple_junit_suffixes -$runner multiple_junit_prefixes -$runner test_scala_junit_test_can_fail -$runner junit_generates_xml_logs -$runner scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix -$runner multiple_junit_patterns -$runner test_junit_test_must_have_prefix_or_suffix -$runner test_junit_test_errors_when_no_tests_found -$runner scala_library_jar_without_srcs_must_include_direct_file_resources -$runner scala_library_jar_without_srcs_must_include_filegroup_resources -$runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath -$runner scala_test_test_filters -$runner scala_junit_test_test_filter +$runner test_scala_library_expect_warning_on_missing_direct_deps +#$runner bazel build test/... +#$runner bazel test test/... +#$runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges +#$runner bazel run test:JavaBinary +#$runner bazel run test:JavaBinary2 +#$runner bazel run test:MixJavaScalaLibBinary +#$runner bazel run test:MixJavaScalaSrcjarLibBinary +#$runner bazel run test:ScalaBinary +#$runner bazel run test:ScalaLibBinary +#$runner test_disappearing_class +#$runner find -L ./bazel-testlogs -iname "*.xml" +#$runner xmllint_test +#$runner test_build_is_identical +#$runner test_transitive_deps +#$runner test_scala_library_suite +#$runner test_repl +#$runner bazel run test:JavaOnlySources +#$runner test_benchmark_jmh +#$runner multiple_junit_suffixes +#$runner multiple_junit_prefixes +#$runner test_scala_junit_test_can_fail +#$runner junit_generates_xml_logs +#$runner scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix +#$runner multiple_junit_patterns +#$runner test_junit_test_must_have_prefix_or_suffix +#$runner test_junit_test_errors_when_no_tests_found +#$runner scala_library_jar_without_srcs_must_include_direct_file_resources +#$runner scala_library_jar_without_srcs_must_include_filegroup_resources +#$runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath +#$runner scala_test_test_filters +#$runner scala_junit_test_test_filter From a827601b7b1746578620411b05b8b0dfeff8855d Mon Sep 17 00:00:00 2001 From: natans Date: Thu, 29 Jun 2017 12:15:56 +0300 Subject: [PATCH 04/24] fix escaping bugs --- scala/scala.bzl | 6 +- .../rulesscala/scalac/ScalacProcessor.java | 16 +++-- test_run.sh | 64 +++++++++---------- 3 files changed, 45 insertions(+), 41 deletions(-) diff --git a/scala/scala.bzl b/scala/scala.bzl index 955bc1320..d0e451b20 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -166,9 +166,9 @@ def _compile(ctx, cjars, dep_srcjars, buildijar, rjars=[], labels = {}): all_jars = cjars + rjars compiler_classpath = ":".join([j.path for j in all_jars]) - direct_jars = ":".join([j.path for j in cjars]) - indirect_jars = ":".join([j.path for j in rjars]) - indirect_targets = ":".join([labels[j.path] for j in rjars]) + direct_jars = ",".join([j.path for j in cjars]) + indirect_jars = ",".join([j.path for j in rjars]) + indirect_targets = ",".join([labels[j.path] for j in rjars]) scalac_args = """ Classpath: {cp} diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index a4b816696..c11c05464 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -17,11 +17,7 @@ import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Enumeration; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -187,7 +183,15 @@ private static boolean matchesFileExtensions(String fileName, String[] extension return false; } + private static String[] encodeBazelTargets(String[] targets) { + return Arrays.stream(targets) + .map(label -> label.replace(":", ";")) + .toArray(String[]::new); + } + private static void compileScalaSources(CompileOptions ops, String[] scalaSources, Path tmpPath) throws IllegalAccessException { + String[] targets = encodeBazelTargets(ops.indirectTargets); + String[] constParams = { "-classpath", ops.classpath, @@ -195,7 +199,7 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource tmpPath.toString(), "-P:classpath-shrinker:direct-jars:" + String.join(":", ops.directJars), "-P:classpath-shrinker:indirect-jars:" + String.join(":", ops.indirectJars), - "-P:classpath-shrinker:indirect-targets:" + String.join(":", ops.indirectTargets), + "-P:classpath-shrinker:indirect-targets:" + String.join(":", targets), }; String[] compilerArgs = GenericWorker.merge( diff --git a/test_run.sh b/test_run.sh index 2396dd7da..4c2fa5f4e 100755 --- a/test_run.sh +++ b/test_run.sh @@ -72,7 +72,7 @@ test_scala_library_suite() { } test_scala_library_expect_warning_on_missing_direct_deps() { - expected_message="target 'transitive_dependency' should be added to deps" + expected_message="target '//test_expect_logs/missing_direct_deps:direct_dependency' should be added to deps" command='bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' output=$($command 2>&1) echo "$output" @@ -353,34 +353,34 @@ else fi $runner test_scala_library_expect_warning_on_missing_direct_deps -#$runner bazel build test/... -#$runner bazel test test/... -#$runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges -#$runner bazel run test:JavaBinary -#$runner bazel run test:JavaBinary2 -#$runner bazel run test:MixJavaScalaLibBinary -#$runner bazel run test:MixJavaScalaSrcjarLibBinary -#$runner bazel run test:ScalaBinary -#$runner bazel run test:ScalaLibBinary -#$runner test_disappearing_class -#$runner find -L ./bazel-testlogs -iname "*.xml" -#$runner xmllint_test -#$runner test_build_is_identical -#$runner test_transitive_deps -#$runner test_scala_library_suite -#$runner test_repl -#$runner bazel run test:JavaOnlySources -#$runner test_benchmark_jmh -#$runner multiple_junit_suffixes -#$runner multiple_junit_prefixes -#$runner test_scala_junit_test_can_fail -#$runner junit_generates_xml_logs -#$runner scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix -#$runner multiple_junit_patterns -#$runner test_junit_test_must_have_prefix_or_suffix -#$runner test_junit_test_errors_when_no_tests_found -#$runner scala_library_jar_without_srcs_must_include_direct_file_resources -#$runner scala_library_jar_without_srcs_must_include_filegroup_resources -#$runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath -#$runner scala_test_test_filters -#$runner scala_junit_test_test_filter +$runner bazel build test/... +$runner bazel test test/... +$runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges +$runner bazel run test:JavaBinary +$runner bazel run test:JavaBinary2 +$runner bazel run test:MixJavaScalaLibBinary +$runner bazel run test:MixJavaScalaSrcjarLibBinary +$runner bazel run test:ScalaBinary +$runner bazel run test:ScalaLibBinary +$runner test_disappearing_class +$runner find -L ./bazel-testlogs -iname "*.xml" +$runner xmllint_test +$runner test_build_is_identical +$runner test_transitive_deps +$runner test_scala_library_suite +$runner test_repl +$runner bazel run test:JavaOnlySources +$runner test_benchmark_jmh +$runner multiple_junit_suffixes +$runner multiple_junit_prefixes +$runner test_scala_junit_test_can_fail +$runner junit_generates_xml_logs +$runner scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix +$runner multiple_junit_patterns +$runner test_junit_test_must_have_prefix_or_suffix +$runner test_junit_test_errors_when_no_tests_found +$runner scala_library_jar_without_srcs_must_include_direct_file_resources +$runner scala_library_jar_without_srcs_must_include_filegroup_resources +$runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath +$runner scala_test_test_filters +$runner scala_junit_test_test_filter From f0a96ffb4540046958c106c1fe1aeb30406ff3a8 Mon Sep 17 00:00:00 2001 From: natans Date: Sun, 2 Jul 2017 21:16:29 +0300 Subject: [PATCH 05/24] attempt to put dependency-analyzer inside rules_scala --- plugin/src/WORKSPACE | 38 +++++ plugin/src/main/BUILD | 20 +++ plugin/src/main/resources/scalac-plugin.xml | 4 + plugin/src/main/resources/toolbox.classpath | 1 + .../DependencyAnalyzer.scala | 100 +++++++++++ .../retronym/dependencyanalyzer/Compat.scala | 11 ++ .../retronym/dependencyanalyzer/Compat.scala | 11 ++ plugin/src/test/BUILD | 27 +++ plugin/src/test/resources/BUILD | 9 + plugin/src/test/resources/toolbox.plugin | 1 + .../DependencyAnalyzerSpec.scala | 92 +++++++++++ .../dependencyanalyzer/TestUtil.scala | 114 +++++++++++++ scala/scala.bzl | 14 +- .../rulesscala/scalac/CompileOptions.java | 3 + .../rulesscala/scalac/ScalacProcessor.java | 155 ++++++++++-------- test_expect_logs/missing_direct_deps/BUILD | 6 +- 16 files changed, 532 insertions(+), 74 deletions(-) create mode 100644 plugin/src/WORKSPACE create mode 100644 plugin/src/main/BUILD create mode 100644 plugin/src/main/resources/scalac-plugin.xml create mode 100644 plugin/src/main/resources/toolbox.classpath create mode 100644 plugin/src/main/scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzer.scala create mode 100644 plugin/src/main/scala_2_11/io/github/retronym/dependencyanalyzer/Compat.scala create mode 100644 plugin/src/main/scala_2_12/io/github/retronym/dependencyanalyzer/Compat.scala create mode 100644 plugin/src/test/BUILD create mode 100644 plugin/src/test/resources/BUILD create mode 100644 plugin/src/test/resources/toolbox.plugin create mode 100644 plugin/src/test/scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzerSpec.scala create mode 100644 plugin/src/test/scala/io/github/retronym/dependencyanalyzer/TestUtil.scala diff --git a/plugin/src/WORKSPACE b/plugin/src/WORKSPACE new file mode 100644 index 000000000..a0022ec09 --- /dev/null +++ b/plugin/src/WORKSPACE @@ -0,0 +1,38 @@ +rules_scala_version="3070353d06bb726141e6cea241e5d6e355a4d14f" # update this as needed + +http_archive( + name = "io_bazel_rules_scala", + url = "https://github.com/wix/rules_scala/archive/%s.zip"%rules_scala_version, + type = "zip", + strip_prefix= "rules_scala-%s" % rules_scala_version +) + +load("@io_bazel_rules_scala//scala:scala.bzl", "scala_repositories") +scala_repositories() +load("@io_bazel_rules_scala//specs2:specs2_junit.bzl","specs2_junit_repositories") +specs2_junit_repositories() + +maven_jar( + name = "io_get_coursier_coursier_cache", + artifact = "io.get-coursier:coursier-cache_2.11:jar:1.0.0-M15", +) + +maven_jar( + name = "io_get_coursier_coursier", + artifact = "io.get-coursier:coursier_2.11:jar:1.0.0-M15", +) + +maven_jar( + name = "org_scalaz_scalaz_concurrent_2_11", + artifact = "org.scalaz:scalaz-concurrent_2.11:jar:7.2.7" +) + +maven_jar( + name = "org_scalaz_scalaz_core_2_11", + artifact = "org.scalaz:scalaz-core_2.11:jar:7.2.7" +) + +maven_jar( + name = "org_scalaz_scala_effect_2_11", + artifact = "org.scalaz:scalaz-effect_2.11:jar:7.2.7" +) \ No newline at end of file diff --git a/plugin/src/main/BUILD b/plugin/src/main/BUILD new file mode 100644 index 000000000..1bd098752 --- /dev/null +++ b/plugin/src/main/BUILD @@ -0,0 +1,20 @@ +load("@io_bazel_rules_scala//scala:scala.bzl", "scala_library") + +scala_library( + name = "dependency_analyzer", + srcs = ["scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzer.scala", + "scala_2_11/io/github/retronym/dependencyanalyzer/Compat.scala", + ], + resources = ["resources/scalac-plugin.xml"], + visibility = ["//visibility:public"], + deps = [ + "//external:io_bazel_rules_scala/dependency/scala/scala_compiler" + ], +# enable_dependency_analyzer = False +) + +scala_library( + name = "resources", + resources = ["resources/toolbox.classpath"], + visibility = ["//visibility:public"], +) \ No newline at end of file diff --git a/plugin/src/main/resources/scalac-plugin.xml b/plugin/src/main/resources/scalac-plugin.xml new file mode 100644 index 000000000..d1daeb6d4 --- /dev/null +++ b/plugin/src/main/resources/scalac-plugin.xml @@ -0,0 +1,4 @@ + + dependency-analyzer + plugin.src.main.scala.io.github.retronym.dependencyanalyzer.DependencyAnalyzer + \ No newline at end of file diff --git a/plugin/src/main/resources/toolbox.classpath b/plugin/src/main/resources/toolbox.classpath new file mode 100644 index 000000000..88e7c419e --- /dev/null +++ b/plugin/src/main/resources/toolbox.classpath @@ -0,0 +1 @@ +/Users/natans/Work/rules_scala/bazel-genfiles/external/scala/_ijar/scala-compiler/external/scala/lib/scala-compiler-ijar.jar:/Users/natans/Work/rules_scala/bazel-genfiles/external/scala/_ijar/scala-library/external/scala/lib/scala-library-ijar.jar \ No newline at end of file diff --git a/plugin/src/main/scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzer.scala b/plugin/src/main/scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzer.scala new file mode 100644 index 000000000..b50500a85 --- /dev/null +++ b/plugin/src/main/scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzer.scala @@ -0,0 +1,100 @@ +package plugin.src.main.scala.io.github.retronym.dependencyanalyzer + +import plugin.src.main.scala_2_11.io.github.retronym.dependencyanalyzer.Compat + +import scala.reflect.io.AbstractFile +import scala.tools.nsc.plugins.{Plugin, PluginComponent} +import scala.tools.nsc.{Global, Phase} + +class DependencyAnalyzer(val global: Global) extends Plugin with Compat { + + val name = "dependency-analyzer" + val description = + "Warns about classpath entries that are not directly needed." + val components = List[PluginComponent](Component) + + var indirect: Map[String, String] = Map.empty + var direct: Set[String] = Set.empty + + override def processOptions(options: List[String], error: (String) => Unit): Unit = { + var indirectJars: Seq[String] = Seq.empty + var indirectTargets: Seq[String] = Seq.empty + + for (option <- options) { + option.split(":").toList match { + case "direct-jars" :: data => direct = data.toSet + case "indirect-jars" :: data => indirectJars = data; + case "indirect-targets" :: data => indirectTargets = data.map(_.replace(";", ":")) + case unknown :: _ => error(s"unknown param $unknown") + case Nil => + } + } + indirect = indirectJars.zip(indirectTargets).toMap + } + + + private object Component extends PluginComponent { + val global: DependencyAnalyzer.this.global.type = + DependencyAnalyzer.this.global + + import global._ + + override val runsAfter = List("jvm") + + val phaseName = DependencyAnalyzer.this.name + + override def newPhase(prev: Phase): StdPhase = new StdPhase(prev) { + override def run(): Unit = { + + super.run() + + val usedJars = findUsedJars + + warnOnIndirectTargetsFoundIn(usedJars) + } + + private def warnOnIndirectTargetsFoundIn(usedJars: Set[AbstractFile]) = { + for (usedJar <- usedJars; + usedJarPath = usedJar.path; + target <- indirect.get(usedJarPath) + if !direct.contains(usedJarPath)) { + reporter.error(NoPosition, s"Target '$target' is used but isn't explicitly declared, please add it to the deps") + } + } + + override def apply(unit: CompilationUnit): Unit = () + } + + } + + import global._ + + private def findUsedJars: Set[AbstractFile] = { + val jars = collection.mutable.Set[AbstractFile]() + + def walkTopLevels(root: Symbol): Unit = { + def safeInfo(sym: Symbol): Type = + if (sym.hasRawInfo && sym.rawInfo.isComplete) sym.info else NoType + + def packageClassOrSelf(sym: Symbol): Symbol = + if (sym.hasPackageFlag && !sym.isModuleClass) sym.moduleClass else sym + + for (x <- safeInfo(packageClassOrSelf(root)).decls) { + if (x == root) () + else if (x.hasPackageFlag) walkTopLevels(x) + else if (x.owner != root) { // exclude package class members + if (x.hasRawInfo && x.rawInfo.isComplete) { + val assocFile = x.associatedFile + if (assocFile.path.endsWith(".class") && assocFile.underlyingSource.isDefined) + assocFile.underlyingSource.foreach(jars += _) + } + } + } + } + + exitingTyper { + walkTopLevels(RootClass) + } + jars.toSet + } +} diff --git a/plugin/src/main/scala_2_11/io/github/retronym/dependencyanalyzer/Compat.scala b/plugin/src/main/scala_2_11/io/github/retronym/dependencyanalyzer/Compat.scala new file mode 100644 index 000000000..3c76be229 --- /dev/null +++ b/plugin/src/main/scala_2_11/io/github/retronym/dependencyanalyzer/Compat.scala @@ -0,0 +1,11 @@ +package plugin.src.main.scala_2_11.io.github.retronym.dependencyanalyzer + +import scala.tools.nsc.Settings +import scala.tools.nsc.classpath.FlatClassPathFactory + +/** + * Provides compatibility stubs for 2.11 and 2.12 Scala compilers. + */ +trait Compat { + def getClassPathFrom(settings: Settings) = new FlatClassPathFactory(settings) +} diff --git a/plugin/src/main/scala_2_12/io/github/retronym/dependencyanalyzer/Compat.scala b/plugin/src/main/scala_2_12/io/github/retronym/dependencyanalyzer/Compat.scala new file mode 100644 index 000000000..a8e08bac9 --- /dev/null +++ b/plugin/src/main/scala_2_12/io/github/retronym/dependencyanalyzer/Compat.scala @@ -0,0 +1,11 @@ +package plugin.src.main.scala_2_12.io.github.retronym.dependencyanalyzer + +import scala.tools.nsc.Settings +import scala.tools.nsc.classpath.ClassPathFactory + +/** + * Provides compatibility stubs for 2.11 and 2.12 Scala compilers. + */ +trait Compat { + def getClassPathFrom(settings: Settings) = new ClassPathFactory(settings) +} diff --git a/plugin/src/test/BUILD b/plugin/src/test/BUILD new file mode 100644 index 000000000..a3ac5aa93 --- /dev/null +++ b/plugin/src/test/BUILD @@ -0,0 +1,27 @@ +load("@io_bazel_rules_scala//scala:scala.bzl", + "scala_binary", + "scala_library", + "scala_test", + "scala_macro_library", + "scala_specs2_junit_test", + "scala_export_to_java", + "scala_junit_test") + +scala_junit_test( + name = "dependency_analyzer_test", + srcs = ["scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzerSpec.scala", + "scala/io/github/retronym/dependencyanalyzer/TestUtil.scala"], + suffixes = ["Test"], + size = "small", + deps = ["//main:dependency_analyzer", + "@io_get_coursier_coursier_cache//jar", + "@io_get_coursier_coursier//jar", + "@org_scalaz_scalaz_concurrent_2_11//jar", + "//external:io_bazel_rules_scala/dependency/scalaz/scalaz_effect", + "//external:io_bazel_rules_scala/dependency/scalaz/scalaz_core", + "//external:io_bazel_rules_scala/dependency/scala/scala_xml", + "//external:io_bazel_rules_scala/dependency/scala/scala_compiler", + "//main:resources", + "//test/resources:resources" + ] +) \ No newline at end of file diff --git a/plugin/src/test/resources/BUILD b/plugin/src/test/resources/BUILD new file mode 100644 index 000000000..74fdc7b49 --- /dev/null +++ b/plugin/src/test/resources/BUILD @@ -0,0 +1,9 @@ +load("@io_bazel_rules_scala//scala:scala.bzl", "scala_library") + +package(default_visibility = ["//visibility:public"]) + +scala_library( + name = "resources", + resources = native.glob(["**"],exclude=["BUILD"]), + visibility = ["//visibility:public"], +) \ No newline at end of file diff --git a/plugin/src/test/resources/toolbox.plugin b/plugin/src/test/resources/toolbox.plugin new file mode 100644 index 000000000..7ec9ae852 --- /dev/null +++ b/plugin/src/test/resources/toolbox.plugin @@ -0,0 +1 @@ +-Xplugin:/Users/natans/Work/rules_scala/plugin/src/bazel-bin/main/dependency_analyzer.jar -Jdummy=1499012573000 \ No newline at end of file diff --git a/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzerSpec.scala b/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzerSpec.scala new file mode 100644 index 000000000..8572b8b78 --- /dev/null +++ b/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/DependencyAnalyzerSpec.scala @@ -0,0 +1,92 @@ +package plugin.src.test.scala.io.github.retronym.dependencyanalyzer + +import coursier.{Dependency, Module} +import plugin.src.test.scala.io.github.retronym.dependencyanalyzer.TestUtil._ +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(classOf[JUnit4]) +class DependencyAnalyzerTest { + + object Dependencies { + val commons = + Dependency(Module("org.apache.commons", "commons-lang3"), "3.5") + val guava = Dependency(Module("com.google.guava", "guava"), "21.0") + } + + import Dependencies._ + + @Test + def `error on indirect dependency target`(): Unit = { + val testCode = + """object Foo { + | org.apache.commons.lang3.ArrayUtils.EMPTY_BOOLEAN_ARRAY.length + |} + """.stripMargin + val commonsPath = Coursier.getArtifact(commons) + val commonsTarget = "//commons:Target".encode() + val indirect = Map(commonsPath -> commonsTarget) + run(testCode, withIndirect = indirect).expectErrorOn(indirect(commonsPath).decoded) + } + + @Test + def `error on multiple indirect dependency targets`(): Unit = { + val testCode = + """object Foo { + | org.apache.commons.lang3.ArrayUtils.EMPTY_BOOLEAN_ARRAY.length + | com.google.common.base.Strings.commonPrefix("abc", "abcd") + |} + """.stripMargin + val commonsPath = Coursier.getArtifact(commons) + val commonsTarget = "commonsTarget" + + val guavaPath = Coursier.getArtifact(guava) + val guavaTarget = "guavaTarget" + + val indirect = Map(commonsPath -> commonsTarget, guavaPath -> guavaTarget) + run(testCode, withIndirect = indirect).expectErrorOn(commonsTarget, guavaTarget) + } + + @Test + def `do not give error on direct dependency target`(): Unit = { + val testCode = + """object Foo { + | org.apache.commons.lang3.ArrayUtils.EMPTY_BOOLEAN_ARRAY.length + |} + """.stripMargin + val commonsPath = Coursier.getArtifact(commons) + val commonsTarget = "commonsTarget" + + val direct = Seq(commonsPath) + val indirect = Map(commonsPath -> commonsTarget) + run(testCode, withDirect = direct, withIndirect = indirect).noErrorOn(commonsTarget) + } + + + implicit class `nice errors on sequence of strings`(infos: Seq[String]) { + + private def checkErrorContainsMessage(target: String) = (_: String).contains(targetErrorMessage(target)) + + private def targetErrorMessage(target: String) = s"Target '$target' is used but isn't explicitly declared, please add it to the deps" + + def expectErrorOn(targets: String*) = targets.foreach(target => assert( + infos.exists(checkErrorContainsMessage(target)), + s"expected an error on $target to appear in errors!") + ) + + def noErrorOn(target: String) = assert( + !infos.exists(checkErrorContainsMessage(target)), + s"error on $target should not appear in errors!") + } + + implicit class `decode bazel lables`(targetLabel: String) { + def decoded() = { + targetLabel.replace(";", ":") + } + + def encode() = { + targetLabel.replace(":", ";") + } + } +} diff --git a/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/TestUtil.scala b/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/TestUtil.scala new file mode 100644 index 000000000..585bd6f78 --- /dev/null +++ b/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/TestUtil.scala @@ -0,0 +1,114 @@ +package plugin.src.test.scala.io.github.retronym.dependencyanalyzer + +import java.io.File + +import coursier.maven.MavenRepository +import coursier.{Cache, Dependency, Fetch, Resolution} + +import scala.reflect.internal.util.{BatchSourceFile, NoPosition} +import scala.reflect.io.VirtualDirectory +import scala.tools.cmd.CommandLineParser +import scala.tools.nsc.reporters.StoreReporter +import scala.tools.nsc.{CompilerCommand, Global, Settings} +import scalaz.concurrent.Task + +object TestUtil { + + import scala.language.postfixOps + + def run(code: String, withDirect: Seq[String] = Seq.empty, withIndirect: Map[String, String] = Map.empty): Seq[String] = { + val compileOptions = Seq( + constructParam("direct-jars", withDirect), + constructParam("indirect-jars", withIndirect.keys), + constructParam("indirect-targets", withIndirect.values) + ).mkString(" ") + + val extraClasspath = withDirect ++ withIndirect.keys + + val reporter: StoreReporter = runCompilation(code, compileOptions, extraClasspath) + reporter.infos.collect({ case msg if msg.severity == reporter.ERROR => msg.msg }).toSeq + } + + private def runCompilation(code: String, compileOptions: String, extraClasspath: Seq[String]) = { + val fullClasspath: String = { + val extraClasspathString = extraClasspath.mkString(":") + if (toolboxClasspath.isEmpty) extraClasspathString + else s"$toolboxClasspath:$extraClasspathString" + } + val basicOptions = + createBasicCompileOptions(fullClasspath, toolboxPluginOptions) + + println("basicOptions: " + basicOptions) + + eval(code, s"$basicOptions $compileOptions") + } + + /** Evaluate using global instance instead of toolbox because toolbox seems + * to fail to typecheck code that comes from external dependencies. */ + private def eval(code: String, compileOptions: String = ""): StoreReporter = { + // TODO: Optimize and cache global. + val options = CommandLineParser.tokenize(compileOptions) + val reporter = new StoreReporter() + val settings = new Settings(println) + val _ = new CompilerCommand(options, settings) + settings.outputDirs.setSingleOutput(new VirtualDirectory("(memory)", None)) + val global = new Global(settings, reporter) + val run = new global.Run + val toCompile = new BatchSourceFile("", code) + run.compileSources(List(toCompile)) + reporter + } + + def getResourceContent(resourceName: String): String = { + println("getResourceContent "+ resourceName) + val resource = getClass.getClassLoader.getResourceAsStream(resourceName) + println("getResourceContent resource: "+ resource) + + val file = scala.io.Source.fromInputStream(resource) + file.getLines.mkString + } + + lazy val toolboxClasspath: String = getResourceContent("toolbox.classpath") + lazy val toolboxPluginOptions: String = getResourceContent("toolbox.plugin") + + private def createBasicCompileOptions(classpath: String, usePluginOptions: String) = + s"-classpath $classpath $usePluginOptions" + + private def constructParam(name: String, values: Iterable[String]) = { + if (values.isEmpty) "" + else s"-P:dependency-analyzer:$name:${values.mkString(":")}" + } + + object Coursier { + private final val repositories = Seq( + Cache.ivy2Local, + MavenRepository("https://repo1.maven.org/maven2") + ) + + def getArtifact(dependency: Dependency) = getArtifacts(Seq(dependency)).head + + private def getArtifacts(deps: Seq[Dependency]): Seq[String] = + getArtifacts(deps, toAbsolutePath) + + private def getArtifacts(deps: Seq[Dependency], fileToString: File => String): Seq[String] = { + val toResolve = Resolution(deps.toSet) + val fetch = Fetch.from(repositories, Cache.fetch()) + val resolution = toResolve.process.run(fetch).run + val resolutionErrors = resolution.errors + if (resolutionErrors.nonEmpty) + sys.error(s"Modules could not be resolved:\n$resolutionErrors.") + val errorsOrJars = Task + .gatherUnordered(resolution.artifacts.map(Cache.file(_).run)) + .unsafePerformSync + val onlyErrors = errorsOrJars.filter(_.isLeft) + if (onlyErrors.nonEmpty) + sys.error(s"Jars could not be fetched from cache:\n$onlyErrors") + errorsOrJars.flatMap(_.map(fileToString).toList) + } + + private def toAbsolutePath(f: File): String = + f.getAbsolutePath + + } + +} diff --git a/scala/scala.bzl b/scala/scala.bzl index d0e451b20..cd518971f 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -162,6 +162,11 @@ def _compile(ctx, cjars, dep_srcjars, buildijar, rjars=[], labels = {}): all_srcjars = set(srcjars + list(dep_srcjars)) # look for any plugins: plugins = _collect_plugin_paths(ctx.attr.plugins) + + if ctx.attr.enable_dependency_analyzer: + dep_plugin = ctx.attr.dependency_analyzer_plugin + plugins += [f.path for f in dep_plugin.files] + plugin_arg = ",".join(list(plugins)) all_jars = cjars + rjars @@ -193,6 +198,7 @@ SourceJars: {srcjars} DirectJars: {direct_jars} IndirectJars: {indirect_jars} IndirectTargets: {indirect_targets} +EnableDependencyAnalyzer: {enable_dependency_analyzer} """.format( out=ctx.outputs.jar.path, manifest=ctx.outputs.manifest.path, @@ -218,7 +224,8 @@ IndirectTargets: {indirect_targets} resource_jars=",".join([f.path for f in ctx.files.resource_jars]), direct_jars=direct_jars, indirect_jars=indirect_jars, - indirect_targets=indirect_targets + indirect_targets=indirect_targets, + enable_dependency_analyzer = ctx.attr.enable_dependency_analyzer, ) argfile = ctx.new_file( ctx.outputs.jar, @@ -238,6 +245,7 @@ IndirectTargets: {indirect_targets} list(sources) + ctx.files.srcs + ctx.files.plugins + + ctx.files.dependency_analyzer_plugin + ctx.files.resources + ctx.files.resource_jars + ctx.files._jdk + @@ -266,7 +274,7 @@ IndirectTargets: {indirect_targets} ) -def _compile_or_empty(ctx, jars, srcjars, buildijar, transitive_jars, jars2labels): +def _compile_or_empty(ctx, jars, srcjars, buildijar, transitive_jars = [], jars2labels = []): # We assume that if a srcjar is present, it is not empty if len(ctx.files.srcs) + len(srcjars) == 0: _build_nosrc_jar(ctx, buildijar) @@ -652,6 +660,8 @@ scala_library = rule( attrs={ "main_class": attr.string(), "exports": attr.label_list(allow_files=False), + "enable_dependency_analyzer": attr.bool(default=True, mandatory=False), + "dependency_analyzer_plugin": attr.label(default=Label("@classpath_shrinker//jar"), allow_files=_jar_filetype, mandatory=False), } + _implicit_deps + _common_attrs, outputs={ "jar": "%{name}.jar", diff --git a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java index 10f74ac1d..a3442b5d1 100644 --- a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java +++ b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java @@ -26,6 +26,7 @@ public class CompileOptions { final public String[] directJars; final public String[] indirectJars; final public String[] indirectTargets; + final public boolean enableDependencyAnalyzer; public CompileOptions(List args) { Map argMap = buildArgMap(args); @@ -61,6 +62,8 @@ public CompileOptions(List args) { directJars = getCommaList(argMap, "DirectJars"); indirectJars = getCommaList(argMap, "IndirectJars"); indirectTargets = getCommaList(argMap, "IndirectTargets"); + + enableDependencyAnalyzer = booleanGetOrFalse(argMap, "EnableDependencyAnalyzer"); } private static Map getResources(Map args) { diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index c11c05464..c94721a8c 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -3,6 +3,7 @@ import io.bazel.rulesscala.jar.JarCreator; import io.bazel.rulesscala.worker.GenericWorker; import io.bazel.rulesscala.worker.Processor; + import java.io.BufferedWriter; import java.io.File; import java.io.FileNotFoundException; @@ -21,6 +22,7 @@ import java.util.Map.Entry; import java.util.jar.JarEntry; import java.util.jar.JarFile; + import scala.tools.nsc.Driver; import scala.tools.nsc.MainClass; import scala.tools.nsc.reporters.ConsoleReporter; @@ -30,12 +32,12 @@ class ScalacProcessor implements Processor { * This is the reporter field for scalac, which we want to access */ private static Field reporterField; + static { try { reporterField = Driver.class.getDeclaredField("reporter"); //NoSuchFieldException reporterField.setAccessible(true); - } - catch (NoSuchFieldException ex) { + } catch (NoSuchFieldException ex) { throw new RuntimeException("could not access reporter field on Driver", ex); } } @@ -85,28 +87,27 @@ public void processRequest(List args) throws Exception { * Now build the output jar */ String[] jarCreatorArgs = { - "-m", - ops.manifestPath, - outputPath.toString(), - tmpPath.toString() + "-m", + ops.manifestPath, + outputPath.toString(), + tmpPath.toString() }; JarCreator.buildJar(jarCreatorArgs); /** * Now build the output ijar */ - if(ops.iJarEnabled) { + if (ops.iJarEnabled) { Process iostat = new ProcessBuilder() - .command(ops.ijarCmdPath, ops.outputName, ops.ijarOutput) - .inheritIO() - .start(); + .command(ops.ijarCmdPath, ops.outputName, ops.ijarOutput) + .inheritIO() + .start(); int exitCode = iostat.waitFor(); - if(exitCode != 0) { + if (exitCode != 0) { throw new RuntimeException("ijar process failed!"); } } - } - finally { + } finally { removeTmp(tmpPath); } } @@ -118,7 +119,7 @@ private static String[] collectSrcJarSources(String[] files, List scalaJar private static List filterFilesByExtension(List files, String extension) { List filtered = new ArrayList(); - for (File f: files) { + for (File f : files) { if (f.toString().endsWith(extension)) { filtered.add(f); } @@ -127,11 +128,12 @@ private static List filterFilesByExtension(List files, String extens } static private String[] sourceExtensions = {".scala", ".java"}; + static private List extractSourceJars(CompileOptions opts, Path tmpParent) throws IOException { List sourceFiles = new ArrayList(); - for(String jarPath : opts.sourceJars) { - if (jarPath.length() > 0){ + for (String jarPath : opts.sourceJars) { + if (jarPath.length() > 0) { Path tmpPath = Files.createTempDirectory(tmpParent, "tmp"); sourceFiles.addAll(extractJar(jarPath, tmpPath.toString(), sourceExtensions)); } @@ -141,8 +143,8 @@ static private List extractSourceJars(CompileOptions opts, Path tmpParent) } private static List extractJar(String jarPath, - String outputFolder, - String[] extensions) throws IOException, FileNotFoundException { + String outputFolder, + String[] extensions) throws IOException, FileNotFoundException { List outputPaths = new ArrayList(); JarFile jar = new JarFile(jarPath); @@ -175,7 +177,7 @@ private static List extractJar(String jarPath, } private static boolean matchesFileExtensions(String fileName, String[] extensions) { - for (String e: extensions) { + for (String e : extensions) { if (fileName.endsWith(e)) { return true; } @@ -192,21 +194,33 @@ private static String[] encodeBazelTargets(String[] targets) { private static void compileScalaSources(CompileOptions ops, String[] scalaSources, Path tmpPath) throws IllegalAccessException { String[] targets = encodeBazelTargets(ops.indirectTargets); + String[] pluginParamsInUse = { + "-P:classpath-shrinker:direct-jars:" + String.join(":", ops.directJars), + "-P:classpath-shrinker:indirect-jars:" + String.join(":", ops.indirectJars), + "-P:classpath-shrinker:indirect-targets:" + String.join(":", targets), + }; + + String[] pluginParams; + if (ops.enableDependencyAnalyzer) { + pluginParams = pluginParamsInUse; + } else { + pluginParams = new String[0]; + } + String[] constParams = { - "-classpath", - ops.classpath, - "-d", - tmpPath.toString(), - "-P:classpath-shrinker:direct-jars:" + String.join(":", ops.directJars), - "-P:classpath-shrinker:indirect-jars:" + String.join(":", ops.indirectJars), - "-P:classpath-shrinker:indirect-targets:" + String.join(":", targets), + "-classpath", + ops.classpath, + "-d", + tmpPath.toString() }; + String[] compilerArgs = GenericWorker.merge( - ops.scalaOpts, - ops.pluginArgs, - constParams, - scalaSources); + ops.scalaOpts, + ops.pluginArgs, + constParams, + pluginParams, + scalaSources); MainClass comp = new MainClass(); long start = System.currentTimeMillis(); @@ -243,14 +257,13 @@ private static void compileJavaSources(CompileOptions ops, String[] javaSources, commandParts.add("@" + normalizeSlash(argsFile.toFile().getAbsolutePath())); try { Process iostat = new ProcessBuilder(commandParts) - .inheritIO() - .start(); + .inheritIO() + .start(); int exitCode = iostat.waitFor(); - if(exitCode != 0) { + if (exitCode != 0) { throw new RuntimeException("javac process failed!"); } - } - finally { + } finally { removeTmp(argsFile); } } @@ -263,9 +276,10 @@ private static final String escapeSpaces(String str) { return '\"' + normalizeSlash(str) + '\"'; } - /** collects javac compile options into an 'argfile' - * http://docs.oracle.com/javase/8/docs/technotes/tools/windows/javac.html#BHCJEIBB - */ + /** + * collects javac compile options into an 'argfile' + * http://docs.oracle.com/javase/8/docs/technotes/tools/windows/javac.html#BHCJEIBB + */ private static final Path newArgFile(CompileOptions ops, String[] javaSources, Path tmpPath) throws IOException { Path argsFile = Files.createTempFile("argfile", null); List args = new ArrayList<>(); @@ -275,7 +289,7 @@ private static final Path newArgFile(CompileOptions ops, String[] javaSources, P args.add("-classpath " + ops.classpath + ":" + tmpPath.toString()); args.add("-d " + tmpPath.toString()); - for(String javaFile : javaSources) { + for (String javaFile : javaSources) { args.add(escapeSpaces(javaFile.toString())); } String contents = String.join("\n", args); @@ -288,38 +302,39 @@ private static final Path newArgFile(CompileOptions ops, String[] javaSources, P private static void removeTmp(Path tmp) throws IOException { if (tmp != null) { Files.walkFileTree(tmp, new SimpleFileVisitor() { - @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { - Files.delete(file); - return FileVisitResult.CONTINUE; - } - - @Override - public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { - Files.delete(dir); - return FileVisitResult.CONTINUE; - } + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + Files.delete(file); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + Files.delete(dir); + return FileVisitResult.CONTINUE; + } }); } } + private static void copyResources( - Map resources, - String resourceStripPrefix, - Path dest) throws IOException { - for(Entry e : resources.entrySet()) { + Map resources, + String resourceStripPrefix, + Path dest) throws IOException { + for (Entry e : resources.entrySet()) { Path source = Paths.get(e.getKey()); String dstr; // Check if we need to modify resource destination path if (!"".equals(resourceStripPrefix)) { - /** - * NOTE: We are not using the Resource Hash Value as the destination path - * when `resource_strip_prefix` present. The path in the hash value is computed - * by the `_adjust_resources_path` in `scala.bzl`. These are the default paths, - * ie, path that are automatically computed when there is no `resource_strip_prefix` - * present. But when `resource_strip_prefix` is present, we need to strip the prefix - * from the Source Path and use that as the new destination path - * Refer Bazel -> BazelJavaRuleClasses.java#L227 for details - */ + /** + * NOTE: We are not using the Resource Hash Value as the destination path + * when `resource_strip_prefix` present. The path in the hash value is computed + * by the `_adjust_resources_path` in `scala.bzl`. These are the default paths, + * ie, path that are automatically computed when there is no `resource_strip_prefix` + * present. But when `resource_strip_prefix` is present, we need to strip the prefix + * from the Source Path and use that as the new destination path + * Refer Bazel -> BazelJavaRuleClasses.java#L227 for details + */ dstr = getResourcePath(source, resourceStripPrefix); } else { dstr = e.getValue(); @@ -331,23 +346,25 @@ private static void copyResources( Files.copy(source, target); } } + private static String getResourcePath( - Path source, - String resourceStripPrefix) throws RuntimeException { + Path source, + String resourceStripPrefix) throws RuntimeException { String sourcePath = source.toString(); // check if the Resource file is under the specified prefix to strip if (!sourcePath.startsWith(resourceStripPrefix)) { // Resource File is not under the specified prefix to strip throw new RuntimeException("Resource File " - + sourcePath - + " is not under the specified strip prefix " - + resourceStripPrefix); + + sourcePath + + " is not under the specified strip prefix " + + resourceStripPrefix); } String newResPath = sourcePath.substring(resourceStripPrefix.length()); return newResPath; } + private static void copyResourceJars(String[] resourceJars, Path dest) throws IOException { - for (String jarPath: resourceJars) { + for (String jarPath : resourceJars) { extractJar(jarPath, dest.toString(), null); } } diff --git a/test_expect_logs/missing_direct_deps/BUILD b/test_expect_logs/missing_direct_deps/BUILD index a9c4a8074..b0556d6e9 100644 --- a/test_expect_logs/missing_direct_deps/BUILD +++ b/test_expect_logs/missing_direct_deps/BUILD @@ -7,7 +7,7 @@ scala_library( "A.scala", ], deps = ["direct_dependency"], - plugins = ["@classpath_shrinker//jar"], +# plugins = ["@classpath_shrinker//jar"], ) scala_library( @@ -16,7 +16,7 @@ scala_library( "B.scala", ], deps = ["transitive_dependency"], - plugins = ["@classpath_shrinker//jar"], +# plugins = ["@classpath_shrinker//jar"], ) @@ -25,6 +25,6 @@ scala_library( srcs=[ "C.scala", ], - plugins = ["@classpath_shrinker//jar"], +# plugins = ["@classpath_shrinker//jar"], ) \ No newline at end of file From 22cb9d2463c571147a068bde5f5f10331aaae78a Mon Sep 17 00:00:00 2001 From: natans Date: Mon, 3 Jul 2017 12:42:23 +0300 Subject: [PATCH 06/24] merge workspaces --- WORKSPACE | 28 ++++++++++++-- plugin/src/WORKSPACE | 38 ------------------- plugin/src/test/BUILD | 15 ++------ plugin/src/test/resources/toolbox.plugin | 2 +- scala/scala.bzl | 6 +-- .../rulesscala/scalac/ScalacProcessor.java | 6 +-- .../missing_direct_deps/A.scala | 8 ++++ .../missing_direct_deps/B.scala | 7 ++++ .../missing_direct_deps/BUILD | 5 --- .../missing_direct_deps/C.scala | 7 ++++ test_expect_logs/missing_direct_deps/A.scala | 8 ---- test_expect_logs/missing_direct_deps/B.scala | 7 ---- test_expect_logs/missing_direct_deps/C.scala | 7 ---- test_run.sh | 15 +++++--- 14 files changed, 68 insertions(+), 91 deletions(-) delete mode 100644 plugin/src/WORKSPACE create mode 100644 test_expect_failure/missing_direct_deps/A.scala create mode 100644 test_expect_failure/missing_direct_deps/B.scala rename {test_expect_logs => test_expect_failure}/missing_direct_deps/BUILD (75%) create mode 100644 test_expect_failure/missing_direct_deps/C.scala delete mode 100644 test_expect_logs/missing_direct_deps/A.scala delete mode 100644 test_expect_logs/missing_direct_deps/B.scala delete mode 100644 test_expect_logs/missing_direct_deps/C.scala diff --git a/WORKSPACE b/WORKSPACE index 4204edd68..962b96f76 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -1,5 +1,16 @@ workspace(name = "io_bazel_rules_scala") + +####### previous rules scala for building of scala compiler plugin +rules_scala_version="3070353d06bb726141e6cea241e5d6e355a4d14f" # update this as needed + +http_archive( + name = "io_bazel_rules_scala", + url = "https://github.com/wix/rules_scala/archive/%s.zip"%rules_scala_version, + type = "zip", + strip_prefix= "rules_scala-%s" % rules_scala_version +) + load("//scala:scala.bzl", "scala_repositories", "scala_mvn_artifact") scala_repositories() @@ -28,8 +39,19 @@ maven_jar( artifact = scala_mvn_artifact("org.psywerx.hairyfotr:linter:0.1.13"), sha1 = "e5b3e2753d0817b622c32aedcb888bcf39e275b4") +# test of a scala plugin +maven_jar( + name = "io_get_coursier_coursier_cache", + artifact = "io.get-coursier:coursier-cache_2.11:jar:1.0.0-M15", + sha1 = "cbf7a208ccd4c3ad44efa3886e237ecbf96a6fd9") + +maven_jar( + name = "io_get_coursier_coursier", + artifact = "io.get-coursier:coursier_2.11:jar:1.0.0-M15", + sha1 = "9c6281274f9964a786cba4a5df62740c07b07046") -http_jar( - name = "classpath_shrinker", - url = "file:///Users/natans/work/classpath-shrinker/plugin/target/scala-2.11/classpath-shrinker_2.11-0.1.1.jar", +maven_jar( + name = "org_scalaz_scalaz_concurrent_2_11", + artifact = "org.scalaz:scalaz-concurrent_2.11:jar:7.2.7", + sha1 = "abaea3aa04f11301f63099d96cf47f91ec229ed4" ) \ No newline at end of file diff --git a/plugin/src/WORKSPACE b/plugin/src/WORKSPACE deleted file mode 100644 index a0022ec09..000000000 --- a/plugin/src/WORKSPACE +++ /dev/null @@ -1,38 +0,0 @@ -rules_scala_version="3070353d06bb726141e6cea241e5d6e355a4d14f" # update this as needed - -http_archive( - name = "io_bazel_rules_scala", - url = "https://github.com/wix/rules_scala/archive/%s.zip"%rules_scala_version, - type = "zip", - strip_prefix= "rules_scala-%s" % rules_scala_version -) - -load("@io_bazel_rules_scala//scala:scala.bzl", "scala_repositories") -scala_repositories() -load("@io_bazel_rules_scala//specs2:specs2_junit.bzl","specs2_junit_repositories") -specs2_junit_repositories() - -maven_jar( - name = "io_get_coursier_coursier_cache", - artifact = "io.get-coursier:coursier-cache_2.11:jar:1.0.0-M15", -) - -maven_jar( - name = "io_get_coursier_coursier", - artifact = "io.get-coursier:coursier_2.11:jar:1.0.0-M15", -) - -maven_jar( - name = "org_scalaz_scalaz_concurrent_2_11", - artifact = "org.scalaz:scalaz-concurrent_2.11:jar:7.2.7" -) - -maven_jar( - name = "org_scalaz_scalaz_core_2_11", - artifact = "org.scalaz:scalaz-core_2.11:jar:7.2.7" -) - -maven_jar( - name = "org_scalaz_scala_effect_2_11", - artifact = "org.scalaz:scalaz-effect_2.11:jar:7.2.7" -) \ No newline at end of file diff --git a/plugin/src/test/BUILD b/plugin/src/test/BUILD index a3ac5aa93..6eabb50e1 100644 --- a/plugin/src/test/BUILD +++ b/plugin/src/test/BUILD @@ -1,11 +1,4 @@ -load("@io_bazel_rules_scala//scala:scala.bzl", - "scala_binary", - "scala_library", - "scala_test", - "scala_macro_library", - "scala_specs2_junit_test", - "scala_export_to_java", - "scala_junit_test") +load("@io_bazel_rules_scala//scala:scala.bzl", "scala_junit_test") scala_junit_test( name = "dependency_analyzer_test", @@ -13,7 +6,9 @@ scala_junit_test( "scala/io/github/retronym/dependencyanalyzer/TestUtil.scala"], suffixes = ["Test"], size = "small", - deps = ["//main:dependency_analyzer", + deps = ["//plugin/src/main:dependency_analyzer", + "//plugin/src/main:resources", + "//plugin/src/test/resources:resources", "@io_get_coursier_coursier_cache//jar", "@io_get_coursier_coursier//jar", "@org_scalaz_scalaz_concurrent_2_11//jar", @@ -21,7 +16,5 @@ scala_junit_test( "//external:io_bazel_rules_scala/dependency/scalaz/scalaz_core", "//external:io_bazel_rules_scala/dependency/scala/scala_xml", "//external:io_bazel_rules_scala/dependency/scala/scala_compiler", - "//main:resources", - "//test/resources:resources" ] ) \ No newline at end of file diff --git a/plugin/src/test/resources/toolbox.plugin b/plugin/src/test/resources/toolbox.plugin index 7ec9ae852..c45d7ed1e 100644 --- a/plugin/src/test/resources/toolbox.plugin +++ b/plugin/src/test/resources/toolbox.plugin @@ -1 +1 @@ --Xplugin:/Users/natans/Work/rules_scala/plugin/src/bazel-bin/main/dependency_analyzer.jar -Jdummy=1499012573000 \ No newline at end of file +-Xplugin:/Users/natans/Work/rules_scala/bazel-bin/plugin/src/main/dependency_analyzer.jar -Jdummy=1499012573000 \ No newline at end of file diff --git a/scala/scala.bzl b/scala/scala.bzl index cd518971f..bbff1ada0 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -163,7 +163,7 @@ def _compile(ctx, cjars, dep_srcjars, buildijar, rjars=[], labels = {}): # look for any plugins: plugins = _collect_plugin_paths(ctx.attr.plugins) - if ctx.attr.enable_dependency_analyzer: + if hasattr(ctx.attr, 'enable_dependency_analyzer') and ctx.attr.enable_dependency_analyzer: dep_plugin = ctx.attr.dependency_analyzer_plugin plugins += [f.path for f in dep_plugin.files] @@ -653,6 +653,8 @@ _common_attrs = { "javacopts":attr.string_list(), "jvm_flags": attr.string_list(), "print_compile_time": attr.bool(default=False, mandatory=False), + "enable_dependency_analyzer": attr.bool(default=True, mandatory=False), + "dependency_analyzer_plugin": attr.label(default=Label("//plugin/src/main:dependency_analyzer"), allow_files=_jar_filetype, mandatory=False), } scala_library = rule( @@ -660,8 +662,6 @@ scala_library = rule( attrs={ "main_class": attr.string(), "exports": attr.label_list(allow_files=False), - "enable_dependency_analyzer": attr.bool(default=True, mandatory=False), - "dependency_analyzer_plugin": attr.label(default=Label("@classpath_shrinker//jar"), allow_files=_jar_filetype, mandatory=False), } + _implicit_deps + _common_attrs, outputs={ "jar": "%{name}.jar", diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index c94721a8c..19a6fd720 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -195,9 +195,9 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource String[] targets = encodeBazelTargets(ops.indirectTargets); String[] pluginParamsInUse = { - "-P:classpath-shrinker:direct-jars:" + String.join(":", ops.directJars), - "-P:classpath-shrinker:indirect-jars:" + String.join(":", ops.indirectJars), - "-P:classpath-shrinker:indirect-targets:" + String.join(":", targets), + "-P:dependency-analyzer:direct-jars:" + String.join(":", ops.directJars), + "-P:dependency-analyzer:indirect-jars:" + String.join(":", ops.indirectJars), + "-P:dependency-analyzer:indirect-targets:" + String.join(":", targets), }; String[] pluginParams; diff --git a/test_expect_failure/missing_direct_deps/A.scala b/test_expect_failure/missing_direct_deps/A.scala new file mode 100644 index 000000000..e7898ff5f --- /dev/null +++ b/test_expect_failure/missing_direct_deps/A.scala @@ -0,0 +1,8 @@ +package test_expect_failure.missing_direct_deps + +object A { + def foo = { + B.foo + C.foo + } +} \ No newline at end of file diff --git a/test_expect_failure/missing_direct_deps/B.scala b/test_expect_failure/missing_direct_deps/B.scala new file mode 100644 index 000000000..2cbf5e5f0 --- /dev/null +++ b/test_expect_failure/missing_direct_deps/B.scala @@ -0,0 +1,7 @@ +package test_expect_failure.missing_direct_deps + +object B { + def foo = { + C.foo + } +} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/BUILD b/test_expect_failure/missing_direct_deps/BUILD similarity index 75% rename from test_expect_logs/missing_direct_deps/BUILD rename to test_expect_failure/missing_direct_deps/BUILD index b0556d6e9..40d5bc8c8 100644 --- a/test_expect_logs/missing_direct_deps/BUILD +++ b/test_expect_failure/missing_direct_deps/BUILD @@ -7,7 +7,6 @@ scala_library( "A.scala", ], deps = ["direct_dependency"], -# plugins = ["@classpath_shrinker//jar"], ) scala_library( @@ -16,8 +15,6 @@ scala_library( "B.scala", ], deps = ["transitive_dependency"], -# plugins = ["@classpath_shrinker//jar"], - ) scala_library( @@ -25,6 +22,4 @@ scala_library( srcs=[ "C.scala", ], -# plugins = ["@classpath_shrinker//jar"], - ) \ No newline at end of file diff --git a/test_expect_failure/missing_direct_deps/C.scala b/test_expect_failure/missing_direct_deps/C.scala new file mode 100644 index 000000000..8f60eb518 --- /dev/null +++ b/test_expect_failure/missing_direct_deps/C.scala @@ -0,0 +1,7 @@ +package test_expect_failure.missing_direct_deps + +object C { + def foo = { + println("in C") + } +} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/A.scala b/test_expect_logs/missing_direct_deps/A.scala deleted file mode 100644 index 58b66b63f..000000000 --- a/test_expect_logs/missing_direct_deps/A.scala +++ /dev/null @@ -1,8 +0,0 @@ -package test_expect_logs.missing_direct_deps - -object A { - def foo = { - B.foo - C.foo - } -} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/B.scala b/test_expect_logs/missing_direct_deps/B.scala deleted file mode 100644 index 7884bc65f..000000000 --- a/test_expect_logs/missing_direct_deps/B.scala +++ /dev/null @@ -1,7 +0,0 @@ -package test_expect_logs.missing_direct_deps - -object B { - def foo = { - C.foo - } -} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/C.scala b/test_expect_logs/missing_direct_deps/C.scala deleted file mode 100644 index ae9d5f28b..000000000 --- a/test_expect_logs/missing_direct_deps/C.scala +++ /dev/null @@ -1,7 +0,0 @@ -package test_expect_logs.missing_direct_deps - -object C { - def foo = { - println("in C") - } -} \ No newline at end of file diff --git a/test_run.sh b/test_run.sh index 4c2fa5f4e..0372e69e6 100755 --- a/test_run.sh +++ b/test_run.sh @@ -71,14 +71,19 @@ test_scala_library_suite() { exit 0 } -test_scala_library_expect_warning_on_missing_direct_deps() { - expected_message="target '//test_expect_logs/missing_direct_deps:direct_dependency' should be added to deps" - command='bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' +test_scala_library_expect_failure_on_missing_direct_deps() { + expected_message="Target '//test_expect_failure/missing_direct_deps:direct_dependency' is used but isn't explicitly declared, please add it to the deps" + command='bazel build test_expect_failure/missing_direct_deps:transitive_dependency_user' output=$($command 2>&1) + if [ $? -eq 0 ]; then + echo "$output" + echo "'bazel build of scala_library with missing direct deps should have failed." + exit 1 + fi echo "$output" echo $output | grep "$expected_message" if [ $? -ne 0 ]; then - echo "'bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' should have logged \"$expected_message\"." + echo "'bazel build test_expect_failure/missing_direct_deps:transitive_dependency_user' should have logged \"$expected_message\"." exit 1 fi exit 0 @@ -352,7 +357,6 @@ else runner="run_test_ci" fi -$runner test_scala_library_expect_warning_on_missing_direct_deps $runner bazel build test/... $runner bazel test test/... $runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges @@ -384,3 +388,4 @@ $runner scala_library_jar_without_srcs_must_include_filegroup_resources $runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath $runner scala_test_test_filters $runner scala_junit_test_test_filter +$runner test_scala_library_expect_failure_on_missing_direct_deps From 3ec634d2c4644f67b4bd4b20b06c542e81597556 Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Tue, 20 Jun 2017 19:08:48 +0300 Subject: [PATCH 07/24] renamed MoreScala* targets to ResourcesStripScala* targets (#232) --- test/BUILD | 18 +++++++++--------- ...y.scala => ResourcesStripScalaBinary.scala} | 8 ++++---- ...rces.scala => ResourcesStripScalaLib.scala} | 2 +- test_run.sh | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) rename test/src/main/scala/scala/test/{MoreScalaLibBinary.scala => ResourcesStripScalaBinary.scala} (77%) rename test/src/main/scala/scala/test/{MoreScalaLibResources.scala => ResourcesStripScalaLib.scala} (96%) diff --git a/test/BUILD b/test/BUILD index b1f03b243..08fe5dcd4 100644 --- a/test/BUILD +++ b/test/BUILD @@ -151,8 +151,8 @@ scala_library( ) scala_library( - name = "MoreScalaLibResources", - srcs = ["src/main/scala/scala/test/MoreScalaLibResources.scala"], + name = "ResourcesStripScalaLib", + srcs = ["src/main/scala/scala/test/ResourcesStripScalaLib.scala"], resources = [ "//test/data:more.txt", "//test/data:foo.txt", @@ -170,10 +170,10 @@ scala_binary( ) scala_binary( - name = "MoreScalaLibBinary", - srcs = ["src/main/scala/scala/test/MoreScalaLibBinary.scala"], - main_class = "scala.test.MoreScalaLibBinary", - deps = ["MoreScalaLibResources"], + name = "ResourcesStripScalaBinary", + srcs = ["src/main/scala/scala/test/ResourcesStripScalaBinary.scala"], + main_class = "scala.test.ResourcesStripScalaBinary", + deps = ["ResourcesStripScalaLib"], ) scala_repl( @@ -181,8 +181,8 @@ scala_repl( deps = [":ScalaLibBinary"]) scala_repl( - name = "MoreScalaLibBinaryRepl", - deps = [":MoreScalaLibBinary"]) + name = "ResourcesStripScalaBinaryRepl", + deps = [":ResourcesStripScalaBinary"]) scala_repl( name = "ReplWithSources", @@ -292,7 +292,7 @@ scala_specs2_junit_test( "JavaBinary2", "ScalaBinary", "ScalaLibBinary", - "MoreScalaLibBinary", + "ResourcesStripScalaBinary", "MixJavaScalaLibBinary", "JavaOnlySources", ]] diff --git a/test/src/main/scala/scala/test/MoreScalaLibBinary.scala b/test/src/main/scala/scala/test/ResourcesStripScalaBinary.scala similarity index 77% rename from test/src/main/scala/scala/test/MoreScalaLibBinary.scala rename to test/src/main/scala/scala/test/ResourcesStripScalaBinary.scala index 3960872de..7f161db15 100644 --- a/test/src/main/scala/scala/test/MoreScalaLibBinary.scala +++ b/test/src/main/scala/scala/test/ResourcesStripScalaBinary.scala @@ -14,10 +14,10 @@ package scala.test -object MoreScalaLibBinary { +object ResourcesStripScalaBinary { def main(args:Array[String]) { - MoreScalaLibResources.getGreetings foreach println - MoreScalaLibResources.getFarewells foreach println - MoreScalaLibResources.getData foreach println + ResourcesStripScalaLib.getGreetings foreach println + ResourcesStripScalaLib.getFarewells foreach println + ResourcesStripScalaLib.getData foreach println } } diff --git a/test/src/main/scala/scala/test/MoreScalaLibResources.scala b/test/src/main/scala/scala/test/ResourcesStripScalaLib.scala similarity index 96% rename from test/src/main/scala/scala/test/MoreScalaLibResources.scala rename to test/src/main/scala/scala/test/ResourcesStripScalaLib.scala index 902012b82..2ddacb829 100644 --- a/test/src/main/scala/scala/test/MoreScalaLibResources.scala +++ b/test/src/main/scala/scala/test/ResourcesStripScalaLib.scala @@ -14,7 +14,7 @@ package scala.test -object MoreScalaLibResources { +object ResourcesStripScalaLib { def getGreetings() = get("/src/main/resources/scala/test/more-hellos") def getFarewells = get("/src/main/resources/scala/test/more-byes") diff --git a/test_run.sh b/test_run.sh index 0372e69e6..24de3d387 100755 --- a/test_run.sh +++ b/test_run.sh @@ -105,7 +105,7 @@ test_repl() { echo "import scala.test._; HelloLib.printMessage(\"foo\")" | bazel-bin/test/HelloLibRepl | grep "foo java" && echo "import scala.test._; TestUtil.foo" | bazel-bin/test/HelloLibTestRepl | grep "bar" && echo "import scala.test._; ScalaLibBinary.main(Array())" | bazel-bin/test/ScalaLibBinaryRepl | grep "A hui hou" && - echo "import scala.test._; MoreScalaLibBinary.main(Array())" | bazel-bin/test/MoreScalaLibBinaryRepl | grep "More Hello" + echo "import scala.test._; ResourcesStripScalaBinary.main(Array())" | bazel-bin/test/ResourcesStripScalaBinaryRepl | grep "More Hello" echo "import scala.test._; A.main(Array())" | bazel-bin/test/ReplWithSources | grep "4 8 15" } From 9aa0499a5fda3f77707f04498c8c5148b72139b6 Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Sun, 25 Jun 2017 16:30:11 +0300 Subject: [PATCH 08/24] introduced scalac_jvm_flags instead of overloading jvm_flags for both scalac and executable (breaking change) --- scala/scala.bzl | 3 ++- test_expect_failure/compilers_jvm_flags/BUILD | 6 ++++++ .../WillNotCompileScalaSinceXmxTooLow.scala | 1 + test_run.sh | 12 ++++++++++++ 4 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 test_expect_failure/compilers_jvm_flags/BUILD create mode 100644 test_expect_failure/compilers_jvm_flags/WillNotCompileScalaSinceXmxTooLow.scala diff --git a/scala/scala.bzl b/scala/scala.bzl index bbff1ada0..de20fd5b0 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -270,7 +270,7 @@ EnableDependencyAnalyzer: {enable_dependency_analyzer} # be correctly handled since the executable is a jvm app that will # consume the flags on startup. - arguments=["--jvm_flag=%s" % flag for flag in ctx.attr.jvm_flags] + ["@" + argfile.path], + arguments=["--jvm_flag=%s" % flag for flag in ctx.attr.scalac_jvm_flags] + ["@" + argfile.path], ) @@ -652,6 +652,7 @@ _common_attrs = { "scalacopts":attr.string_list(), "javacopts":attr.string_list(), "jvm_flags": attr.string_list(), + "scalac_jvm_flags": attr.string_list(), "print_compile_time": attr.bool(default=False, mandatory=False), "enable_dependency_analyzer": attr.bool(default=True, mandatory=False), "dependency_analyzer_plugin": attr.label(default=Label("//plugin/src/main:dependency_analyzer"), allow_files=_jar_filetype, mandatory=False), diff --git a/test_expect_failure/compilers_jvm_flags/BUILD b/test_expect_failure/compilers_jvm_flags/BUILD new file mode 100644 index 000000000..fc3e3ccdd --- /dev/null +++ b/test_expect_failure/compilers_jvm_flags/BUILD @@ -0,0 +1,6 @@ +load("//scala:scala.bzl", "scala_library") +scala_library( + name = "can_configure_jvm_flags_for_scalac", + srcs = ["WillNotCompileScalaSinceXmxTooLow.scala"], + scalac_jvm_flags = ["-Xmx1M"], +) \ No newline at end of file diff --git a/test_expect_failure/compilers_jvm_flags/WillNotCompileScalaSinceXmxTooLow.scala b/test_expect_failure/compilers_jvm_flags/WillNotCompileScalaSinceXmxTooLow.scala new file mode 100644 index 000000000..572f0d6c5 --- /dev/null +++ b/test_expect_failure/compilers_jvm_flags/WillNotCompileScalaSinceXmxTooLow.scala @@ -0,0 +1 @@ +class WillNotCompileScalaSinceXmxTooLow \ No newline at end of file diff --git a/test_run.sh b/test_run.sh index 24de3d387..3a479b931 100755 --- a/test_run.sh +++ b/test_run.sh @@ -350,6 +350,17 @@ scala_junit_test_test_filter(){ done } +scalac_jvm_flags_are_configured(){ + set +e + + bazel build //test_expect_failure/compilers_jvm_flags:can_configure_jvm_flags_for_scalac + if [ $? -eq 0 ]; then + echo "'bazel build test_expect_failure/compiler_jvm_flags:can_configure_jvm_flags_for_scalac' should have failed." + exit 1 + fi + set -e + exit 0 +} if [ "$1" != "ci" ]; then runner="run_test_local" @@ -388,4 +399,5 @@ $runner scala_library_jar_without_srcs_must_include_filegroup_resources $runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath $runner scala_test_test_filters $runner scala_junit_test_test_filter +$runner scalac_jvm_flags_are_configured $runner test_scala_library_expect_failure_on_missing_direct_deps From 3e31081b5a46ced8bc92eba13444c1a770e7f45b Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Sun, 25 Jun 2017 16:50:05 +0300 Subject: [PATCH 09/24] introduced javac_jvm_flags --- scala/scala.bzl | 8 ++++---- .../bazel/rulesscala/scalac/CompileOptions.java | 2 -- .../bazel/rulesscala/scalac/ScalacProcessor.java | 1 - test_expect_failure/compilers_jvm_flags/BUILD | 5 +++++ .../WillNotCompileJavaSinceXmxTooLow.java | 1 + test_run.sh | 15 ++++++++++++++- 6 files changed, 24 insertions(+), 8 deletions(-) create mode 100644 test_expect_failure/compilers_jvm_flags/WillNotCompileJavaSinceXmxTooLow.java diff --git a/scala/scala.bzl b/scala/scala.bzl index de20fd5b0..77a51adcc 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -185,7 +185,6 @@ JarOutput: {out} JavacOpts: -encoding utf8 {javac_opts} JavacPath: {javac_path} JavaFiles: {java_files} -JvmFlags: {jvm_flags} Manifest: {manifest} Plugins: {plugin_arg} PrintCompileTime: {print_compile_time} @@ -211,11 +210,11 @@ EnableDependencyAnalyzer: {enable_dependency_analyzer} ijar_out=ijar_output_path, ijar_cmd_path=ijar_cmd_path, srcjars=",".join([f.path for f in all_srcjars]), - javac_opts=" ".join(ctx.attr.javacopts), + javac_opts=" ".join(ctx.attr.javacopts) + + # these are the flags passed to javac, which needs them prefixed by -J + " ".join(["-J" + flag for flag in ctx.attr.javac_jvm_flags]), javac_path=ctx.executable._javac.path, java_files=",".join([f.path for f in java_srcs]), - # these are the flags passed to javac, which needs them prefixed by -J - jvm_flags=",".join(["-J" + flag for flag in ctx.attr.jvm_flags]), resource_src=",".join([f.path for f in ctx.files.resources]), resource_dest=",".join( [_adjust_resources_path_by_default_prefixes(f.path)[1] for f in ctx.files.resources] @@ -653,6 +652,7 @@ _common_attrs = { "javacopts":attr.string_list(), "jvm_flags": attr.string_list(), "scalac_jvm_flags": attr.string_list(), + "javac_jvm_flags": attr.string_list(), "print_compile_time": attr.bool(default=False, mandatory=False), "enable_dependency_analyzer": attr.bool(default=True, mandatory=False), "dependency_analyzer_plugin": attr.label(default=Label("//plugin/src/main:dependency_analyzer"), allow_files=_jar_filetype, mandatory=False), diff --git a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java index a3442b5d1..589d358bd 100644 --- a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java +++ b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java @@ -19,7 +19,6 @@ public class CompileOptions { final public String[] javaFiles; final public String javacPath; final public String javacOpts; - final public String[] jvmFlags; final public Map resourceFiles; final public String resourceStripPrefix; final public String[] resourceJars; @@ -43,7 +42,6 @@ public CompileOptions(List args) { javaFiles = getCommaList(argMap, "JavaFiles"); javacPath = getOrEmpty(argMap, "JavacPath"); javacOpts = getOrEmpty(argMap, "JavacOpts"); - jvmFlags = getCommaList(argMap, "JvmFlags"); sourceJars = getCommaList(argMap, "SourceJars"); iJarEnabled = booleanGetOrFalse(argMap, "EnableIjar"); diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index 19a6fd720..08dd37014 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -252,7 +252,6 @@ private static void compileJavaSources(CompileOptions ops, String[] javaSources, ArrayList commandParts = new ArrayList<>(); commandParts.add(ops.javacPath); - Collections.addAll(commandParts, ops.jvmFlags); Path argsFile = newArgFile(ops, javaSources, tmpPath); commandParts.add("@" + normalizeSlash(argsFile.toFile().getAbsolutePath())); try { diff --git a/test_expect_failure/compilers_jvm_flags/BUILD b/test_expect_failure/compilers_jvm_flags/BUILD index fc3e3ccdd..511e9d58d 100644 --- a/test_expect_failure/compilers_jvm_flags/BUILD +++ b/test_expect_failure/compilers_jvm_flags/BUILD @@ -3,4 +3,9 @@ scala_library( name = "can_configure_jvm_flags_for_scalac", srcs = ["WillNotCompileScalaSinceXmxTooLow.scala"], scalac_jvm_flags = ["-Xmx1M"], +) +scala_library( + name = "can_configure_jvm_flags_for_javac", + srcs = ["WillNotCompileJavaSinceXmxTooLow.java"], + javac_jvm_flags = ["-Xmx1M"], ) \ No newline at end of file diff --git a/test_expect_failure/compilers_jvm_flags/WillNotCompileJavaSinceXmxTooLow.java b/test_expect_failure/compilers_jvm_flags/WillNotCompileJavaSinceXmxTooLow.java new file mode 100644 index 000000000..62d76d842 --- /dev/null +++ b/test_expect_failure/compilers_jvm_flags/WillNotCompileJavaSinceXmxTooLow.java @@ -0,0 +1 @@ +class WillNotCompileJavaSinceXmxTooLow {} \ No newline at end of file diff --git a/test_run.sh b/test_run.sh index 3a479b931..f0823c80c 100755 --- a/test_run.sh +++ b/test_run.sh @@ -362,6 +362,18 @@ scalac_jvm_flags_are_configured(){ exit 0 } +javac_jvm_flags_are_configured(){ + set +e + + bazel build //test_expect_failure/compilers_jvm_flags:can_configure_jvm_flags_for_javac + if [ $? -eq 0 ]; then + echo "'bazel build test_expect_failure/compiler_jvm_flags:can_configure_jvm_flags_for_scalac' should have failed." + exit 1 + fi + set -e + exit 0 +} + if [ "$1" != "ci" ]; then runner="run_test_local" else @@ -400,4 +412,5 @@ $runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath $runner scala_test_test_filters $runner scala_junit_test_test_filter $runner scalac_jvm_flags_are_configured -$runner test_scala_library_expect_failure_on_missing_direct_deps +$runner javac_jvm_flags_are_configured +$runner test_scala_library_expect_failure_on_missing_direct_deps \ No newline at end of file From 78d6ed6a32eb1fcf8c531ccdb10523e7b579cfc6 Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Sun, 25 Jun 2017 17:16:02 +0300 Subject: [PATCH 10/24] removed duplication of failing tests --- test_run.sh | 111 +++++++++++++++++----------------------------------- 1 file changed, 36 insertions(+), 75 deletions(-) diff --git a/test_run.sh b/test_run.sh index f0823c80c..8443f20da 100755 --- a/test_run.sh +++ b/test_run.sh @@ -60,45 +60,11 @@ test_transitive_deps() { } test_scala_library_suite() { - set +e - - bazel build test_expect_failure/scala_library_suite:library_suite_dep_on_children - if [ $? -eq 0 ]; then - echo "'bazel build test_expect_failure/scala_library_suite:library_suite_dep_on_children' should have failed." - exit 1 - fi - set -e - exit 0 -} - -test_scala_library_expect_failure_on_missing_direct_deps() { - expected_message="Target '//test_expect_failure/missing_direct_deps:direct_dependency' is used but isn't explicitly declared, please add it to the deps" - command='bazel build test_expect_failure/missing_direct_deps:transitive_dependency_user' - output=$($command 2>&1) - if [ $? -eq 0 ]; then - echo "$output" - echo "'bazel build of scala_library with missing direct deps should have failed." - exit 1 - fi - echo "$output" - echo $output | grep "$expected_message" - if [ $? -ne 0 ]; then - echo "'bazel build test_expect_failure/missing_direct_deps:transitive_dependency_user' should have logged \"$expected_message\"." - exit 1 - fi - exit 0 + action_should_fail build test_expect_failure/scala_library_suite:library_suite_dep_on_children } test_scala_junit_test_can_fail() { - set +e - - bazel test test_expect_failure/scala_junit_test:failing_test - if [ $? -eq 0 ]; then - echo "'bazel build test_expect_failure/scala_junit_test:failing_test' should have failed." - exit 1 - fi - set -e - exit 0 + action_should_fail test test_expect_failure/scala_junit_test:failing_test } test_repl() { @@ -193,6 +159,20 @@ run_test_local() { fi } +action_should_fail() { + # runs the tests locally + set +e + TEST_ARG=$@ + $(bazel $TEST_ARG) + RESPONSE_CODE=$? + if [ $RESPONSE_CODE -eq 0 ]; then + echo -e "${RED} \"bazel $TEST_ARG\" should have failed but passed. $NC" + exit -1 + else + exit 0 + fi +} + xmllint_test() { find -L ./bazel-testlogs -iname "*.xml" | xargs -n1 xmllint > /dev/null } @@ -241,27 +221,11 @@ junit_generates_xml_logs() { } test_junit_test_must_have_prefix_or_suffix() { - set +e - - bazel test test_expect_failure/scala_junit_test:no_prefix_or_suffix - if [ $? -eq 0 ]; then - echo "'bazel build test_expect_failure/scala_junit_test:no_prefix_or_suffix' should have failed." - exit 1 - fi - set -e - exit 0 + action_should_fail test test_expect_failure/scala_junit_test:no_prefix_or_suffix } test_junit_test_errors_when_no_tests_found() { - set +e - - bazel test test_expect_failure/scala_junit_test:no_tests_found - if [ $? -eq 0 ]; then - echo "'bazel build test_expect_failure/scala_junit_test:no_tests_found' should have failed." - exit 1 - fi - set -e - exit 0 + action_should_fail test test_expect_failure/scala_junit_test:no_tests_found } test_resources() { @@ -282,14 +246,7 @@ scala_library_jar_without_srcs_must_include_filegroup_resources(){ } scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix() { - set +e - bazel build test_expect_failure/wrong_resource_strip_prefix:noSrcsJarWithWrongStripPrefix - if [ $? -eq 0 ]; then - echo "'bazel build of scala_library with invalid resource_strip_prefix should have failed." - exit 1 - fi - set -e - exit 0 + action_should_fail build test_expect_failure/wrong_resource_strip_prefix:noSrcsJarWithWrongStripPrefix } scala_test_test_filters() { @@ -351,26 +308,30 @@ scala_junit_test_test_filter(){ } scalac_jvm_flags_are_configured(){ - set +e + action_should_fail build //test_expect_failure/compilers_jvm_flags:can_configure_jvm_flags_for_scalac - bazel build //test_expect_failure/compilers_jvm_flags:can_configure_jvm_flags_for_scalac - if [ $? -eq 0 ]; then - echo "'bazel build test_expect_failure/compiler_jvm_flags:can_configure_jvm_flags_for_scalac' should have failed." - exit 1 - fi - set -e - exit 0 } javac_jvm_flags_are_configured(){ - set +e + action_should_fail build //test_expect_failure/compilers_jvm_flags:can_configure_jvm_flags_for_javac - bazel build //test_expect_failure/compilers_jvm_flags:can_configure_jvm_flags_for_javac - if [ $? -eq 0 ]; then - echo "'bazel build test_expect_failure/compiler_jvm_flags:can_configure_jvm_flags_for_scalac' should have failed." +} + +test_scala_library_expect_failure_on_missing_direct_deps() { + expected_message="Target '//test_expect_failure/missing_direct_deps:direct_dependency' is used but isn't explicitly declared, please add it to the deps" + command='bazel build test_expect_failure/missing_direct_deps:transitive_dependency_user' + output=$($command 2>&1) + if [ $? -eq 0 ]; then + echo "$output" + echo "'bazel build of scala_library with missing direct deps should have failed." + exit 1 + fi + echo "$output" + echo $output | grep "$expected_message" + if [ $? -ne 0 ]; then + echo "'bazel build test_expect_failure/missing_direct_deps:transitive_dependency_user' should have logged \"$expected_message\"." exit 1 fi - set -e exit 0 } From 0f8bb37ea154e8fb381f3cbb2f0f14f6af5a6fa9 Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Wed, 28 Jun 2017 00:58:07 +0300 Subject: [PATCH 11/24] changed docs to include scalac_jvm_flags and javac_jvm_flags and to mention deprecation of jvm_flags --- README.md | 57 +++++++++++++++++-- test_expect_failure/compilers_jvm_flags/BUILD | 2 +- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 19b822715..b60d479e4 100644 --- a/README.md +++ b/README.md @@ -58,8 +58,8 @@ to your command line, or to enable by default for building/testing add it to you ## scala\_library / scala\_macro_library ```python -scala_library(name, srcs, deps, runtime_deps, exports, data, main_class, resources, resource_strip_prefix, scalacopts, jvm_flags) -scala_macro_library(name, srcs, deps, runtime_deps, exports, data, main_class, resources, resource_strip_prefix, scalacopts, jvm_flags) +scala_library(name, srcs, deps, runtime_deps, exports, data, main_class, resources, resource_strip_prefix, scalacopts, jvm_flags, scalac_jvm_flags, javac_jvm_flags) +scala_macro_library(name, srcs, deps, runtime_deps, exports, data, main_class, resources, resource_strip_prefix, scalacopts, jvm_flags, scalac_jvm_flags, javac_jvm_flags) ``` `scala_library` generates a `.jar` file from `.scala` source files. This rule @@ -174,6 +174,15 @@ In order to make a java rule use this jar file, use the `java_import` rule. jvm_flags + +

List of strings; optional; deprecated

+

+ Deprecated, superseded by scalac_jvm_flags and javac_jvm_flags. Is not used and is kept as backwards compatibility for the near future. Effectively jvm_flags is now an executable target attribute only. +

+ + + + scalac_jvm_flags

List of strings; optional

@@ -185,6 +194,19 @@ In order to make a java rule use this jar file, use the `java_import` rule.

+ + javac_jvm_flags + +

List of strings; optional

+

+ List of JVM flags to be passed to javac after the + javacopts. Subject to + Make variable + substitution and + Bourne shell tokenization. +

+ + @@ -192,7 +214,7 @@ In order to make a java rule use this jar file, use the `java_import` rule. ## scala_binary ```python -scala_binary(name, srcs, deps, runtime_deps, data, main_class, resources, resource_strip_prefix, scalacopts, jvm_flags) +scala_binary(name, srcs, deps, runtime_deps, data, main_class, resources, resource_strip_prefix, scalacopts, jvm_flags, scalac_jvm_flags, javac_jvm_flags) ``` `scala_binary` generates a Scala executable. It may depend on `scala_library`, `scala_macro_library` @@ -292,6 +314,18 @@ A `scala_binary` requires a `main_class` attribute. jvm_flags + +

List of strings; optional

+

+ List of JVM flags to be passed to the executing JVM. Subject to + Make variable + substitution and + Bourne shell tokenization. +

+ + + + scalac_jvm_flags

List of strings; optional

@@ -303,6 +337,19 @@ A `scala_binary` requires a `main_class` attribute.

+ + javac_jvm_flags + +

List of strings; optional

+

+ List of JVM flags to be passed to javac after the + javacopts. Subject to + Make variable + substitution and + Bourne shell tokenization. +

+ + @@ -310,7 +357,7 @@ A `scala_binary` requires a `main_class` attribute. ## scala_test ```python -scala_test(name, srcs, suites, deps, data, main_class, resources, resource_strip_prefix, scalacopts, jvm_flags) +scala_test(name, srcs, suites, deps, data, main_class, resources, resource_strip_prefix, scalacopts, jvm_flags, scalac_jvm_flags, javac_jvm_flags) ``` `scala_test` generates a Scala executable which runs unit test suites written @@ -326,7 +373,7 @@ populated and tests are not run. ## scala_repl ```python -scala_repl(name, deps, scalacopts, jvm_flags) +scala_repl(name, deps, scalacopts, jvm_flags, scalac_jvm_flags, javac_jvm_flags) ``` A scala repl allows you to add library dependendencies (not currently `scala_binary` targets) to generate a script to run which starts a REPL. diff --git a/test_expect_failure/compilers_jvm_flags/BUILD b/test_expect_failure/compilers_jvm_flags/BUILD index 511e9d58d..286d9bec0 100644 --- a/test_expect_failure/compilers_jvm_flags/BUILD +++ b/test_expect_failure/compilers_jvm_flags/BUILD @@ -8,4 +8,4 @@ scala_library( name = "can_configure_jvm_flags_for_javac", srcs = ["WillNotCompileJavaSinceXmxTooLow.java"], javac_jvm_flags = ["-Xmx1M"], -) \ No newline at end of file +) From 6fccf576a8ad3090e0dd1a07aa72e2e96226e6e3 Mon Sep 17 00:00:00 2001 From: Stephen Twigg Date: Wed, 28 Jun 2017 03:18:26 -0700 Subject: [PATCH 12/24] Restructure scala provider, add JavaProvider (#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 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 15525cc6d..8c1a2ac77 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,7 +12,7 @@ os: env: - V=HEAD - - V=0.4.5 + - V=0.5.2 before_install: - | From 733045170f0ccfa483425400b3c97474d57c32fa Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Wed, 28 Jun 2017 15:07:04 +0300 Subject: [PATCH 13/24] update rules_scala version to latest in readme (#237) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b60d479e4..ce24501cc 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ you must have bazel 0.3.1 or later and add the following to your WORKSPACE file: ```python -rules_scala_version="d916599d38de29085e5ca9eae167716c4f150a02" # update this as needed +rules_scala_version="031e73c02e0d8bfcd06c6e4086cdfc7f3a3061a8" # update this as needed http_archive( name = "io_bazel_rules_scala", From 3e2f34d8d42f36cb2797b742e3830fb7a9f62993 Mon Sep 17 00:00:00 2001 From: "P. Oscar Boykin" Date: Wed, 28 Jun 2017 08:08:04 -1000 Subject: [PATCH 14/24] update README.md note we need 0.5.2 now. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ce24501cc..6d23da6fd 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ and `scala_test`. ## Getting started In order to use `scala_library`, `scala_macro_library`, and `scala_binary`, -you must have bazel 0.3.1 or later and add the following to your WORKSPACE file: +you must have bazel 0.5.2 or later and add the following to your WORKSPACE file: ```python From 64598793c58f39bbe06abe547a549b7f86d64762 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Wed, 28 Jun 2017 18:33:51 -1000 Subject: [PATCH 15/24] don't put signatures in deploy jar (#229) * don't put signatures in fat jar * Add tests --- .../io/bazel/rulesscala/jar/JarCreator.java | 10 ++++++++ .../io/bazel/rulesscala/jar/JarHelper.java | 8 ++++-- test/BUILD | 24 ++++++++++++++++++ test/fake_sig.jar | Bin 0 -> 492 bytes test/jar_lister.py | 5 ++++ test/no_sigs.sh | 13 ++++++++++ 6 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 test/fake_sig.jar create mode 100644 test/jar_lister.py create mode 100755 test/no_sigs.sh diff --git a/src/java/io/bazel/rulesscala/jar/JarCreator.java b/src/java/io/bazel/rulesscala/jar/JarCreator.java index c98c3f6ab..33d002a47 100644 --- a/src/java/io/bazel/rulesscala/jar/JarCreator.java +++ b/src/java/io/bazel/rulesscala/jar/JarCreator.java @@ -173,6 +173,16 @@ public void execute() throws IOException { } } + @Override + protected boolean ignoreFileName(String name) { + /* + * It does not make sense to copy signature files + * into a fat jar because the jar signature will + * be broken + */ + return (name.endsWith(".DSA") || name.endsWith(".RSA")); + } + public static void buildJar(String[] args) throws IOException { if (args.length < 1) { System.err.println("usage: CreateJar [-m manifest] output [root directories]"); diff --git a/src/java/io/bazel/rulesscala/jar/JarHelper.java b/src/java/io/bazel/rulesscala/jar/JarHelper.java index d51956972..44707413a 100644 --- a/src/java/io/bazel/rulesscala/jar/JarHelper.java +++ b/src/java/io/bazel/rulesscala/jar/JarHelper.java @@ -177,17 +177,21 @@ protected void writeManifestEntry(byte[] content) throws IOException { } } + protected boolean ignoreFileName(String name) { + return false; + } + /** * This copies the contents of jarFile into out * This is a static method to make it clear what is mutated (and it * was written by someone who really likes to minimize state changes). */ - static private void copyJar(JarFile nameJf, Set names, JarOutputStream out) throws IOException { + private void copyJar(JarFile nameJf, Set names, JarOutputStream out) throws IOException { byte[] buffer = new byte[2048]; for (Enumeration e = nameJf.entries(); e.hasMoreElements();) { JarEntry existing = e.nextElement(); String name = existing.getName(); - if (!names.contains(name)) { + if (!(ignoreFileName(name) || names.contains(name))) { JarEntry outEntry = new JarEntry(name); outEntry.setTime(existing.getTime()); outEntry.setSize(existing.getSize()); diff --git a/test/BUILD b/test/BUILD index 08fe5dcd4..4ad2d2a17 100644 --- a/test/BUILD +++ b/test/BUILD @@ -374,3 +374,27 @@ scala_library( srcs = ["src/main/scala/scala/test/utf8/JavaClassWithUtf8.java", "src/main/scala/scala/test/utf8/ScalaClassWithUtf8.scala"] ) + +# make sure making a fat jar strips signatures +java_import( + name = "fakejar", + jars = ["fake_sig.jar"]) + +scala_binary( + name = "ScalaBinary_with_fake", + srcs = ["ScalaBinary.scala"], + main_class = "scala.test.ScalaBinary", + deps = [":HelloLib", ":MacroTest", ":fakejar"], +) + +py_binary( + name = "jar_lister", + srcs = ["jar_lister.py"] +) + +sh_test( + name = "no_sig", + srcs = ["no_sigs.sh"], + args = ["$(location //test:jar_lister)", "$(location //test:ScalaBinary_with_fake_deploy.jar)"], + data = [":ScalaBinary_with_fake_deploy.jar", ":jar_lister"], +) diff --git a/test/fake_sig.jar b/test/fake_sig.jar new file mode 100644 index 0000000000000000000000000000000000000000..b76b2261cef1a908dade50a1b08b4bae352a0de2 GIT binary patch literal 492 zcmWIWW@h1H00GvLJ6>Q0l;C8LVeoYgan$wnbJGtE;bdU$^a_cb4aB7t+zgB?Ul|z~ zSVVw|1K>uyFGe*&5ZMSfM{id>mtaS*QHelfK*r%T>K#F&f}lpl<1&hoNsbwp*Ce2x z6kvGk2x206lojGp43{B{!}KQ1I0l9#jn2r%Av_H<7UXF>#$tLI+1Ne|W5EFcG!_&9 U7{;=)fxN{GgnxnbO%R6x07d3ay8r+H literal 0 HcmV?d00001 diff --git a/test/jar_lister.py b/test/jar_lister.py new file mode 100644 index 000000000..3c3992bcc --- /dev/null +++ b/test/jar_lister.py @@ -0,0 +1,5 @@ +import zipfile +import sys + +for n in zipfile.ZipFile(sys.argv[1]).namelist(): + print n diff --git a/test/no_sigs.sh b/test/no_sigs.sh new file mode 100755 index 000000000..756ecfd7c --- /dev/null +++ b/test/no_sigs.sh @@ -0,0 +1,13 @@ +#!/bin/sh + +OUTPUT1=`$1 $2 | grep DSA` +OUTPUT2=`$1 $2 | grep RSA` + +if [[ $OUTPUT1 ]]; then + echo $OUTPUT1 + exit 1 +fi +if [[ $OUTPUT2 ]]; then + echo $OUTPUT2 + exit 1 +fi From 0dc305a2c385540a5bafb8fe4f979b508f6c4861 Mon Sep 17 00:00:00 2001 From: "P. Oscar Boykin" Date: Wed, 28 Jun 2017 19:31:04 -1000 Subject: [PATCH 16/24] Add a provider for binary targets (#238) Add a provider for binary targets --- scala/scala.bzl | 7 +++++++ test/BUILD | 12 ++++++++++++ test/LibToBin.scala | 5 +++++ test/LibToTest.scala | 5 +++++ 4 files changed, 29 insertions(+) create mode 100644 test/LibToBin.scala create mode 100644 test/LibToTest.scala diff --git a/scala/scala.bzl b/scala/scala.bzl index 77a51adcc..54a9f30e8 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -511,8 +511,15 @@ def _scala_binary_common(ctx, cjars, rjars): compile_jars = depset([outputs.class_jar]), transitive_runtime_jars = 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.executable]), + providers = [java_provider], scala = scalaattr, runfiles=runfiles) diff --git a/test/BUILD b/test/BUILD index 4ad2d2a17..2590fea5f 100644 --- a/test/BUILD +++ b/test/BUILD @@ -76,6 +76,12 @@ scala_test( ], ) +# test that a library can depend on a test: +scala_library( + name = "lib_to_test", + srcs = ["LibToTest.scala"], + deps = [":HelloLibTest"]) + scala_test_suite( name = "HelloLibTestSuite", size = "small", # Not a macro, can pass test-specific attributes. @@ -176,6 +182,12 @@ scala_binary( deps = ["ResourcesStripScalaLib"], ) +# test that a library can depend on a binary: +scala_library( + name = "lib_to_bin", + srcs = ["LibToBin.scala"], + deps = [":ScalaLibBinary"]) + scala_repl( name = "ScalaLibBinaryRepl", deps = [":ScalaLibBinary"]) diff --git a/test/LibToBin.scala b/test/LibToBin.scala new file mode 100644 index 000000000..bac33d16a --- /dev/null +++ b/test/LibToBin.scala @@ -0,0 +1,5 @@ +package scala.test + +object LibToBin { + def foo = ScalaLibBinary.main(Array("foo")) +} diff --git a/test/LibToTest.scala b/test/LibToTest.scala new file mode 100644 index 000000000..636ab0373 --- /dev/null +++ b/test/LibToTest.scala @@ -0,0 +1,5 @@ +package scala.test + +object LibToTest { + def foo = TestUtil.foo +} From b700d96abd06b3c3db34cb5d785d202da4d50f12 Mon Sep 17 00:00:00 2001 From: Matt DuVall Date: Thu, 29 Jun 2017 15:51:20 -0700 Subject: [PATCH 17/24] misc: add line for test and build formatting --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 6d23da6fd..6475c1d41 100644 --- a/README.md +++ b/README.md @@ -466,6 +466,7 @@ thrift_library(name, srcs, deps, absolute_prefix, absolute_prefixes) + ## Building from source Test & Build: ``` From d51e465b01c78408577e46cf7cdcc15960a32359 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Fri, 30 Jun 2017 15:11:32 -1000 Subject: [PATCH 18/24] Use workspace name in tut rule --- tut_rule/tut.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tut_rule/tut.bzl b/tut_rule/tut.bzl index 4ca712575..a56db468a 100644 --- a/tut_rule/tut.bzl +++ b/tut_rule/tut.bzl @@ -23,7 +23,7 @@ def scala_tut_doc(**kw): name = tool, main_class = "io.bazel.rules_scala.tut_support.TutCompiler", deps = deps + [ - "//src/scala/io/bazel/rules_scala/tut_support:tut_compiler_lib" + "@io_bazel_rules_scala//src/scala/io/bazel/rules_scala/tut_support:tut_compiler_lib" ], ) native.genrule( From 091e8414030d5447bf6cd26071503b6727c6112f Mon Sep 17 00:00:00 2001 From: Stephen Twigg Date: Sun, 2 Apr 2017 20:24:01 -0700 Subject: [PATCH 19/24] 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. --- scala/scala.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/scala.bzl b/scala/scala.bzl index 54a9f30e8..12ec934bd 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -210,7 +210,7 @@ EnableDependencyAnalyzer: {enable_dependency_analyzer} ijar_out=ijar_output_path, ijar_cmd_path=ijar_cmd_path, srcjars=",".join([f.path for f in all_srcjars]), - javac_opts=" ".join(ctx.attr.javacopts) + + javac_opts=" ".join(ctx.attr.javacopts) + # these are the flags passed to javac, which needs them prefixed by -J " ".join(["-J" + flag for flag in ctx.attr.javac_jvm_flags]), javac_path=ctx.executable._javac.path, From d725ad60d9413b14d88f61013dbdb0741d49898a Mon Sep 17 00:00:00 2001 From: natans Date: Wed, 28 Jun 2017 18:30:15 +0300 Subject: [PATCH 20/24] using classpath shrinker plugin to warn about indirect dependency usage --- .../rulesscala/scalac/ScalacProcessor.java | 5 +- test_expect_logs/missing_direct_deps/A.scala | 8 ++ test_expect_logs/missing_direct_deps/B.scala | 7 ++ test_expect_logs/missing_direct_deps/BUILD | 30 ++++++++ test_expect_logs/missing_direct_deps/C.scala | 7 ++ test_run.sh | 76 +++++++++++-------- 6 files changed, 101 insertions(+), 32 deletions(-) create mode 100644 test_expect_logs/missing_direct_deps/A.scala create mode 100644 test_expect_logs/missing_direct_deps/B.scala create mode 100644 test_expect_logs/missing_direct_deps/BUILD create mode 100644 test_expect_logs/missing_direct_deps/C.scala diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index 08dd37014..3c1f80fd7 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -211,7 +211,10 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource "-classpath", ops.classpath, "-d", - tmpPath.toString() + tmpPath.toString(), + "-P:classpath-shrinker:direct-jars:" + String.join(":", ops.directJars), + "-P:classpath-shrinker:indirect-jars:" + String.join(":", ops.indirectJars), + "-P:classpath-shrinker:indirect-targets:" + String.join(":", ops.indirectTargets), }; diff --git a/test_expect_logs/missing_direct_deps/A.scala b/test_expect_logs/missing_direct_deps/A.scala new file mode 100644 index 000000000..58b66b63f --- /dev/null +++ b/test_expect_logs/missing_direct_deps/A.scala @@ -0,0 +1,8 @@ +package test_expect_logs.missing_direct_deps + +object A { + def foo = { + B.foo + C.foo + } +} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/B.scala b/test_expect_logs/missing_direct_deps/B.scala new file mode 100644 index 000000000..7884bc65f --- /dev/null +++ b/test_expect_logs/missing_direct_deps/B.scala @@ -0,0 +1,7 @@ +package test_expect_logs.missing_direct_deps + +object B { + def foo = { + C.foo + } +} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/BUILD b/test_expect_logs/missing_direct_deps/BUILD new file mode 100644 index 000000000..a9c4a8074 --- /dev/null +++ b/test_expect_logs/missing_direct_deps/BUILD @@ -0,0 +1,30 @@ +package(default_visibility = ["//visibility:public"]) +load("//scala:scala.bzl", "scala_library", "scala_test") + +scala_library( + name="transitive_dependency_user", + srcs=[ + "A.scala", + ], + deps = ["direct_dependency"], + plugins = ["@classpath_shrinker//jar"], +) + +scala_library( + name="direct_dependency", + srcs=[ + "B.scala", + ], + deps = ["transitive_dependency"], + plugins = ["@classpath_shrinker//jar"], + +) + +scala_library( + name="transitive_dependency", + srcs=[ + "C.scala", + ], + plugins = ["@classpath_shrinker//jar"], + +) \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/C.scala b/test_expect_logs/missing_direct_deps/C.scala new file mode 100644 index 000000000..ae9d5f28b --- /dev/null +++ b/test_expect_logs/missing_direct_deps/C.scala @@ -0,0 +1,7 @@ +package test_expect_logs.missing_direct_deps + +object C { + def foo = { + println("in C") + } +} \ No newline at end of file diff --git a/test_run.sh b/test_run.sh index 8443f20da..c0793751d 100755 --- a/test_run.sh +++ b/test_run.sh @@ -63,6 +63,19 @@ test_scala_library_suite() { action_should_fail build test_expect_failure/scala_library_suite:library_suite_dep_on_children } +test_scala_library_expect_warning_on_missing_direct_deps() { + expected_message="target 'transitive_dependency' should be added to deps" + command='bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' + output=$($command 2>&1) + echo "$output" + echo $output | grep "$expected_message" + if [ $? -ne 0 ]; then + echo "'bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' should have logged \"$expected_message\"." + exit 1 + fi + exit 0 +} + test_scala_junit_test_can_fail() { action_should_fail test test_expect_failure/scala_junit_test:failing_test } @@ -341,37 +354,38 @@ else runner="run_test_ci" fi -$runner bazel build test/... -$runner bazel test test/... -$runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges -$runner bazel run test:JavaBinary -$runner bazel run test:JavaBinary2 -$runner bazel run test:MixJavaScalaLibBinary -$runner bazel run test:MixJavaScalaSrcjarLibBinary -$runner bazel run test:ScalaBinary -$runner bazel run test:ScalaLibBinary -$runner test_disappearing_class -$runner find -L ./bazel-testlogs -iname "*.xml" -$runner xmllint_test -$runner test_build_is_identical -$runner test_transitive_deps -$runner test_scala_library_suite -$runner test_repl -$runner bazel run test:JavaOnlySources -$runner test_benchmark_jmh -$runner multiple_junit_suffixes -$runner multiple_junit_prefixes -$runner test_scala_junit_test_can_fail -$runner junit_generates_xml_logs -$runner scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix -$runner multiple_junit_patterns -$runner test_junit_test_must_have_prefix_or_suffix -$runner test_junit_test_errors_when_no_tests_found -$runner scala_library_jar_without_srcs_must_include_direct_file_resources -$runner scala_library_jar_without_srcs_must_include_filegroup_resources -$runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath -$runner scala_test_test_filters -$runner scala_junit_test_test_filter +$runner test_scala_library_expect_warning_on_missing_direct_deps +#$runner bazel build test/... +#$runner bazel test test/... +#$runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges +#$runner bazel run test:JavaBinary +#$runner bazel run test:JavaBinary2 +#$runner bazel run test:MixJavaScalaLibBinary +#$runner bazel run test:MixJavaScalaSrcjarLibBinary +#$runner bazel run test:ScalaBinary +#$runner bazel run test:ScalaLibBinary +#$runner test_disappearing_class +#$runner find -L ./bazel-testlogs -iname "*.xml" +#$runner xmllint_test +#$runner test_build_is_identical +#$runner test_transitive_deps +#$runner test_scala_library_suite +#$runner test_repl +#$runner bazel run test:JavaOnlySources +#$runner test_benchmark_jmh +#$runner multiple_junit_suffixes +#$runner multiple_junit_prefixes +#$runner test_scala_junit_test_can_fail +#$runner junit_generates_xml_logs +#$runner scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix +#$runner multiple_junit_patterns +#$runner test_junit_test_must_have_prefix_or_suffix +#$runner test_junit_test_errors_when_no_tests_found +#$runner scala_library_jar_without_srcs_must_include_direct_file_resources +#$runner scala_library_jar_without_srcs_must_include_filegroup_resources +#$runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath +#$runner scala_test_test_filters +#$runner scala_junit_test_test_filter $runner scalac_jvm_flags_are_configured $runner javac_jvm_flags_are_configured $runner test_scala_library_expect_failure_on_missing_direct_deps \ No newline at end of file From 303af8098a755ed67ce917a54f2881c6bc3ae589 Mon Sep 17 00:00:00 2001 From: natans Date: Thu, 29 Jun 2017 12:15:56 +0300 Subject: [PATCH 21/24] fix escaping bugs --- .../rulesscala/scalac/ScalacProcessor.java | 2 +- test_run.sh | 64 +++++++++---------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index 3c1f80fd7..31702d2e8 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -214,7 +214,7 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource tmpPath.toString(), "-P:classpath-shrinker:direct-jars:" + String.join(":", ops.directJars), "-P:classpath-shrinker:indirect-jars:" + String.join(":", ops.indirectJars), - "-P:classpath-shrinker:indirect-targets:" + String.join(":", ops.indirectTargets), + "-P:classpath-shrinker:indirect-targets:" + String.join(":", targets), }; diff --git a/test_run.sh b/test_run.sh index c0793751d..0f77e327e 100755 --- a/test_run.sh +++ b/test_run.sh @@ -64,7 +64,7 @@ test_scala_library_suite() { } test_scala_library_expect_warning_on_missing_direct_deps() { - expected_message="target 'transitive_dependency' should be added to deps" + expected_message="target '//test_expect_logs/missing_direct_deps:direct_dependency' should be added to deps" command='bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' output=$($command 2>&1) echo "$output" @@ -355,37 +355,37 @@ else fi $runner test_scala_library_expect_warning_on_missing_direct_deps -#$runner bazel build test/... -#$runner bazel test test/... -#$runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges -#$runner bazel run test:JavaBinary -#$runner bazel run test:JavaBinary2 -#$runner bazel run test:MixJavaScalaLibBinary -#$runner bazel run test:MixJavaScalaSrcjarLibBinary -#$runner bazel run test:ScalaBinary -#$runner bazel run test:ScalaLibBinary -#$runner test_disappearing_class -#$runner find -L ./bazel-testlogs -iname "*.xml" -#$runner xmllint_test -#$runner test_build_is_identical -#$runner test_transitive_deps -#$runner test_scala_library_suite -#$runner test_repl -#$runner bazel run test:JavaOnlySources -#$runner test_benchmark_jmh -#$runner multiple_junit_suffixes -#$runner multiple_junit_prefixes -#$runner test_scala_junit_test_can_fail -#$runner junit_generates_xml_logs -#$runner scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix -#$runner multiple_junit_patterns -#$runner test_junit_test_must_have_prefix_or_suffix -#$runner test_junit_test_errors_when_no_tests_found -#$runner scala_library_jar_without_srcs_must_include_direct_file_resources -#$runner scala_library_jar_without_srcs_must_include_filegroup_resources -#$runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath -#$runner scala_test_test_filters -#$runner scala_junit_test_test_filter +$runner bazel build test/... +$runner bazel test test/... +$runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges +$runner bazel run test:JavaBinary +$runner bazel run test:JavaBinary2 +$runner bazel run test:MixJavaScalaLibBinary +$runner bazel run test:MixJavaScalaSrcjarLibBinary +$runner bazel run test:ScalaBinary +$runner bazel run test:ScalaLibBinary +$runner test_disappearing_class +$runner find -L ./bazel-testlogs -iname "*.xml" +$runner xmllint_test +$runner test_build_is_identical +$runner test_transitive_deps +$runner test_scala_library_suite +$runner test_repl +$runner bazel run test:JavaOnlySources +$runner test_benchmark_jmh +$runner multiple_junit_suffixes +$runner multiple_junit_prefixes +$runner test_scala_junit_test_can_fail +$runner junit_generates_xml_logs +$runner scala_library_jar_without_srcs_must_fail_on_mismatching_resource_strip_prefix +$runner multiple_junit_patterns +$runner test_junit_test_must_have_prefix_or_suffix +$runner test_junit_test_errors_when_no_tests_found +$runner scala_library_jar_without_srcs_must_include_direct_file_resources +$runner scala_library_jar_without_srcs_must_include_filegroup_resources +$runner bazel run test/src/main/scala/scala/test/large_classpath:largeClasspath +$runner scala_test_test_filters +$runner scala_junit_test_test_filter $runner scalac_jvm_flags_are_configured $runner javac_jvm_flags_are_configured $runner test_scala_library_expect_failure_on_missing_direct_deps \ No newline at end of file From 6d1de1739f63ea34fd44f4fbf6dcf3ab9be5b368 Mon Sep 17 00:00:00 2001 From: natans Date: Mon, 3 Jul 2017 12:42:23 +0300 Subject: [PATCH 22/24] merge workspaces --- test_expect_logs/missing_direct_deps/A.scala | 8 ------ test_expect_logs/missing_direct_deps/B.scala | 7 ----- test_expect_logs/missing_direct_deps/BUILD | 30 -------------------- test_expect_logs/missing_direct_deps/C.scala | 7 ----- test_run.sh | 14 +++++---- 5 files changed, 9 insertions(+), 57 deletions(-) delete mode 100644 test_expect_logs/missing_direct_deps/A.scala delete mode 100644 test_expect_logs/missing_direct_deps/B.scala delete mode 100644 test_expect_logs/missing_direct_deps/BUILD delete mode 100644 test_expect_logs/missing_direct_deps/C.scala diff --git a/test_expect_logs/missing_direct_deps/A.scala b/test_expect_logs/missing_direct_deps/A.scala deleted file mode 100644 index 58b66b63f..000000000 --- a/test_expect_logs/missing_direct_deps/A.scala +++ /dev/null @@ -1,8 +0,0 @@ -package test_expect_logs.missing_direct_deps - -object A { - def foo = { - B.foo - C.foo - } -} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/B.scala b/test_expect_logs/missing_direct_deps/B.scala deleted file mode 100644 index 7884bc65f..000000000 --- a/test_expect_logs/missing_direct_deps/B.scala +++ /dev/null @@ -1,7 +0,0 @@ -package test_expect_logs.missing_direct_deps - -object B { - def foo = { - C.foo - } -} \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/BUILD b/test_expect_logs/missing_direct_deps/BUILD deleted file mode 100644 index a9c4a8074..000000000 --- a/test_expect_logs/missing_direct_deps/BUILD +++ /dev/null @@ -1,30 +0,0 @@ -package(default_visibility = ["//visibility:public"]) -load("//scala:scala.bzl", "scala_library", "scala_test") - -scala_library( - name="transitive_dependency_user", - srcs=[ - "A.scala", - ], - deps = ["direct_dependency"], - plugins = ["@classpath_shrinker//jar"], -) - -scala_library( - name="direct_dependency", - srcs=[ - "B.scala", - ], - deps = ["transitive_dependency"], - plugins = ["@classpath_shrinker//jar"], - -) - -scala_library( - name="transitive_dependency", - srcs=[ - "C.scala", - ], - plugins = ["@classpath_shrinker//jar"], - -) \ No newline at end of file diff --git a/test_expect_logs/missing_direct_deps/C.scala b/test_expect_logs/missing_direct_deps/C.scala deleted file mode 100644 index ae9d5f28b..000000000 --- a/test_expect_logs/missing_direct_deps/C.scala +++ /dev/null @@ -1,7 +0,0 @@ -package test_expect_logs.missing_direct_deps - -object C { - def foo = { - println("in C") - } -} \ No newline at end of file diff --git a/test_run.sh b/test_run.sh index 0f77e327e..3a2f07984 100755 --- a/test_run.sh +++ b/test_run.sh @@ -63,14 +63,19 @@ test_scala_library_suite() { action_should_fail build test_expect_failure/scala_library_suite:library_suite_dep_on_children } -test_scala_library_expect_warning_on_missing_direct_deps() { - expected_message="target '//test_expect_logs/missing_direct_deps:direct_dependency' should be added to deps" - command='bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' +test_scala_library_expect_failure_on_missing_direct_deps() { + expected_message="Target '//test_expect_failure/missing_direct_deps:direct_dependency' is used but isn't explicitly declared, please add it to the deps" + command='bazel build test_expect_failure/missing_direct_deps:transitive_dependency_user' output=$($command 2>&1) + if [ $? -eq 0 ]; then + echo "$output" + echo "'bazel build of scala_library with missing direct deps should have failed." + exit 1 + fi echo "$output" echo $output | grep "$expected_message" if [ $? -ne 0 ]; then - echo "'bazel build test_expect_logs/missing_direct_deps:transitive_dependency_user' should have logged \"$expected_message\"." + echo "'bazel build test_expect_failure/missing_direct_deps:transitive_dependency_user' should have logged \"$expected_message\"." exit 1 fi exit 0 @@ -354,7 +359,6 @@ else runner="run_test_ci" fi -$runner test_scala_library_expect_warning_on_missing_direct_deps $runner bazel build test/... $runner bazel test test/... $runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges From a46aefe2bfc4c2129627a029c43a6058ffe47688 Mon Sep 17 00:00:00 2001 From: natans Date: Mon, 3 Jul 2017 17:58:41 +0300 Subject: [PATCH 23/24] fix bugs --- WORKSPACE | 2 +- plugin/src/main/BUILD | 2 +- plugin/src/test/BUILD | 2 +- plugin/src/test/resources/BUILD | 2 +- scala/scala.bzl | 23 +++++++++++++++++------ test_run.sh | 1 + 6 files changed, 22 insertions(+), 10 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 962b96f76..38c0b429d 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -5,7 +5,7 @@ workspace(name = "io_bazel_rules_scala") rules_scala_version="3070353d06bb726141e6cea241e5d6e355a4d14f" # update this as needed http_archive( - name = "io_bazel_rules_scala", + name = "io_bazel_rules_scala_for_plugin_bootstrapping", url = "https://github.com/wix/rules_scala/archive/%s.zip"%rules_scala_version, type = "zip", strip_prefix= "rules_scala-%s" % rules_scala_version diff --git a/plugin/src/main/BUILD b/plugin/src/main/BUILD index 1bd098752..aa1344414 100644 --- a/plugin/src/main/BUILD +++ b/plugin/src/main/BUILD @@ -1,4 +1,4 @@ -load("@io_bazel_rules_scala//scala:scala.bzl", "scala_library") +load("@io_bazel_rules_scala_for_plugin_bootstrapping//scala:scala.bzl", "scala_library") scala_library( name = "dependency_analyzer", diff --git a/plugin/src/test/BUILD b/plugin/src/test/BUILD index 6eabb50e1..9ebf172ba 100644 --- a/plugin/src/test/BUILD +++ b/plugin/src/test/BUILD @@ -1,4 +1,4 @@ -load("@io_bazel_rules_scala//scala:scala.bzl", "scala_junit_test") +load("@io_bazel_rules_scala_for_plugin_bootstrapping//scala:scala.bzl", "scala_junit_test") scala_junit_test( name = "dependency_analyzer_test", diff --git a/plugin/src/test/resources/BUILD b/plugin/src/test/resources/BUILD index 74fdc7b49..b2b204379 100644 --- a/plugin/src/test/resources/BUILD +++ b/plugin/src/test/resources/BUILD @@ -1,4 +1,4 @@ -load("@io_bazel_rules_scala//scala:scala.bzl", "scala_library") +load("@io_bazel_rules_scala_for_plugin_bootstrapping//scala:scala.bzl", "scala_library") package(default_visibility = ["//visibility:public"]) diff --git a/scala/scala.bzl b/scala/scala.bzl index 12ec934bd..bbd11e027 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -164,16 +164,27 @@ def _compile(ctx, cjars, dep_srcjars, buildijar, rjars=[], labels = {}): plugins = _collect_plugin_paths(ctx.attr.plugins) if hasattr(ctx.attr, 'enable_dependency_analyzer') and ctx.attr.enable_dependency_analyzer: + enable_dependency_analyzer = ctx.attr.enable_dependency_analyzer dep_plugin = ctx.attr.dependency_analyzer_plugin plugins += [f.path for f in dep_plugin.files] + dependency_analyzer_plugin_jars = ctx.files.dependency_analyzer_plugin + else: + enable_dependency_analyzer = False + dependency_analyzer_plugin_jars = [] plugin_arg = ",".join(list(plugins)) all_jars = cjars + rjars compiler_classpath = ":".join([j.path for j in all_jars]) direct_jars = ",".join([j.path for j in cjars]) - indirect_jars = ",".join([j.path for j in rjars]) - indirect_targets = ",".join([labels[j.path] for j in rjars]) + + valid_jar_paths = [] + for j in rjars: + if j.path in labels: + valid_jar_paths.append(j.path) + + indirect_jars = ",".join(valid_jar_paths) + indirect_targets = ",".join([labels[path] for path in valid_jar_paths]) scalac_args = """ Classpath: {cp} @@ -224,7 +235,7 @@ EnableDependencyAnalyzer: {enable_dependency_analyzer} direct_jars=direct_jars, indirect_jars=indirect_jars, indirect_targets=indirect_targets, - enable_dependency_analyzer = ctx.attr.enable_dependency_analyzer, + enable_dependency_analyzer = enable_dependency_analyzer, ) argfile = ctx.new_file( ctx.outputs.jar, @@ -244,7 +255,7 @@ EnableDependencyAnalyzer: {enable_dependency_analyzer} list(sources) + ctx.files.srcs + ctx.files.plugins + - ctx.files.dependency_analyzer_plugin + + dependency_analyzer_plugin_jars + ctx.files.resources + ctx.files.resource_jars + ctx.files._jdk + @@ -661,8 +672,6 @@ _common_attrs = { "scalac_jvm_flags": attr.string_list(), "javac_jvm_flags": attr.string_list(), "print_compile_time": attr.bool(default=False, mandatory=False), - "enable_dependency_analyzer": attr.bool(default=True, mandatory=False), - "dependency_analyzer_plugin": attr.label(default=Label("//plugin/src/main:dependency_analyzer"), allow_files=_jar_filetype, mandatory=False), } scala_library = rule( @@ -670,6 +679,8 @@ scala_library = rule( attrs={ "main_class": attr.string(), "exports": attr.label_list(allow_files=False), + "enable_dependency_analyzer": attr.bool(default=True, mandatory=False), + "dependency_analyzer_plugin": attr.label(default=Label("//plugin/src/main:dependency_analyzer"), allow_files=_jar_filetype, mandatory=False), } + _implicit_deps + _common_attrs, outputs={ "jar": "%{name}.jar", diff --git a/test_run.sh b/test_run.sh index 3a2f07984..f84587c0d 100755 --- a/test_run.sh +++ b/test_run.sh @@ -361,6 +361,7 @@ fi $runner bazel build test/... $runner bazel test test/... +$runner bazel test plugin/... $runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges $runner bazel run test:JavaBinary $runner bazel run test:JavaBinary2 From 93db8c36eef43fcb24cfe8efdcc9c16ca56285c1 Mon Sep 17 00:00:00 2001 From: natans Date: Tue, 4 Jul 2017 11:59:20 +0300 Subject: [PATCH 24/24] remove hardcoded jar paths --- plugin/src/main/resources/toolbox.classpath | 2 +- plugin/src/test/BUILD | 6 +++-- plugin/src/test/resources/BUILD | 9 ------- plugin/src/test/resources/toolbox.plugin | 1 - .../dependencyanalyzer/TestUtil.scala | 26 +++++++++++++++---- .../rulesscala/scalac/ScalacProcessor.java | 5 +--- test_run.sh | 2 +- 7 files changed, 28 insertions(+), 23 deletions(-) delete mode 100644 plugin/src/test/resources/BUILD delete mode 100644 plugin/src/test/resources/toolbox.plugin diff --git a/plugin/src/main/resources/toolbox.classpath b/plugin/src/main/resources/toolbox.classpath index 88e7c419e..8aaa11a74 100644 --- a/plugin/src/main/resources/toolbox.classpath +++ b/plugin/src/main/resources/toolbox.classpath @@ -1 +1 @@ -/Users/natans/Work/rules_scala/bazel-genfiles/external/scala/_ijar/scala-compiler/external/scala/lib/scala-compiler-ijar.jar:/Users/natans/Work/rules_scala/bazel-genfiles/external/scala/_ijar/scala-library/external/scala/lib/scala-library-ijar.jar \ No newline at end of file +/Users/natans/Work/rules_scala/bazel-bin/plugin/src/test/dependency_analyzer_test.runfiles/io_bazel_rules_scala/external/scala/lib/scala-library.jar \ No newline at end of file diff --git a/plugin/src/test/BUILD b/plugin/src/test/BUILD index 9ebf172ba..a05c4c15f 100644 --- a/plugin/src/test/BUILD +++ b/plugin/src/test/BUILD @@ -8,7 +8,6 @@ scala_junit_test( size = "small", deps = ["//plugin/src/main:dependency_analyzer", "//plugin/src/main:resources", - "//plugin/src/test/resources:resources", "@io_get_coursier_coursier_cache//jar", "@io_get_coursier_coursier//jar", "@org_scalaz_scalaz_concurrent_2_11//jar", @@ -16,5 +15,8 @@ scala_junit_test( "//external:io_bazel_rules_scala/dependency/scalaz/scalaz_core", "//external:io_bazel_rules_scala/dependency/scala/scala_xml", "//external:io_bazel_rules_scala/dependency/scala/scala_compiler", - ] + "//external:io_bazel_rules_scala/dependency/scala/scala_library", + ], + jvm_flags = ["-Dplugin.jar.location=$(location //plugin/src/main:dependency_analyzer)", + "-Dscala.library.location=$(location //external:io_bazel_rules_scala/dependency/scala/scala_library)"], ) \ No newline at end of file diff --git a/plugin/src/test/resources/BUILD b/plugin/src/test/resources/BUILD deleted file mode 100644 index b2b204379..000000000 --- a/plugin/src/test/resources/BUILD +++ /dev/null @@ -1,9 +0,0 @@ -load("@io_bazel_rules_scala_for_plugin_bootstrapping//scala:scala.bzl", "scala_library") - -package(default_visibility = ["//visibility:public"]) - -scala_library( - name = "resources", - resources = native.glob(["**"],exclude=["BUILD"]), - visibility = ["//visibility:public"], -) \ No newline at end of file diff --git a/plugin/src/test/resources/toolbox.plugin b/plugin/src/test/resources/toolbox.plugin deleted file mode 100644 index c45d7ed1e..000000000 --- a/plugin/src/test/resources/toolbox.plugin +++ /dev/null @@ -1 +0,0 @@ --Xplugin:/Users/natans/Work/rules_scala/bazel-bin/plugin/src/main/dependency_analyzer.jar -Jdummy=1499012573000 \ No newline at end of file diff --git a/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/TestUtil.scala b/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/TestUtil.scala index 585bd6f78..62ba440bd 100644 --- a/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/TestUtil.scala +++ b/plugin/src/test/scala/io/github/retronym/dependencyanalyzer/TestUtil.scala @@ -1,11 +1,12 @@ package plugin.src.test.scala.io.github.retronym.dependencyanalyzer import java.io.File +import java.nio.file.Paths import coursier.maven.MavenRepository import coursier.{Cache, Dependency, Fetch, Resolution} -import scala.reflect.internal.util.{BatchSourceFile, NoPosition} +import scala.reflect.internal.util.BatchSourceFile import scala.reflect.io.VirtualDirectory import scala.tools.cmd.CommandLineParser import scala.tools.nsc.reporters.StoreReporter @@ -60,16 +61,31 @@ object TestUtil { } def getResourceContent(resourceName: String): String = { - println("getResourceContent "+ resourceName) + println("getResourceContent " + resourceName) val resource = getClass.getClassLoader.getResourceAsStream(resourceName) - println("getResourceContent resource: "+ resource) + println("getResourceContent resource: " + resource) val file = scala.io.Source.fromInputStream(resource) file.getLines.mkString } - lazy val toolboxClasspath: String = getResourceContent("toolbox.classpath") - lazy val toolboxPluginOptions: String = getResourceContent("toolbox.plugin") + lazy val baseDir = { + val workingDir = System.getProperty("user.dir") + val workspaceStart = workingDir.indexOf("bazel-out/") + workingDir.substring(0, workspaceStart) + } + + lazy val toolboxClasspath: String = { + val jar = System.getProperty("scala.library.location") + val pluginPath = Paths.get(baseDir, jar).toAbsolutePath + pluginPath.toString + } + + lazy val toolboxPluginOptions: String = { + val jar = System.getProperty("plugin.jar.location") + val pluginPath = Paths.get(baseDir, jar).toAbsolutePath + s"-Xplugin:${pluginPath} -Jdummy=${pluginPath.toFile.lastModified}" + } private def createBasicCompileOptions(classpath: String, usePluginOptions: String) = s"-classpath $classpath $usePluginOptions" diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index 31702d2e8..08dd37014 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -211,10 +211,7 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource "-classpath", ops.classpath, "-d", - tmpPath.toString(), - "-P:classpath-shrinker:direct-jars:" + String.join(":", ops.directJars), - "-P:classpath-shrinker:indirect-jars:" + String.join(":", ops.indirectJars), - "-P:classpath-shrinker:indirect-targets:" + String.join(":", targets), + tmpPath.toString() }; diff --git a/test_run.sh b/test_run.sh index f84587c0d..436b4c803 100755 --- a/test_run.sh +++ b/test_run.sh @@ -361,7 +361,7 @@ fi $runner bazel build test/... $runner bazel test test/... -$runner bazel test plugin/... +$runner bazel test plugin/... --spawn_strategy=standalone $runner bazel run test/src/main/scala/scala/test/twitter_scrooge:justscrooges $runner bazel run test:JavaBinary $runner bazel run test:JavaBinary2