Skip to content

Commit

Permalink
final phase is inlined (bazelbuild#947)
Browse files Browse the repository at this point in the history
all phases are equal
DefaultInfo is returned by a default_info phase
Other external providers are passed by their respective phases
  • Loading branch information
ittaiz authored and Andre Rocha committed Jul 6, 2020
1 parent 7fd79d1 commit 317214c
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 60 deletions.
6 changes: 3 additions & 3 deletions docs/customizable_phase.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_<PHASE_NAME>.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_<PHASE_NAME>.bzl`: phase definition file
Expand Down
6 changes: 3 additions & 3 deletions scala/private/phases/api.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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)

Expand All @@ -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):
Expand Down
1 change: 1 addition & 0 deletions scala/private/phases/phase_collect_jars.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions scala/private/phases/phase_compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
36 changes: 36 additions & 0 deletions scala/private/phases/phase_default_info.bzl
Original file line number Diff line number Diff line change
@@ -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),
),
],
)
27 changes: 0 additions & 27 deletions scala/private/phases/phase_final.bzl

This file was deleted.

16 changes: 8 additions & 8 deletions scala/private/phases/phases.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
5 changes: 2 additions & 3 deletions scala/private/rules/scala_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 = {
Expand Down
5 changes: 2 additions & 3 deletions scala/private/rules/scala_junit_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 = {
Expand Down
11 changes: 4 additions & 7 deletions scala/private/rules/scala_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 = {}
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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 = {
Expand Down
5 changes: 2 additions & 3 deletions scala/private/rules/scala_repl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 = {
Expand Down
5 changes: 2 additions & 3 deletions scala/private/rules/scala_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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 = {
Expand Down

0 comments on commit 317214c

Please sign in to comment.