From 45d307c9b9f51947d72adf93247ba8c9aa5e39a4 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Fri, 25 Oct 2019 16:04:16 -0600 Subject: [PATCH 01/32] Add configurable phases --- scala/private/phases/api.bzl | 10 ++ scala/private/phases/phase_collect_jars.bzl | 71 +++++++++ scala/private/phases/phase_compile.bzl | 144 ++++++++++++++++++ .../phases/phase_coverage_runfiles.bzl | 28 ++++ .../phases/phase_declare_executable.bzl | 12 ++ scala/private/phases/phase_final.bzl | 38 +++++ scala/private/phases/phase_init.bzl | 39 +++++ scala/private/phases/phase_java_wrapper.bzl | 46 ++++++ scala/private/phases/phase_jvm_flags.bzl | 52 +++++++ scala/private/phases/phase_merge_jars.bzl | 19 +++ scala/private/phases/phase_runfiles.bzl | 65 ++++++++ scala/private/phases/phase_scala_provider.bzl | 45 ++++++ .../phases/phase_unused_deps_checker.bzl | 12 ++ .../private/phases/phase_write_executable.bzl | 72 +++++++++ scala/private/phases/phases.bzl | 127 +++++++++++++++ 15 files changed, 780 insertions(+) create mode 100644 scala/private/phases/api.bzl create mode 100644 scala/private/phases/phase_collect_jars.bzl create mode 100644 scala/private/phases/phase_compile.bzl create mode 100644 scala/private/phases/phase_coverage_runfiles.bzl create mode 100644 scala/private/phases/phase_declare_executable.bzl create mode 100644 scala/private/phases/phase_final.bzl create mode 100644 scala/private/phases/phase_init.bzl create mode 100644 scala/private/phases/phase_java_wrapper.bzl create mode 100644 scala/private/phases/phase_jvm_flags.bzl create mode 100644 scala/private/phases/phase_merge_jars.bzl create mode 100644 scala/private/phases/phase_runfiles.bzl create mode 100644 scala/private/phases/phase_scala_provider.bzl create mode 100644 scala/private/phases/phase_unused_deps_checker.bzl create mode 100644 scala/private/phases/phase_write_executable.bzl create mode 100644 scala/private/phases/phases.bzl diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl new file mode 100644 index 000000000..afbcee020 --- /dev/null +++ b/scala/private/phases/api.bzl @@ -0,0 +1,10 @@ +def run_phases(ctx, phases): + global_provider = {} + current_provider = struct(**global_provider) + for (name, function) in phases: + new_provider = function(ctx, current_provider) + if new_provider != None: + global_provider[name] = new_provider + current_provider = struct(**global_provider) + + return current_provider diff --git a/scala/private/phases/phase_collect_jars.bzl b/scala/private/phases/phase_collect_jars.bzl new file mode 100644 index 000000000..6eac37475 --- /dev/null +++ b/scala/private/phases/phase_collect_jars.bzl @@ -0,0 +1,71 @@ +# +# PHASE: collect jars +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "collect_jars_from_common_ctx", +) + +def phase_test_collect_jars(ctx, p): + args = struct( + base_classpath = p.init.scalac_provider.default_classpath + [ctx.attr._scalatest], + extra_runtime_deps = [ + ctx.attr._scalatest_reporter, + ctx.attr._scalatest_runner, + ], + ) + return phase_common_collect_jars(ctx, p, args) + +def phase_repl_collect_jars(ctx, p): + args = struct( + base_classpath = p.init.scalac_provider.default_repl_classpath, + ) + return phase_common_collect_jars(ctx, p, args) + +def phase_macro_library_collect_jars(ctx, p): + args = struct( + base_classpath = p.init.scalac_provider.default_macro_classpath, + ) + return phase_common_collect_jars(ctx, p, args) + +def phase_junit_test_collect_jars(ctx, p): + args = struct( + extra_deps = [ + ctx.attr._junit, + ctx.attr._hamcrest, + ctx.attr.suite_label, + ctx.attr._bazel_test_runner, + ], + ) + return phase_common_collect_jars(ctx, p, args) + +def phase_library_for_plugin_bootstrapping_collect_jars(ctx, p): + args = struct( + unused_dependency_checker_mode = "off", + ) + return phase_common_collect_jars(ctx, p, args) + +def phase_common_collect_jars(ctx, p, _args = struct()): + return _phase_collect_jars( + ctx, + _args.base_classpath if hasattr(_args, "base_classpath") else p.init.scalac_provider.default_classpath, + _args.extra_deps if hasattr(_args, "extra_deps") else [], + _args.extra_runtime_deps if hasattr(_args, "extra_runtime_deps") else [], + _args.unused_dependency_checker_mode if hasattr(_args, "unused_dependency_checker_mode") else p.unused_deps_checker, + ) + +def _phase_collect_jars( + ctx, + base_classpath, + extra_deps, + extra_runtime_deps, + unused_dependency_checker_mode): + return collect_jars_from_common_ctx( + ctx, + base_classpath, + extra_deps, + extra_runtime_deps, + unused_dependency_checker_mode == "off", + ) diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl new file mode 100644 index 000000000..9027c67f4 --- /dev/null +++ b/scala/private/phases/phase_compile.bzl @@ -0,0 +1,144 @@ +# +# PHASE: compile +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "compile_or_empty", + "pack_source_jars", +) + +def phase_binary_compile(ctx, p): + args = struct( + unused_dependency_checker_ignored_targets = [ + target.label + for target in p.init.scalac_provider.default_classpath + + ctx.attr.unused_dependency_checker_ignored_targets + ], + ) + return phase_common_compile(ctx, p, args) + +def phase_library_compile(ctx, p): + args = struct( + srcjars = p.init.srcjars, + buildijar = True, + unused_dependency_checker_ignored_targets = [ + target.label + for target in p.init.scalac_provider.default_classpath + ctx.attr.exports + + ctx.attr.unused_dependency_checker_ignored_targets + ], + ) + return phase_common_compile(ctx, p, args) + +def phase_library_for_plugin_bootstrapping_compile(ctx, p): + args = struct( + buildijar = True, + unused_dependency_checker_ignored_targets = [ + target.label + for target in p.init.scalac_provider.default_classpath + ctx.attr.exports + ], + unused_dependency_checker_mode = "off", + ) + return phase_common_compile(ctx, p, args) + +def phase_macro_library_compile(ctx, p): + args = struct( + unused_dependency_checker_ignored_targets = [ + target.label + for target in p.init.scalac_provider.default_macro_classpath + ctx.attr.exports + + ctx.attr.unused_dependency_checker_ignored_targets + ], + ) + return phase_common_compile(ctx, p, args) + +def phase_junit_test_compile(ctx, p): + args = struct( + implicit_junit_deps_needed_for_java_compilation = [ + ctx.attr._junit, + ctx.attr._hamcrest, + ], + unused_dependency_checker_ignored_targets = [ + target.label + for target in p.init.scalac_provider.default_classpath + + ctx.attr.unused_dependency_checker_ignored_targets + ] + [ + ctx.attr._junit.label, + ctx.attr._hamcrest.label, + ctx.attr.suite_label.label, + ctx.attr._bazel_test_runner.label, + ], + ) + return phase_common_compile(ctx, p, args) + +def phase_repl_compile(ctx, p): + args = struct( + unused_dependency_checker_ignored_targets = [ + target.label + for target in p.init.scalac_provider.default_repl_classpath + + ctx.attr.unused_dependency_checker_ignored_targets + ], + ) + return phase_common_compile(ctx, p, args) + +def phase_test_compile(ctx, p): + args = struct( + unused_dependency_checker_ignored_targets = [ + target.label + for target in p.init.scalac_provider.default_classpath + + ctx.attr.unused_dependency_checker_ignored_targets + ], + ) + return phase_common_compile(ctx, p, args) + +def phase_common_compile(ctx, p, _args = struct()): + return _phase_compile( + ctx, + p, + _args.srcjars if hasattr(_args, "srcjars") else depset(), + _args.buildijar if hasattr(_args, "buildijar") else False, + _args.implicit_junit_deps_needed_for_java_compilation if hasattr(_args, "implicit_junit_deps_needed_for_java_compilation") else [], + _args.unused_dependency_checker_ignored_targets if hasattr(_args, "unused_dependency_checker_ignored_targets") else [], + _args.unused_dependency_checker_mode if hasattr(_args, "unused_dependency_checker_mode") else p.unused_deps_checker, + ) + +def _phase_compile( + ctx, + p, + srcjars, + buildijar, + implicit_junit_deps_needed_for_java_compilation, + unused_dependency_checker_ignored_targets, + unused_dependency_checker_mode): + manifest = ctx.outputs.manifest + jars = p.collect_jars.compile_jars + rjars = p.collect_jars.transitive_runtime_jars + transitive_compile_jars = p.collect_jars.transitive_compile_jars + jars2labels = p.collect_jars.jars2labels.jars_to_labels + deps_providers = p.collect_jars.deps_providers + + out = compile_or_empty( + ctx, + manifest, + jars, + srcjars, + buildijar, + transitive_compile_jars, + jars2labels, + implicit_junit_deps_needed_for_java_compilation, + unused_dependency_checker_mode, + unused_dependency_checker_ignored_targets, + deps_providers, + ) + + return struct( + class_jar = out.class_jar, + coverage = out.coverage, + full_jars = out.full_jars, + ijar = out.ijar, + ijars = out.ijars, + rjars = depset(out.full_jars, transitive = [rjars]), + java_jar = out.java_jar, + source_jars = pack_source_jars(ctx) + out.source_jars, + merged_provider = out.merged_provider, + ) diff --git a/scala/private/phases/phase_coverage_runfiles.bzl b/scala/private/phases/phase_coverage_runfiles.bzl new file mode 100644 index 000000000..00e50cf44 --- /dev/null +++ b/scala/private/phases/phase_coverage_runfiles.bzl @@ -0,0 +1,28 @@ +# +# PHASE: coverage runfiles +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:coverage_replacements_provider.bzl", + _coverage_replacements_provider = "coverage_replacements_provider", +) + +def phase_coverage_runfiles(ctx, p): + coverage_runfiles = [] + rjars = p.compile.rjars + if ctx.configuration.coverage_enabled and _coverage_replacements_provider.is_enabled(ctx): + coverage_replacements = _coverage_replacements_provider.from_ctx( + ctx, + base = p.compile.coverage.replacements, + ).replacements + + rjars = depset([ + coverage_replacements[jar] if jar in coverage_replacements else jar + for jar in rjars.to_list() + ]) + coverage_runfiles = ctx.files._jacocorunner + ctx.files._lcov_merger + coverage_replacements.values() + return struct( + coverage_runfiles = coverage_runfiles, + rjars = rjars, + ) diff --git a/scala/private/phases/phase_declare_executable.bzl b/scala/private/phases/phase_declare_executable.bzl new file mode 100644 index 000000000..53d0cdd50 --- /dev/null +++ b/scala/private/phases/phase_declare_executable.bzl @@ -0,0 +1,12 @@ +# +# PHASE: declare executable +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "declare_executable", +) + +def phase_declare_executable(ctx, p): + return declare_executable(ctx) diff --git a/scala/private/phases/phase_final.bzl b/scala/private/phases/phase_final.bzl new file mode 100644 index 000000000..771002dcb --- /dev/null +++ b/scala/private/phases/phase_final.bzl @@ -0,0 +1,38 @@ +# +# PHASE: final +# +# DOCUMENT THIS +# +def phase_binary_final(ctx, p): + return struct( + executable = p.declare_executable, + coverage = p.compile.coverage, + files = depset([p.declare_executable, ctx.outputs.jar]), + instrumented_files = p.compile.coverage.instrumented_files, + providers = [p.compile.merged_provider, p.collect_jars.jars2labels] + p.compile.coverage.providers, + runfiles = p.runfiles.runfiles, + scala = p.scala_provider, + transitive_rjars = p.compile.rjars, #calling rules need this for the classpath in the launcher + ) + +def phase_library_final(ctx, p): + return struct( + files = depset([ctx.outputs.jar] + p.compile.full_jars), # Here is the default output + instrumented_files = p.compile.coverage.instrumented_files, + jars_to_labels = p.collect_jars.jars2labels, + providers = [p.compile.merged_provider, p.collect_jars.jars2labels] + p.compile.coverage.providers, + runfiles = p.runfiles.runfiles, + scala = p.scala_provider, + ) + +def phase_test_final(ctx, p): + coverage_runfiles = p.coverage_runfiles.coverage_runfiles + coverage_runfiles.extend(p.write_executable) + return struct( + executable = p.declare_executable, + files = depset([p.declare_executable, ctx.outputs.jar]), + instrumented_files = p.compile.coverage.instrumented_files, + providers = [p.compile.merged_provider, p.collect_jars.jars2labels] + p.compile.coverage.providers, + runfiles = ctx.runfiles(coverage_runfiles, transitive_files = p.runfiles.runfiles.files), + scala = p.scala_provider, + ) diff --git a/scala/private/phases/phase_init.bzl b/scala/private/phases/phase_init.bzl new file mode 100644 index 000000000..f80494bef --- /dev/null +++ b/scala/private/phases/phase_init.bzl @@ -0,0 +1,39 @@ +# +# PHASE: init +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "get_scalac_provider", +) +load( + "@io_bazel_rules_scala//scala/private:common.bzl", + "collect_jars", + "collect_srcjars", + "write_manifest", +) + +def phase_library_init(ctx, p): + # This will be used to pick up srcjars from non-scala library + # targets (like thrift code generation) + srcjars = collect_srcjars(ctx.attr.deps) + + # 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) + + args = phase_common_init(ctx, p) + + return struct( + srcjars = srcjars, + exports_jars = exports_jars, + scalac_provider = args.scalac_provider, + ) + +def phase_common_init(ctx, p): + write_manifest(ctx) + return struct( + scalac_provider = get_scalac_provider(ctx), + ) diff --git a/scala/private/phases/phase_java_wrapper.bzl b/scala/private/phases/phase_java_wrapper.bzl new file mode 100644 index 000000000..64c7334cc --- /dev/null +++ b/scala/private/phases/phase_java_wrapper.bzl @@ -0,0 +1,46 @@ +# +# PHASE: java wrapper +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "write_java_wrapper", +) + +def phase_repl_java_wrapper(ctx, p): + args = struct( + args = " ".join(ctx.attr.scalacopts), + wrapper_preamble = """ +# save stty like in bin/scala +saved_stty=$(stty -g 2>/dev/null) +if [[ ! $? ]]; then + saved_stty="" +fi +function finish() { + if [[ "$saved_stty" != "" ]]; then + stty $saved_stty + saved_stty="" + fi +} +trap finish EXIT +""", + ) + return phase_common_java_wrapper(ctx, p, args) + +def phase_common_java_wrapper(ctx, p, _args = struct()): + return _phase_java_wrapper( + ctx, + _args.args if hasattr(_args, "args") else "", + _args.wrapper_preamble if hasattr(_args, "wrapper_preamble") else "", + ) + +def _phase_java_wrapper( + ctx, + args, + wrapper_preamble): + return write_java_wrapper( + ctx, + args, + wrapper_preamble, + ) diff --git a/scala/private/phases/phase_jvm_flags.bzl b/scala/private/phases/phase_jvm_flags.bzl new file mode 100644 index 000000000..9f17a95dd --- /dev/null +++ b/scala/private/phases/phase_jvm_flags.bzl @@ -0,0 +1,52 @@ +# +# PHASE: jvm flags +# +# DOCUMENT THIS +# +def phase_jvm_flags(ctx, p): + if ctx.attr.tests_from: + archives = _get_test_archive_jars(ctx, ctx.attr.tests_from) + else: + archives = [archive.class_jar for archive in p.scala_provider.outputs.jars] + + serialized_archives = _serialize_archives_short_path(archives) + test_suite = _gen_test_suite_flags_based_on_prefixes_and_suffixes( + ctx, + serialized_archives, + ) + return [ + "-ea", + test_suite.archiveFlag, + test_suite.prefixesFlag, + test_suite.suffixesFlag, + test_suite.printFlag, + test_suite.testSuiteFlag, + ] + +def _gen_test_suite_flags_based_on_prefixes_and_suffixes(ctx, archives): + return struct( + archiveFlag = "-Dbazel.discover.classes.archives.file.paths=%s" % + archives, + prefixesFlag = "-Dbazel.discover.classes.prefixes=%s" % ",".join( + ctx.attr.prefixes, + ), + printFlag = "-Dbazel.discover.classes.print.discovered=%s" % + ctx.attr.print_discovered_classes, + suffixesFlag = "-Dbazel.discover.classes.suffixes=%s" % ",".join( + ctx.attr.suffixes, + ), + testSuiteFlag = "-Dbazel.test_suite=%s" % ctx.attr.suite_class, + ) + +def _serialize_archives_short_path(archives): + archives_short_path = "" + for archive in archives: + archives_short_path += archive.short_path + "," + return archives_short_path[:-1] #remove redundant comma + +def _get_test_archive_jars(ctx, test_archives): + flattened_list = [] + for archive in test_archives: + class_jars = [java_output.class_jar for java_output in archive[JavaInfo].outputs.jars] + flattened_list.extend(class_jars) + return flattened_list diff --git a/scala/private/phases/phase_merge_jars.bzl b/scala/private/phases/phase_merge_jars.bzl new file mode 100644 index 000000000..bec2194f5 --- /dev/null +++ b/scala/private/phases/phase_merge_jars.bzl @@ -0,0 +1,19 @@ +# +# PHASE: merge jars +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "merge_jars", +) + +def phase_merge_jars(ctx, p): + merge_jars( + actions = ctx.actions, + deploy_jar = ctx.outputs.deploy_jar, + singlejar_executable = ctx.executable._singlejar, + jars_list = p.compile.rjars.to_list(), + main_class = getattr(ctx.attr, "main_class", ""), + progress_message = "Merging Scala jar: %s" % ctx.label, + ) diff --git a/scala/private/phases/phase_runfiles.bzl b/scala/private/phases/phase_runfiles.bzl new file mode 100644 index 000000000..bdc602524 --- /dev/null +++ b/scala/private/phases/phase_runfiles.bzl @@ -0,0 +1,65 @@ +# +# PHASE: runfiles +# +# DOCUMENT THIS +# +def phase_library_runfiles(ctx, p): + args = struct( + # Using transitive_files since transitive_rjars a depset and avoiding linearization + transitive_files = p.compile.rjars, + ) + return phase_common_runfiles(ctx, p, args) + +def phase_test_runfiles(ctx, p): + args = "\n".join([ + "-R", + ctx.outputs.jar.short_path, + _scala_test_flags(ctx), + "-C", + "io.bazel.rules.scala.JUnitXmlReporter", + ]) + args_file = ctx.actions.declare_file("%s.args" % ctx.label.name) + ctx.actions.write(args_file, args) + runfiles_ext = [args_file] + + args = struct( + transitive_files = depset( + [p.declare_executable, p.java_wrapper] + ctx.files._java_runtime + runfiles_ext, + transitive = [p.compile.rjars], + ), + args_file = args_file, + ) + return phase_common_runfiles(ctx, p, args) + +def phase_common_runfiles(ctx, p, _args = struct()): + return _phase_runfiles( + ctx, + _args.transitive_files if hasattr(_args, "transitive_files") else depset( + [p.declare_executable, p.java_wrapper] + ctx.files._java_runtime, + transitive = [p.compile.rjars], + ), + _args.args_file if hasattr(_args, "args_file") else None, + ) + +def _phase_runfiles( + ctx, + transitive_files, + args_file): + return struct( + runfiles = ctx.runfiles( + transitive_files = transitive_files, + collect_data = True, + ), + args_file = args_file, + ) + +def _scala_test_flags(ctx): + # output report test duration + flags = "-oD" + if ctx.attr.full_stacktraces: + flags += "F" + else: + flags += "S" + if not ctx.attr.colors: + flags += "W" + return flags diff --git a/scala/private/phases/phase_scala_provider.bzl b/scala/private/phases/phase_scala_provider.bzl new file mode 100644 index 000000000..3a4f821f6 --- /dev/null +++ b/scala/private/phases/phase_scala_provider.bzl @@ -0,0 +1,45 @@ +# +# PHASE: scala provider +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala:providers.bzl", + "create_scala_provider", +) + +def phase_library_scala_provider(ctx, p): + args = struct( + rjars = depset( + transitive = [p.compile.rjars, p.init.exports_jars.transitive_runtime_jars], + ), + compile_jars = depset( + p.compile.ijars, + transitive = [p.init.exports_jars.compile_jars], + ), + ) + return phase_common_scala_provider(ctx, p, args) + +def phase_common_scala_provider(ctx, p, _args = struct()): + return _phase_scala_provider( + ctx, + p, + _args.rjars if hasattr(_args, "rjars") else p.compile.rjars, + _args.compile_jars if hasattr(_args, "compile_jars") else depset(p.compile.ijars), + ) + +def _phase_scala_provider( + ctx, + p, + rjars, + compile_jars): + return create_scala_provider( + class_jar = p.compile.class_jar, + compile_jars = compile_jars, + deploy_jar = ctx.outputs.deploy_jar, + full_jars = p.compile.full_jars, + ijar = p.compile.class_jar, # we aren't using ijar here + source_jars = p.compile.source_jars, + statsfile = ctx.outputs.statsfile, + transitive_runtime_jars = rjars, + ) diff --git a/scala/private/phases/phase_unused_deps_checker.bzl b/scala/private/phases/phase_unused_deps_checker.bzl new file mode 100644 index 000000000..4757118cf --- /dev/null +++ b/scala/private/phases/phase_unused_deps_checker.bzl @@ -0,0 +1,12 @@ +# +# PHASE: unused deps checker +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "get_unused_dependency_checker_mode", +) + +def phase_unused_deps_checker(ctx, p): + return get_unused_dependency_checker_mode(ctx) diff --git a/scala/private/phases/phase_write_executable.bzl b/scala/private/phases/phase_write_executable.bzl new file mode 100644 index 000000000..93a6784b5 --- /dev/null +++ b/scala/private/phases/phase_write_executable.bzl @@ -0,0 +1,72 @@ +# +# PHASE: write executable +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "expand_location", + "first_non_empty", + "write_executable", +) + +def phase_test_write_executable(ctx, p): + # jvm_flags passed in on the target override scala_test_jvm_flags passed in on the + # toolchain + final_jvm_flags = first_non_empty( + ctx.attr.jvm_flags, + ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].scala_test_jvm_flags, + ) + args = struct( + rjars = p.coverage_runfiles.rjars, + jvm_flags = [ + "-DRULES_SCALA_MAIN_WS_NAME=%s" % ctx.workspace_name, + "-DRULES_SCALA_ARGS_FILE=%s" % p.runfiles.args_file.short_path, + ] + expand_location(ctx, final_jvm_flags), + use_jacoco = ctx.configuration.coverage_enabled, + ) + return phase_common_write_executable(ctx, p, args) + +def phase_repl_write_executable(ctx, p): + args = struct( + jvm_flags = ["-Dscala.usejavacp=true"] + ctx.attr.jvm_flags, + main_class = "scala.tools.nsc.MainGenericRunner", + ) + return phase_common_write_executable(ctx, p, args) + +def phase_junit_test_write_executable(ctx, p): + args = struct( + jvm_flags = p.jvm_flags + ctx.attr.jvm_flags, + main_class = "com.google.testing.junit.runner.BazelTestRunner", + ) + return phase_common_write_executable(ctx, p, args) + +def phase_common_write_executable(ctx, p, _args = struct()): + return _phase_write_executable( + ctx, + p, + _args.rjars if hasattr(_args, "rjars") else p.compile.rjars, + _args.jvm_flags if hasattr(_args, "jvm_flags") else ctx.attr.jvm_flags, + _args.use_jacoco if hasattr(_args, "use_jacoco") else False, + _args.main_class if hasattr(_args, "main_class") else ctx.attr.main_class, + ) + +def _phase_write_executable( + ctx, + p, + rjars, + jvm_flags, + use_jacoco, + main_class): + executable = p.declare_executable + wrapper = p.java_wrapper + + return write_executable( + ctx, + executable, + rjars, + main_class, + jvm_flags, + wrapper, + use_jacoco, + ) diff --git a/scala/private/phases/phases.bzl b/scala/private/phases/phases.bzl new file mode 100644 index 000000000..12d03ab3c --- /dev/null +++ b/scala/private/phases/phases.bzl @@ -0,0 +1,127 @@ +load( + "@io_bazel_rules_scala//scala/private:phases/api.bzl", + _run_phases = "run_phases", +) +load( + "@io_bazel_rules_scala//scala/private:phases/phase_write_executable.bzl", + _phase_common_write_executable = "phase_common_write_executable", + _phase_junit_test_write_executable = "phase_junit_test_write_executable", + _phase_repl_write_executable = "phase_repl_write_executable", + _phase_test_write_executable = "phase_test_write_executable", +) +load( + "@io_bazel_rules_scala//scala/private:phases/phase_java_wrapper.bzl", + _phase_common_java_wrapper = "phase_common_java_wrapper", + _phase_repl_java_wrapper = "phase_repl_java_wrapper", +) +load( + "@io_bazel_rules_scala//scala/private:phases/phase_collect_jars.bzl", + _phase_common_collect_jars = "phase_common_collect_jars", + _phase_junit_test_collect_jars = "phase_junit_test_collect_jars", + _phase_library_for_plugin_bootstrapping_collect_jars = "phase_library_for_plugin_bootstrapping_collect_jars", + _phase_macro_library_collect_jars = "phase_macro_library_collect_jars", + _phase_repl_collect_jars = "phase_repl_collect_jars", + _phase_test_collect_jars = "phase_test_collect_jars", +) +load( + "@io_bazel_rules_scala//scala/private:phases/phase_compile.bzl", + _phase_binary_compile = "phase_binary_compile", + _phase_common_compile = "phase_common_compile", + _phase_junit_test_compile = "phase_junit_test_compile", + _phase_library_compile = "phase_library_compile", + _phase_library_for_plugin_bootstrapping_compile = "phase_library_for_plugin_bootstrapping_compile", + _phase_macro_library_compile = "phase_macro_library_compile", + _phase_repl_compile = "phase_repl_compile", + _phase_test_compile = "phase_test_compile", +) +load( + "@io_bazel_rules_scala//scala/private:phases/phase_scala_provider.bzl", + _phase_common_scala_provider = "phase_common_scala_provider", + _phase_library_scala_provider = "phase_library_scala_provider", +) +load( + "@io_bazel_rules_scala//scala/private:phases/phase_runfiles.bzl", + _phase_common_runfiles = "phase_common_runfiles", + _phase_library_runfiles = "phase_library_runfiles", + _phase_test_runfiles = "phase_test_runfiles", +) +load( + "@io_bazel_rules_scala//scala/private:phases/phase_final.bzl", + _phase_binary_final = "phase_binary_final", + _phase_library_final = "phase_library_final", + _phase_test_final = "phase_test_final", +) +load( + "@io_bazel_rules_scala//scala/private:phases/phase_init.bzl", + _phase_common_init = "phase_common_init", + _phase_library_init = "phase_library_init", +) +load("@io_bazel_rules_scala//scala/private:phases/phase_unused_deps_checker.bzl", _phase_unused_deps_checker = "phase_unused_deps_checker") +load("@io_bazel_rules_scala//scala/private:phases/phase_declare_executable.bzl", _phase_declare_executable = "phase_declare_executable") +load("@io_bazel_rules_scala//scala/private:phases/phase_merge_jars.bzl", _phase_merge_jars = "phase_merge_jars") +load("@io_bazel_rules_scala//scala/private:phases/phase_jvm_flags.bzl", _phase_jvm_flags = "phase_jvm_flags") +load("@io_bazel_rules_scala//scala/private:phases/phase_coverage_runfiles.bzl", _phase_coverage_runfiles = "phase_coverage_runfiles") + +# API +run_phases = _run_phases + +# init +phase_common_init = _phase_common_init +phase_library_init = _phase_library_init + +# unused_deps_checker +phase_unused_deps_checker = _phase_unused_deps_checker + +# declare_executable +phase_declare_executable = _phase_declare_executable + +# merge_jars +phase_merge_jars = _phase_merge_jars + +# jvm_flags +phase_jvm_flags = _phase_jvm_flags + +# coverage_runfiles +phase_coverage_runfiles = _phase_coverage_runfiles + +# write_executable +phase_test_write_executable = _phase_test_write_executable +phase_repl_write_executable = _phase_repl_write_executable +phase_junit_test_write_executable = _phase_junit_test_write_executable +phase_common_write_executable = _phase_common_write_executable + +# java_wrapper +phase_repl_java_wrapper = _phase_repl_java_wrapper +phase_common_java_wrapper = _phase_common_java_wrapper + +# collect_jars +phase_test_collect_jars = _phase_test_collect_jars +phase_repl_collect_jars = _phase_repl_collect_jars +phase_macro_library_collect_jars = _phase_macro_library_collect_jars +phase_junit_test_collect_jars = _phase_junit_test_collect_jars +phase_library_for_plugin_bootstrapping_collect_jars = _phase_library_for_plugin_bootstrapping_collect_jars +phase_common_collect_jars = _phase_common_collect_jars + +# compile +phase_binary_compile = _phase_binary_compile +phase_library_compile = _phase_library_compile +phase_library_for_plugin_bootstrapping_compile = _phase_library_for_plugin_bootstrapping_compile +phase_macro_library_compile = _phase_macro_library_compile +phase_junit_test_compile = _phase_junit_test_compile +phase_repl_compile = _phase_repl_compile +phase_test_compile = _phase_test_compile +phase_common_compile = _phase_common_compile + +# scala_provider +phase_library_scala_provider = _phase_library_scala_provider +phase_common_scala_provider = _phase_common_scala_provider + +# runfiles +phase_library_runfiles = _phase_library_runfiles +phase_test_runfiles = _phase_test_runfiles +phase_common_runfiles = _phase_common_runfiles + +# final +phase_binary_final = _phase_binary_final +phase_library_final = _phase_library_final +phase_test_final = _phase_test_final From df374ec76f04aaa9db65ec5c958dbdfe63d0bc97 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Fri, 25 Oct 2019 16:04:48 -0600 Subject: [PATCH 02/32] Refactor rules implementation into configurable phases --- scala/private/rule_impls.bzl | 73 ---------- scala/private/rules/scala_binary.bzl | 76 ++++------ scala/private/rules/scala_junit_test.bzl | 145 ++++--------------- scala/private/rules/scala_library.bzl | 171 ++++++----------------- scala/private/rules/scala_repl.bzl | 99 ++++--------- scala/private/rules/scala_test.bzl | 158 ++++----------------- 6 files changed, 157 insertions(+), 565 deletions(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index f57f905e3..925179430 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -811,79 +811,6 @@ def get_unused_dependency_checker_mode(ctx): else: return ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].unused_dependency_checker_mode -# Common code shared by all scala binary implementations. -def scala_binary_common( - ctx, - executable, - cjars, - rjars, - transitive_compile_time_jars, - jars2labels, - java_wrapper, - unused_dependency_checker_mode, - unused_dependency_checker_ignored_targets, - deps_providers, - implicit_junit_deps_needed_for_java_compilation = [], - runfiles_ext = []): - write_manifest(ctx) - outputs = compile_or_empty( - ctx, - ctx.outputs.manifest, - cjars, - depset(), - False, - transitive_compile_time_jars, - jars2labels.jars_to_labels, - implicit_junit_deps_needed_for_java_compilation, - unused_dependency_checker_ignored_targets = - unused_dependency_checker_ignored_targets, - unused_dependency_checker_mode = unused_dependency_checker_mode, - deps_providers = deps_providers, - ) # no need to build an ijar for an executable - rjars = depset(outputs.full_jars, transitive = [rjars]) - - merge_jars( - actions = ctx.actions, - deploy_jar = ctx.outputs.deploy_jar, - singlejar_executable = ctx.executable._singlejar, - jars_list = rjars.to_list(), - main_class = getattr(ctx.attr, "main_class", ""), - progress_message = "Merging Scala binary jar: %s" % ctx.label, - ) - - runfiles = ctx.runfiles( - transitive_files = depset( - [executable, java_wrapper] + ctx.files._java_runtime + runfiles_ext, - transitive = [rjars], - ), - collect_data = True, - ) - - source_jars = pack_source_jars(ctx) + outputs.source_jars - - scalaattr = create_scala_provider( - class_jar = outputs.class_jar, - compile_jars = depset(outputs.ijars), - deploy_jar = ctx.outputs.deploy_jar, - full_jars = outputs.full_jars, - ijar = outputs.class_jar, # we aren't using ijar here - source_jars = source_jars, - statsfile = ctx.outputs.statsfile, - transitive_runtime_jars = rjars, - ) - - return struct( - executable = executable, - coverage = outputs.coverage, - files = depset([executable, ctx.outputs.jar]), - instrumented_files = outputs.coverage.instrumented_files, - providers = [outputs.merged_provider, jars2labels] + outputs.coverage.providers, - runfiles = runfiles, - scala = scalaattr, - transitive_rjars = - rjars, #calling rules need this for the classpath in the launcher - ) - def _pack_source_jar(ctx): # collect .scala sources and pack a source jar for Scala scala_sources = [ diff --git a/scala/private/rules/scala_binary.bzl b/scala/private/rules/scala_binary.bzl index df4dcbea5..cbcadc907 100644 --- a/scala/private/rules/scala_binary.bzl +++ b/scala/private/rules/scala_binary.bzl @@ -9,58 +9,36 @@ load( ) load("@io_bazel_rules_scala//scala/private:common_outputs.bzl", "common_outputs") load( - "@io_bazel_rules_scala//scala/private:rule_impls.bzl", - "collect_jars_from_common_ctx", - "declare_executable", - "get_scalac_provider", - "get_unused_dependency_checker_mode", - "scala_binary_common", - "write_executable", - "write_java_wrapper", + "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "phase_binary_compile", + "phase_binary_final", + "phase_common_collect_jars", + "phase_common_init", + "phase_common_java_wrapper", + "phase_common_runfiles", + "phase_common_scala_provider", + "phase_common_write_executable", + "phase_declare_executable", + "phase_merge_jars", + "phase_unused_deps_checker", + "run_phases", ) def _scala_binary_impl(ctx): - scalac_provider = get_scalac_provider(ctx) - unused_dependency_checker_mode = get_unused_dependency_checker_mode(ctx) - unused_dependency_checker_is_off = unused_dependency_checker_mode == "off" - - jars = collect_jars_from_common_ctx( - ctx, - scalac_provider.default_classpath, - unused_dependency_checker_is_off = unused_dependency_checker_is_off, - ) - (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) - - wrapper = write_java_wrapper(ctx, "", "") - - executable = declare_executable(ctx) - - out = scala_binary_common( - ctx, - executable, - cjars, - transitive_rjars, - jars.transitive_compile_jars, - jars.jars2labels, - wrapper, - unused_dependency_checker_ignored_targets = [ - target.label - for target in scalac_provider.default_classpath + - ctx.attr.unused_dependency_checker_ignored_targets - ], - unused_dependency_checker_mode = unused_dependency_checker_mode, - deps_providers = jars.deps_providers, - ) - write_executable( - ctx = ctx, - executable = executable, - jvm_flags = ctx.attr.jvm_flags, - main_class = ctx.attr.main_class, - rjars = out.transitive_rjars, - use_jacoco = False, - wrapper = wrapper, - ) - return out + return run_phases(ctx, [ + ("init", phase_common_init), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_common_collect_jars), + ("java_wrapper", phase_common_java_wrapper), + ("declare_executable", phase_declare_executable), + # no need to build an ijar for an executable + ("compile", phase_binary_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_common_runfiles), + ("scala_provider", phase_common_scala_provider), + ("write_executable", phase_common_write_executable), + ("final", phase_binary_final), + ]).final _scala_binary_attrs = { "main_class": attr.string(mandatory = True), diff --git a/scala/private/rules/scala_junit_test.bzl b/scala/private/rules/scala_junit_test.bzl index a36b8082c..d43938be9 100644 --- a/scala/private/rules/scala_junit_test.bzl +++ b/scala/private/rules/scala_junit_test.bzl @@ -8,129 +8,42 @@ load( ) load("@io_bazel_rules_scala//scala/private:common_outputs.bzl", "common_outputs") load( - "@io_bazel_rules_scala//scala/private:rule_impls.bzl", - "collect_jars_from_common_ctx", - "declare_executable", - "get_scalac_provider", - "get_unused_dependency_checker_mode", - "scala_binary_common", - "write_executable", - "write_java_wrapper", + "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "phase_binary_final", + "phase_common_init", + "phase_common_java_wrapper", + "phase_common_runfiles", + "phase_common_scala_provider", + "phase_declare_executable", + "phase_junit_test_collect_jars", + "phase_junit_test_compile", + "phase_junit_test_write_executable", + "phase_jvm_flags", + "phase_merge_jars", + "phase_unused_deps_checker", + "run_phases", ) -def _gen_test_suite_flags_based_on_prefixes_and_suffixes(ctx, archives): - return struct( - archiveFlag = "-Dbazel.discover.classes.archives.file.paths=%s" % - archives, - prefixesFlag = "-Dbazel.discover.classes.prefixes=%s" % ",".join( - ctx.attr.prefixes, - ), - printFlag = "-Dbazel.discover.classes.print.discovered=%s" % - ctx.attr.print_discovered_classes, - suffixesFlag = "-Dbazel.discover.classes.suffixes=%s" % ",".join( - ctx.attr.suffixes, - ), - testSuiteFlag = "-Dbazel.test_suite=%s" % ctx.attr.suite_class, - ) - -def _serialize_archives_short_path(archives): - archives_short_path = "" - for archive in archives: - archives_short_path += archive.short_path + "," - return archives_short_path[:-1] #remove redundant comma - -def _get_test_archive_jars(ctx, test_archives): - flattened_list = [] - for archive in test_archives: - class_jars = [java_output.class_jar for java_output in archive[JavaInfo].outputs.jars] - flattened_list.extend(class_jars) - return flattened_list - def _scala_junit_test_impl(ctx): if (not (ctx.attr.prefixes) and not (ctx.attr.suffixes)): fail( "Setting at least one of the attributes ('prefixes','suffixes') is required", ) - scalac_provider = get_scalac_provider(ctx) - - unused_dependency_checker_mode = get_unused_dependency_checker_mode(ctx) - unused_dependency_checker_ignored_targets = [ - target.label - for target in scalac_provider.default_classpath + - ctx.attr.unused_dependency_checker_ignored_targets - ] + [ - ctx.attr._junit.label, - ctx.attr._hamcrest.label, - ctx.attr.suite_label.label, - ctx.attr._bazel_test_runner.label, - ] - unused_dependency_checker_is_off = unused_dependency_checker_mode == "off" - - jars = collect_jars_from_common_ctx( - ctx, - scalac_provider.default_classpath, - extra_deps = [ - ctx.attr._junit, - ctx.attr._hamcrest, - ctx.attr.suite_label, - ctx.attr._bazel_test_runner, - ], - unused_dependency_checker_is_off = unused_dependency_checker_is_off, - ) - (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) - implicit_junit_deps_needed_for_java_compilation = [ - ctx.attr._junit, - ctx.attr._hamcrest, - ] - - executable = declare_executable(ctx) - - wrapper = write_java_wrapper(ctx, "", "") - out = scala_binary_common( - ctx, - executable, - cjars, - transitive_rjars, - jars.transitive_compile_jars, - jars.jars2labels, - wrapper, - implicit_junit_deps_needed_for_java_compilation = - implicit_junit_deps_needed_for_java_compilation, - unused_dependency_checker_ignored_targets = - unused_dependency_checker_ignored_targets, - unused_dependency_checker_mode = unused_dependency_checker_mode, - deps_providers = jars.deps_providers, - ) - - if ctx.attr.tests_from: - archives = _get_test_archive_jars(ctx, ctx.attr.tests_from) - else: - archives = [archive.class_jar for archive in out.scala.outputs.jars] - - serialized_archives = _serialize_archives_short_path(archives) - test_suite = _gen_test_suite_flags_based_on_prefixes_and_suffixes( - ctx, - serialized_archives, - ) - launcherJvmFlags = [ - "-ea", - test_suite.archiveFlag, - test_suite.prefixesFlag, - test_suite.suffixesFlag, - test_suite.printFlag, - test_suite.testSuiteFlag, - ] - write_executable( - ctx = ctx, - executable = executable, - jvm_flags = launcherJvmFlags + ctx.attr.jvm_flags, - main_class = "com.google.testing.junit.runner.BazelTestRunner", - rjars = out.transitive_rjars, - use_jacoco = False, - wrapper = wrapper, - ) - - return out + return run_phases(ctx, [ + ("init", phase_common_init), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_junit_test_collect_jars), + ("java_wrapper", phase_common_java_wrapper), + ("declare_executable", phase_declare_executable), + # no need to build an ijar for an executable + ("compile", phase_junit_test_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_common_runfiles), + ("scala_provider", phase_common_scala_provider), + ("jvm_flags", phase_jvm_flags), + ("write_executable", phase_junit_test_write_executable), + ("final", phase_binary_final), + ]).final _scala_junit_test_attrs = { "prefixes": attr.string_list(default = []), diff --git a/scala/private/rules/scala_library.bzl b/scala/private/rules/scala_library.bzl index 5978cb333..d49a6abf0 100644 --- a/scala/private/rules/scala_library.bzl +++ b/scala/private/rules/scala_library.bzl @@ -1,10 +1,6 @@ -load("@io_bazel_rules_scala//scala:providers.bzl", "create_scala_provider") load( "@io_bazel_rules_scala//scala/private:common.bzl", - "collect_jars", - "collect_srcjars", "sanitize_string_for_usage", - "write_manifest", ) load( "@io_bazel_rules_scala//scala/private:common_attributes.bzl", @@ -19,13 +15,20 @@ load( _coverage_replacements_provider = "coverage_replacements_provider", ) load( - "@io_bazel_rules_scala//scala/private:rule_impls.bzl", - "collect_jars_from_common_ctx", - "compile_or_empty", - "get_scalac_provider", - "get_unused_dependency_checker_mode", - "merge_jars", - "pack_source_jars", + "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "phase_common_collect_jars", + "phase_library_compile", + "phase_library_final", + "phase_library_for_plugin_bootstrapping_collect_jars", + "phase_library_for_plugin_bootstrapping_compile", + "phase_library_init", + "phase_library_runfiles", + "phase_library_scala_provider", + "phase_macro_library_collect_jars", + "phase_macro_library_compile", + "phase_merge_jars", + "phase_unused_deps_checker", + "run_phases", ) ## @@ -40,110 +43,22 @@ _library_attrs = { ), } -def _lib( - ctx, - base_classpath, - non_macro_lib, - unused_dependency_checker_mode, - unused_dependency_checker_ignored_targets): - # 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) - - unused_dependency_checker_is_off = unused_dependency_checker_mode == "off" - jars = collect_jars_from_common_ctx( - ctx, - base_classpath, - unused_dependency_checker_is_off = unused_dependency_checker_is_off, - ) - - (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) - - write_manifest(ctx) - outputs = compile_or_empty( - ctx, - ctx.outputs.manifest, - cjars, - srcjars, - non_macro_lib, - jars.transitive_compile_jars, - jars.jars2labels.jars_to_labels, - [], - unused_dependency_checker_ignored_targets = [ - target.label - for target in base_classpath + ctx.attr.exports + - unused_dependency_checker_ignored_targets - ], - unused_dependency_checker_mode = unused_dependency_checker_mode, - deps_providers = jars.deps_providers, - ) - - transitive_rjars = depset(outputs.full_jars, transitive = [transitive_rjars]) - - merge_jars( - actions = ctx.actions, - deploy_jar = ctx.outputs.deploy_jar, - singlejar_executable = ctx.executable._singlejar, - jars_list = transitive_rjars.to_list(), - main_class = getattr(ctx.attr, "main_class", ""), - progress_message = "Merging Scala library jar: %s" % ctx.label, - ) - - # Using transitive_files since transitive_rjars a depset and avoiding linearization - runfiles = ctx.runfiles( - 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) - transitive_rjars = depset( - transitive = [transitive_rjars, exports_jars.transitive_runtime_jars], - ) - - source_jars = pack_source_jars(ctx) + outputs.source_jars - - scalaattr = create_scala_provider( - class_jar = outputs.class_jar, - compile_jars = depset( - outputs.ijars, - transitive = [exports_jars.compile_jars], - ), - deploy_jar = ctx.outputs.deploy_jar, - full_jars = outputs.full_jars, - ijar = outputs.ijar, - source_jars = source_jars, - statsfile = ctx.outputs.statsfile, - transitive_runtime_jars = transitive_rjars, - ) - - return struct( - files = depset([ctx.outputs.jar] + outputs.full_jars), # Here is the default output - instrumented_files = outputs.coverage.instrumented_files, - jars_to_labels = jars.jars2labels, - providers = [outputs.merged_provider, jars.jars2labels] + outputs.coverage.providers, - runfiles = runfiles, - scala = scalaattr, - ) - ## # scala_library ## def _scala_library_impl(ctx): - scalac_provider = get_scalac_provider(ctx) - unused_dependency_checker_mode = get_unused_dependency_checker_mode(ctx) - return _lib( - ctx, - scalac_provider.default_classpath, - True, - unused_dependency_checker_mode, - ctx.attr.unused_dependency_checker_ignored_targets, - ) + # Build up information from dependency-like attributes + return run_phases(ctx, [ + ("init", phase_library_init), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_common_collect_jars), + ("compile", phase_library_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_library_runfiles), + ("scala_provider", phase_library_scala_provider), + ("final", phase_library_final), + ]).final _scala_library_attrs = {} @@ -195,14 +110,15 @@ def scala_library_suite( ## def _scala_library_for_plugin_bootstrapping_impl(ctx): - scalac_provider = get_scalac_provider(ctx) - return _lib( - ctx, - scalac_provider.default_classpath, - True, - unused_dependency_checker_ignored_targets = [], - unused_dependency_checker_mode = "off", - ) + return run_phases(ctx, [ + ("init", phase_library_init), + ("collect_jars", phase_library_for_plugin_bootstrapping_collect_jars), + ("compile", phase_library_for_plugin_bootstrapping_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_library_runfiles), + ("scala_provider", phase_library_scala_provider), + ("final", phase_library_final), + ]).final # the scala compiler plugin used for dependency analysis is compiled using `scala_library`. # in order to avoid cyclic dependencies `scala_library_for_plugin_bootstrapping` was created for this purpose, @@ -232,15 +148,16 @@ scala_library_for_plugin_bootstrapping = rule( ## def _scala_macro_library_impl(ctx): - scalac_provider = get_scalac_provider(ctx) - unused_dependency_checker_mode = get_unused_dependency_checker_mode(ctx) - return _lib( - ctx, - scalac_provider.default_macro_classpath, - False, # don't build the ijar for macros - unused_dependency_checker_mode, - ctx.attr.unused_dependency_checker_ignored_targets, - ) + return run_phases(ctx, [ + ("init", phase_library_init), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_macro_library_collect_jars), + ("compile", phase_macro_library_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_library_runfiles), + ("scala_provider", phase_library_scala_provider), + ("final", phase_library_final), + ]).final _scala_macro_library_attrs = { "main_class": attr.string(), diff --git a/scala/private/rules/scala_repl.bzl b/scala/private/rules/scala_repl.bzl index 78630ff3b..b4e0d1155 100644 --- a/scala/private/rules/scala_repl.bzl +++ b/scala/private/rules/scala_repl.bzl @@ -9,80 +9,37 @@ load( ) load("@io_bazel_rules_scala//scala/private:common_outputs.bzl", "common_outputs") load( - "@io_bazel_rules_scala//scala/private:rule_impls.bzl", - "collect_jars_from_common_ctx", - "declare_executable", - "get_scalac_provider", - "get_unused_dependency_checker_mode", - "scala_binary_common", - "write_executable", - "write_java_wrapper", + "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "phase_binary_final", + "phase_common_init", + "phase_common_runfiles", + "phase_common_scala_provider", + "phase_declare_executable", + "phase_merge_jars", + "phase_repl_collect_jars", + "phase_repl_compile", + "phase_repl_java_wrapper", + "phase_repl_write_executable", + "phase_unused_deps_checker", + "run_phases", ) def _scala_repl_impl(ctx): - scalac_provider = get_scalac_provider(ctx) - - unused_dependency_checker_mode = get_unused_dependency_checker_mode(ctx) - unused_dependency_checker_is_off = unused_dependency_checker_mode == "off" - - # need scala-compiler for MainGenericRunner below - jars = collect_jars_from_common_ctx( - ctx, - scalac_provider.default_repl_classpath, - unused_dependency_checker_is_off = unused_dependency_checker_is_off, - ) - (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) - - args = " ".join(ctx.attr.scalacopts) - - executable = declare_executable(ctx) - - wrapper = write_java_wrapper( - ctx, - args, - wrapper_preamble = """ -# save stty like in bin/scala -saved_stty=$(stty -g 2>/dev/null) -if [[ ! $? ]]; then - saved_stty="" -fi -function finish() { - if [[ "$saved_stty" != "" ]]; then - stty $saved_stty - saved_stty="" - fi -} -trap finish EXIT -""", - ) - - out = scala_binary_common( - ctx, - executable, - cjars, - transitive_rjars, - jars.transitive_compile_jars, - jars.jars2labels, - wrapper, - unused_dependency_checker_ignored_targets = [ - target.label - for target in scalac_provider.default_repl_classpath + - ctx.attr.unused_dependency_checker_ignored_targets - ], - unused_dependency_checker_mode = unused_dependency_checker_mode, - deps_providers = jars.deps_providers, - ) - write_executable( - ctx = ctx, - executable = executable, - jvm_flags = ["-Dscala.usejavacp=true"] + ctx.attr.jvm_flags, - main_class = "scala.tools.nsc.MainGenericRunner", - rjars = out.transitive_rjars, - use_jacoco = False, - wrapper = wrapper, - ) - - return out + return run_phases(ctx, [ + ("init", phase_common_init), + ("unused_deps_checker", phase_unused_deps_checker), + # need scala-compiler for MainGenericRunner below + ("collect_jars", phase_repl_collect_jars), + ("java_wrapper", phase_repl_java_wrapper), + ("declare_executable", phase_declare_executable), + # no need to build an ijar for an executable + ("compile", phase_repl_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_common_runfiles), + ("scala_provider", phase_common_scala_provider), + ("write_executable", phase_repl_write_executable), + ("final", phase_binary_final), + ]).final _scala_repl_attrs = { "jvm_flags": attr.string_list(), diff --git a/scala/private/rules/scala_test.bzl b/scala/private/rules/scala_test.bzl index 7d86b2e75..80b156c38 100644 --- a/scala/private/rules/scala_test.bzl +++ b/scala/private/rules/scala_test.bzl @@ -9,138 +9,38 @@ load( load("@io_bazel_rules_scala//scala/private:common.bzl", "sanitize_string_for_usage") load("@io_bazel_rules_scala//scala/private:common_outputs.bzl", "common_outputs") load( - "@io_bazel_rules_scala//scala/private:coverage_replacements_provider.bzl", - _coverage_replacements_provider = "coverage_replacements_provider", -) -load( - "@io_bazel_rules_scala//scala/private:rule_impls.bzl", - "collect_jars_from_common_ctx", - "declare_executable", - "expand_location", - "first_non_empty", - "get_scalac_provider", - "get_unused_dependency_checker_mode", - "scala_binary_common", - "write_executable", - "write_java_wrapper", + "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "phase_common_init", + "phase_common_java_wrapper", + "phase_common_scala_provider", + "phase_coverage_runfiles", + "phase_declare_executable", + "phase_merge_jars", + "phase_test_collect_jars", + "phase_test_compile", + "phase_test_final", + "phase_test_runfiles", + "phase_test_write_executable", + "phase_unused_deps_checker", + "run_phases", ) -def _scala_test_flags(ctx): - # output report test duration - flags = "-oD" - if ctx.attr.full_stacktraces: - flags += "F" - else: - flags += "S" - if not ctx.attr.colors: - flags += "W" - return flags - def _scala_test_impl(ctx): - scalac_provider = get_scalac_provider(ctx) - - unused_dependency_checker_mode = get_unused_dependency_checker_mode(ctx) - unused_dependency_checker_ignored_targets = [ - target.label - for target in scalac_provider.default_classpath + - ctx.attr.unused_dependency_checker_ignored_targets - ] - unused_dependency_checker_is_off = unused_dependency_checker_mode == "off" - - scalatest_base_classpath = scalac_provider.default_classpath + [ctx.attr._scalatest] - jars = collect_jars_from_common_ctx( - ctx, - scalatest_base_classpath, - extra_runtime_deps = [ - ctx.attr._scalatest_reporter, - ctx.attr._scalatest_runner, - ], - unused_dependency_checker_is_off = unused_dependency_checker_is_off, - ) - ( - cjars, - transitive_rjars, - transitive_compile_jars, - jars_to_labels, - ) = ( - jars.compile_jars, - jars.transitive_runtime_jars, - jars.transitive_compile_jars, - jars.jars2labels, - ) - - args = "\n".join([ - "-R", - ctx.outputs.jar.short_path, - _scala_test_flags(ctx), - "-C", - "io.bazel.rules.scala.JUnitXmlReporter", - ]) - - argsFile = ctx.actions.declare_file("%s.args" % ctx.label.name) - ctx.actions.write(argsFile, args) - - executable = declare_executable(ctx) - - wrapper = write_java_wrapper(ctx, "", "") - out = scala_binary_common( - ctx, - executable, - cjars, - transitive_rjars, - transitive_compile_jars, - jars_to_labels, - wrapper, - unused_dependency_checker_ignored_targets = - unused_dependency_checker_ignored_targets, - unused_dependency_checker_mode = unused_dependency_checker_mode, - runfiles_ext = [argsFile], - deps_providers = jars.deps_providers, - ) - - rjars = out.transitive_rjars - - coverage_runfiles = [] - if ctx.configuration.coverage_enabled and _coverage_replacements_provider.is_enabled(ctx): - coverage_replacements = _coverage_replacements_provider.from_ctx( - ctx, - base = out.coverage.replacements, - ).replacements - - rjars = depset([ - coverage_replacements[jar] if jar in coverage_replacements else jar - for jar in rjars.to_list() - ]) - coverage_runfiles = ctx.files._jacocorunner + ctx.files._lcov_merger + coverage_replacements.values() - - # jvm_flags passed in on the target override scala_test_jvm_flags passed in on the - # toolchain - final_jvm_flags = first_non_empty( - ctx.attr.jvm_flags, - ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].scala_test_jvm_flags, - ) - - coverage_runfiles.extend(write_executable( - ctx = ctx, - executable = executable, - jvm_flags = [ - "-DRULES_SCALA_MAIN_WS_NAME=%s" % ctx.workspace_name, - "-DRULES_SCALA_ARGS_FILE=%s" % argsFile.short_path, - ] + expand_location(ctx, final_jvm_flags), - main_class = ctx.attr.main_class, - rjars = rjars, - use_jacoco = ctx.configuration.coverage_enabled, - wrapper = wrapper, - )) - - return struct( - executable = executable, - files = out.files, - instrumented_files = out.instrumented_files, - providers = out.providers, - runfiles = ctx.runfiles(coverage_runfiles, transitive_files = out.runfiles.files), - scala = out.scala, - ) + return run_phases(ctx, [ + ("init", phase_common_init), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_test_collect_jars), + ("java_wrapper", phase_common_java_wrapper), + ("declare_executable", phase_declare_executable), + # no need to build an ijar for an executable + ("compile", phase_test_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_test_runfiles), + ("scala_provider", phase_common_scala_provider), + ("coverage_runfiles", phase_coverage_runfiles), + ("write_executable", phase_test_write_executable), + ("final", phase_test_final), + ]).final _scala_test_attrs = { "main_class": attr.string( From 5d052a37dc0873435946735ccca057e5710be97b Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Fri, 25 Oct 2019 15:21:45 -0600 Subject: [PATCH 03/32] Customizable phases --- WORKSPACE | 23 ++++---- scala/private/phases/api.bzl | 37 ++++++++++++ scala/private/phases/phases.bzl | 2 + scala/private/rules/scala_binary.bzl | 28 ++++++--- scala/private/rules/scala_junit_test.bzl | 28 ++++++--- scala/private/rules/scala_library.bzl | 74 +++++++++++++++++------- scala/private/rules/scala_repl.bzl | 28 ++++++--- scala/private/rules/scala_test.bzl | 30 +++++++--- scala/providers.bzl | 7 +++ scala/scala.bzl | 15 +++++ 10 files changed, 207 insertions(+), 65 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 692dcfd4e..44227568f 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -3,6 +3,18 @@ workspace(name = "io_bazel_rules_scala") load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") load("@bazel_tools//tools/build_defs/repo:jvm.bzl", "jvm_maven_import_external") + +http_archive( + name = "com_github_bazelbuild_buildtools", + sha256 = "cdaac537b56375f658179ee2f27813cac19542443f4722b6730d84e4125355e6", + strip_prefix = "buildtools-f27d1753c8b3210d9e87cdc9c45bc2739ae2c2db", + url = "https://github.com/bazelbuild/buildtools/archive/f27d1753c8b3210d9e87cdc9c45bc2739ae2c2db.zip", +) + +load("@com_github_bazelbuild_buildtools//buildifier:deps.bzl", "buildifier_dependencies") + +buildifier_dependencies() + load("//scala:scala.bzl", "scala_repositories") scala_repositories() @@ -162,13 +174,6 @@ http_archive( url = "https://github.com/bazelbuild/rules_go/releases/download/0.18.7/rules_go-0.18.7.tar.gz", ) -http_archive( - name = "com_github_bazelbuild_buildtools", - sha256 = "cdaac537b56375f658179ee2f27813cac19542443f4722b6730d84e4125355e6", - strip_prefix = "buildtools-f27d1753c8b3210d9e87cdc9c45bc2739ae2c2db", - url = "https://github.com/bazelbuild/buildtools/archive/f27d1753c8b3210d9e87cdc9c45bc2739ae2c2db.zip", -) - load( "@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", @@ -179,10 +184,6 @@ go_rules_dependencies() go_register_toolchains() -load("@com_github_bazelbuild_buildtools//buildifier:deps.bzl", "buildifier_dependencies") - -buildifier_dependencies() - http_archive( name = "bazel_toolchains", sha256 = "5962fe677a43226c409316fcb321d668fc4b7fa97cb1f9ef45e7dc2676097b26", diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index afbcee020..76e41c73a 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -1,4 +1,33 @@ +load( + "@io_bazel_rules_scala//scala:providers.bzl", + _ScalaRulePhase = "ScalaRulePhase", +) + +def _adjust_phases(phases, adjustments): + if len(adjustments) == 0: + return phases + phases = phases[:] + for (relation, peer_name, name, function) in adjustments: + for idx, (needle, _) in enumerate(phases): + if needle == peer_name: + if relation in ["-", "before"]: + phases.insert(idx, (name, function)) + elif relation in ["+", "after"]: + phases.insert(idx + 1, (name, function)) + elif relation in ["=", "replace"]: + phases[idx] = (name, function) + return phases + def run_phases(ctx, phases): + phase_providers = [ + p[_ScalaRulePhase] + for p in ctx.attr._phase_providers + if _ScalaRulePhase in p + ] + + if phase_providers != []: + phases = _adjust_phases(phases, [p for pp in phase_providers for p in pp.phases]) + global_provider = {} current_provider = struct(**global_provider) for (name, function) in phases: @@ -8,3 +37,11 @@ def run_phases(ctx, phases): current_provider = struct(**global_provider) return current_provider + +def extras_phases(extras): + return { + "_phase_providers": attr.label_list( + default = [pp for extra in extras for pp in extra["phase_providers"]], + providers = [_ScalaRulePhase], + ), + } diff --git a/scala/private/phases/phases.bzl b/scala/private/phases/phases.bzl index 12d03ab3c..b41ca1825 100644 --- a/scala/private/phases/phases.bzl +++ b/scala/private/phases/phases.bzl @@ -1,5 +1,6 @@ load( "@io_bazel_rules_scala//scala/private:phases/api.bzl", + _extras_phases = "extras_phases", _run_phases = "run_phases", ) load( @@ -64,6 +65,7 @@ load("@io_bazel_rules_scala//scala/private:phases/phase_coverage_runfiles.bzl", # API run_phases = _run_phases +extras_phases = _extras_phases # init phase_common_init = _phase_common_init diff --git a/scala/private/rules/scala_binary.bzl b/scala/private/rules/scala_binary.bzl index cbcadc907..177fc9c27 100644 --- a/scala/private/rules/scala_binary.bzl +++ b/scala/private/rules/scala_binary.bzl @@ -1,5 +1,6 @@ """Builds Scala binaries""" +load("@bazel_skylib//lib:dicts.bzl", _dicts = "dicts") load( "@io_bazel_rules_scala//scala/private:common_attributes.bzl", "common_attrs", @@ -10,6 +11,7 @@ load( load("@io_bazel_rules_scala//scala/private:common_outputs.bzl", "common_outputs") load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "extras_phases", "phase_binary_compile", "phase_binary_final", "phase_common_collect_jars", @@ -54,11 +56,21 @@ _scala_binary_attrs.update(common_attrs) _scala_binary_attrs.update(resolve_deps) -scala_binary = rule( - attrs = _scala_binary_attrs, - executable = True, - fragments = ["java"], - outputs = common_outputs, - toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], - implementation = _scala_binary_impl, -) +def make_scala_binary(*extras): + return rule( + attrs = _dicts.add( + _scala_binary_attrs, + extras_phases(extras), + *[extra["attrs"] for extra in extras] + ), + executable = True, + fragments = ["java"], + outputs = _dicts.add( + common_outputs, + *[extra["outputs"] for extra in extras] + ), + toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], + implementation = _scala_binary_impl, + ) + +scala_binary = make_scala_binary() diff --git a/scala/private/rules/scala_junit_test.bzl b/scala/private/rules/scala_junit_test.bzl index d43938be9..2cab5e168 100644 --- a/scala/private/rules/scala_junit_test.bzl +++ b/scala/private/rules/scala_junit_test.bzl @@ -1,5 +1,6 @@ """Rules for writing tests with JUnit""" +load("@bazel_skylib//lib:dicts.bzl", _dicts = "dicts") load( "@io_bazel_rules_scala//scala/private:common_attributes.bzl", "common_attrs", @@ -9,6 +10,7 @@ load( load("@io_bazel_rules_scala//scala/private:common_outputs.bzl", "common_outputs") load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "extras_phases", "phase_binary_final", "phase_common_init", "phase_common_java_wrapper", @@ -106,11 +108,21 @@ _scala_junit_test_attrs.update({ "tests_from": attr.label_list(providers = [[JavaInfo]]), }) -scala_junit_test = rule( - attrs = _scala_junit_test_attrs, - fragments = ["java"], - outputs = common_outputs, - test = True, - toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], - implementation = _scala_junit_test_impl, -) +def make_scala_junit_test(*extras): + return rule( + attrs = _dicts.add( + _scala_junit_test_attrs, + extras_phases(extras), + *[extra["attrs"] for extra in extras] + ), + fragments = ["java"], + outputs = _dicts.add( + common_outputs, + *[extra["outputs"] for extra in extras] + ), + test = True, + toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], + implementation = _scala_junit_test_impl, + ) + +scala_junit_test = make_scala_junit_test() diff --git a/scala/private/rules/scala_library.bzl b/scala/private/rules/scala_library.bzl index d49a6abf0..81adfcd5f 100644 --- a/scala/private/rules/scala_library.bzl +++ b/scala/private/rules/scala_library.bzl @@ -1,3 +1,4 @@ +load("@bazel_skylib//lib:dicts.bzl", _dicts = "dicts") load( "@io_bazel_rules_scala//scala/private:common.bzl", "sanitize_string_for_usage", @@ -16,6 +17,7 @@ load( ) load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "extras_phases", "phase_common_collect_jars", "phase_library_compile", "phase_library_final", @@ -70,13 +72,23 @@ _scala_library_attrs.update(_library_attrs) _scala_library_attrs.update(resolve_deps) -scala_library = rule( - attrs = _scala_library_attrs, - fragments = ["java"], - outputs = common_outputs, - toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], - implementation = _scala_library_impl, -) +def make_scala_library(*extras): + return rule( + attrs = _dicts.add( + _scala_library_attrs, + extras_phases(extras), + *[extra["attrs"] for extra in extras] + ), + fragments = ["java"], + outputs = _dicts.add( + common_outputs, + *[extra["outputs"] for extra in extras] + ), + toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], + implementation = _scala_library_impl, + ) + +scala_library = make_scala_library() # Scala library suite generates a series of scala libraries # then it depends on them with a meta one which exports all the sub targets @@ -135,13 +147,23 @@ _scala_library_for_plugin_bootstrapping_attrs.update( common_attrs_for_plugin_bootstrapping, ) -scala_library_for_plugin_bootstrapping = rule( - attrs = _scala_library_for_plugin_bootstrapping_attrs, - fragments = ["java"], - outputs = common_outputs, - toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], - implementation = _scala_library_for_plugin_bootstrapping_impl, -) +def make_scala_library_for_plugin_bootstrapping(*extras): + return rule( + attrs = _dicts.add( + _scala_library_for_plugin_bootstrapping_attrs, + extras_phases(extras), + *[extra["attrs"] for extra in extras] + ), + fragments = ["java"], + outputs = _dicts.add( + common_outputs, + *[extra["outputs"] for extra in extras] + ), + toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], + implementation = _scala_library_for_plugin_bootstrapping_impl, + ) + +scala_library_for_plugin_bootstrapping = make_scala_library_for_plugin_bootstrapping() ## # scala_macro_library @@ -184,10 +206,20 @@ _scala_macro_library_attrs["unused_dependency_checker_mode"] = attr.string( mandatory = False, ) -scala_macro_library = rule( - attrs = _scala_macro_library_attrs, - fragments = ["java"], - outputs = common_outputs, - toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], - implementation = _scala_macro_library_impl, -) +def make_scala_macro_library(*extras): + return rule( + attrs = _dicts.add( + _scala_macro_library_attrs, + extras_phases(extras), + *[extra["attrs"] for extra in extras] + ), + fragments = ["java"], + outputs = _dicts.add( + common_outputs, + *[extra["outputs"] for extra in extras] + ), + toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], + implementation = _scala_macro_library_impl, + ) + +scala_macro_library = make_scala_macro_library() diff --git a/scala/private/rules/scala_repl.bzl b/scala/private/rules/scala_repl.bzl index b4e0d1155..6ee79c1b4 100644 --- a/scala/private/rules/scala_repl.bzl +++ b/scala/private/rules/scala_repl.bzl @@ -1,5 +1,6 @@ """Rule for launching a Scala REPL with dependencies""" +load("@bazel_skylib//lib:dicts.bzl", _dicts = "dicts") load( "@io_bazel_rules_scala//scala/private:common_attributes.bzl", "common_attrs", @@ -10,6 +11,7 @@ load( load("@io_bazel_rules_scala//scala/private:common_outputs.bzl", "common_outputs") load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "extras_phases", "phase_binary_final", "phase_common_init", "phase_common_runfiles", @@ -53,11 +55,21 @@ _scala_repl_attrs.update(common_attrs) _scala_repl_attrs.update(resolve_deps) -scala_repl = rule( - attrs = _scala_repl_attrs, - executable = True, - fragments = ["java"], - outputs = common_outputs, - toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], - implementation = _scala_repl_impl, -) +def make_scala_repl(*extras): + return rule( + attrs = _dicts.add( + _scala_repl_attrs, + extras_phases(extras), + *[extra["attrs"] for extra in extras] + ), + executable = True, + fragments = ["java"], + outputs = _dicts.add( + common_outputs, + *[extra["outputs"] for extra in extras] + ), + toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], + implementation = _scala_repl_impl, + ) + +scala_repl = make_scala_repl() diff --git a/scala/private/rules/scala_test.bzl b/scala/private/rules/scala_test.bzl index 80b156c38..79a86b658 100644 --- a/scala/private/rules/scala_test.bzl +++ b/scala/private/rules/scala_test.bzl @@ -1,5 +1,6 @@ """Rules for writing tests with ScalaTest""" +load("@bazel_skylib//lib:dicts.bzl", _dicts = "dicts") load( "@io_bazel_rules_scala//scala/private:common_attributes.bzl", "common_attrs", @@ -10,6 +11,7 @@ load("@io_bazel_rules_scala//scala/private:common.bzl", "sanitize_string_for_usa load("@io_bazel_rules_scala//scala/private:common_outputs.bzl", "common_outputs") load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", + "extras_phases", "phase_common_init", "phase_common_java_wrapper", "phase_common_scala_provider", @@ -91,15 +93,25 @@ _scala_test_attrs.update(common_attrs) _scala_test_attrs.update(_test_resolve_deps) -scala_test = rule( - attrs = _scala_test_attrs, - executable = True, - fragments = ["java"], - outputs = common_outputs, - test = True, - toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], - implementation = _scala_test_impl, -) +def make_scala_test(*extras): + return rule( + attrs = _dicts.add( + _scala_test_attrs, + extras_phases(extras), + *[extra["attrs"] for extra in extras] + ), + executable = True, + fragments = ["java"], + outputs = _dicts.add( + common_outputs, + *[extra["outputs"] for extra in extras] + ), + test = True, + toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], + implementation = _scala_test_impl, + ) + +scala_test = make_scala_test() # This auto-generates a test suite based on the passed set of targets # we will add a root test_suite with the name of the passed name diff --git a/scala/providers.bzl b/scala/providers.bzl index 5a92d4525..5a23ebfe3 100644 --- a/scala/providers.bzl +++ b/scala/providers.bzl @@ -57,3 +57,10 @@ declare_scalac_provider = rule( "default_macro_classpath": attr.label_list(allow_files = True), }, ) + +ScalaRulePhase = provider( + doc = "A Scala compiler plugin", + fields = { + "phases": "the phases to add", + }, +) diff --git a/scala/scala.bzl b/scala/scala.bzl index fb4a9eeaf..f63e1521b 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -8,6 +8,7 @@ load( ) load( "@io_bazel_rules_scala//scala/private:rules/scala_binary.bzl", + _make_scala_binary = "make_scala_binary", _scala_binary = "scala_binary", ) load( @@ -16,10 +17,14 @@ load( ) load( "@io_bazel_rules_scala//scala/private:rules/scala_junit_test.bzl", + _make_scala_junit_test = "make_scala_junit_test", _scala_junit_test = "scala_junit_test", ) load( "@io_bazel_rules_scala//scala/private:rules/scala_library.bzl", + _make_scala_library = "make_scala_library", + _make_scala_library_for_plugin_bootstrapping = "make_scala_library_for_plugin_bootstrapping", + _make_scala_macro_library = "make_scala_macro_library", _scala_library = "scala_library", _scala_library_for_plugin_bootstrapping = "scala_library_for_plugin_bootstrapping", _scala_library_suite = "scala_library_suite", @@ -27,10 +32,12 @@ load( ) load( "@io_bazel_rules_scala//scala/private:rules/scala_repl.bzl", + _make_scala_repl = "make_scala_repl", _scala_repl = "scala_repl", ) load( "@io_bazel_rules_scala//scala/private:rules/scala_test.bzl", + _make_scala_test = "make_scala_test", _scala_test = "scala_test", _scala_test_suite = "scala_test_suite", ) @@ -60,3 +67,11 @@ scala_repl = _scala_repl scala_repositories = _scala_repositories scala_test = _scala_test scala_test_suite = _scala_test_suite + +make_scala_binary = _make_scala_binary +make_scala_library = _make_scala_library +make_scala_library_for_plugin_bootstrapping = _make_scala_library_for_plugin_bootstrapping +make_scala_macro_library = _make_scala_macro_library +make_scala_repl = _make_scala_repl +make_scala_junit_test = _make_scala_junit_test +make_scala_test = _make_scala_test From 584f8458a75ba957ba4286e60da604415c116605 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Fri, 25 Oct 2019 15:21:58 -0600 Subject: [PATCH 04/32] Customizable phases tests --- test/phase/BUILD | 30 +++++++++++++++ test/phase/HelloBinary.scala | 7 ++++ test/phase/HelloLibrary.scala | 5 +++ test/phase/HelloTest.scala | 10 +++++ test/phase/customizability_test.bzl | 45 ++++++++++++++++++++++ test/phase/phase_customizability_test.bzl | 10 +++++ test/shell/test_phase.sh | 47 +++++++++++++++++++++++ test_rules_scala.sh | 1 + 8 files changed, 155 insertions(+) create mode 100644 test/phase/BUILD create mode 100644 test/phase/HelloBinary.scala create mode 100644 test/phase/HelloLibrary.scala create mode 100644 test/phase/HelloTest.scala create mode 100644 test/phase/customizability_test.bzl create mode 100644 test/phase/phase_customizability_test.bzl create mode 100755 test/shell/test_phase.sh diff --git a/test/phase/BUILD b/test/phase/BUILD new file mode 100644 index 000000000..7c47a162b --- /dev/null +++ b/test/phase/BUILD @@ -0,0 +1,30 @@ +load( + "//test/phase:customizability_test.bzl", + "add_phase_customizability_test_singleton", + "customizability_test_scala_binary", + "customizability_test_scala_library", + "customizability_test_scala_test", +) + +add_phase_customizability_test_singleton( + name = "phase_customizability_test", + visibility = ["//visibility:public"], +) + +customizability_test_scala_binary( + name = "HelloBinary", + srcs = ["HelloBinary.scala"], + main_class = "scalarules.test.phase.HelloBinary", +) + +customizability_test_scala_library( + name = "HelloLibrary", + srcs = ["HelloLibrary.scala"], + custom_content = "This is custom content in library", +) + +customizability_test_scala_test( + name = "HelloTest", + srcs = ["HelloTest.scala"], + custom_content = "This is custom content in test", +) diff --git a/test/phase/HelloBinary.scala b/test/phase/HelloBinary.scala new file mode 100644 index 000000000..4ca6fa9a5 --- /dev/null +++ b/test/phase/HelloBinary.scala @@ -0,0 +1,7 @@ +package scalarules.test.phase + +object HelloBinary { + def main(args: Array[String]) { + val message = "You can customize binary phases!" + } +} diff --git a/test/phase/HelloLibrary.scala b/test/phase/HelloLibrary.scala new file mode 100644 index 000000000..58737692a --- /dev/null +++ b/test/phase/HelloLibrary.scala @@ -0,0 +1,5 @@ +package scalarules.test.phase + +object HelloLibrary { + val message = "You can customize library phases!" +} diff --git a/test/phase/HelloTest.scala b/test/phase/HelloTest.scala new file mode 100644 index 000000000..2df1d5879 --- /dev/null +++ b/test/phase/HelloTest.scala @@ -0,0 +1,10 @@ +package scalarules.test.phase + +import org.scalatest._ + +class HelloTest extends FlatSpec { + val message = "You can customize test phases!" + "HelloTest" should "be able to customize test phases!" in { + assert(message.equals("You can customize test phases!")) + } +} diff --git a/test/phase/customizability_test.bzl b/test/phase/customizability_test.bzl new file mode 100644 index 000000000..05ace37a3 --- /dev/null +++ b/test/phase/customizability_test.bzl @@ -0,0 +1,45 @@ +load( + "//scala:providers.bzl", + _ScalaRulePhase = "ScalaRulePhase", +) +load( + "//test/phase:phase_customizability_test.bzl", + _phase_customizability_test = "phase_customizability_test", +) +load( + "//scala:scala.bzl", + _make_scala_binary = "make_scala_binary", + _make_scala_library = "make_scala_library", + _make_scala_test = "make_scala_test", +) + +ext_add_phase_customizability_test = { + "attrs": { + "custom_content": attr.string( + default = "This is custom content", + ), + }, + "outputs": { + "custom_output": "%{name}.custom-output", + }, + "phase_providers": [ + "//test/phase:phase_customizability_test", + ], +} + +def _add_phase_customizability_test_singleton_implementation(ctx): + return [ + _ScalaRulePhase( + phases = [ + ("-", "final", "customizability_test", _phase_customizability_test), + ], + ), + ] + +add_phase_customizability_test_singleton = rule( + implementation = _add_phase_customizability_test_singleton_implementation, +) + +customizability_test_scala_binary = _make_scala_binary(ext_add_phase_customizability_test) +customizability_test_scala_library = _make_scala_library(ext_add_phase_customizability_test) +customizability_test_scala_test = _make_scala_test(ext_add_phase_customizability_test) diff --git a/test/phase/phase_customizability_test.bzl b/test/phase/phase_customizability_test.bzl new file mode 100644 index 000000000..15f01e09e --- /dev/null +++ b/test/phase/phase_customizability_test.bzl @@ -0,0 +1,10 @@ +# +# PHASE: customizability test +# +# A dummy test phase to make sure rules are customizable +# +def phase_customizability_test(ctx, p): + ctx.actions.write( + output = ctx.outputs.custom_output, + content = ctx.attr.custom_content, + ) diff --git a/test/shell/test_phase.sh b/test/shell/test_phase.sh new file mode 100755 index 000000000..ac5844a1e --- /dev/null +++ b/test/shell/test_phase.sh @@ -0,0 +1,47 @@ +# shellcheck source=./test_runner.sh +dir=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd ) +. "${dir}"/test_runner.sh +. "${dir}"/test_helper.sh +runner=$(get_test_runner "${1:-local}") + +output_file_should_contain_message() { + set +e + MSG=$1 + TEST_ARG=${@:2} + OUTPUT_FILE=$(echo ${@:3} | sed 's#//#/#g;s#:#/#g') + OUTPUT_PATH=$(bazel info bazel-bin)/$OUTPUT_FILE + bazel $TEST_ARG + RESPONSE_CODE=$? + cat $OUTPUT_PATH | grep -- "$MSG" + GREP_RES=$? + if [ $RESPONSE_CODE -ne 0 ]; then + echo -e "${RED} \"bazel $TEST_ARG\" should pass but failed. $NC" + exit 1 + elif [ $GREP_RES -ne 0 ]; then + echo -e "${RED} \"bazel $TEST_ARG\" should pass with \"$MSG\" in file \"$OUTPUT_FILE\" but did not. $NC" + exit 1 + else + exit 0 + fi +} +test_scala_binary_with_extra_phase() { + output_file_should_contain_message \ + "This is custom content" \ + build //test/phase:HelloBinary.custom-output +} + +test_scala_library_with_extra_phase_and_custom_content() { + output_file_should_contain_message \ + "This is custom content in library" \ + build //test/phase:HelloLibrary.custom-output +} + +test_scala_test_with_extra_phase_and_custom_content() { + output_file_should_contain_message \ + "This is custom content in test" \ + build //test/phase:HelloTest.custom-output +} + +$runner test_scala_binary_with_extra_phase +$runner test_scala_library_with_extra_phase_and_custom_content +$runner test_scala_test_with_extra_phase_and_custom_content diff --git a/test_rules_scala.sh b/test_rules_scala.sh index 04b1ced74..e8456fabf 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -29,6 +29,7 @@ $runner bazel test //test/... --extra_toolchains="//test_expect_failure/plus_one . "${test_dir}"/test_javac_jvm_flags.sh . "${test_dir}"/test_junit.sh . "${test_dir}"/test_misc.sh +. "${test_dir}"/test_phase.sh . "${test_dir}"/test_scala_binary.sh . "${test_dir}"/test_scalac_jvm_flags.sh . "${test_dir}"/test_scala_classpath.sh From 7d26cab898b3329dabf8f695d8bf7a4e4dffa285 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Thu, 31 Oct 2019 10:47:28 -0600 Subject: [PATCH 05/32] Break up init to more reasonable phases --- .../phases/phase_collect_exports_jars.bzl | 15 +++++++ scala/private/phases/phase_collect_jars.bzl | 8 ++-- .../private/phases/phase_collect_srcjars.bzl | 14 +++++++ scala/private/phases/phase_compile.bzl | 16 ++++---- scala/private/phases/phase_init.bzl | 39 ------------------- scala/private/phases/phase_scala_provider.bzl | 4 +- .../private/phases/phase_scalac_provider.bzl | 12 ++++++ scala/private/phases/phase_write_manifest.bzl | 12 ++++++ scala/private/phases/phases.bzl | 23 +++++++---- scala/private/rules/scala_binary.bzl | 6 ++- scala/private/rules/scala_junit_test.bzl | 6 ++- scala/private/rules/scala_library.bzl | 20 ++++++++-- scala/private/rules/scala_repl.bzl | 6 ++- scala/private/rules/scala_test.bzl | 6 ++- 14 files changed, 114 insertions(+), 73 deletions(-) create mode 100644 scala/private/phases/phase_collect_exports_jars.bzl create mode 100644 scala/private/phases/phase_collect_srcjars.bzl delete mode 100644 scala/private/phases/phase_init.bzl create mode 100644 scala/private/phases/phase_scalac_provider.bzl create mode 100644 scala/private/phases/phase_write_manifest.bzl diff --git a/scala/private/phases/phase_collect_exports_jars.bzl b/scala/private/phases/phase_collect_exports_jars.bzl new file mode 100644 index 000000000..35a83dbe3 --- /dev/null +++ b/scala/private/phases/phase_collect_exports_jars.bzl @@ -0,0 +1,15 @@ +# +# PHASE: collect exports jars +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:common.bzl", + "collect_jars", +) + +def phase_collect_exports_jars(ctx, p): + # 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 + return collect_jars(ctx.attr.exports) diff --git a/scala/private/phases/phase_collect_jars.bzl b/scala/private/phases/phase_collect_jars.bzl index 6eac37475..b667158f2 100644 --- a/scala/private/phases/phase_collect_jars.bzl +++ b/scala/private/phases/phase_collect_jars.bzl @@ -10,7 +10,7 @@ load( def phase_test_collect_jars(ctx, p): args = struct( - base_classpath = p.init.scalac_provider.default_classpath + [ctx.attr._scalatest], + base_classpath = p.scalac_provider.default_classpath + [ctx.attr._scalatest], extra_runtime_deps = [ ctx.attr._scalatest_reporter, ctx.attr._scalatest_runner, @@ -20,13 +20,13 @@ def phase_test_collect_jars(ctx, p): def phase_repl_collect_jars(ctx, p): args = struct( - base_classpath = p.init.scalac_provider.default_repl_classpath, + base_classpath = p.scalac_provider.default_repl_classpath, ) return phase_common_collect_jars(ctx, p, args) def phase_macro_library_collect_jars(ctx, p): args = struct( - base_classpath = p.init.scalac_provider.default_macro_classpath, + base_classpath = p.scalac_provider.default_macro_classpath, ) return phase_common_collect_jars(ctx, p, args) @@ -50,7 +50,7 @@ def phase_library_for_plugin_bootstrapping_collect_jars(ctx, p): def phase_common_collect_jars(ctx, p, _args = struct()): return _phase_collect_jars( ctx, - _args.base_classpath if hasattr(_args, "base_classpath") else p.init.scalac_provider.default_classpath, + _args.base_classpath if hasattr(_args, "base_classpath") else p.scalac_provider.default_classpath, _args.extra_deps if hasattr(_args, "extra_deps") else [], _args.extra_runtime_deps if hasattr(_args, "extra_runtime_deps") else [], _args.unused_dependency_checker_mode if hasattr(_args, "unused_dependency_checker_mode") else p.unused_deps_checker, diff --git a/scala/private/phases/phase_collect_srcjars.bzl b/scala/private/phases/phase_collect_srcjars.bzl new file mode 100644 index 000000000..020ce99cb --- /dev/null +++ b/scala/private/phases/phase_collect_srcjars.bzl @@ -0,0 +1,14 @@ +# +# PHASE: collect srcjars +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:common.bzl", + "collect_srcjars", +) + +def phase_collect_srcjars(ctx, p): + # This will be used to pick up srcjars from non-scala library + # targets (like thrift code generation) + return collect_srcjars(ctx.attr.deps) diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index 9027c67f4..699b11401 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -13,7 +13,7 @@ def phase_binary_compile(ctx, p): args = struct( unused_dependency_checker_ignored_targets = [ target.label - for target in p.init.scalac_provider.default_classpath + + for target in p.scalac_provider.default_classpath + ctx.attr.unused_dependency_checker_ignored_targets ], ) @@ -21,11 +21,11 @@ def phase_binary_compile(ctx, p): def phase_library_compile(ctx, p): args = struct( - srcjars = p.init.srcjars, + srcjars = p.collect_srcjars, buildijar = True, unused_dependency_checker_ignored_targets = [ target.label - for target in p.init.scalac_provider.default_classpath + ctx.attr.exports + + for target in p.scalac_provider.default_classpath + ctx.attr.exports + ctx.attr.unused_dependency_checker_ignored_targets ], ) @@ -36,7 +36,7 @@ def phase_library_for_plugin_bootstrapping_compile(ctx, p): buildijar = True, unused_dependency_checker_ignored_targets = [ target.label - for target in p.init.scalac_provider.default_classpath + ctx.attr.exports + for target in p.scalac_provider.default_classpath + ctx.attr.exports ], unused_dependency_checker_mode = "off", ) @@ -46,7 +46,7 @@ def phase_macro_library_compile(ctx, p): args = struct( unused_dependency_checker_ignored_targets = [ target.label - for target in p.init.scalac_provider.default_macro_classpath + ctx.attr.exports + + for target in p.scalac_provider.default_macro_classpath + ctx.attr.exports + ctx.attr.unused_dependency_checker_ignored_targets ], ) @@ -60,7 +60,7 @@ def phase_junit_test_compile(ctx, p): ], unused_dependency_checker_ignored_targets = [ target.label - for target in p.init.scalac_provider.default_classpath + + for target in p.scalac_provider.default_classpath + ctx.attr.unused_dependency_checker_ignored_targets ] + [ ctx.attr._junit.label, @@ -75,7 +75,7 @@ def phase_repl_compile(ctx, p): args = struct( unused_dependency_checker_ignored_targets = [ target.label - for target in p.init.scalac_provider.default_repl_classpath + + for target in p.scalac_provider.default_repl_classpath + ctx.attr.unused_dependency_checker_ignored_targets ], ) @@ -85,7 +85,7 @@ def phase_test_compile(ctx, p): args = struct( unused_dependency_checker_ignored_targets = [ target.label - for target in p.init.scalac_provider.default_classpath + + for target in p.scalac_provider.default_classpath + ctx.attr.unused_dependency_checker_ignored_targets ], ) diff --git a/scala/private/phases/phase_init.bzl b/scala/private/phases/phase_init.bzl deleted file mode 100644 index f80494bef..000000000 --- a/scala/private/phases/phase_init.bzl +++ /dev/null @@ -1,39 +0,0 @@ -# -# PHASE: init -# -# DOCUMENT THIS -# -load( - "@io_bazel_rules_scala//scala/private:rule_impls.bzl", - "get_scalac_provider", -) -load( - "@io_bazel_rules_scala//scala/private:common.bzl", - "collect_jars", - "collect_srcjars", - "write_manifest", -) - -def phase_library_init(ctx, p): - # This will be used to pick up srcjars from non-scala library - # targets (like thrift code generation) - srcjars = collect_srcjars(ctx.attr.deps) - - # 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) - - args = phase_common_init(ctx, p) - - return struct( - srcjars = srcjars, - exports_jars = exports_jars, - scalac_provider = args.scalac_provider, - ) - -def phase_common_init(ctx, p): - write_manifest(ctx) - return struct( - scalac_provider = get_scalac_provider(ctx), - ) diff --git a/scala/private/phases/phase_scala_provider.bzl b/scala/private/phases/phase_scala_provider.bzl index 3a4f821f6..636a9db65 100644 --- a/scala/private/phases/phase_scala_provider.bzl +++ b/scala/private/phases/phase_scala_provider.bzl @@ -11,11 +11,11 @@ load( def phase_library_scala_provider(ctx, p): args = struct( rjars = depset( - transitive = [p.compile.rjars, p.init.exports_jars.transitive_runtime_jars], + transitive = [p.compile.rjars, p.collect_exports_jars.transitive_runtime_jars], ), compile_jars = depset( p.compile.ijars, - transitive = [p.init.exports_jars.compile_jars], + transitive = [p.collect_exports_jars.compile_jars], ), ) return phase_common_scala_provider(ctx, p, args) diff --git a/scala/private/phases/phase_scalac_provider.bzl b/scala/private/phases/phase_scalac_provider.bzl new file mode 100644 index 000000000..86e453618 --- /dev/null +++ b/scala/private/phases/phase_scalac_provider.bzl @@ -0,0 +1,12 @@ +# +# PHASE: scalac provider +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "get_scalac_provider", +) + +def phase_scalac_provider(ctx, p): + return get_scalac_provider(ctx) diff --git a/scala/private/phases/phase_write_manifest.bzl b/scala/private/phases/phase_write_manifest.bzl new file mode 100644 index 000000000..13a9df179 --- /dev/null +++ b/scala/private/phases/phase_write_manifest.bzl @@ -0,0 +1,12 @@ +# +# PHASE: write manifest +# +# DOCUMENT THIS +# +load( + "@io_bazel_rules_scala//scala/private:common.bzl", + "write_manifest", +) + +def phase_write_manifest(ctx, p): + write_manifest(ctx) diff --git a/scala/private/phases/phases.bzl b/scala/private/phases/phases.bzl index b41ca1825..d4ccffbaa 100644 --- a/scala/private/phases/phases.bzl +++ b/scala/private/phases/phases.bzl @@ -52,11 +52,10 @@ load( _phase_library_final = "phase_library_final", _phase_test_final = "phase_test_final", ) -load( - "@io_bazel_rules_scala//scala/private:phases/phase_init.bzl", - _phase_common_init = "phase_common_init", - _phase_library_init = "phase_library_init", -) +load("@io_bazel_rules_scala//scala/private:phases/phase_scalac_provider.bzl", _phase_scalac_provider = "phase_scalac_provider") +load("@io_bazel_rules_scala//scala/private:phases/phase_write_manifest.bzl", _phase_write_manifest = "phase_write_manifest") +load("@io_bazel_rules_scala//scala/private:phases/phase_collect_srcjars.bzl", _phase_collect_srcjars = "phase_collect_srcjars") +load("@io_bazel_rules_scala//scala/private:phases/phase_collect_exports_jars.bzl", _phase_collect_exports_jars = "phase_collect_exports_jars") load("@io_bazel_rules_scala//scala/private:phases/phase_unused_deps_checker.bzl", _phase_unused_deps_checker = "phase_unused_deps_checker") load("@io_bazel_rules_scala//scala/private:phases/phase_declare_executable.bzl", _phase_declare_executable = "phase_declare_executable") load("@io_bazel_rules_scala//scala/private:phases/phase_merge_jars.bzl", _phase_merge_jars = "phase_merge_jars") @@ -67,9 +66,17 @@ load("@io_bazel_rules_scala//scala/private:phases/phase_coverage_runfiles.bzl", run_phases = _run_phases extras_phases = _extras_phases -# init -phase_common_init = _phase_common_init -phase_library_init = _phase_library_init +# scalac_provider +phase_scalac_provider = _phase_scalac_provider + +# collect_srcjars +phase_collect_srcjars = _phase_collect_srcjars + +# collect_exports_jars +phase_collect_exports_jars = _phase_collect_exports_jars + +# write_manifest +phase_write_manifest = _phase_write_manifest # unused_deps_checker phase_unused_deps_checker = _phase_unused_deps_checker diff --git a/scala/private/rules/scala_binary.bzl b/scala/private/rules/scala_binary.bzl index 177fc9c27..777f558b8 100644 --- a/scala/private/rules/scala_binary.bzl +++ b/scala/private/rules/scala_binary.bzl @@ -15,20 +15,22 @@ load( "phase_binary_compile", "phase_binary_final", "phase_common_collect_jars", - "phase_common_init", "phase_common_java_wrapper", "phase_common_runfiles", "phase_common_scala_provider", "phase_common_write_executable", "phase_declare_executable", "phase_merge_jars", + "phase_scalac_provider", "phase_unused_deps_checker", + "phase_write_manifest", "run_phases", ) def _scala_binary_impl(ctx): return run_phases(ctx, [ - ("init", phase_common_init), + ("scalac_provider", phase_scalac_provider), + ("write_manifest", phase_write_manifest), ("unused_deps_checker", phase_unused_deps_checker), ("collect_jars", phase_common_collect_jars), ("java_wrapper", phase_common_java_wrapper), diff --git a/scala/private/rules/scala_junit_test.bzl b/scala/private/rules/scala_junit_test.bzl index 2cab5e168..f2e02ed7b 100644 --- a/scala/private/rules/scala_junit_test.bzl +++ b/scala/private/rules/scala_junit_test.bzl @@ -12,7 +12,6 @@ load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", "extras_phases", "phase_binary_final", - "phase_common_init", "phase_common_java_wrapper", "phase_common_runfiles", "phase_common_scala_provider", @@ -22,7 +21,9 @@ load( "phase_junit_test_write_executable", "phase_jvm_flags", "phase_merge_jars", + "phase_scalac_provider", "phase_unused_deps_checker", + "phase_write_manifest", "run_phases", ) @@ -32,7 +33,8 @@ def _scala_junit_test_impl(ctx): "Setting at least one of the attributes ('prefixes','suffixes') is required", ) return run_phases(ctx, [ - ("init", phase_common_init), + ("scalac_provider", phase_scalac_provider), + ("write_manifest", phase_write_manifest), ("unused_deps_checker", phase_unused_deps_checker), ("collect_jars", phase_junit_test_collect_jars), ("java_wrapper", phase_common_java_wrapper), diff --git a/scala/private/rules/scala_library.bzl b/scala/private/rules/scala_library.bzl index 81adfcd5f..b547d72f4 100644 --- a/scala/private/rules/scala_library.bzl +++ b/scala/private/rules/scala_library.bzl @@ -18,18 +18,21 @@ load( load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", "extras_phases", + "phase_collect_exports_jars", + "phase_collect_srcjars", "phase_common_collect_jars", "phase_library_compile", "phase_library_final", "phase_library_for_plugin_bootstrapping_collect_jars", "phase_library_for_plugin_bootstrapping_compile", - "phase_library_init", "phase_library_runfiles", "phase_library_scala_provider", "phase_macro_library_collect_jars", "phase_macro_library_compile", "phase_merge_jars", + "phase_scalac_provider", "phase_unused_deps_checker", + "phase_write_manifest", "run_phases", ) @@ -52,7 +55,10 @@ _library_attrs = { def _scala_library_impl(ctx): # Build up information from dependency-like attributes return run_phases(ctx, [ - ("init", phase_library_init), + ("scalac_provider", phase_scalac_provider), + ("collect_srcjars", phase_collect_srcjars), + ("collect_exports_jars", phase_collect_exports_jars), + ("write_manifest", phase_write_manifest), ("unused_deps_checker", phase_unused_deps_checker), ("collect_jars", phase_common_collect_jars), ("compile", phase_library_compile), @@ -123,7 +129,10 @@ def scala_library_suite( def _scala_library_for_plugin_bootstrapping_impl(ctx): return run_phases(ctx, [ - ("init", phase_library_init), + ("scalac_provider", phase_scalac_provider), + ("collect_srcjars", phase_collect_srcjars), + ("collect_exports_jars", phase_collect_exports_jars), + ("write_manifest", phase_write_manifest), ("collect_jars", phase_library_for_plugin_bootstrapping_collect_jars), ("compile", phase_library_for_plugin_bootstrapping_compile), ("merge_jars", phase_merge_jars), @@ -171,7 +180,10 @@ scala_library_for_plugin_bootstrapping = make_scala_library_for_plugin_bootstrap def _scala_macro_library_impl(ctx): return run_phases(ctx, [ - ("init", phase_library_init), + ("scalac_provider", phase_scalac_provider), + ("collect_srcjars", phase_collect_srcjars), + ("collect_exports_jars", phase_collect_exports_jars), + ("write_manifest", phase_write_manifest), ("unused_deps_checker", phase_unused_deps_checker), ("collect_jars", phase_macro_library_collect_jars), ("compile", phase_macro_library_compile), diff --git a/scala/private/rules/scala_repl.bzl b/scala/private/rules/scala_repl.bzl index 6ee79c1b4..37dbf1e2a 100644 --- a/scala/private/rules/scala_repl.bzl +++ b/scala/private/rules/scala_repl.bzl @@ -13,7 +13,6 @@ load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", "extras_phases", "phase_binary_final", - "phase_common_init", "phase_common_runfiles", "phase_common_scala_provider", "phase_declare_executable", @@ -22,13 +21,16 @@ load( "phase_repl_compile", "phase_repl_java_wrapper", "phase_repl_write_executable", + "phase_scalac_provider", "phase_unused_deps_checker", + "phase_write_manifest", "run_phases", ) def _scala_repl_impl(ctx): return run_phases(ctx, [ - ("init", phase_common_init), + ("scalac_provider", phase_scalac_provider), + ("write_manifest", phase_write_manifest), ("unused_deps_checker", phase_unused_deps_checker), # need scala-compiler for MainGenericRunner below ("collect_jars", phase_repl_collect_jars), diff --git a/scala/private/rules/scala_test.bzl b/scala/private/rules/scala_test.bzl index 79a86b658..68a38ef53 100644 --- a/scala/private/rules/scala_test.bzl +++ b/scala/private/rules/scala_test.bzl @@ -12,24 +12,26 @@ load("@io_bazel_rules_scala//scala/private:common_outputs.bzl", "common_outputs" load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", "extras_phases", - "phase_common_init", "phase_common_java_wrapper", "phase_common_scala_provider", "phase_coverage_runfiles", "phase_declare_executable", "phase_merge_jars", + "phase_scalac_provider", "phase_test_collect_jars", "phase_test_compile", "phase_test_final", "phase_test_runfiles", "phase_test_write_executable", "phase_unused_deps_checker", + "phase_write_manifest", "run_phases", ) def _scala_test_impl(ctx): return run_phases(ctx, [ - ("init", phase_common_init), + ("scalac_provider", phase_scalac_provider), + ("write_manifest", phase_write_manifest), ("unused_deps_checker", phase_unused_deps_checker), ("collect_jars", phase_test_collect_jars), ("java_wrapper", phase_common_java_wrapper), From a50a114e0fc1248eb63ea3dadf228177bdcda5e5 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Fri, 1 Nov 2019 10:43:46 -0600 Subject: [PATCH 06/32] Move final to non-configurable phase --- scala/private/phases/api.bzl | 12 ++-- scala/private/rules/scala_binary.bzl | 33 +++++---- scala/private/rules/scala_junit_test.bzl | 35 +++++----- scala/private/rules/scala_library.bzl | 85 ++++++++++++++---------- scala/private/rules/scala_repl.bzl | 35 +++++----- scala/private/rules/scala_test.bzl | 35 +++++----- test/phase/customizability_test.bzl | 2 +- 7 files changed, 138 insertions(+), 99 deletions(-) diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index 76e41c73a..dcec17aad 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -9,7 +9,11 @@ def _adjust_phases(phases, adjustments): phases = phases[:] for (relation, peer_name, name, function) in adjustments: for idx, (needle, _) in enumerate(phases): - if needle == peer_name: + if relation in ["^", "first"]: + phases.insert(0, (name, function)) + elif relation in ["$", "last"]: + phases.append((name, function)) + elif needle == peer_name: if relation in ["-", "before"]: phases.insert(idx, (name, function)) elif relation in ["+", "after"]: @@ -18,7 +22,7 @@ def _adjust_phases(phases, adjustments): phases[idx] = (name, function) return phases -def run_phases(ctx, phases): +def run_phases(ctx, customizable_phases, fixed_phase): phase_providers = [ p[_ScalaRulePhase] for p in ctx.attr._phase_providers @@ -26,11 +30,11 @@ def run_phases(ctx, phases): ] if phase_providers != []: - phases = _adjust_phases(phases, [p for pp in phase_providers for p in pp.phases]) + customizable_phases = _adjust_phases(customizable_phases, [p for pp in phase_providers for p in pp.phases]) global_provider = {} current_provider = struct(**global_provider) - for (name, function) in phases: + for (name, function) in customizable_phases + [fixed_phase]: new_provider = function(ctx, current_provider) if new_provider != None: global_provider[name] = new_provider diff --git a/scala/private/rules/scala_binary.bzl b/scala/private/rules/scala_binary.bzl index 777f558b8..bef295df2 100644 --- a/scala/private/rules/scala_binary.bzl +++ b/scala/private/rules/scala_binary.bzl @@ -28,21 +28,26 @@ load( ) def _scala_binary_impl(ctx): - return run_phases(ctx, [ - ("scalac_provider", phase_scalac_provider), - ("write_manifest", phase_write_manifest), - ("unused_deps_checker", phase_unused_deps_checker), - ("collect_jars", phase_common_collect_jars), - ("java_wrapper", phase_common_java_wrapper), - ("declare_executable", phase_declare_executable), - # no need to build an ijar for an executable - ("compile", phase_binary_compile), - ("merge_jars", phase_merge_jars), - ("runfiles", phase_common_runfiles), - ("scala_provider", phase_common_scala_provider), - ("write_executable", phase_common_write_executable), + return run_phases( + ctx, + # customizable phases + [ + ("scalac_provider", phase_scalac_provider), + ("write_manifest", phase_write_manifest), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_common_collect_jars), + ("java_wrapper", phase_common_java_wrapper), + ("declare_executable", phase_declare_executable), + # no need to build an ijar for an executable + ("compile", phase_binary_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_common_runfiles), + ("scala_provider", phase_common_scala_provider), + ("write_executable", phase_common_write_executable), + ], + # fixed phase ("final", phase_binary_final), - ]).final + ).final _scala_binary_attrs = { "main_class": attr.string(mandatory = True), diff --git a/scala/private/rules/scala_junit_test.bzl b/scala/private/rules/scala_junit_test.bzl index f2e02ed7b..45922eb39 100644 --- a/scala/private/rules/scala_junit_test.bzl +++ b/scala/private/rules/scala_junit_test.bzl @@ -32,22 +32,27 @@ def _scala_junit_test_impl(ctx): fail( "Setting at least one of the attributes ('prefixes','suffixes') is required", ) - return run_phases(ctx, [ - ("scalac_provider", phase_scalac_provider), - ("write_manifest", phase_write_manifest), - ("unused_deps_checker", phase_unused_deps_checker), - ("collect_jars", phase_junit_test_collect_jars), - ("java_wrapper", phase_common_java_wrapper), - ("declare_executable", phase_declare_executable), - # no need to build an ijar for an executable - ("compile", phase_junit_test_compile), - ("merge_jars", phase_merge_jars), - ("runfiles", phase_common_runfiles), - ("scala_provider", phase_common_scala_provider), - ("jvm_flags", phase_jvm_flags), - ("write_executable", phase_junit_test_write_executable), + return run_phases( + ctx, + # customizable phases + [ + ("scalac_provider", phase_scalac_provider), + ("write_manifest", phase_write_manifest), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_junit_test_collect_jars), + ("java_wrapper", phase_common_java_wrapper), + ("declare_executable", phase_declare_executable), + # no need to build an ijar for an executable + ("compile", phase_junit_test_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_common_runfiles), + ("scala_provider", phase_common_scala_provider), + ("jvm_flags", phase_jvm_flags), + ("write_executable", phase_junit_test_write_executable), + ], + # fixed phase ("final", phase_binary_final), - ]).final + ).final _scala_junit_test_attrs = { "prefixes": attr.string_list(default = []), diff --git a/scala/private/rules/scala_library.bzl b/scala/private/rules/scala_library.bzl index b547d72f4..1792eb9d4 100644 --- a/scala/private/rules/scala_library.bzl +++ b/scala/private/rules/scala_library.bzl @@ -54,19 +54,24 @@ _library_attrs = { def _scala_library_impl(ctx): # Build up information from dependency-like attributes - return run_phases(ctx, [ - ("scalac_provider", phase_scalac_provider), - ("collect_srcjars", phase_collect_srcjars), - ("collect_exports_jars", phase_collect_exports_jars), - ("write_manifest", phase_write_manifest), - ("unused_deps_checker", phase_unused_deps_checker), - ("collect_jars", phase_common_collect_jars), - ("compile", phase_library_compile), - ("merge_jars", phase_merge_jars), - ("runfiles", phase_library_runfiles), - ("scala_provider", phase_library_scala_provider), + return run_phases( + ctx, + # customizable phases + [ + ("scalac_provider", phase_scalac_provider), + ("collect_srcjars", phase_collect_srcjars), + ("collect_exports_jars", phase_collect_exports_jars), + ("write_manifest", phase_write_manifest), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_common_collect_jars), + ("compile", phase_library_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_library_runfiles), + ("scala_provider", phase_library_scala_provider), + ], + # fixed phase ("final", phase_library_final), - ]).final + ).final _scala_library_attrs = {} @@ -128,18 +133,23 @@ def scala_library_suite( ## def _scala_library_for_plugin_bootstrapping_impl(ctx): - return run_phases(ctx, [ - ("scalac_provider", phase_scalac_provider), - ("collect_srcjars", phase_collect_srcjars), - ("collect_exports_jars", phase_collect_exports_jars), - ("write_manifest", phase_write_manifest), - ("collect_jars", phase_library_for_plugin_bootstrapping_collect_jars), - ("compile", phase_library_for_plugin_bootstrapping_compile), - ("merge_jars", phase_merge_jars), - ("runfiles", phase_library_runfiles), - ("scala_provider", phase_library_scala_provider), + return run_phases( + ctx, + # customizable phases + [ + ("scalac_provider", phase_scalac_provider), + ("collect_srcjars", phase_collect_srcjars), + ("collect_exports_jars", phase_collect_exports_jars), + ("write_manifest", phase_write_manifest), + ("collect_jars", phase_library_for_plugin_bootstrapping_collect_jars), + ("compile", phase_library_for_plugin_bootstrapping_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_library_runfiles), + ("scala_provider", phase_library_scala_provider), + ], + # fixed phase ("final", phase_library_final), - ]).final + ).final # the scala compiler plugin used for dependency analysis is compiled using `scala_library`. # in order to avoid cyclic dependencies `scala_library_for_plugin_bootstrapping` was created for this purpose, @@ -179,19 +189,24 @@ scala_library_for_plugin_bootstrapping = make_scala_library_for_plugin_bootstrap ## def _scala_macro_library_impl(ctx): - return run_phases(ctx, [ - ("scalac_provider", phase_scalac_provider), - ("collect_srcjars", phase_collect_srcjars), - ("collect_exports_jars", phase_collect_exports_jars), - ("write_manifest", phase_write_manifest), - ("unused_deps_checker", phase_unused_deps_checker), - ("collect_jars", phase_macro_library_collect_jars), - ("compile", phase_macro_library_compile), - ("merge_jars", phase_merge_jars), - ("runfiles", phase_library_runfiles), - ("scala_provider", phase_library_scala_provider), + return run_phases( + ctx, + # customizable phases + [ + ("scalac_provider", phase_scalac_provider), + ("collect_srcjars", phase_collect_srcjars), + ("collect_exports_jars", phase_collect_exports_jars), + ("write_manifest", phase_write_manifest), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_macro_library_collect_jars), + ("compile", phase_macro_library_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_library_runfiles), + ("scala_provider", phase_library_scala_provider), + ], + # fixed phase ("final", phase_library_final), - ]).final + ).final _scala_macro_library_attrs = { "main_class": attr.string(), diff --git a/scala/private/rules/scala_repl.bzl b/scala/private/rules/scala_repl.bzl index 37dbf1e2a..32aae92ac 100644 --- a/scala/private/rules/scala_repl.bzl +++ b/scala/private/rules/scala_repl.bzl @@ -28,22 +28,27 @@ load( ) def _scala_repl_impl(ctx): - return run_phases(ctx, [ - ("scalac_provider", phase_scalac_provider), - ("write_manifest", phase_write_manifest), - ("unused_deps_checker", phase_unused_deps_checker), - # need scala-compiler for MainGenericRunner below - ("collect_jars", phase_repl_collect_jars), - ("java_wrapper", phase_repl_java_wrapper), - ("declare_executable", phase_declare_executable), - # no need to build an ijar for an executable - ("compile", phase_repl_compile), - ("merge_jars", phase_merge_jars), - ("runfiles", phase_common_runfiles), - ("scala_provider", phase_common_scala_provider), - ("write_executable", phase_repl_write_executable), + return run_phases( + ctx, + # customizable phases + [ + ("scalac_provider", phase_scalac_provider), + ("write_manifest", phase_write_manifest), + ("unused_deps_checker", phase_unused_deps_checker), + # need scala-compiler for MainGenericRunner below + ("collect_jars", phase_repl_collect_jars), + ("java_wrapper", phase_repl_java_wrapper), + ("declare_executable", phase_declare_executable), + # no need to build an ijar for an executable + ("compile", phase_repl_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_common_runfiles), + ("scala_provider", phase_common_scala_provider), + ("write_executable", phase_repl_write_executable), + ], + # fixed phase ("final", phase_binary_final), - ]).final + ).final _scala_repl_attrs = { "jvm_flags": attr.string_list(), diff --git a/scala/private/rules/scala_test.bzl b/scala/private/rules/scala_test.bzl index 68a38ef53..e630be2a8 100644 --- a/scala/private/rules/scala_test.bzl +++ b/scala/private/rules/scala_test.bzl @@ -29,22 +29,27 @@ load( ) def _scala_test_impl(ctx): - return run_phases(ctx, [ - ("scalac_provider", phase_scalac_provider), - ("write_manifest", phase_write_manifest), - ("unused_deps_checker", phase_unused_deps_checker), - ("collect_jars", phase_test_collect_jars), - ("java_wrapper", phase_common_java_wrapper), - ("declare_executable", phase_declare_executable), - # no need to build an ijar for an executable - ("compile", phase_test_compile), - ("merge_jars", phase_merge_jars), - ("runfiles", phase_test_runfiles), - ("scala_provider", phase_common_scala_provider), - ("coverage_runfiles", phase_coverage_runfiles), - ("write_executable", phase_test_write_executable), + return run_phases( + ctx, + # customizable phases + [ + ("scalac_provider", phase_scalac_provider), + ("write_manifest", phase_write_manifest), + ("unused_deps_checker", phase_unused_deps_checker), + ("collect_jars", phase_test_collect_jars), + ("java_wrapper", phase_common_java_wrapper), + ("declare_executable", phase_declare_executable), + # no need to build an ijar for an executable + ("compile", phase_test_compile), + ("merge_jars", phase_merge_jars), + ("runfiles", phase_test_runfiles), + ("scala_provider", phase_common_scala_provider), + ("coverage_runfiles", phase_coverage_runfiles), + ("write_executable", phase_test_write_executable), + ], + # fixed phase ("final", phase_test_final), - ]).final + ).final _scala_test_attrs = { "main_class": attr.string( diff --git a/test/phase/customizability_test.bzl b/test/phase/customizability_test.bzl index 05ace37a3..c1c5d3a6d 100644 --- a/test/phase/customizability_test.bzl +++ b/test/phase/customizability_test.bzl @@ -31,7 +31,7 @@ def _add_phase_customizability_test_singleton_implementation(ctx): return [ _ScalaRulePhase( phases = [ - ("-", "final", "customizability_test", _phase_customizability_test), + ("$", "", "customizability_test", _phase_customizability_test), ], ), ] From 89f202a8a316dcc29e8f5605e0d2aee27fd28a8d Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 11 Nov 2019 17:20:15 -0700 Subject: [PATCH 07/32] Rename parameter builtin_customizable_phases --- scala/private/phases/api.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index dcec17aad..9c84fa059 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -22,7 +22,7 @@ def _adjust_phases(phases, adjustments): phases[idx] = (name, function) return phases -def run_phases(ctx, customizable_phases, fixed_phase): +def run_phases(ctx, builtin_customizable_phases, fixed_phase): phase_providers = [ p[_ScalaRulePhase] for p in ctx.attr._phase_providers @@ -30,11 +30,11 @@ def run_phases(ctx, customizable_phases, fixed_phase): ] if phase_providers != []: - customizable_phases = _adjust_phases(customizable_phases, [p for pp in phase_providers for p in pp.phases]) + builtin_customizable_phases = _adjust_phases(builtin_customizable_phases, [p for pp in phase_providers for p in pp.phases]) global_provider = {} current_provider = struct(**global_provider) - for (name, function) in customizable_phases + [fixed_phase]: + for (name, function) in builtin_customizable_phases + [fixed_phase]: new_provider = function(ctx, current_provider) if new_provider != None: global_provider[name] = new_provider From ac9e1dff767db194be95c875717d05783f7c7327 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 11 Nov 2019 23:03:01 -0700 Subject: [PATCH 08/32] Fix ijar --- scala/private/phases/phase_scala_provider.bzl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scala/private/phases/phase_scala_provider.bzl b/scala/private/phases/phase_scala_provider.bzl index 636a9db65..e5fb7add6 100644 --- a/scala/private/phases/phase_scala_provider.bzl +++ b/scala/private/phases/phase_scala_provider.bzl @@ -17,6 +17,7 @@ def phase_library_scala_provider(ctx, p): p.compile.ijars, transitive = [p.collect_exports_jars.compile_jars], ), + ijar = p.compile.ijar, ) return phase_common_scala_provider(ctx, p, args) @@ -26,19 +27,21 @@ def phase_common_scala_provider(ctx, p, _args = struct()): p, _args.rjars if hasattr(_args, "rjars") else p.compile.rjars, _args.compile_jars if hasattr(_args, "compile_jars") else depset(p.compile.ijars), + _args.ijar if hasattr(_args, "ijar") else p.compile.class_jar, # we aren't using ijar here ) def _phase_scala_provider( ctx, p, rjars, - compile_jars): + compile_jars, + ijar): return create_scala_provider( class_jar = p.compile.class_jar, compile_jars = compile_jars, deploy_jar = ctx.outputs.deploy_jar, full_jars = p.compile.full_jars, - ijar = p.compile.class_jar, # we aren't using ijar here + ijar = ijar, source_jars = p.compile.source_jars, statsfile = ctx.outputs.statsfile, transitive_runtime_jars = rjars, From 988a24ca19d5ace1eed6a6d18faec48a366eb31c Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Tue, 12 Nov 2019 13:48:18 -0700 Subject: [PATCH 09/32] Switch default for buildijar --- scala/private/phases/phase_compile.bzl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index 699b11401..23a626c5e 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -11,6 +11,7 @@ load( def phase_binary_compile(ctx, p): args = struct( + buildijar = False, unused_dependency_checker_ignored_targets = [ target.label for target in p.scalac_provider.default_classpath + @@ -22,7 +23,6 @@ def phase_binary_compile(ctx, p): def phase_library_compile(ctx, p): args = struct( srcjars = p.collect_srcjars, - buildijar = True, unused_dependency_checker_ignored_targets = [ target.label for target in p.scalac_provider.default_classpath + ctx.attr.exports + @@ -33,7 +33,6 @@ def phase_library_compile(ctx, p): def phase_library_for_plugin_bootstrapping_compile(ctx, p): args = struct( - buildijar = True, unused_dependency_checker_ignored_targets = [ target.label for target in p.scalac_provider.default_classpath + ctx.attr.exports @@ -44,6 +43,7 @@ def phase_library_for_plugin_bootstrapping_compile(ctx, p): def phase_macro_library_compile(ctx, p): args = struct( + buildijar = False, unused_dependency_checker_ignored_targets = [ target.label for target in p.scalac_provider.default_macro_classpath + ctx.attr.exports + @@ -54,6 +54,7 @@ def phase_macro_library_compile(ctx, p): def phase_junit_test_compile(ctx, p): args = struct( + buildijar = False, implicit_junit_deps_needed_for_java_compilation = [ ctx.attr._junit, ctx.attr._hamcrest, @@ -73,6 +74,7 @@ def phase_junit_test_compile(ctx, p): def phase_repl_compile(ctx, p): args = struct( + buildijar = False, unused_dependency_checker_ignored_targets = [ target.label for target in p.scalac_provider.default_repl_classpath + @@ -83,6 +85,7 @@ def phase_repl_compile(ctx, p): def phase_test_compile(ctx, p): args = struct( + buildijar = False, unused_dependency_checker_ignored_targets = [ target.label for target in p.scalac_provider.default_classpath + @@ -96,7 +99,7 @@ def phase_common_compile(ctx, p, _args = struct()): ctx, p, _args.srcjars if hasattr(_args, "srcjars") else depset(), - _args.buildijar if hasattr(_args, "buildijar") else False, + _args.buildijar if hasattr(_args, "buildijar") else True, _args.implicit_junit_deps_needed_for_java_compilation if hasattr(_args, "implicit_junit_deps_needed_for_java_compilation") else [], _args.unused_dependency_checker_ignored_targets if hasattr(_args, "unused_dependency_checker_ignored_targets") else [], _args.unused_dependency_checker_mode if hasattr(_args, "unused_dependency_checker_mode") else p.unused_deps_checker, From ec75a896539514e50c99320944de70872a656798 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Tue, 12 Nov 2019 13:51:30 -0700 Subject: [PATCH 10/32] Add TODOs --- scala/private/phases/phase_compile.bzl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index 23a626c5e..70793aa51 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -110,6 +110,7 @@ def _phase_compile( p, srcjars, buildijar, + # TODO: generalize this hack implicit_junit_deps_needed_for_java_compilation, unused_dependency_checker_ignored_targets, unused_dependency_checker_mode): @@ -134,6 +135,7 @@ def _phase_compile( deps_providers, ) + # TODO: simplify the return values and use provider return struct( class_jar = out.class_jar, coverage = out.coverage, From 9854fcc68cf75262d93f0e85cfce634c389703a0 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Fri, 15 Nov 2019 10:19:45 -0700 Subject: [PATCH 11/32] Rename provider --- scala/providers.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/providers.bzl b/scala/providers.bzl index 5a23ebfe3..1e6acdbd0 100644 --- a/scala/providers.bzl +++ b/scala/providers.bzl @@ -59,7 +59,7 @@ declare_scalac_provider = rule( ) ScalaRulePhase = provider( - doc = "A Scala compiler plugin", + doc = "A custom phase plugin", fields = { "phases": "the phases to add", }, From 54a5db36a0251c5a695b6ca37982684a2dac10af Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 25 Nov 2019 12:52:21 -0700 Subject: [PATCH 12/32] Move to advanced_usage --- scala/advanced_usage/providers.bzl | 6 ++++++ scala/advanced_usage/scala.bzl | 30 +++++++++++++++++++++++++++++ scala/private/phases/api.bzl | 2 +- scala/providers.bzl | 7 ------- scala/scala.bzl | 15 --------------- test/phase/customizability_test.bzl | 4 ++-- 6 files changed, 39 insertions(+), 25 deletions(-) create mode 100644 scala/advanced_usage/providers.bzl create mode 100644 scala/advanced_usage/scala.bzl diff --git a/scala/advanced_usage/providers.bzl b/scala/advanced_usage/providers.bzl new file mode 100644 index 000000000..eaefcf4dc --- /dev/null +++ b/scala/advanced_usage/providers.bzl @@ -0,0 +1,6 @@ +ScalaRulePhase = provider( + doc = "A custom phase plugin", + fields = { + "phases": "the phases to add", + }, +) diff --git a/scala/advanced_usage/scala.bzl b/scala/advanced_usage/scala.bzl new file mode 100644 index 000000000..046523709 --- /dev/null +++ b/scala/advanced_usage/scala.bzl @@ -0,0 +1,30 @@ +load( + "@io_bazel_rules_scala//scala/private:rules/scala_binary.bzl", + _make_scala_binary = "make_scala_binary", +) +load( + "@io_bazel_rules_scala//scala/private:rules/scala_junit_test.bzl", + _make_scala_junit_test = "make_scala_junit_test", +) +load( + "@io_bazel_rules_scala//scala/private:rules/scala_library.bzl", + _make_scala_library = "make_scala_library", + _make_scala_library_for_plugin_bootstrapping = "make_scala_library_for_plugin_bootstrapping", + _make_scala_macro_library = "make_scala_macro_library", +) +load( + "@io_bazel_rules_scala//scala/private:rules/scala_repl.bzl", + _make_scala_repl = "make_scala_repl", +) +load( + "@io_bazel_rules_scala//scala/private:rules/scala_test.bzl", + _make_scala_test = "make_scala_test", +) + +make_scala_binary = _make_scala_binary +make_scala_library = _make_scala_library +make_scala_library_for_plugin_bootstrapping = _make_scala_library_for_plugin_bootstrapping +make_scala_macro_library = _make_scala_macro_library +make_scala_repl = _make_scala_repl +make_scala_junit_test = _make_scala_junit_test +make_scala_test = _make_scala_test diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index 9c84fa059..c2f8078b3 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -1,5 +1,5 @@ load( - "@io_bazel_rules_scala//scala:providers.bzl", + "@io_bazel_rules_scala//scala:advanced_usage/providers.bzl", _ScalaRulePhase = "ScalaRulePhase", ) diff --git a/scala/providers.bzl b/scala/providers.bzl index 1e6acdbd0..5a92d4525 100644 --- a/scala/providers.bzl +++ b/scala/providers.bzl @@ -57,10 +57,3 @@ declare_scalac_provider = rule( "default_macro_classpath": attr.label_list(allow_files = True), }, ) - -ScalaRulePhase = provider( - doc = "A custom phase plugin", - fields = { - "phases": "the phases to add", - }, -) diff --git a/scala/scala.bzl b/scala/scala.bzl index f63e1521b..fb4a9eeaf 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -8,7 +8,6 @@ load( ) load( "@io_bazel_rules_scala//scala/private:rules/scala_binary.bzl", - _make_scala_binary = "make_scala_binary", _scala_binary = "scala_binary", ) load( @@ -17,14 +16,10 @@ load( ) load( "@io_bazel_rules_scala//scala/private:rules/scala_junit_test.bzl", - _make_scala_junit_test = "make_scala_junit_test", _scala_junit_test = "scala_junit_test", ) load( "@io_bazel_rules_scala//scala/private:rules/scala_library.bzl", - _make_scala_library = "make_scala_library", - _make_scala_library_for_plugin_bootstrapping = "make_scala_library_for_plugin_bootstrapping", - _make_scala_macro_library = "make_scala_macro_library", _scala_library = "scala_library", _scala_library_for_plugin_bootstrapping = "scala_library_for_plugin_bootstrapping", _scala_library_suite = "scala_library_suite", @@ -32,12 +27,10 @@ load( ) load( "@io_bazel_rules_scala//scala/private:rules/scala_repl.bzl", - _make_scala_repl = "make_scala_repl", _scala_repl = "scala_repl", ) load( "@io_bazel_rules_scala//scala/private:rules/scala_test.bzl", - _make_scala_test = "make_scala_test", _scala_test = "scala_test", _scala_test_suite = "scala_test_suite", ) @@ -67,11 +60,3 @@ scala_repl = _scala_repl scala_repositories = _scala_repositories scala_test = _scala_test scala_test_suite = _scala_test_suite - -make_scala_binary = _make_scala_binary -make_scala_library = _make_scala_library -make_scala_library_for_plugin_bootstrapping = _make_scala_library_for_plugin_bootstrapping -make_scala_macro_library = _make_scala_macro_library -make_scala_repl = _make_scala_repl -make_scala_junit_test = _make_scala_junit_test -make_scala_test = _make_scala_test diff --git a/test/phase/customizability_test.bzl b/test/phase/customizability_test.bzl index c1c5d3a6d..002f7e7db 100644 --- a/test/phase/customizability_test.bzl +++ b/test/phase/customizability_test.bzl @@ -1,5 +1,5 @@ load( - "//scala:providers.bzl", + "//scala:advanced_usage/providers.bzl", _ScalaRulePhase = "ScalaRulePhase", ) load( @@ -7,7 +7,7 @@ load( _phase_customizability_test = "phase_customizability_test", ) load( - "//scala:scala.bzl", + "//scala:advanced_usage/scala.bzl", _make_scala_binary = "make_scala_binary", _make_scala_library = "make_scala_library", _make_scala_test = "make_scala_test", From e10bcb35ed766f0ec61b91a177c7fb2bd435fc79 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 25 Nov 2019 13:20:58 -0700 Subject: [PATCH 13/32] rename custom_phases --- scala/advanced_usage/providers.bzl | 2 +- scala/private/phases/api.bzl | 2 +- test/phase/customizability_test.bzl | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scala/advanced_usage/providers.bzl b/scala/advanced_usage/providers.bzl index eaefcf4dc..a7ad19f29 100644 --- a/scala/advanced_usage/providers.bzl +++ b/scala/advanced_usage/providers.bzl @@ -1,6 +1,6 @@ ScalaRulePhase = provider( doc = "A custom phase plugin", fields = { - "phases": "the phases to add", + "custom_phases": "the phases to add", }, ) diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index c2f8078b3..125de9707 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -30,7 +30,7 @@ def run_phases(ctx, builtin_customizable_phases, fixed_phase): ] if phase_providers != []: - builtin_customizable_phases = _adjust_phases(builtin_customizable_phases, [p for pp in phase_providers for p in pp.phases]) + builtin_customizable_phases = _adjust_phases(builtin_customizable_phases, [p for pp in phase_providers for p in pp.custom_phases]) global_provider = {} current_provider = struct(**global_provider) diff --git a/test/phase/customizability_test.bzl b/test/phase/customizability_test.bzl index 002f7e7db..31226f74b 100644 --- a/test/phase/customizability_test.bzl +++ b/test/phase/customizability_test.bzl @@ -30,7 +30,7 @@ ext_add_phase_customizability_test = { def _add_phase_customizability_test_singleton_implementation(ctx): return [ _ScalaRulePhase( - phases = [ + custom_phases = [ ("$", "", "customizability_test", _phase_customizability_test), ], ), From c6deba3e1246c0513a004200f7eb556d47995b51 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 25 Nov 2019 14:10:30 -0700 Subject: [PATCH 14/32] Make default phase private --- scala/private/phases/phase_collect_jars.bzl | 15 +++++++++------ scala/private/phases/phase_compile.bzl | 19 +++++++++++-------- scala/private/phases/phase_java_wrapper.bzl | 7 +++++-- scala/private/phases/phase_runfiles.bzl | 9 ++++++--- scala/private/phases/phase_scala_provider.bzl | 7 +++++-- .../private/phases/phase_write_executable.bzl | 11 +++++++---- 6 files changed, 43 insertions(+), 25 deletions(-) diff --git a/scala/private/phases/phase_collect_jars.bzl b/scala/private/phases/phase_collect_jars.bzl index b667158f2..20b7cf1a3 100644 --- a/scala/private/phases/phase_collect_jars.bzl +++ b/scala/private/phases/phase_collect_jars.bzl @@ -16,19 +16,19 @@ def phase_test_collect_jars(ctx, p): ctx.attr._scalatest_runner, ], ) - return phase_common_collect_jars(ctx, p, args) + return _phase_default_collect_jars(ctx, p, args) def phase_repl_collect_jars(ctx, p): args = struct( base_classpath = p.scalac_provider.default_repl_classpath, ) - return phase_common_collect_jars(ctx, p, args) + return _phase_default_collect_jars(ctx, p, args) def phase_macro_library_collect_jars(ctx, p): args = struct( base_classpath = p.scalac_provider.default_macro_classpath, ) - return phase_common_collect_jars(ctx, p, args) + return _phase_default_collect_jars(ctx, p, args) def phase_junit_test_collect_jars(ctx, p): args = struct( @@ -39,15 +39,18 @@ def phase_junit_test_collect_jars(ctx, p): ctx.attr._bazel_test_runner, ], ) - return phase_common_collect_jars(ctx, p, args) + return _phase_default_collect_jars(ctx, p, args) def phase_library_for_plugin_bootstrapping_collect_jars(ctx, p): args = struct( unused_dependency_checker_mode = "off", ) - return phase_common_collect_jars(ctx, p, args) + return _phase_default_collect_jars(ctx, p, args) -def phase_common_collect_jars(ctx, p, _args = struct()): +def phase_common_collect_jars(ctx, p): + return _phase_default_collect_jars(ctx, p) + +def _phase_default_collect_jars(ctx, p, _args = struct()): return _phase_collect_jars( ctx, _args.base_classpath if hasattr(_args, "base_classpath") else p.scalac_provider.default_classpath, diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index 70793aa51..dc3afc950 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -18,7 +18,7 @@ def phase_binary_compile(ctx, p): ctx.attr.unused_dependency_checker_ignored_targets ], ) - return phase_common_compile(ctx, p, args) + return _phase_default_compile(ctx, p, args) def phase_library_compile(ctx, p): args = struct( @@ -29,7 +29,7 @@ def phase_library_compile(ctx, p): ctx.attr.unused_dependency_checker_ignored_targets ], ) - return phase_common_compile(ctx, p, args) + return _phase_default_compile(ctx, p, args) def phase_library_for_plugin_bootstrapping_compile(ctx, p): args = struct( @@ -39,7 +39,7 @@ def phase_library_for_plugin_bootstrapping_compile(ctx, p): ], unused_dependency_checker_mode = "off", ) - return phase_common_compile(ctx, p, args) + return _phase_default_compile(ctx, p, args) def phase_macro_library_compile(ctx, p): args = struct( @@ -50,7 +50,7 @@ def phase_macro_library_compile(ctx, p): ctx.attr.unused_dependency_checker_ignored_targets ], ) - return phase_common_compile(ctx, p, args) + return _phase_default_compile(ctx, p, args) def phase_junit_test_compile(ctx, p): args = struct( @@ -70,7 +70,7 @@ def phase_junit_test_compile(ctx, p): ctx.attr._bazel_test_runner.label, ], ) - return phase_common_compile(ctx, p, args) + return _phase_default_compile(ctx, p, args) def phase_repl_compile(ctx, p): args = struct( @@ -81,7 +81,7 @@ def phase_repl_compile(ctx, p): ctx.attr.unused_dependency_checker_ignored_targets ], ) - return phase_common_compile(ctx, p, args) + return _phase_default_compile(ctx, p, args) def phase_test_compile(ctx, p): args = struct( @@ -92,9 +92,12 @@ def phase_test_compile(ctx, p): ctx.attr.unused_dependency_checker_ignored_targets ], ) - return phase_common_compile(ctx, p, args) + return _phase_default_compile(ctx, p, args) -def phase_common_compile(ctx, p, _args = struct()): +def phase_common_compile(ctx, p): + return _phase_default_compile(ctx, p) + +def _phase_default_compile(ctx, p, _args = struct()): return _phase_compile( ctx, p, diff --git a/scala/private/phases/phase_java_wrapper.bzl b/scala/private/phases/phase_java_wrapper.bzl index 64c7334cc..eda478a1f 100644 --- a/scala/private/phases/phase_java_wrapper.bzl +++ b/scala/private/phases/phase_java_wrapper.bzl @@ -26,9 +26,12 @@ function finish() { trap finish EXIT """, ) - return phase_common_java_wrapper(ctx, p, args) + return _phase_default_java_wrapper(ctx, p, args) -def phase_common_java_wrapper(ctx, p, _args = struct()): +def phase_common_java_wrapper(ctx, p): + return _phase_default_java_wrapper(ctx, p) + +def _phase_default_java_wrapper(ctx, p, _args = struct()): return _phase_java_wrapper( ctx, _args.args if hasattr(_args, "args") else "", diff --git a/scala/private/phases/phase_runfiles.bzl b/scala/private/phases/phase_runfiles.bzl index bdc602524..a1c8317d6 100644 --- a/scala/private/phases/phase_runfiles.bzl +++ b/scala/private/phases/phase_runfiles.bzl @@ -8,7 +8,7 @@ def phase_library_runfiles(ctx, p): # Using transitive_files since transitive_rjars a depset and avoiding linearization transitive_files = p.compile.rjars, ) - return phase_common_runfiles(ctx, p, args) + return _phase_default_runfiles(ctx, p, args) def phase_test_runfiles(ctx, p): args = "\n".join([ @@ -29,9 +29,12 @@ def phase_test_runfiles(ctx, p): ), args_file = args_file, ) - return phase_common_runfiles(ctx, p, args) + return _phase_default_runfiles(ctx, p, args) -def phase_common_runfiles(ctx, p, _args = struct()): +def phase_common_runfiles(ctx, p): + return _phase_default_runfiles(ctx, p) + +def _phase_default_runfiles(ctx, p, _args = struct()): return _phase_runfiles( ctx, _args.transitive_files if hasattr(_args, "transitive_files") else depset( diff --git a/scala/private/phases/phase_scala_provider.bzl b/scala/private/phases/phase_scala_provider.bzl index e5fb7add6..b50ec6a5d 100644 --- a/scala/private/phases/phase_scala_provider.bzl +++ b/scala/private/phases/phase_scala_provider.bzl @@ -19,9 +19,12 @@ def phase_library_scala_provider(ctx, p): ), ijar = p.compile.ijar, ) - return phase_common_scala_provider(ctx, p, args) + return _phase_default_scala_provider(ctx, p, args) -def phase_common_scala_provider(ctx, p, _args = struct()): +def phase_common_scala_provider(ctx, p): + return _phase_default_scala_provider(ctx, p) + +def _phase_default_scala_provider(ctx, p, _args = struct()): return _phase_scala_provider( ctx, p, diff --git a/scala/private/phases/phase_write_executable.bzl b/scala/private/phases/phase_write_executable.bzl index 93a6784b5..fea230d1a 100644 --- a/scala/private/phases/phase_write_executable.bzl +++ b/scala/private/phases/phase_write_executable.bzl @@ -25,23 +25,26 @@ def phase_test_write_executable(ctx, p): ] + expand_location(ctx, final_jvm_flags), use_jacoco = ctx.configuration.coverage_enabled, ) - return phase_common_write_executable(ctx, p, args) + return _phase_deafult_write_executable(ctx, p, args) def phase_repl_write_executable(ctx, p): args = struct( jvm_flags = ["-Dscala.usejavacp=true"] + ctx.attr.jvm_flags, main_class = "scala.tools.nsc.MainGenericRunner", ) - return phase_common_write_executable(ctx, p, args) + return _phase_deafult_write_executable(ctx, p, args) def phase_junit_test_write_executable(ctx, p): args = struct( jvm_flags = p.jvm_flags + ctx.attr.jvm_flags, main_class = "com.google.testing.junit.runner.BazelTestRunner", ) - return phase_common_write_executable(ctx, p, args) + return _phase_deafult_write_executable(ctx, p, args) -def phase_common_write_executable(ctx, p, _args = struct()): +def phase_common_write_executable(ctx, p): + return _phase_deafult_write_executable(ctx, p) + +def _phase_deafult_write_executable(ctx, p, _args = struct()): return _phase_write_executable( ctx, p, From 364cb8585566467efcaaa55fd40eea112442f41f Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 25 Nov 2019 14:42:23 -0700 Subject: [PATCH 15/32] Fix exports_jars --- scala/private/rules/scala_library.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scala/private/rules/scala_library.bzl b/scala/private/rules/scala_library.bzl index 1792eb9d4..4ffca6882 100644 --- a/scala/private/rules/scala_library.bzl +++ b/scala/private/rules/scala_library.bzl @@ -60,13 +60,13 @@ def _scala_library_impl(ctx): [ ("scalac_provider", phase_scalac_provider), ("collect_srcjars", phase_collect_srcjars), - ("collect_exports_jars", phase_collect_exports_jars), ("write_manifest", phase_write_manifest), ("unused_deps_checker", phase_unused_deps_checker), ("collect_jars", phase_common_collect_jars), ("compile", phase_library_compile), ("merge_jars", phase_merge_jars), ("runfiles", phase_library_runfiles), + ("collect_exports_jars", phase_collect_exports_jars), ("scala_provider", phase_library_scala_provider), ], # fixed phase @@ -139,12 +139,12 @@ def _scala_library_for_plugin_bootstrapping_impl(ctx): [ ("scalac_provider", phase_scalac_provider), ("collect_srcjars", phase_collect_srcjars), - ("collect_exports_jars", phase_collect_exports_jars), ("write_manifest", phase_write_manifest), ("collect_jars", phase_library_for_plugin_bootstrapping_collect_jars), ("compile", phase_library_for_plugin_bootstrapping_compile), ("merge_jars", phase_merge_jars), ("runfiles", phase_library_runfiles), + ("collect_exports_jars", phase_collect_exports_jars), ("scala_provider", phase_library_scala_provider), ], # fixed phase @@ -195,13 +195,13 @@ def _scala_macro_library_impl(ctx): [ ("scalac_provider", phase_scalac_provider), ("collect_srcjars", phase_collect_srcjars), - ("collect_exports_jars", phase_collect_exports_jars), ("write_manifest", phase_write_manifest), ("unused_deps_checker", phase_unused_deps_checker), ("collect_jars", phase_macro_library_collect_jars), ("compile", phase_macro_library_compile), ("merge_jars", phase_merge_jars), ("runfiles", phase_library_runfiles), + ("collect_exports_jars", phase_collect_exports_jars), ("scala_provider", phase_library_scala_provider), ], # fixed phase From 0e777f03b1caf6d415c99b5fba39ade5a6f6d916 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 25 Nov 2019 14:57:35 -0700 Subject: [PATCH 16/32] Adjusted_phases --- scala/private/phases/api.bzl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index 125de9707..84720662d 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -29,12 +29,11 @@ def run_phases(ctx, builtin_customizable_phases, fixed_phase): if _ScalaRulePhase in p ] - if phase_providers != []: - builtin_customizable_phases = _adjust_phases(builtin_customizable_phases, [p for pp in phase_providers for p in pp.custom_phases]) + adjusted_phases = _adjust_phases(builtin_customizable_phases, [p for pp in phase_providers for p in pp.custom_phases]) global_provider = {} current_provider = struct(**global_provider) - for (name, function) in builtin_customizable_phases + [fixed_phase]: + for (name, function) in adjusted_phases + [fixed_phase]: new_provider = function(ctx, current_provider) if new_provider != None: global_provider[name] = new_provider From e4fb12c1d4743e0856e2d9bc32933f9294692e45 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 25 Nov 2019 15:41:12 -0700 Subject: [PATCH 17/32] Rename p to be more clear --- scala/private/phases/api.bzl | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index 84720662d..121e39348 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -24,12 +24,19 @@ def _adjust_phases(phases, adjustments): def run_phases(ctx, builtin_customizable_phases, fixed_phase): phase_providers = [ - p[_ScalaRulePhase] - for p in ctx.attr._phase_providers - if _ScalaRulePhase in p + phase_provider[_ScalaRulePhase] + for phase_provider in ctx.attr._phase_providers + if _ScalaRulePhase in phase_provider ] - adjusted_phases = _adjust_phases(builtin_customizable_phases, [p for pp in phase_providers for p in pp.custom_phases]) + adjusted_phases = _adjust_phases( + builtin_customizable_phases, + [ + phase + for phase_provider in phase_providers + for phase in phase_provider.custom_phases + ], + ) global_provider = {} current_provider = struct(**global_provider) @@ -44,7 +51,11 @@ def run_phases(ctx, builtin_customizable_phases, fixed_phase): def extras_phases(extras): return { "_phase_providers": attr.label_list( - default = [pp for extra in extras for pp in extra["phase_providers"]], + default = [ + phase_provider + for extra in extras + for phase_provider in extra["phase_providers"] + ], providers = [_ScalaRulePhase], ), } From e64a73937862fb0b1524a252e573d19337e4dbfa Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Wed, 27 Nov 2019 11:38:56 -0700 Subject: [PATCH 18/32] Add in-line comments --- scala/advanced_usage/providers.bzl | 5 +++++ scala/advanced_usage/scala.bzl | 5 +++++ scala/private/phases/api.bzl | 23 +++++++++++++++++++++++ scala/private/phases/phases.bzl | 4 ++++ test/phase/customizability_test.bzl | 7 +++++++ 5 files changed, 44 insertions(+) diff --git a/scala/advanced_usage/providers.bzl b/scala/advanced_usage/providers.bzl index a7ad19f29..deeeeefa2 100644 --- a/scala/advanced_usage/providers.bzl +++ b/scala/advanced_usage/providers.bzl @@ -1,3 +1,8 @@ +""" +A phase provider for customizable rules +It is used only when you intend to add functionalities to existing default rules +""" + ScalaRulePhase = provider( doc = "A custom phase plugin", fields = { diff --git a/scala/advanced_usage/scala.bzl b/scala/advanced_usage/scala.bzl index 046523709..552532880 100644 --- a/scala/advanced_usage/scala.bzl +++ b/scala/advanced_usage/scala.bzl @@ -1,3 +1,8 @@ +""" +Re-expose the customizable rules +It is used only when you intend to add functionalities to existing default rules +""" + load( "@io_bazel_rules_scala//scala/private:rules/scala_binary.bzl", _make_scala_binary = "make_scala_binary", diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index 121e39348..29bf619e3 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -1,12 +1,25 @@ +""" +The phase API for rules implementation +""" + load( "@io_bazel_rules_scala//scala:advanced_usage/providers.bzl", _ScalaRulePhase = "ScalaRulePhase", ) +# A method to modify the built-in phase list +# - Insert new phases to the first/last position +# - Insert new phases before/after existing phases +# - Replace existing phases def _adjust_phases(phases, adjustments): + # Return when no adjustment needed if len(adjustments) == 0: return phases phases = phases[:] + # relation: the position to add a new phase + # peer_name: the existing phase to compare the position with + # name: the name of the new phase + # function: the function of the new phase for (relation, peer_name, name, function) in adjustments: for idx, (needle, _) in enumerate(phases): if relation in ["^", "first"]: @@ -22,13 +35,17 @@ def _adjust_phases(phases, adjustments): phases[idx] = (name, function) return phases +# Execute phases def run_phases(ctx, builtin_customizable_phases, fixed_phase): + # Loading custom phases + # Phases must be passed in by provider phase_providers = [ phase_provider[_ScalaRulePhase] for phase_provider in ctx.attr._phase_providers if _ScalaRulePhase in phase_provider ] + # Modify the built-in phase list adjusted_phases = _adjust_phases( builtin_customizable_phases, [ @@ -38,16 +55,22 @@ def run_phases(ctx, builtin_customizable_phases, fixed_phase): ], ) + # A placeholder for data shared with later phases global_provider = {} current_provider = struct(**global_provider) for (name, function) in adjusted_phases + [fixed_phase]: + # Run a phase new_provider = function(ctx, current_provider) + # If a phase returns data, append it to global_provider + # for later phases to access if new_provider != None: global_provider[name] = new_provider current_provider = struct(**global_provider) + # The final return of rules implementation return current_provider +# A method to pass in phase provider def extras_phases(extras): return { "_phase_providers": attr.label_list( diff --git a/scala/private/phases/phases.bzl b/scala/private/phases/phases.bzl index d4ccffbaa..6e02a09e1 100644 --- a/scala/private/phases/phases.bzl +++ b/scala/private/phases/phases.bzl @@ -1,3 +1,7 @@ +""" +Re-expose all the phase APIs and built-in phases +""" + load( "@io_bazel_rules_scala//scala/private:phases/api.bzl", _extras_phases = "extras_phases", diff --git a/test/phase/customizability_test.bzl b/test/phase/customizability_test.bzl index 31226f74b..92194aa11 100644 --- a/test/phase/customizability_test.bzl +++ b/test/phase/customizability_test.bzl @@ -1,3 +1,7 @@ +""" +This test makes sure custom phases can be inserted to the desired position through phase API +""" + load( "//scala:advanced_usage/providers.bzl", _ScalaRulePhase = "ScalaRulePhase", @@ -13,6 +17,7 @@ load( _make_scala_test = "make_scala_test", ) +# Inputs for the customizable rules ext_add_phase_customizability_test = { "attrs": { "custom_content": attr.string( @@ -27,6 +32,7 @@ ext_add_phase_customizability_test = { ], } +# The rule implementation for phase provider def _add_phase_customizability_test_singleton_implementation(ctx): return [ _ScalaRulePhase( @@ -36,6 +42,7 @@ def _add_phase_customizability_test_singleton_implementation(ctx): ), ] +# The rule for phase provider add_phase_customizability_test_singleton = rule( implementation = _add_phase_customizability_test_singleton_implementation, ) From 4ad873dce17efa4fe71e2561dab6b51a1d9176f6 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Wed, 27 Nov 2019 13:58:47 -0700 Subject: [PATCH 19/32] Fix lint --- scala/private/phases/api.bzl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index 29bf619e3..c56f4ff0c 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -16,6 +16,7 @@ def _adjust_phases(phases, adjustments): if len(adjustments) == 0: return phases phases = phases[:] + # relation: the position to add a new phase # peer_name: the existing phase to compare the position with # name: the name of the new phase @@ -61,6 +62,7 @@ def run_phases(ctx, builtin_customizable_phases, fixed_phase): for (name, function) in adjusted_phases + [fixed_phase]: # Run a phase new_provider = function(ctx, current_provider) + # If a phase returns data, append it to global_provider # for later phases to access if new_provider != None: From f569ea0cffcc2f362030d57b9705c2026353bdcf Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Wed, 27 Nov 2019 14:40:05 -0700 Subject: [PATCH 20/32] Add doc for phases --- README.md | 9 +++++++++ docs/customizable_phase.md | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 docs/customizable_phase.md diff --git a/README.md b/README.md index 0dcdd22d8..6094db40e 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,15 @@ Unused dependency checking can either be enabled globally for all targets using in these cases you can enable unused dependency checking globally through a toolchain and override individual misbehaving targets using the attribute. +## Advanced Configurable Rules +To make the ruleset more flexible and configurable, we introduce a phase architecture. By using a phase architecture, where rule implementations are defined as a list of phases that are executed sequentially, functionality can easily be added (or modified) by adding (or swapping) phases. + +Phases provide two major benefits: + - Contributors are able to implement new functionality by creating additional default phases. Phases also give us more clear idea what steps are shared across rules. + - Consumers are able to configure the rules to their specific use cases by defining new phases within their workspace without impacting other consumers. + +See [Customizable Phase](docs/customizable_phase.md) for more info. + ## Building from source Test & Build: ``` diff --git a/docs/customizable_phase.md b/docs/customizable_phase.md new file mode 100644 index 000000000..df6c532e7 --- /dev/null +++ b/docs/customizable_phase.md @@ -0,0 +1,25 @@ +# Customizable Phase + +## Overview +Phases increase configurability. + +## Who needs customizable phases +Customizable phases is an advanced feature for people who want the rules to do more. + +If you don't need to customize your rules and just need the default setup to work correctly, then just load the following file for default rules: +``` +load("@io_bazel_rules_scala//scala:scala.bzl") +``` +You may skip the rest of the documentation. + +## As a Consumer +You need to load the following two files: +``` +load("@io_bazel_rules_scala//scala:advanced_usage/providers.bzl", _ScalaRulePhase = "ScalaRulePhase") +load("@io_bazel_rules_scala//scala:advanced_usage/scala.bzl") +``` + +## As a Contributor +These are the relevant files + - `scala/private/phases/api.bzl` + - `scala/private/phases/phases.bzl` From 38482f94d93c3b0b64f3c869940f3aac10384217 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 2 Dec 2019 11:16:24 -0700 Subject: [PATCH 21/32] Doc for consumers --- docs/customizable_phase.md | 92 ++++++++++++++++++++++++++++++++++-- scala/private/phases/api.bzl | 16 +++---- 2 files changed, 95 insertions(+), 13 deletions(-) diff --git a/docs/customizable_phase.md b/docs/customizable_phase.md index df6c532e7..4f1da808c 100644 --- a/docs/customizable_phase.md +++ b/docs/customizable_phase.md @@ -1,23 +1,105 @@ # Customizable Phase ## Overview -Phases increase configurability. +Phases increase configurability. Rule implementations are defined as a list of phases. Each phase defines a specific step, which helps breaking up implementation into smaller and more readable groups. Some phases are independent from others, which means the order doesn't matter. However, some phases depend on outputs of previous phases, in this case, we should make sure it meets all the prerequisites before executing phases. + +The biggest benefit of phases is that it is customizable. If default phase A is not doing what you expect, you may switch it with your self-defined phase A. One use case is to write your own compilation phase with your favorite Scala compiler. You may also extend the default phase list for more functionality. One use case is to check the Scala format. ## Who needs customizable phases -Customizable phases is an advanced feature for people who want the rules to do more. +Customizable phases is an advanced feature for people who want the rules to do more. If you are an experienced Bazel rules developer, we make this powerful API public for you to do custom work without impacting other users. If you have no experience on writing Bazel rules, we are happy to help but be aware it may be frucstrating at first. If you don't need to customize your rules and just need the default setup to work correctly, then just load the following file for default rules: ``` load("@io_bazel_rules_scala//scala:scala.bzl") ``` -You may skip the rest of the documentation. ## As a Consumer -You need to load the following two files: +You need to load the following 2 files: ``` load("@io_bazel_rules_scala//scala:advanced_usage/providers.bzl", _ScalaRulePhase = "ScalaRulePhase") -load("@io_bazel_rules_scala//scala:advanced_usage/scala.bzl") +load("@io_bazel_rules_scala//scala:advanced_usage/scala.bzl", _make_scala_binary = "make_scala_binary") +``` +`ScalaRulePhase` is a phase provider to pass in custom phase. Rules with `make_` prefix, like `make_scala_binary`, are customizable rules. `make_`s take a dictionary as input. It currently supports appending `attrs` and `outputs` to default rules, as well as modifying the phase list. + +For example: +``` +ext_add_custom_phase = { + "attrs": { + "custom_content": attr.string( + default = "This is custom content", + ), + }, + "outputs": { + "custom_output": "%{name}.custom-output", + }, + "phase_providers": [ + "//custom/phase:phase_custom_write_extra_file", + ], +} + +custom_scala_binary = _make_scala_binary(ext_add_custom_phase) +``` +The usage of `attrs` and `outputs` is straight forward. `make_`s append these 2 fields to the default rules definition. All items in `attrs` can be accessed by `ctx.attr`, and all items in `outputs` can be accessed by `ctx.outputs`. `phase_providers` takes a list of targets which define how do you want to modify phase list. +``` +def _add_custom_phase_singleton_implementation(ctx): + return [ + _ScalaRulePhase( + custom_phases = [ + ("last", "", "custom_write_extra_file", phase_custom_write_extra_file), + ], + ), + ] + +add_custom_phase_singleton = rule( + implementation = _add_custom_phase_singleton_implementation, +) +``` +`add_custom_phase_singleton` is a simple rule solely to pass in custom phases using `_ScalaRulePhase`. The `custom_phases` field in `_ScalaRulePhase` take a list of tuples. Each tuple has 4 elements: +``` +(relation, peer_name, phase_name, phase_function) +``` + - relation: the position to add a new phase + - peer_name: the existing phase to compare the position with + - phase_name: the name of the new phase, also used to access phase information + - phase_function: the function of the new phase + +There are 5 possible relations: + - `^` or `first` + - `$` or `last` + - `-` or `before` + - `+` or `after` + - `=` or `replace` + +The symbols and words are interchangable. If `first` or `last` is used, it puts your custom phase at the beginning or the end of the phase list, `peer_name` is not needed. + +Then you have to call the rule in a `BUILD` +``` +add_custom_phase_singleton( + name = "phase_custom_write_extra_file", + visibility = ["//visibility:public"], +) +``` + +You may now see `phase_providers` in `ext_add_custom_phase` is pointing to this target. + +The last step is to write the function of the phase. For example: +``` +def phase_custom_write_extra_file(ctx, p): + ctx.actions.write( + output = ctx.outputs.custom_output, + content = ctx.attr.custom_content, + ) +``` +Every phase has 2 arguments, `ctx` and `p`. `ctx` gives you access to the fields defined in rules. `p` is the global provider, which contains information from initial state as well as all the previous phases. You may access the information from previous phases by `p..`. For example, if the previous phase, said `phase_jar` with phase name `jar`, returns a struct +``` +def phase_jar(ctx, p): + # Some works to get the jars + return struct( + class_jar = class_jar, + ijar = ijar, + ) ``` +You are able to access information like `p.jar.class_jar` in `phase_custom_write_extra_file`. You can provide the information for later phases in the same way, then they can access it by `p.custom_write_extra_file.`. ## As a Contributor These are the relevant files diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index c56f4ff0c..cc8a340af 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -19,21 +19,21 @@ def _adjust_phases(phases, adjustments): # relation: the position to add a new phase # peer_name: the existing phase to compare the position with - # name: the name of the new phase - # function: the function of the new phase - for (relation, peer_name, name, function) in adjustments: + # phase_name: the name of the new phase, also used to access phase information + # phase_function: the function of the new phase + for (relation, peer_name, phase_name, phase_function) in adjustments: for idx, (needle, _) in enumerate(phases): if relation in ["^", "first"]: - phases.insert(0, (name, function)) + phases.insert(0, (phase_name, phase_function)) elif relation in ["$", "last"]: - phases.append((name, function)) + phases.append((phase_name, phase_function)) elif needle == peer_name: if relation in ["-", "before"]: - phases.insert(idx, (name, function)) + phases.insert(idx, (phase_name, phase_function)) elif relation in ["+", "after"]: - phases.insert(idx + 1, (name, function)) + phases.insert(idx + 1, (phase_name, phase_function)) elif relation in ["=", "replace"]: - phases[idx] = (name, function) + phases[idx] = (phase_name, phase_function) return phases # Execute phases From 22aa4525f0e21509935e8b22740a5154c47656f6 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 2 Dec 2019 14:49:32 -0700 Subject: [PATCH 22/32] Doc for contributors --- docs/customizable_phase.md | 56 +++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/docs/customizable_phase.md b/docs/customizable_phase.md index 4f1da808c..1f43f095c 100644 --- a/docs/customizable_phase.md +++ b/docs/customizable_phase.md @@ -5,21 +5,21 @@ Phases increase configurability. Rule implementations are defined as a list of p The biggest benefit of phases is that it is customizable. If default phase A is not doing what you expect, you may switch it with your self-defined phase A. One use case is to write your own compilation phase with your favorite Scala compiler. You may also extend the default phase list for more functionality. One use case is to check the Scala format. -## Who needs customizable phases -Customizable phases is an advanced feature for people who want the rules to do more. If you are an experienced Bazel rules developer, we make this powerful API public for you to do custom work without impacting other users. If you have no experience on writing Bazel rules, we are happy to help but be aware it may be frucstrating at first. +## Who needs customizable phase? +Customizable phase is an advanced feature for people who want the rules to do more. If you are an experienced Bazel rules developer, we make this powerful API public for you to do custom work without impacting other users. If you have no experience on writing Bazel rules, we are happy to help but be aware it may be frucstrating at first. If you don't need to customize your rules and just need the default setup to work correctly, then just load the following file for default rules: ``` load("@io_bazel_rules_scala//scala:scala.bzl") ``` -## As a Consumer +## As a consumer You need to load the following 2 files: ``` -load("@io_bazel_rules_scala//scala:advanced_usage/providers.bzl", _ScalaRulePhase = "ScalaRulePhase") -load("@io_bazel_rules_scala//scala:advanced_usage/scala.bzl", _make_scala_binary = "make_scala_binary") +load("@io_bazel_rules_scala//scala:advanced_usage/providers.bzl", "ScalaRulePhase") +load("@io_bazel_rules_scala//scala:advanced_usage/scala.bzl", "make_scala_binary") ``` -`ScalaRulePhase` is a phase provider to pass in custom phase. Rules with `make_` prefix, like `make_scala_binary`, are customizable rules. `make_`s take a dictionary as input. It currently supports appending `attrs` and `outputs` to default rules, as well as modifying the phase list. +`ScalaRulePhase` is a phase provider to pass in custom phase. Rules with `make_` prefix, like `make_scala_binary`, are customizable rules. `make_`s take a dictionary as input. It currently supports appending `attrs` and `outputs` to default rules, as well as modifying the phase list. For example: ``` @@ -37,13 +37,13 @@ ext_add_custom_phase = { ], } -custom_scala_binary = _make_scala_binary(ext_add_custom_phase) +custom_scala_binary = make_scala_binary(ext_add_custom_phase) ``` -The usage of `attrs` and `outputs` is straight forward. `make_`s append these 2 fields to the default rules definition. All items in `attrs` can be accessed by `ctx.attr`, and all items in `outputs` can be accessed by `ctx.outputs`. `phase_providers` takes a list of targets which define how do you want to modify phase list. +`make_`s append `attrs` and `outputs` to the default rules definition. All items in `attrs` can be accessed by `ctx.attr`, and all items in `outputs` can be accessed by `ctx.outputs`. `phase_providers` takes a list of targets which define how you want to modify phase list. ``` def _add_custom_phase_singleton_implementation(ctx): return [ - _ScalaRulePhase( + ScalaRulePhase( custom_phases = [ ("last", "", "custom_write_extra_file", phase_custom_write_extra_file), ], @@ -54,7 +54,7 @@ add_custom_phase_singleton = rule( implementation = _add_custom_phase_singleton_implementation, ) ``` -`add_custom_phase_singleton` is a simple rule solely to pass in custom phases using `_ScalaRulePhase`. The `custom_phases` field in `_ScalaRulePhase` take a list of tuples. Each tuple has 4 elements: +`add_custom_phase_singleton` is a rule solely to pass in custom phases using `ScalaRulePhase`. The `custom_phases` field in `ScalaRulePhase` takes a list of tuples. Each tuple has 4 elements: ``` (relation, peer_name, phase_name, phase_function) ``` @@ -101,7 +101,37 @@ def phase_jar(ctx, p): ``` You are able to access information like `p.jar.class_jar` in `phase_custom_write_extra_file`. You can provide the information for later phases in the same way, then they can access it by `p.custom_write_extra_file.`. -## As a Contributor +You should be able to define the files above entirely in your own workspace without making change to the [bazelbuild/rules_scala](https://github.com/bazelbuild/rules_scala). If you believe your custom phase will be valuable to the community, please refer to [As a contributor](#as-a-contributor). Pull requests are welcome. + +## As a contributor +Besides the basics in [As a consumer](#as-a-consumer), the followings help you understand how phases are setup if you plan to contribute to [bazelbuild/rules_scala](https://github.com/bazelbuild/rules_scala). + These are the relevant files - - `scala/private/phases/api.bzl` - - `scala/private/phases/phases.bzl` + - `scala/private/phases/api.bzl`: the API of executing and modifying the phase list + - `scala/private/phases/phases.bzl`: re-expose phases for convenience + - `scala/private/phases/phase_.bzl`: all the phase definitions + +Currently phase architecture is used by 7 rules: + - scala_library + - scala_macro_library + - scala_library_for_plugin_bootstrapping + - scala_binary + - scala_test + - scala_junit_test + - scala_repl + +In each of the rule implementation, it calls `run_phases` and returns the information from `phase_final`, which groups the final returns of the rule. To prevent users from accidently removing `phase_final` from the list, we make it a non-customizable phase. + +To make a new phase, you have to define a new `phase_.bzl` in `scala/private/phases/`. Function definition should have 2 arguments, `ctx` and `p`. You may expose the information for later phases by returning a `struct`. In some phases, there are multiple phase functions since different rules may take slightly different input arguemnts. You may want to re-expose the phase definition in `scala/private/phases/phases.bzl`, so it's more convenient to access in rule files. + +In the rule implementations, put your new phase in `builtin_customizable_phases` list. The phases are executed sequentially, the order matters if the new phase depends on previous phases. + +### Phase naming convention +Files in `scala/private/phases/` + - `phase_.bzl`: phase definition file + +Function names in `phase_.bzl` + - `phase__`: function with custom inputs of specific rule + - `phase_common_`: function without custom inputs + - `_phase_default_`: private function that takes `_args` for custom inputs + - `_phase_`: private function with the actual logic From f3b89721d25b1fc4d4a4b35605172464956ed21f Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 2 Dec 2019 15:23:18 -0700 Subject: [PATCH 23/32] Add more content --- README.md | 7 ++++--- docs/customizable_phase.md | 21 ++++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 6094db40e..f50f81cc1 100644 --- a/README.md +++ b/README.md @@ -189,12 +189,13 @@ Unused dependency checking can either be enabled globally for all targets using in these cases you can enable unused dependency checking globally through a toolchain and override individual misbehaving targets using the attribute. -## Advanced Configurable Rules +## Advanced configurable rules To make the ruleset more flexible and configurable, we introduce a phase architecture. By using a phase architecture, where rule implementations are defined as a list of phases that are executed sequentially, functionality can easily be added (or modified) by adding (or swapping) phases. -Phases provide two major benefits: - - Contributors are able to implement new functionality by creating additional default phases. Phases also give us more clear idea what steps are shared across rules. +Phases provide 3 major benefits: - Consumers are able to configure the rules to their specific use cases by defining new phases within their workspace without impacting other consumers. + - Contributors are able to implement new functionalities by creating additional default phases. + - Phases give us more clear idea what steps are shared across rules. See [Customizable Phase](docs/customizable_phase.md) for more info. diff --git a/docs/customizable_phase.md b/docs/customizable_phase.md index 1f43f095c..b41c90ef8 100644 --- a/docs/customizable_phase.md +++ b/docs/customizable_phase.md @@ -1,12 +1,19 @@ # Customizable Phase +## Contents +* [Overview](#overview) +* [Who needs customizable phase?](#who-needs-customizable-phase?) +* [As a consumer](#as-a-consumer) +* [As a contributor](#as-a-contributor) + * [Phase naming convention](#phase-naming-convention) + ## Overview Phases increase configurability. Rule implementations are defined as a list of phases. Each phase defines a specific step, which helps breaking up implementation into smaller and more readable groups. Some phases are independent from others, which means the order doesn't matter. However, some phases depend on outputs of previous phases, in this case, we should make sure it meets all the prerequisites before executing phases. -The biggest benefit of phases is that it is customizable. If default phase A is not doing what you expect, you may switch it with your self-defined phase A. One use case is to write your own compilation phase with your favorite Scala compiler. You may also extend the default phase list for more functionality. One use case is to check the Scala format. +The biggest benefit of phases is that it is customizable. If default phase A is not doing what you expect, you may switch it with your self-defined phase A. One use case is to write your own compilation phase with your favorite Scala compiler. You may also extend the default phase list for more functionalities. One use case is to check the Scala format. ## Who needs customizable phase? -Customizable phase is an advanced feature for people who want the rules to do more. If you are an experienced Bazel rules developer, we make this powerful API public for you to do custom work without impacting other users. If you have no experience on writing Bazel rules, we are happy to help but be aware it may be frucstrating at first. +Customizable phase is an advanced feature for people who want the rules to do more. If you are an experienced Bazel rules developer, we make this powerful API public for you to do custom work without impacting other consumers. If you have no experience on writing Bazel rules, we are happy to help but be aware it may be frucstrating at first. If you don't need to customize your rules and just need the default setup to work correctly, then just load the following file for default rules: ``` @@ -19,7 +26,7 @@ You need to load the following 2 files: load("@io_bazel_rules_scala//scala:advanced_usage/providers.bzl", "ScalaRulePhase") load("@io_bazel_rules_scala//scala:advanced_usage/scala.bzl", "make_scala_binary") ``` -`ScalaRulePhase` is a phase provider to pass in custom phase. Rules with `make_` prefix, like `make_scala_binary`, are customizable rules. `make_`s take a dictionary as input. It currently supports appending `attrs` and `outputs` to default rules, as well as modifying the phase list. +`ScalaRulePhase` is a phase provider to pass in custom phases. Rules with `make_` prefix, like `make_scala_binary`, are customizable rules. `make_`s take a dictionary as input. It currently supports appending `attrs` and `outputs` to default rules, as well as modifying the phase list. For example: ``` @@ -39,7 +46,7 @@ ext_add_custom_phase = { custom_scala_binary = make_scala_binary(ext_add_custom_phase) ``` -`make_`s append `attrs` and `outputs` to the default rules definition. All items in `attrs` can be accessed by `ctx.attr`, and all items in `outputs` can be accessed by `ctx.outputs`. `phase_providers` takes a list of targets which define how you want to modify phase list. +`make_`s append `attrs` and `outputs` to the default rule definitions. All items in `attrs` can be accessed by `ctx.attr`, and all items in `outputs` can be accessed by `ctx.outputs`. `phase_providers` takes a list of targets which define how you want to modify phase list. ``` def _add_custom_phase_singleton_implementation(ctx): return [ @@ -120,12 +127,14 @@ Currently phase architecture is used by 7 rules: - scala_junit_test - scala_repl -In each of the rule implementation, it calls `run_phases` and returns the information from `phase_final`, which groups the final returns of the rule. To prevent users from accidently removing `phase_final` from the list, we make it a non-customizable phase. +In each of the rule implementation, it calls `run_phases` and returns the information from `phase_final`, which groups the final returns of the rule. To prevent consumers from accidently removing `phase_final` from the list, we make it a non-customizable phase. To make a new phase, you have to define a new `phase_.bzl` in `scala/private/phases/`. Function definition should have 2 arguments, `ctx` and `p`. You may expose the information for later phases by returning a `struct`. In some phases, there are multiple phase functions since different rules may take slightly different input arguemnts. You may want to re-expose the phase definition in `scala/private/phases/phases.bzl`, so it's more convenient to access in rule files. In the rule implementations, put your new phase in `builtin_customizable_phases` list. The phases are executed sequentially, the order matters if the new phase depends on previous phases. +If you are making new return fields of the rule, remember to modify `phase_final`. + ### Phase naming convention Files in `scala/private/phases/` - `phase_.bzl`: phase definition file @@ -135,3 +144,5 @@ Function names in `phase_.bzl` - `phase_common_`: function without custom inputs - `_phase_default_`: private function that takes `_args` for custom inputs - `_phase_`: private function with the actual logic + +See [phase_compile](scala/private/phases/phase_compile.bzl) for example. From eb14ea025bd6ad7049b1b66497fbfc48f85a63f8 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 2 Dec 2019 15:27:28 -0700 Subject: [PATCH 24/32] Fix md --- docs/customizable_phase.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/customizable_phase.md b/docs/customizable_phase.md index b41c90ef8..39b89c0ee 100644 --- a/docs/customizable_phase.md +++ b/docs/customizable_phase.md @@ -2,7 +2,7 @@ ## Contents * [Overview](#overview) -* [Who needs customizable phase?](#who-needs-customizable-phase?) +* [Who needs customizable phase](#who-needs-customizable-phase) * [As a consumer](#as-a-consumer) * [As a contributor](#as-a-contributor) * [Phase naming convention](#phase-naming-convention) @@ -12,7 +12,7 @@ Phases increase configurability. Rule implementations are defined as a list of p The biggest benefit of phases is that it is customizable. If default phase A is not doing what you expect, you may switch it with your self-defined phase A. One use case is to write your own compilation phase with your favorite Scala compiler. You may also extend the default phase list for more functionalities. One use case is to check the Scala format. -## Who needs customizable phase? +## Who needs customizable phase Customizable phase is an advanced feature for people who want the rules to do more. If you are an experienced Bazel rules developer, we make this powerful API public for you to do custom work without impacting other consumers. If you have no experience on writing Bazel rules, we are happy to help but be aware it may be frucstrating at first. If you don't need to customize your rules and just need the default setup to work correctly, then just load the following file for default rules: @@ -145,4 +145,4 @@ Function names in `phase_.bzl` - `_phase_default_`: private function that takes `_args` for custom inputs - `_phase_`: private function with the actual logic -See [phase_compile](scala/private/phases/phase_compile.bzl) for example. +See `phase_compile.bzl` for example. From 5dde9db053568d0d54fd60c50b0b0b30a6c05e9c Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 2 Dec 2019 16:31:55 -0700 Subject: [PATCH 25/32] Test for all rules --- test/phase/BUILD | 30 ---------- test/phase/add_phase_to_all_rules/BUILD | 59 +++++++++++++++++++ .../HelloBinary.scala | 0 .../HelloLibrary.scala | 0 .../HelloTest.scala | 0 .../customizability_test.bzl | 18 ++++-- .../phase_customizability_test.bzl | 0 test/shell/test_phase.sh | 35 ++++++++++- 8 files changed, 104 insertions(+), 38 deletions(-) delete mode 100644 test/phase/BUILD create mode 100644 test/phase/add_phase_to_all_rules/BUILD rename test/phase/{ => add_phase_to_all_rules}/HelloBinary.scala (100%) rename test/phase/{ => add_phase_to_all_rules}/HelloLibrary.scala (100%) rename test/phase/{ => add_phase_to_all_rules}/HelloTest.scala (100%) rename test/phase/{ => add_phase_to_all_rules}/customizability_test.bzl (63%) rename test/phase/{ => add_phase_to_all_rules}/phase_customizability_test.bzl (100%) diff --git a/test/phase/BUILD b/test/phase/BUILD deleted file mode 100644 index 7c47a162b..000000000 --- a/test/phase/BUILD +++ /dev/null @@ -1,30 +0,0 @@ -load( - "//test/phase:customizability_test.bzl", - "add_phase_customizability_test_singleton", - "customizability_test_scala_binary", - "customizability_test_scala_library", - "customizability_test_scala_test", -) - -add_phase_customizability_test_singleton( - name = "phase_customizability_test", - visibility = ["//visibility:public"], -) - -customizability_test_scala_binary( - name = "HelloBinary", - srcs = ["HelloBinary.scala"], - main_class = "scalarules.test.phase.HelloBinary", -) - -customizability_test_scala_library( - name = "HelloLibrary", - srcs = ["HelloLibrary.scala"], - custom_content = "This is custom content in library", -) - -customizability_test_scala_test( - name = "HelloTest", - srcs = ["HelloTest.scala"], - custom_content = "This is custom content in test", -) diff --git a/test/phase/add_phase_to_all_rules/BUILD b/test/phase/add_phase_to_all_rules/BUILD new file mode 100644 index 000000000..5fe234056 --- /dev/null +++ b/test/phase/add_phase_to_all_rules/BUILD @@ -0,0 +1,59 @@ +load( + "//test/phase/add_phase_to_all_rules:customizability_test.bzl", + "add_phase_customizability_test_singleton", + "customizability_test_scala_binary", + "customizability_test_scala_library", + "customizability_test_scala_library_for_plugin_bootstrapping", + "customizability_test_scala_macro_library", + "customizability_test_scala_test", + "customizability_test_scala_junit_test", + "customizability_test_scala_repl", +) + +add_phase_customizability_test_singleton( + name = "phase_customizability_test", + visibility = ["//visibility:public"], +) + +customizability_test_scala_binary( + name = "HelloBinary", + srcs = ["HelloBinary.scala"], + main_class = "scalarules.test.phase.HelloBinary", +) + +customizability_test_scala_library( + name = "HelloLibrary", + srcs = ["HelloLibrary.scala"], + custom_content = "This is custom content in library", +) + +customizability_test_scala_library_for_plugin_bootstrapping( + name = "HelloLibraryForPluginBootstrapping", + srcs = ["HelloLibrary.scala"], + custom_content = "This is custom content in library_for_plugin_bootstrapping", +) + +customizability_test_scala_macro_library( + name = "HelloMacroLibrary", + srcs = ["HelloLibrary.scala"], + custom_content = "This is custom content in macro_library", +) + +customizability_test_scala_test( + name = "HelloTest", + srcs = ["HelloTest.scala"], + custom_content = "This is custom content in test", +) + +customizability_test_scala_junit_test( + name = "HelloJunitTest", + srcs = ["HelloTest.scala"], + prefixes = ["custom_phase"], + custom_content = "This is custom content in junit_test", +) + +customizability_test_scala_repl( + name = "HelloRepl", + srcs = ["HelloLibrary.scala"], + custom_content = "This is custom content in repl", +) diff --git a/test/phase/HelloBinary.scala b/test/phase/add_phase_to_all_rules/HelloBinary.scala similarity index 100% rename from test/phase/HelloBinary.scala rename to test/phase/add_phase_to_all_rules/HelloBinary.scala diff --git a/test/phase/HelloLibrary.scala b/test/phase/add_phase_to_all_rules/HelloLibrary.scala similarity index 100% rename from test/phase/HelloLibrary.scala rename to test/phase/add_phase_to_all_rules/HelloLibrary.scala diff --git a/test/phase/HelloTest.scala b/test/phase/add_phase_to_all_rules/HelloTest.scala similarity index 100% rename from test/phase/HelloTest.scala rename to test/phase/add_phase_to_all_rules/HelloTest.scala diff --git a/test/phase/customizability_test.bzl b/test/phase/add_phase_to_all_rules/customizability_test.bzl similarity index 63% rename from test/phase/customizability_test.bzl rename to test/phase/add_phase_to_all_rules/customizability_test.bzl index 92194aa11..2a3dae9ba 100644 --- a/test/phase/customizability_test.bzl +++ b/test/phase/add_phase_to_all_rules/customizability_test.bzl @@ -6,15 +6,19 @@ load( "//scala:advanced_usage/providers.bzl", _ScalaRulePhase = "ScalaRulePhase", ) -load( - "//test/phase:phase_customizability_test.bzl", - _phase_customizability_test = "phase_customizability_test", -) load( "//scala:advanced_usage/scala.bzl", _make_scala_binary = "make_scala_binary", _make_scala_library = "make_scala_library", + _make_scala_library_for_plugin_bootstrapping = "make_scala_library_for_plugin_bootstrapping", + _make_scala_macro_library = "make_scala_macro_library", _make_scala_test = "make_scala_test", + _make_scala_junit_test = "make_scala_junit_test", + _make_scala_repl = "make_scala_repl", +) +load( + "//test/phase/add_phase_to_all_rules:phase_customizability_test.bzl", + _phase_customizability_test = "phase_customizability_test", ) # Inputs for the customizable rules @@ -28,7 +32,7 @@ ext_add_phase_customizability_test = { "custom_output": "%{name}.custom-output", }, "phase_providers": [ - "//test/phase:phase_customizability_test", + "//test/phase/add_phase_to_all_rules:phase_customizability_test", ], } @@ -49,4 +53,8 @@ add_phase_customizability_test_singleton = rule( customizability_test_scala_binary = _make_scala_binary(ext_add_phase_customizability_test) customizability_test_scala_library = _make_scala_library(ext_add_phase_customizability_test) +customizability_test_scala_library_for_plugin_bootstrapping = _make_scala_library_for_plugin_bootstrapping(ext_add_phase_customizability_test) +customizability_test_scala_macro_library = _make_scala_macro_library(ext_add_phase_customizability_test) customizability_test_scala_test = _make_scala_test(ext_add_phase_customizability_test) +customizability_test_scala_junit_test = _make_scala_junit_test(ext_add_phase_customizability_test) +customizability_test_scala_repl = _make_scala_repl(ext_add_phase_customizability_test) diff --git a/test/phase/phase_customizability_test.bzl b/test/phase/add_phase_to_all_rules/phase_customizability_test.bzl similarity index 100% rename from test/phase/phase_customizability_test.bzl rename to test/phase/add_phase_to_all_rules/phase_customizability_test.bzl diff --git a/test/shell/test_phase.sh b/test/shell/test_phase.sh index ac5844a1e..95c26e1fd 100755 --- a/test/shell/test_phase.sh +++ b/test/shell/test_phase.sh @@ -24,24 +24,53 @@ output_file_should_contain_message() { exit 0 fi } + test_scala_binary_with_extra_phase() { output_file_should_contain_message \ "This is custom content" \ - build //test/phase:HelloBinary.custom-output + build //test/phase/add_phase_to_all_rules:HelloBinary.custom-output } test_scala_library_with_extra_phase_and_custom_content() { output_file_should_contain_message \ "This is custom content in library" \ - build //test/phase:HelloLibrary.custom-output + build //test/phase/add_phase_to_all_rules:HelloLibrary.custom-output +} + +test_scala_library_for_plugin_bootstrapping_with_extra_phase_and_custom_content() { + output_file_should_contain_message \ + "This is custom content in library_for_plugin_bootstrapping" \ + build //test/phase/add_phase_to_all_rules:HelloLibraryForPluginBootstrapping.custom-output +} + +test_scala_macro_library_with_extra_phase_and_custom_content() { + output_file_should_contain_message \ + "This is custom content in macro_library" \ + build //test/phase/add_phase_to_all_rules:HelloMacroLibrary.custom-output } test_scala_test_with_extra_phase_and_custom_content() { output_file_should_contain_message \ "This is custom content in test" \ - build //test/phase:HelloTest.custom-output + build //test/phase/add_phase_to_all_rules:HelloTest.custom-output +} + +test_scala_junit_test_with_extra_phase_and_custom_content() { + output_file_should_contain_message \ + "This is custom content in junit_test" \ + build //test/phase/add_phase_to_all_rules:HelloJunitTest.custom-output +} + +test_scala_repl_with_extra_phase_and_custom_content() { + output_file_should_contain_message \ + "This is custom content in repl" \ + build //test/phase/add_phase_to_all_rules:HelloRepl.custom-output } $runner test_scala_binary_with_extra_phase $runner test_scala_library_with_extra_phase_and_custom_content +$runner test_scala_library_for_plugin_bootstrapping_with_extra_phase_and_custom_content +$runner test_scala_macro_library_with_extra_phase_and_custom_content $runner test_scala_test_with_extra_phase_and_custom_content +$runner test_scala_junit_test_with_extra_phase_and_custom_content +$runner test_scala_repl_with_extra_phase_and_custom_content From 37d6904030e3fa7ab08ca9241981032dc1677afa Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 2 Dec 2019 16:46:05 -0700 Subject: [PATCH 26/32] Fix junit test --- test/phase/add_phase_to_all_rules/BUILD | 4 ++-- test/phase/add_phase_to_all_rules/HelloJunitTest.scala | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 test/phase/add_phase_to_all_rules/HelloJunitTest.scala diff --git a/test/phase/add_phase_to_all_rules/BUILD b/test/phase/add_phase_to_all_rules/BUILD index 5fe234056..b780bbf15 100644 --- a/test/phase/add_phase_to_all_rules/BUILD +++ b/test/phase/add_phase_to_all_rules/BUILD @@ -47,8 +47,8 @@ customizability_test_scala_test( customizability_test_scala_junit_test( name = "HelloJunitTest", - srcs = ["HelloTest.scala"], - prefixes = ["custom_phase"], + srcs = ["HelloJunitTest.scala"], + suffixes = ["Test"], custom_content = "This is custom content in junit_test", ) diff --git a/test/phase/add_phase_to_all_rules/HelloJunitTest.scala b/test/phase/add_phase_to_all_rules/HelloJunitTest.scala new file mode 100644 index 000000000..c4a48e9e5 --- /dev/null +++ b/test/phase/add_phase_to_all_rules/HelloJunitTest.scala @@ -0,0 +1,9 @@ +package scalarules.test.phase + +import org.junit.Test + +class HelloJunitTest { + @Test + def someTest: Unit = + println("passing") +} From 7f0fa4ef74c2be1fb403058543b57936a675549c Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Mon, 2 Dec 2019 16:51:11 -0700 Subject: [PATCH 27/32] Fix lint --- test/phase/add_phase_to_all_rules/BUILD | 6 +++--- test/phase/add_phase_to_all_rules/customizability_test.bzl | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/phase/add_phase_to_all_rules/BUILD b/test/phase/add_phase_to_all_rules/BUILD index b780bbf15..82087cecf 100644 --- a/test/phase/add_phase_to_all_rules/BUILD +++ b/test/phase/add_phase_to_all_rules/BUILD @@ -2,12 +2,12 @@ load( "//test/phase/add_phase_to_all_rules:customizability_test.bzl", "add_phase_customizability_test_singleton", "customizability_test_scala_binary", + "customizability_test_scala_junit_test", "customizability_test_scala_library", "customizability_test_scala_library_for_plugin_bootstrapping", "customizability_test_scala_macro_library", - "customizability_test_scala_test", - "customizability_test_scala_junit_test", "customizability_test_scala_repl", + "customizability_test_scala_test", ) add_phase_customizability_test_singleton( @@ -48,8 +48,8 @@ customizability_test_scala_test( customizability_test_scala_junit_test( name = "HelloJunitTest", srcs = ["HelloJunitTest.scala"], - suffixes = ["Test"], custom_content = "This is custom content in junit_test", + suffixes = ["Test"], ) customizability_test_scala_repl( diff --git a/test/phase/add_phase_to_all_rules/customizability_test.bzl b/test/phase/add_phase_to_all_rules/customizability_test.bzl index 2a3dae9ba..b5886349e 100644 --- a/test/phase/add_phase_to_all_rules/customizability_test.bzl +++ b/test/phase/add_phase_to_all_rules/customizability_test.bzl @@ -9,12 +9,12 @@ load( load( "//scala:advanced_usage/scala.bzl", _make_scala_binary = "make_scala_binary", + _make_scala_junit_test = "make_scala_junit_test", _make_scala_library = "make_scala_library", _make_scala_library_for_plugin_bootstrapping = "make_scala_library_for_plugin_bootstrapping", _make_scala_macro_library = "make_scala_macro_library", - _make_scala_test = "make_scala_test", - _make_scala_junit_test = "make_scala_junit_test", _make_scala_repl = "make_scala_repl", + _make_scala_test = "make_scala_test", ) load( "//test/phase/add_phase_to_all_rules:phase_customizability_test.bzl", From aa35cd391886686edd7e49b65c3a4fb762f12cd4 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Tue, 3 Dec 2019 11:37:46 -0700 Subject: [PATCH 28/32] Add more tests --- scala/private/rules/scala_binary.bzl | 4 +- scala/private/rules/scala_junit_test.bzl | 4 +- scala/private/rules/scala_library.bzl | 12 +-- scala/private/rules/scala_repl.bzl | 4 +- scala/private/rules/scala_test.bzl | 4 +- test/phase/add_phase_to_all_rules/BUILD | 59 -------------- .../HelloJunitTest.scala | 9 --- .../add_phase_to_all_rules/HelloLibrary.scala | 5 -- .../customizability_test.bzl | 60 -------------- .../phase_customizability_test.bzl | 10 --- test/phase/add_to_all_rules/BUILD | 59 ++++++++++++++ .../PhaseBinary.scala} | 4 +- .../add_to_all_rules/PhaseJunitTest.scala | 9 +++ .../phase/add_to_all_rules/PhaseLibrary.scala | 5 ++ .../PhaseTest.scala} | 4 +- .../phase_add_to_all_rules.bzl | 10 +++ .../phase_add_to_all_rules_test.bzl | 60 ++++++++++++++ test/phase/adjustment/BUILD | 27 +++++++ test/phase/adjustment/PhaseLibrary.scala | 5 ++ test/phase/adjustment/phase_adjustment.bzl | 42 ++++++++++ .../adjustment/phase_adjustment_test.bzl | 79 +++++++++++++++++++ test/shell/test_phase.sh | 28 +++++-- 22 files changed, 335 insertions(+), 168 deletions(-) delete mode 100644 test/phase/add_phase_to_all_rules/BUILD delete mode 100644 test/phase/add_phase_to_all_rules/HelloJunitTest.scala delete mode 100644 test/phase/add_phase_to_all_rules/HelloLibrary.scala delete mode 100644 test/phase/add_phase_to_all_rules/customizability_test.bzl delete mode 100644 test/phase/add_phase_to_all_rules/phase_customizability_test.bzl create mode 100644 test/phase/add_to_all_rules/BUILD rename test/phase/{add_phase_to_all_rules/HelloBinary.scala => add_to_all_rules/PhaseBinary.scala} (58%) create mode 100644 test/phase/add_to_all_rules/PhaseJunitTest.scala create mode 100644 test/phase/add_to_all_rules/PhaseLibrary.scala rename test/phase/{add_phase_to_all_rules/HelloTest.scala => add_to_all_rules/PhaseTest.scala} (71%) create mode 100644 test/phase/add_to_all_rules/phase_add_to_all_rules.bzl create mode 100644 test/phase/add_to_all_rules/phase_add_to_all_rules_test.bzl create mode 100644 test/phase/adjustment/BUILD create mode 100644 test/phase/adjustment/PhaseLibrary.scala create mode 100644 test/phase/adjustment/phase_adjustment.bzl create mode 100644 test/phase/adjustment/phase_adjustment_test.bzl diff --git a/scala/private/rules/scala_binary.bzl b/scala/private/rules/scala_binary.bzl index bef295df2..1c62193f8 100644 --- a/scala/private/rules/scala_binary.bzl +++ b/scala/private/rules/scala_binary.bzl @@ -68,13 +68,13 @@ def make_scala_binary(*extras): attrs = _dicts.add( _scala_binary_attrs, extras_phases(extras), - *[extra["attrs"] for extra in extras] + *[extra["attrs"] for extra in extras if "attrs" in extra] ), executable = True, fragments = ["java"], outputs = _dicts.add( common_outputs, - *[extra["outputs"] for extra in extras] + *[extra["outputs"] for extra in extras if "outputs" in extra] ), toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], implementation = _scala_binary_impl, diff --git a/scala/private/rules/scala_junit_test.bzl b/scala/private/rules/scala_junit_test.bzl index 45922eb39..6f3bd255a 100644 --- a/scala/private/rules/scala_junit_test.bzl +++ b/scala/private/rules/scala_junit_test.bzl @@ -120,12 +120,12 @@ def make_scala_junit_test(*extras): attrs = _dicts.add( _scala_junit_test_attrs, extras_phases(extras), - *[extra["attrs"] for extra in extras] + *[extra["attrs"] for extra in extras if "attrs" in extra] ), fragments = ["java"], outputs = _dicts.add( common_outputs, - *[extra["outputs"] for extra in extras] + *[extra["outputs"] for extra in extras if "outputs" in extra] ), test = True, toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], diff --git a/scala/private/rules/scala_library.bzl b/scala/private/rules/scala_library.bzl index 4ffca6882..86273f37c 100644 --- a/scala/private/rules/scala_library.bzl +++ b/scala/private/rules/scala_library.bzl @@ -88,12 +88,12 @@ def make_scala_library(*extras): attrs = _dicts.add( _scala_library_attrs, extras_phases(extras), - *[extra["attrs"] for extra in extras] + *[extra["attrs"] for extra in extras if "attrs" in extra] ), fragments = ["java"], outputs = _dicts.add( common_outputs, - *[extra["outputs"] for extra in extras] + *[extra["outputs"] for extra in extras if "outputs" in extra] ), toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], implementation = _scala_library_impl, @@ -171,12 +171,12 @@ def make_scala_library_for_plugin_bootstrapping(*extras): attrs = _dicts.add( _scala_library_for_plugin_bootstrapping_attrs, extras_phases(extras), - *[extra["attrs"] for extra in extras] + *[extra["attrs"] for extra in extras if "attrs" in extra] ), fragments = ["java"], outputs = _dicts.add( common_outputs, - *[extra["outputs"] for extra in extras] + *[extra["outputs"] for extra in extras if "outputs" in extra] ), toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], implementation = _scala_library_for_plugin_bootstrapping_impl, @@ -238,12 +238,12 @@ def make_scala_macro_library(*extras): attrs = _dicts.add( _scala_macro_library_attrs, extras_phases(extras), - *[extra["attrs"] for extra in extras] + *[extra["attrs"] for extra in extras if "attrs" in extra] ), fragments = ["java"], outputs = _dicts.add( common_outputs, - *[extra["outputs"] for extra in extras] + *[extra["outputs"] for extra in extras if "outputs" in extra] ), toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], implementation = _scala_macro_library_impl, diff --git a/scala/private/rules/scala_repl.bzl b/scala/private/rules/scala_repl.bzl index 32aae92ac..8f3fd9625 100644 --- a/scala/private/rules/scala_repl.bzl +++ b/scala/private/rules/scala_repl.bzl @@ -67,13 +67,13 @@ def make_scala_repl(*extras): attrs = _dicts.add( _scala_repl_attrs, extras_phases(extras), - *[extra["attrs"] for extra in extras] + *[extra["attrs"] for extra in extras if "attrs" in extra] ), executable = True, fragments = ["java"], outputs = _dicts.add( common_outputs, - *[extra["outputs"] for extra in extras] + *[extra["outputs"] for extra in extras if "outputs" in extra] ), toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], implementation = _scala_repl_impl, diff --git a/scala/private/rules/scala_test.bzl b/scala/private/rules/scala_test.bzl index e630be2a8..e3927e8e5 100644 --- a/scala/private/rules/scala_test.bzl +++ b/scala/private/rules/scala_test.bzl @@ -105,13 +105,13 @@ def make_scala_test(*extras): attrs = _dicts.add( _scala_test_attrs, extras_phases(extras), - *[extra["attrs"] for extra in extras] + *[extra["attrs"] for extra in extras if "attrs" in extra] ), executable = True, fragments = ["java"], outputs = _dicts.add( common_outputs, - *[extra["outputs"] for extra in extras] + *[extra["outputs"] for extra in extras if "outputs" in extra] ), test = True, toolchains = ["@io_bazel_rules_scala//scala:toolchain_type"], diff --git a/test/phase/add_phase_to_all_rules/BUILD b/test/phase/add_phase_to_all_rules/BUILD deleted file mode 100644 index 82087cecf..000000000 --- a/test/phase/add_phase_to_all_rules/BUILD +++ /dev/null @@ -1,59 +0,0 @@ -load( - "//test/phase/add_phase_to_all_rules:customizability_test.bzl", - "add_phase_customizability_test_singleton", - "customizability_test_scala_binary", - "customizability_test_scala_junit_test", - "customizability_test_scala_library", - "customizability_test_scala_library_for_plugin_bootstrapping", - "customizability_test_scala_macro_library", - "customizability_test_scala_repl", - "customizability_test_scala_test", -) - -add_phase_customizability_test_singleton( - name = "phase_customizability_test", - visibility = ["//visibility:public"], -) - -customizability_test_scala_binary( - name = "HelloBinary", - srcs = ["HelloBinary.scala"], - main_class = "scalarules.test.phase.HelloBinary", -) - -customizability_test_scala_library( - name = "HelloLibrary", - srcs = ["HelloLibrary.scala"], - custom_content = "This is custom content in library", -) - -customizability_test_scala_library_for_plugin_bootstrapping( - name = "HelloLibraryForPluginBootstrapping", - srcs = ["HelloLibrary.scala"], - custom_content = "This is custom content in library_for_plugin_bootstrapping", -) - -customizability_test_scala_macro_library( - name = "HelloMacroLibrary", - srcs = ["HelloLibrary.scala"], - custom_content = "This is custom content in macro_library", -) - -customizability_test_scala_test( - name = "HelloTest", - srcs = ["HelloTest.scala"], - custom_content = "This is custom content in test", -) - -customizability_test_scala_junit_test( - name = "HelloJunitTest", - srcs = ["HelloJunitTest.scala"], - custom_content = "This is custom content in junit_test", - suffixes = ["Test"], -) - -customizability_test_scala_repl( - name = "HelloRepl", - srcs = ["HelloLibrary.scala"], - custom_content = "This is custom content in repl", -) diff --git a/test/phase/add_phase_to_all_rules/HelloJunitTest.scala b/test/phase/add_phase_to_all_rules/HelloJunitTest.scala deleted file mode 100644 index c4a48e9e5..000000000 --- a/test/phase/add_phase_to_all_rules/HelloJunitTest.scala +++ /dev/null @@ -1,9 +0,0 @@ -package scalarules.test.phase - -import org.junit.Test - -class HelloJunitTest { - @Test - def someTest: Unit = - println("passing") -} diff --git a/test/phase/add_phase_to_all_rules/HelloLibrary.scala b/test/phase/add_phase_to_all_rules/HelloLibrary.scala deleted file mode 100644 index 58737692a..000000000 --- a/test/phase/add_phase_to_all_rules/HelloLibrary.scala +++ /dev/null @@ -1,5 +0,0 @@ -package scalarules.test.phase - -object HelloLibrary { - val message = "You can customize library phases!" -} diff --git a/test/phase/add_phase_to_all_rules/customizability_test.bzl b/test/phase/add_phase_to_all_rules/customizability_test.bzl deleted file mode 100644 index b5886349e..000000000 --- a/test/phase/add_phase_to_all_rules/customizability_test.bzl +++ /dev/null @@ -1,60 +0,0 @@ -""" -This test makes sure custom phases can be inserted to the desired position through phase API -""" - -load( - "//scala:advanced_usage/providers.bzl", - _ScalaRulePhase = "ScalaRulePhase", -) -load( - "//scala:advanced_usage/scala.bzl", - _make_scala_binary = "make_scala_binary", - _make_scala_junit_test = "make_scala_junit_test", - _make_scala_library = "make_scala_library", - _make_scala_library_for_plugin_bootstrapping = "make_scala_library_for_plugin_bootstrapping", - _make_scala_macro_library = "make_scala_macro_library", - _make_scala_repl = "make_scala_repl", - _make_scala_test = "make_scala_test", -) -load( - "//test/phase/add_phase_to_all_rules:phase_customizability_test.bzl", - _phase_customizability_test = "phase_customizability_test", -) - -# Inputs for the customizable rules -ext_add_phase_customizability_test = { - "attrs": { - "custom_content": attr.string( - default = "This is custom content", - ), - }, - "outputs": { - "custom_output": "%{name}.custom-output", - }, - "phase_providers": [ - "//test/phase/add_phase_to_all_rules:phase_customizability_test", - ], -} - -# The rule implementation for phase provider -def _add_phase_customizability_test_singleton_implementation(ctx): - return [ - _ScalaRulePhase( - custom_phases = [ - ("$", "", "customizability_test", _phase_customizability_test), - ], - ), - ] - -# The rule for phase provider -add_phase_customizability_test_singleton = rule( - implementation = _add_phase_customizability_test_singleton_implementation, -) - -customizability_test_scala_binary = _make_scala_binary(ext_add_phase_customizability_test) -customizability_test_scala_library = _make_scala_library(ext_add_phase_customizability_test) -customizability_test_scala_library_for_plugin_bootstrapping = _make_scala_library_for_plugin_bootstrapping(ext_add_phase_customizability_test) -customizability_test_scala_macro_library = _make_scala_macro_library(ext_add_phase_customizability_test) -customizability_test_scala_test = _make_scala_test(ext_add_phase_customizability_test) -customizability_test_scala_junit_test = _make_scala_junit_test(ext_add_phase_customizability_test) -customizability_test_scala_repl = _make_scala_repl(ext_add_phase_customizability_test) diff --git a/test/phase/add_phase_to_all_rules/phase_customizability_test.bzl b/test/phase/add_phase_to_all_rules/phase_customizability_test.bzl deleted file mode 100644 index 15f01e09e..000000000 --- a/test/phase/add_phase_to_all_rules/phase_customizability_test.bzl +++ /dev/null @@ -1,10 +0,0 @@ -# -# PHASE: customizability test -# -# A dummy test phase to make sure rules are customizable -# -def phase_customizability_test(ctx, p): - ctx.actions.write( - output = ctx.outputs.custom_output, - content = ctx.attr.custom_content, - ) diff --git a/test/phase/add_to_all_rules/BUILD b/test/phase/add_to_all_rules/BUILD new file mode 100644 index 000000000..fa0b847be --- /dev/null +++ b/test/phase/add_to_all_rules/BUILD @@ -0,0 +1,59 @@ +load( + "//test/phase/add_to_all_rules:phase_add_to_all_rules_test.bzl", + "add_to_all_rules_scala_binary", + "add_to_all_rules_scala_junit_test", + "add_to_all_rules_scala_library", + "add_to_all_rules_scala_library_for_plugin_bootstrapping", + "add_to_all_rules_scala_macro_library", + "add_to_all_rules_scala_repl", + "add_to_all_rules_scala_test", + "add_to_all_rules_singleton", +) + +add_to_all_rules_singleton( + name = "phase_add_to_all_rules", + visibility = ["//visibility:public"], +) + +add_to_all_rules_scala_binary( + name = "PhaseBinary", + srcs = ["PhaseBinary.scala"], + main_class = "scalarules.test.phase.add_to_all_rules.PhaseBinary", +) + +add_to_all_rules_scala_library( + name = "PhaseLibrary", + srcs = ["PhaseLibrary.scala"], + custom_content = "This is custom content in library", +) + +add_to_all_rules_scala_library_for_plugin_bootstrapping( + name = "PhaseLibraryForPluginBootstrapping", + srcs = ["PhaseLibrary.scala"], + custom_content = "This is custom content in library_for_plugin_bootstrapping", +) + +add_to_all_rules_scala_macro_library( + name = "PhaseMacroLibrary", + srcs = ["PhaseLibrary.scala"], + custom_content = "This is custom content in macro_library", +) + +add_to_all_rules_scala_test( + name = "PhaseTest", + srcs = ["PhaseTest.scala"], + custom_content = "This is custom content in test", +) + +add_to_all_rules_scala_junit_test( + name = "PhaseJunitTest", + srcs = ["PhaseJunitTest.scala"], + custom_content = "This is custom content in junit_test", + suffixes = ["Test"], +) + +add_to_all_rules_scala_repl( + name = "PhaseRepl", + srcs = ["PhaseLibrary.scala"], + custom_content = "This is custom content in repl", +) diff --git a/test/phase/add_phase_to_all_rules/HelloBinary.scala b/test/phase/add_to_all_rules/PhaseBinary.scala similarity index 58% rename from test/phase/add_phase_to_all_rules/HelloBinary.scala rename to test/phase/add_to_all_rules/PhaseBinary.scala index 4ca6fa9a5..a1f7bf5da 100644 --- a/test/phase/add_phase_to_all_rules/HelloBinary.scala +++ b/test/phase/add_to_all_rules/PhaseBinary.scala @@ -1,6 +1,6 @@ -package scalarules.test.phase +package scalarules.test.phase.add_to_all_rules -object HelloBinary { +object PhaseBinary { def main(args: Array[String]) { val message = "You can customize binary phases!" } diff --git a/test/phase/add_to_all_rules/PhaseJunitTest.scala b/test/phase/add_to_all_rules/PhaseJunitTest.scala new file mode 100644 index 000000000..5ca350b86 --- /dev/null +++ b/test/phase/add_to_all_rules/PhaseJunitTest.scala @@ -0,0 +1,9 @@ +package scalarules.test.phase.add_to_all_rules + +import org.junit.Test + +class PhaseJunitTest { + @Test + def someTest: Unit = + val message = "You can customize junit test phases!" +} diff --git a/test/phase/add_to_all_rules/PhaseLibrary.scala b/test/phase/add_to_all_rules/PhaseLibrary.scala new file mode 100644 index 000000000..a1b8b1219 --- /dev/null +++ b/test/phase/add_to_all_rules/PhaseLibrary.scala @@ -0,0 +1,5 @@ +package scalarules.test.phase.add_to_all_rules + +object PhaseLibrary { + val message = "You can customize library phases!" +} diff --git a/test/phase/add_phase_to_all_rules/HelloTest.scala b/test/phase/add_to_all_rules/PhaseTest.scala similarity index 71% rename from test/phase/add_phase_to_all_rules/HelloTest.scala rename to test/phase/add_to_all_rules/PhaseTest.scala index 2df1d5879..ca0803a11 100644 --- a/test/phase/add_phase_to_all_rules/HelloTest.scala +++ b/test/phase/add_to_all_rules/PhaseTest.scala @@ -1,8 +1,8 @@ -package scalarules.test.phase +package scalarules.test.phase.add_to_all_rules import org.scalatest._ -class HelloTest extends FlatSpec { +class PhaseTest extends FlatSpec { val message = "You can customize test phases!" "HelloTest" should "be able to customize test phases!" in { assert(message.equals("You can customize test phases!")) diff --git a/test/phase/add_to_all_rules/phase_add_to_all_rules.bzl b/test/phase/add_to_all_rules/phase_add_to_all_rules.bzl new file mode 100644 index 000000000..aa89d08ec --- /dev/null +++ b/test/phase/add_to_all_rules/phase_add_to_all_rules.bzl @@ -0,0 +1,10 @@ +# +# PHASE: add to all rules +# +# A dummy test phase to make sure phase is working for all rules +# +def phase_add_to_all_rules(ctx, p): + ctx.actions.write( + output = ctx.outputs.custom_output, + content = ctx.attr.custom_content, + ) diff --git a/test/phase/add_to_all_rules/phase_add_to_all_rules_test.bzl b/test/phase/add_to_all_rules/phase_add_to_all_rules_test.bzl new file mode 100644 index 000000000..2d9b4c5fc --- /dev/null +++ b/test/phase/add_to_all_rules/phase_add_to_all_rules_test.bzl @@ -0,0 +1,60 @@ +""" +This test makes sure custom phases can be inserted to the desired position through phase API +""" + +load( + "//scala:advanced_usage/providers.bzl", + _ScalaRulePhase = "ScalaRulePhase", +) +load( + "//scala:advanced_usage/scala.bzl", + _make_scala_binary = "make_scala_binary", + _make_scala_junit_test = "make_scala_junit_test", + _make_scala_library = "make_scala_library", + _make_scala_library_for_plugin_bootstrapping = "make_scala_library_for_plugin_bootstrapping", + _make_scala_macro_library = "make_scala_macro_library", + _make_scala_repl = "make_scala_repl", + _make_scala_test = "make_scala_test", +) +load( + "//test/phase/add_to_all_rules:phase_add_to_all_rules.bzl", + _phase_add_to_all_rules = "phase_add_to_all_rules", +) + +# Inputs for the customizable rules +ext_add_to_all_rules = { + "attrs": { + "custom_content": attr.string( + default = "This is custom content", + ), + }, + "outputs": { + "custom_output": "%{name}.custom-output", + }, + "phase_providers": [ + "//test/phase/add_to_all_rules:phase_add_to_all_rules", + ], +} + +# The rule implementation for phase provider +def _add_to_all_rules_singleton_implementation(ctx): + return [ + _ScalaRulePhase( + custom_phases = [ + ("last", "", "add_to_all_rules", _phase_add_to_all_rules), + ], + ), + ] + +# The rule for phase provider +add_to_all_rules_singleton = rule( + implementation = _add_to_all_rules_singleton_implementation, +) + +add_to_all_rules_scala_binary = _make_scala_binary(ext_add_to_all_rules) +add_to_all_rules_scala_library = _make_scala_library(ext_add_to_all_rules) +add_to_all_rules_scala_library_for_plugin_bootstrapping = _make_scala_library_for_plugin_bootstrapping(ext_add_to_all_rules) +add_to_all_rules_scala_macro_library = _make_scala_macro_library(ext_add_to_all_rules) +add_to_all_rules_scala_test = _make_scala_test(ext_add_to_all_rules) +add_to_all_rules_scala_junit_test = _make_scala_junit_test(ext_add_to_all_rules) +add_to_all_rules_scala_repl = _make_scala_repl(ext_add_to_all_rules) diff --git a/test/phase/adjustment/BUILD b/test/phase/adjustment/BUILD new file mode 100644 index 000000000..947440aa0 --- /dev/null +++ b/test/phase/adjustment/BUILD @@ -0,0 +1,27 @@ +load( + "//test/phase/adjustment:phase_adjustment_test.bzl", + "adjustment_replace_scala_library", + "adjustment_replace_singleton", + "adjustment_scala_library", + "adjustment_singleton", +) + +adjustment_singleton( + name = "phase_adjustment", + visibility = ["//visibility:public"], +) + +adjustment_replace_singleton( + name = "phase_adjustment_replace", + visibility = ["//visibility:public"], +) + +adjustment_scala_library( + name = "PhaseLibrary", + srcs = ["PhaseLibrary.scala"], +) + +adjustment_replace_scala_library( + name = "PhaseLibraryReplace", + srcs = ["PhaseLibrary.scala"], +) diff --git a/test/phase/adjustment/PhaseLibrary.scala b/test/phase/adjustment/PhaseLibrary.scala new file mode 100644 index 000000000..20e661203 --- /dev/null +++ b/test/phase/adjustment/PhaseLibrary.scala @@ -0,0 +1,5 @@ +package scalarules.test.phase.adjustment + +object PhaseLibrary { + val message = "You can customize library phases!" +} diff --git a/test/phase/adjustment/phase_adjustment.bzl b/test/phase/adjustment/phase_adjustment.bzl new file mode 100644 index 000000000..ae47fd7d4 --- /dev/null +++ b/test/phase/adjustment/phase_adjustment.bzl @@ -0,0 +1,42 @@ +# +# PHASE: adjustment test +# +# Dummy test phases to make sure phase adjustment is working +# +def phase_first(ctx, p): + return struct( + info_first = "info from phase_first", + ) + +def phase_second(ctx, p): + return struct( + info_first = "phase_second redirect " + p.first.info_first, + info_second = "info from phase_second", + ) + +def phase_third(ctx, p): + ctx.actions.write( + output = ctx.outputs.custom_output, + content = "{} {} {}".format(p.first.info_first, p.second.info_first, p.second.info_second), + ) + +def phase_replace(ctx, p): + return struct( + info = "expected info from phase_replace", + ) + +def phase_being_replaced(ctx, p): + return struct( + info = "unexpected info from phase_being_replaced", + ) + +def phase_check_replacement(ctx, p): + final_info = "" + if getattr(p, "replace"): + final_info += p.replace.info + if hasattr(p, "being_replaced"): + final_info += p.being_replaced.info + ctx.actions.write( + output = ctx.outputs.custom_output, + content = "{} we should only see one info".format(final_info), + ) diff --git a/test/phase/adjustment/phase_adjustment_test.bzl b/test/phase/adjustment/phase_adjustment_test.bzl new file mode 100644 index 000000000..5d6699a4c --- /dev/null +++ b/test/phase/adjustment/phase_adjustment_test.bzl @@ -0,0 +1,79 @@ +""" +This test makes sure custom phases can be inserted to the desired position through phase API +""" + +load( + "//scala:advanced_usage/providers.bzl", + _ScalaRulePhase = "ScalaRulePhase", +) +load( + "//scala:advanced_usage/scala.bzl", + _make_scala_library = "make_scala_library", +) +load( + "//test/phase/adjustment:phase_adjustment.bzl", + _phase_being_replaced = "phase_being_replaced", + _phase_check_replacement = "phase_check_replacement", + _phase_first = "phase_first", + _phase_replace = "phase_replace", + _phase_second = "phase_second", + _phase_third = "phase_third", +) + +# Inputs for the customizable rules +ext_adjustment = { + "outputs": { + "custom_output": "%{name}.custom-output", + }, + "phase_providers": [ + "//test/phase/adjustment:phase_adjustment", + ], +} + +# The rule implementation for phase provider +def _adjustment_singleton_implementation(ctx): + return [ + _ScalaRulePhase( + custom_phases = [ + ("last", "", "second", _phase_second), + ("before", "second", "first", _phase_first), + ("after", "second", "third", _phase_third), + ], + ), + ] + +# The rule for phase provider +adjustment_singleton = rule( + implementation = _adjustment_singleton_implementation, +) + +adjustment_scala_library = _make_scala_library(ext_adjustment) + +# Inputs for the customizable rules +ext_adjustment_replace = { + "outputs": { + "custom_output": "%{name}.custom-output", + }, + "phase_providers": [ + "//test/phase/adjustment:phase_adjustment_replace", + ], +} + +# The rule implementation for phase provider +def _adjustment_replace_singleton_implementation(ctx): + return [ + _ScalaRulePhase( + custom_phases = [ + ("last", "", "check_replacement", _phase_check_replacement), + ("before", "check_replacement", "being_replaced", _phase_being_replaced), + ("replace", "being_replaced", "replace", _phase_replace), + ], + ), + ] + +# The rule for phase provider +adjustment_replace_singleton = rule( + implementation = _adjustment_replace_singleton_implementation, +) + +adjustment_replace_scala_library = _make_scala_library(ext_adjustment_replace) diff --git a/test/shell/test_phase.sh b/test/shell/test_phase.sh index 95c26e1fd..110f752ac 100755 --- a/test/shell/test_phase.sh +++ b/test/shell/test_phase.sh @@ -28,43 +28,55 @@ output_file_should_contain_message() { test_scala_binary_with_extra_phase() { output_file_should_contain_message \ "This is custom content" \ - build //test/phase/add_phase_to_all_rules:HelloBinary.custom-output + build //test/phase/add_to_all_rules:PhaseBinary.custom-output } test_scala_library_with_extra_phase_and_custom_content() { output_file_should_contain_message \ "This is custom content in library" \ - build //test/phase/add_phase_to_all_rules:HelloLibrary.custom-output + build //test/phase/add_to_all_rules:PhaseLibrary.custom-output } test_scala_library_for_plugin_bootstrapping_with_extra_phase_and_custom_content() { output_file_should_contain_message \ "This is custom content in library_for_plugin_bootstrapping" \ - build //test/phase/add_phase_to_all_rules:HelloLibraryForPluginBootstrapping.custom-output + build //test/phase/add_to_all_rules:PhaseLibraryForPluginBootstrapping.custom-output } test_scala_macro_library_with_extra_phase_and_custom_content() { output_file_should_contain_message \ "This is custom content in macro_library" \ - build //test/phase/add_phase_to_all_rules:HelloMacroLibrary.custom-output + build //test/phase/add_to_all_rules:PhaseMacroLibrary.custom-output } test_scala_test_with_extra_phase_and_custom_content() { output_file_should_contain_message \ "This is custom content in test" \ - build //test/phase/add_phase_to_all_rules:HelloTest.custom-output + build //test/phase/add_to_all_rules:PhaseTest.custom-output } test_scala_junit_test_with_extra_phase_and_custom_content() { output_file_should_contain_message \ "This is custom content in junit_test" \ - build //test/phase/add_phase_to_all_rules:HelloJunitTest.custom-output + build //test/phase/add_to_all_rules:PhaseJunitTest.custom-output } test_scala_repl_with_extra_phase_and_custom_content() { output_file_should_contain_message \ "This is custom content in repl" \ - build //test/phase/add_phase_to_all_rules:HelloRepl.custom-output + build //test/phase/add_to_all_rules:PhaseRepl.custom-output +} + +test_phase_adjustment_and_global_provider() { + output_file_should_contain_message \ + "info from phase_first phase_second redirect info from phase_first info from phase_second" \ + build //test/phase/adjustment:PhaseLibrary.custom-output +} + +test_phase_adjustment_replace() { + output_file_should_contain_message \ + "expected info from phase_replace we should only see one info" \ + build //test/phase/adjustment:PhaseLibraryReplace.custom-output } $runner test_scala_binary_with_extra_phase @@ -74,3 +86,5 @@ $runner test_scala_macro_library_with_extra_phase_and_custom_content $runner test_scala_test_with_extra_phase_and_custom_content $runner test_scala_junit_test_with_extra_phase_and_custom_content $runner test_scala_repl_with_extra_phase_and_custom_content +$runner test_phase_adjustment_and_global_provider +$runner test_phase_adjustment_replace From 5b6f4ae0d6ae23faf3daa662ac47fdc73e7eaf4e Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Tue, 3 Dec 2019 11:42:41 -0700 Subject: [PATCH 29/32] Fix junit test --- test/phase/add_to_all_rules/PhaseJunitTest.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/phase/add_to_all_rules/PhaseJunitTest.scala b/test/phase/add_to_all_rules/PhaseJunitTest.scala index 5ca350b86..bc20bbc44 100644 --- a/test/phase/add_to_all_rules/PhaseJunitTest.scala +++ b/test/phase/add_to_all_rules/PhaseJunitTest.scala @@ -4,6 +4,7 @@ import org.junit.Test class PhaseJunitTest { @Test - def someTest: Unit = + def someTest: Unit = { val message = "You can customize junit test phases!" + } } From a581634ae1532cec37b568f3d6ea7c16c31ae74b Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Thu, 19 Dec 2019 12:26:53 -0700 Subject: [PATCH 30/32] Fix doc --- docs/customizable_phase.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/customizable_phase.md b/docs/customizable_phase.md index 39b89c0ee..5d53ac359 100644 --- a/docs/customizable_phase.md +++ b/docs/customizable_phase.md @@ -13,12 +13,13 @@ Phases increase configurability. Rule implementations are defined as a list of p The biggest benefit of phases is that it is customizable. If default phase A is not doing what you expect, you may switch it with your self-defined phase A. One use case is to write your own compilation phase with your favorite Scala compiler. You may also extend the default phase list for more functionalities. One use case is to check the Scala format. ## Who needs customizable phase -Customizable phase is an advanced feature for people who want the rules to do more. If you are an experienced Bazel rules developer, we make this powerful API public for you to do custom work without impacting other consumers. If you have no experience on writing Bazel rules, we are happy to help but be aware it may be frucstrating at first. +Customizable phase is an advanced feature for people who want the rules to do more. If you are an experienced Bazel rules developer, we make this powerful API public for you to do custom work without impacting other consumers. If you have no experience on writing Bazel rules, we are happy to help but be aware it may be frustrating at first. If you don't need to customize your rules and just need the default setup to work correctly, then just load the following file for default rules: ``` load("@io_bazel_rules_scala//scala:scala.bzl") ``` +Otherwise read on: ## As a consumer You need to load the following 2 files: From 4f3dd6450cc779bfb3786f2f2ed9f0795b977fba Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Thu, 19 Dec 2019 13:46:24 -0700 Subject: [PATCH 31/32] Change _test_ to _scalatest_ --- scala/private/phases/phase_collect_jars.bzl | 2 +- scala/private/phases/phase_compile.bzl | 2 +- scala/private/phases/phase_final.bzl | 2 +- scala/private/phases/phase_runfiles.bzl | 2 +- .../private/phases/phase_write_executable.bzl | 2 +- scala/private/phases/phases.bzl | 20 +++++++++---------- scala/private/rules/scala_test.bzl | 20 +++++++++---------- 7 files changed, 25 insertions(+), 25 deletions(-) diff --git a/scala/private/phases/phase_collect_jars.bzl b/scala/private/phases/phase_collect_jars.bzl index 20b7cf1a3..19c22feee 100644 --- a/scala/private/phases/phase_collect_jars.bzl +++ b/scala/private/phases/phase_collect_jars.bzl @@ -8,7 +8,7 @@ load( "collect_jars_from_common_ctx", ) -def phase_test_collect_jars(ctx, p): +def phase_scalatest_collect_jars(ctx, p): args = struct( base_classpath = p.scalac_provider.default_classpath + [ctx.attr._scalatest], extra_runtime_deps = [ diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index dc3afc950..b29ca266c 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -83,7 +83,7 @@ def phase_repl_compile(ctx, p): ) return _phase_default_compile(ctx, p, args) -def phase_test_compile(ctx, p): +def phase_scalatest_compile(ctx, p): args = struct( buildijar = False, unused_dependency_checker_ignored_targets = [ diff --git a/scala/private/phases/phase_final.bzl b/scala/private/phases/phase_final.bzl index 771002dcb..e0c2e16c4 100644 --- a/scala/private/phases/phase_final.bzl +++ b/scala/private/phases/phase_final.bzl @@ -25,7 +25,7 @@ def phase_library_final(ctx, p): scala = p.scala_provider, ) -def phase_test_final(ctx, p): +def phase_scalatest_final(ctx, p): coverage_runfiles = p.coverage_runfiles.coverage_runfiles coverage_runfiles.extend(p.write_executable) return struct( diff --git a/scala/private/phases/phase_runfiles.bzl b/scala/private/phases/phase_runfiles.bzl index a1c8317d6..db5e6e8a2 100644 --- a/scala/private/phases/phase_runfiles.bzl +++ b/scala/private/phases/phase_runfiles.bzl @@ -10,7 +10,7 @@ def phase_library_runfiles(ctx, p): ) return _phase_default_runfiles(ctx, p, args) -def phase_test_runfiles(ctx, p): +def phase_scalatest_runfiles(ctx, p): args = "\n".join([ "-R", ctx.outputs.jar.short_path, diff --git a/scala/private/phases/phase_write_executable.bzl b/scala/private/phases/phase_write_executable.bzl index fea230d1a..a678c1ea2 100644 --- a/scala/private/phases/phase_write_executable.bzl +++ b/scala/private/phases/phase_write_executable.bzl @@ -10,7 +10,7 @@ load( "write_executable", ) -def phase_test_write_executable(ctx, p): +def phase_scalatest_write_executable(ctx, p): # jvm_flags passed in on the target override scala_test_jvm_flags passed in on the # toolchain final_jvm_flags = first_non_empty( diff --git a/scala/private/phases/phases.bzl b/scala/private/phases/phases.bzl index 6e02a09e1..c1b4e4b9a 100644 --- a/scala/private/phases/phases.bzl +++ b/scala/private/phases/phases.bzl @@ -12,7 +12,7 @@ load( _phase_common_write_executable = "phase_common_write_executable", _phase_junit_test_write_executable = "phase_junit_test_write_executable", _phase_repl_write_executable = "phase_repl_write_executable", - _phase_test_write_executable = "phase_test_write_executable", + _phase_scalatest_write_executable = "phase_scalatest_write_executable", ) load( "@io_bazel_rules_scala//scala/private:phases/phase_java_wrapper.bzl", @@ -26,7 +26,7 @@ load( _phase_library_for_plugin_bootstrapping_collect_jars = "phase_library_for_plugin_bootstrapping_collect_jars", _phase_macro_library_collect_jars = "phase_macro_library_collect_jars", _phase_repl_collect_jars = "phase_repl_collect_jars", - _phase_test_collect_jars = "phase_test_collect_jars", + _phase_scalatest_collect_jars = "phase_scalatest_collect_jars", ) load( "@io_bazel_rules_scala//scala/private:phases/phase_compile.bzl", @@ -37,7 +37,7 @@ load( _phase_library_for_plugin_bootstrapping_compile = "phase_library_for_plugin_bootstrapping_compile", _phase_macro_library_compile = "phase_macro_library_compile", _phase_repl_compile = "phase_repl_compile", - _phase_test_compile = "phase_test_compile", + _phase_scalatest_compile = "phase_scalatest_compile", ) load( "@io_bazel_rules_scala//scala/private:phases/phase_scala_provider.bzl", @@ -48,13 +48,13 @@ load( "@io_bazel_rules_scala//scala/private:phases/phase_runfiles.bzl", _phase_common_runfiles = "phase_common_runfiles", _phase_library_runfiles = "phase_library_runfiles", - _phase_test_runfiles = "phase_test_runfiles", + _phase_scalatest_runfiles = "phase_scalatest_runfiles", ) load( "@io_bazel_rules_scala//scala/private:phases/phase_final.bzl", _phase_binary_final = "phase_binary_final", _phase_library_final = "phase_library_final", - _phase_test_final = "phase_test_final", + _phase_scalatest_final = "phase_scalatest_final", ) load("@io_bazel_rules_scala//scala/private:phases/phase_scalac_provider.bzl", _phase_scalac_provider = "phase_scalac_provider") load("@io_bazel_rules_scala//scala/private:phases/phase_write_manifest.bzl", _phase_write_manifest = "phase_write_manifest") @@ -98,7 +98,7 @@ phase_jvm_flags = _phase_jvm_flags phase_coverage_runfiles = _phase_coverage_runfiles # write_executable -phase_test_write_executable = _phase_test_write_executable +phase_scalatest_write_executable = _phase_scalatest_write_executable phase_repl_write_executable = _phase_repl_write_executable phase_junit_test_write_executable = _phase_junit_test_write_executable phase_common_write_executable = _phase_common_write_executable @@ -108,7 +108,7 @@ phase_repl_java_wrapper = _phase_repl_java_wrapper phase_common_java_wrapper = _phase_common_java_wrapper # collect_jars -phase_test_collect_jars = _phase_test_collect_jars +phase_scalatest_collect_jars = _phase_scalatest_collect_jars phase_repl_collect_jars = _phase_repl_collect_jars phase_macro_library_collect_jars = _phase_macro_library_collect_jars phase_junit_test_collect_jars = _phase_junit_test_collect_jars @@ -122,7 +122,7 @@ phase_library_for_plugin_bootstrapping_compile = _phase_library_for_plugin_boots phase_macro_library_compile = _phase_macro_library_compile phase_junit_test_compile = _phase_junit_test_compile phase_repl_compile = _phase_repl_compile -phase_test_compile = _phase_test_compile +phase_scalatest_compile = _phase_scalatest_compile phase_common_compile = _phase_common_compile # scala_provider @@ -131,10 +131,10 @@ phase_common_scala_provider = _phase_common_scala_provider # runfiles phase_library_runfiles = _phase_library_runfiles -phase_test_runfiles = _phase_test_runfiles +phase_scalatest_runfiles = _phase_scalatest_runfiles phase_common_runfiles = _phase_common_runfiles # final phase_binary_final = _phase_binary_final phase_library_final = _phase_library_final -phase_test_final = _phase_test_final +phase_scalatest_final = _phase_scalatest_final diff --git a/scala/private/rules/scala_test.bzl b/scala/private/rules/scala_test.bzl index e3927e8e5..900bd2faa 100644 --- a/scala/private/rules/scala_test.bzl +++ b/scala/private/rules/scala_test.bzl @@ -18,11 +18,11 @@ load( "phase_declare_executable", "phase_merge_jars", "phase_scalac_provider", - "phase_test_collect_jars", - "phase_test_compile", - "phase_test_final", - "phase_test_runfiles", - "phase_test_write_executable", + "phase_scalatest_collect_jars", + "phase_scalatest_compile", + "phase_scalatest_final", + "phase_scalatest_runfiles", + "phase_scalatest_write_executable", "phase_unused_deps_checker", "phase_write_manifest", "run_phases", @@ -36,19 +36,19 @@ def _scala_test_impl(ctx): ("scalac_provider", phase_scalac_provider), ("write_manifest", phase_write_manifest), ("unused_deps_checker", phase_unused_deps_checker), - ("collect_jars", phase_test_collect_jars), + ("collect_jars", phase_scalatest_collect_jars), ("java_wrapper", phase_common_java_wrapper), ("declare_executable", phase_declare_executable), # no need to build an ijar for an executable - ("compile", phase_test_compile), + ("compile", phase_scalatest_compile), ("merge_jars", phase_merge_jars), - ("runfiles", phase_test_runfiles), + ("runfiles", phase_scalatest_runfiles), ("scala_provider", phase_common_scala_provider), ("coverage_runfiles", phase_coverage_runfiles), - ("write_executable", phase_test_write_executable), + ("write_executable", phase_scalatest_write_executable), ], # fixed phase - ("final", phase_test_final), + ("final", phase_scalatest_final), ).final _scala_test_attrs = { From c36cdd0982eb8f45b9f515efef8abc240e508518 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Thu, 19 Dec 2019 14:19:44 -0700 Subject: [PATCH 32/32] More doc on provider --- scala/advanced_usage/providers.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/advanced_usage/providers.bzl b/scala/advanced_usage/providers.bzl index deeeeefa2..da329eccb 100644 --- a/scala/advanced_usage/providers.bzl +++ b/scala/advanced_usage/providers.bzl @@ -6,6 +6,6 @@ It is used only when you intend to add functionalities to existing default rules ScalaRulePhase = provider( doc = "A custom phase plugin", fields = { - "custom_phases": "the phases to add", + "custom_phases": "The phases to add. It takes an array of (relation, peer_name, phase_name, phase_function). Please refer to docs/customizable_phase.md for more details.", }, )