From 76631f8a49b35f35df732bde0811737727b5676a Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Wed, 22 Jan 2020 07:46:12 +0200 Subject: [PATCH] final phase is inlined all phases are equal DefaultInfo is returned by a default_info phase Other external providers are passed by their respective phases --- docs/customizable_phase.md | 6 ++-- scala/private/phases/api.bzl | 6 ++-- scala/private/phases/phase_collect_jars.bzl | 1 + scala/private/phases/phase_compile.bzl | 1 + scala/private/phases/phase_default_info.bzl | 36 +++++++++++++++++++++ scala/private/phases/phase_final.bzl | 27 ---------------- scala/private/phases/phases.bzl | 16 ++++----- scala/private/rules/scala_binary.bzl | 5 ++- scala/private/rules/scala_junit_test.bzl | 5 ++- scala/private/rules/scala_library.bzl | 11 +++---- scala/private/rules/scala_repl.bzl | 5 ++- scala/private/rules/scala_test.bzl | 5 ++- 12 files changed, 64 insertions(+), 60 deletions(-) create mode 100644 scala/private/phases/phase_default_info.bzl delete mode 100644 scala/private/phases/phase_final.bzl diff --git a/docs/customizable_phase.md b/docs/customizable_phase.md index 5d53ac359..58a81648c 100644 --- a/docs/customizable_phase.md +++ b/docs/customizable_phase.md @@ -128,14 +128,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 consumers from accidently removing `phase_final` from the list, we make it a non-customizable phase. +If you need to expose providers to downstream targets you need to return an array of providers from your phase under the `external_providers` attribute. + +In each of the rule implementations, it calls `run_phases` and returns an accumulated `external_providers` array declared by the phases. 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 diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl index 8b2fc7ec9..a1bd4a60c 100644 --- a/scala/private/phases/api.bzl +++ b/scala/private/phases/api.bzl @@ -41,7 +41,7 @@ def _adjust_phases(phases, adjustments): return phases # Execute phases -def run_phases(ctx, builtin_customizable_phases, fixed_phase): +def run_phases(ctx, builtin_customizable_phases): # Loading custom phases # Phases must be passed in by provider phase_providers = [ @@ -64,7 +64,7 @@ def run_phases(ctx, builtin_customizable_phases, fixed_phase): global_provider = {} current_provider = struct(**global_provider) acculmulated_external_providers = [] - for (name, function) in adjusted_phases + [fixed_phase]: + for (name, function) in adjusted_phases: # Run a phase new_provider = function(ctx, current_provider) @@ -77,7 +77,7 @@ def run_phases(ctx, builtin_customizable_phases, fixed_phase): current_provider = struct(**global_provider) # The final return of rules implementation - return acculmulated_external_providers + current_provider.final + return acculmulated_external_providers # A method to pass in phase provider def extras_phases(extras): diff --git a/scala/private/phases/phase_collect_jars.bzl b/scala/private/phases/phase_collect_jars.bzl index 086975ffd..43183826d 100644 --- a/scala/private/phases/phase_collect_jars.bzl +++ b/scala/private/phases/phase_collect_jars.bzl @@ -108,6 +108,7 @@ def _phase_collect_jars( transitive_compile_jars = transitive_compile_jars, transitive_runtime_jars = transitive_rjars, deps_providers = deps_providers, + external_providers = [jars2labels], ) def _collect_runtime_jars(dep_targets): diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index ca846e81a..a3dacc480 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -170,6 +170,7 @@ def _phase_compile( java_jar = out.java_jar, source_jars = _pack_source_jars(ctx) + out.source_jars, merged_provider = out.merged_provider, + external_providers = [out.merged_provider] + out.coverage.providers, ) def _compile_or_empty( diff --git a/scala/private/phases/phase_default_info.bzl b/scala/private/phases/phase_default_info.bzl new file mode 100644 index 000000000..4f5731515 --- /dev/null +++ b/scala/private/phases/phase_default_info.bzl @@ -0,0 +1,36 @@ +# +# PHASE: default_info +# +# DOCUMENT THIS +# +def phase_binary_default_info(ctx, p): + return struct( + external_providers = [ + DefaultInfo( + executable = p.declare_executable, + files = depset([p.declare_executable] + p.compile.full_jars), + runfiles = p.runfiles.runfiles, + ), + ], + ) + +def phase_library_default_info(ctx, p): + return struct( + external_providers = [ + DefaultInfo( + files = depset(p.compile.full_jars), + runfiles = p.runfiles.runfiles, + ), + ], + ) + +def phase_scalatest_default_info(ctx, p): + return struct( + external_providers = [ + DefaultInfo( + executable = p.declare_executable, + files = depset([p.declare_executable] + p.compile.full_jars), + runfiles = ctx.runfiles(p.coverage_runfiles.coverage_runfiles, transitive_files = p.runfiles.runfiles.files), + ), + ], + ) diff --git a/scala/private/phases/phase_final.bzl b/scala/private/phases/phase_final.bzl deleted file mode 100644 index 00d1bd63b..000000000 --- a/scala/private/phases/phase_final.bzl +++ /dev/null @@ -1,27 +0,0 @@ -# -# PHASE: final -# -# DOCUMENT THIS -# -def phase_binary_final(ctx, p): - defaultInfo = DefaultInfo( - executable = p.declare_executable, - files = depset([p.declare_executable] + p.compile.full_jars), - runfiles = p.runfiles.runfiles, - ) - return [defaultInfo, p.compile.merged_provider, p.collect_jars.jars2labels] + p.compile.coverage.providers - -def phase_library_final(ctx, p): - defaultInfo = DefaultInfo( - files = depset(p.compile.full_jars), - runfiles = p.runfiles.runfiles, - ) - return [defaultInfo, p.compile.merged_provider, p.collect_jars.jars2labels] + p.compile.coverage.providers - -def phase_scalatest_final(ctx, p): - defaultInfo = DefaultInfo( - executable = p.declare_executable, - files = depset([p.declare_executable] + p.compile.full_jars), - runfiles = ctx.runfiles(p.coverage_runfiles.coverage_runfiles, transitive_files = p.runfiles.runfiles.files), - ) - return [defaultInfo, p.compile.merged_provider, p.collect_jars.jars2labels] + p.compile.coverage.providers diff --git a/scala/private/phases/phases.bzl b/scala/private/phases/phases.bzl index 95e68b575..6a9b4141f 100644 --- a/scala/private/phases/phases.bzl +++ b/scala/private/phases/phases.bzl @@ -46,10 +46,10 @@ load( _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_scalatest_final = "phase_scalatest_final", + "@io_bazel_rules_scala//scala/private:phases/phase_default_info.bzl", + _phase_binary_default_info = "phase_binary_default_info", + _phase_library_default_info = "phase_library_default_info", + _phase_scalatest_default_info = "phase_scalatest_default_info", ) 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") @@ -125,7 +125,7 @@ phase_library_runfiles = _phase_library_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_scalatest_final = _phase_scalatest_final +# default_info +phase_binary_default_info = _phase_binary_default_info +phase_library_default_info = _phase_library_default_info +phase_scalatest_default_info = _phase_scalatest_default_info diff --git a/scala/private/rules/scala_binary.bzl b/scala/private/rules/scala_binary.bzl index e97027fde..dc9cc801c 100644 --- a/scala/private/rules/scala_binary.bzl +++ b/scala/private/rules/scala_binary.bzl @@ -13,7 +13,7 @@ load( "@io_bazel_rules_scala//scala/private:phases/phases.bzl", "extras_phases", "phase_binary_compile", - "phase_binary_final", + "phase_binary_default_info", "phase_common_collect_jars", "phase_common_java_wrapper", "phase_common_runfiles", @@ -42,9 +42,8 @@ def _scala_binary_impl(ctx): ("merge_jars", phase_merge_jars), ("runfiles", phase_common_runfiles), ("write_executable", phase_common_write_executable), + ("default_info", phase_binary_default_info), ], - # fixed phase - ("final", phase_binary_final), ) _scala_binary_attrs = { diff --git a/scala/private/rules/scala_junit_test.bzl b/scala/private/rules/scala_junit_test.bzl index 5c7806da9..bf8ad4482 100644 --- a/scala/private/rules/scala_junit_test.bzl +++ b/scala/private/rules/scala_junit_test.bzl @@ -11,7 +11,7 @@ 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_binary_default_info", "phase_common_java_wrapper", "phase_common_runfiles", "phase_declare_executable", @@ -47,9 +47,8 @@ def _scala_junit_test_impl(ctx): ("runfiles", phase_common_runfiles), ("jvm_flags", phase_jvm_flags), ("write_executable", phase_junit_test_write_executable), + ("default_info", phase_binary_default_info), ], - # fixed phase - ("final", phase_binary_final), ) _scala_junit_test_attrs = { diff --git a/scala/private/rules/scala_library.bzl b/scala/private/rules/scala_library.bzl index 1cec2c52e..e547c4801 100644 --- a/scala/private/rules/scala_library.bzl +++ b/scala/private/rules/scala_library.bzl @@ -22,7 +22,7 @@ load( "phase_collect_srcjars", "phase_common_collect_jars", "phase_library_compile", - "phase_library_final", + "phase_library_default_info", "phase_library_for_plugin_bootstrapping_collect_jars", "phase_library_for_plugin_bootstrapping_compile", "phase_library_runfiles", @@ -66,9 +66,8 @@ def _scala_library_impl(ctx): ("merge_jars", phase_merge_jars), ("runfiles", phase_library_runfiles), ("collect_exports_jars", phase_collect_exports_jars), + ("default_info", phase_library_default_info), ], - # fixed phase - ("final", phase_library_final), ) _scala_library_attrs = {} @@ -143,9 +142,8 @@ def _scala_library_for_plugin_bootstrapping_impl(ctx): ("merge_jars", phase_merge_jars), ("runfiles", phase_library_runfiles), ("collect_exports_jars", phase_collect_exports_jars), + ("default_info", phase_library_default_info), ], - # fixed phase - ("final", phase_library_final), ) # the scala compiler plugin used for dependency analysis is compiled using `scala_library`. @@ -199,9 +197,8 @@ def _scala_macro_library_impl(ctx): ("merge_jars", phase_merge_jars), ("runfiles", phase_library_runfiles), ("collect_exports_jars", phase_collect_exports_jars), + ("default_info", phase_library_default_info), ], - # fixed phase - ("final", phase_library_final), ) _scala_macro_library_attrs = { diff --git a/scala/private/rules/scala_repl.bzl b/scala/private/rules/scala_repl.bzl index 59928d855..be753d67b 100644 --- a/scala/private/rules/scala_repl.bzl +++ b/scala/private/rules/scala_repl.bzl @@ -12,7 +12,7 @@ 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_binary_default_info", "phase_common_runfiles", "phase_declare_executable", "phase_merge_jars", @@ -43,9 +43,8 @@ def _scala_repl_impl(ctx): ("merge_jars", phase_merge_jars), ("runfiles", phase_common_runfiles), ("write_executable", phase_repl_write_executable), + ("default_info", phase_binary_default_info), ], - # fixed phase - ("final", phase_binary_final), ) _scala_repl_attrs = { diff --git a/scala/private/rules/scala_test.bzl b/scala/private/rules/scala_test.bzl index 2f053a512..afc2b19dc 100644 --- a/scala/private/rules/scala_test.bzl +++ b/scala/private/rules/scala_test.bzl @@ -19,7 +19,7 @@ load( "phase_scalac_provider", "phase_scalatest_collect_jars", "phase_scalatest_compile", - "phase_scalatest_final", + "phase_scalatest_default_info", "phase_scalatest_runfiles", "phase_scalatest_write_executable", "phase_unused_deps_checker", @@ -44,9 +44,8 @@ def _scala_test_impl(ctx): ("runfiles", phase_scalatest_runfiles), ("coverage_runfiles", phase_coverage_runfiles), ("write_executable", phase_scalatest_write_executable), + ("default_info", phase_scalatest_default_info), ], - # fixed phase - ("final", phase_scalatest_final), ) _scala_test_attrs = {