-
-
Notifications
You must be signed in to change notification settings - Fork 287
Refactor rules into configurable phases #865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
ittaiz
merged 32 commits into
bazel-contrib:master
from
lucidsoftware:customizable-phase
Dec 25, 2019
Merged
Changes from all commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
45d307c
Add configurable phases
borkaehw df374ec
Refactor rules implementation into configurable phases
borkaehw 5d052a3
Customizable phases
borkaehw 584f845
Customizable phases tests
borkaehw 7d26cab
Break up init to more reasonable phases
borkaehw a50a114
Move final to non-configurable phase
borkaehw 89f202a
Rename parameter builtin_customizable_phases
borkaehw ac9e1df
Fix ijar
borkaehw 988a24c
Switch default for buildijar
borkaehw ec75a89
Add TODOs
borkaehw 9854fcc
Rename provider
borkaehw 54a5db3
Move to advanced_usage
borkaehw e10bcb3
rename custom_phases
borkaehw c6deba3
Make default phase private
borkaehw 364cb85
Fix exports_jars
borkaehw 0e777f0
Adjusted_phases
borkaehw e4fb12c
Rename p to be more clear
borkaehw e64a739
Add in-line comments
borkaehw 4ad873d
Fix lint
borkaehw f569ea0
Add doc for phases
borkaehw 38482f9
Doc for consumers
borkaehw 22aa452
Doc for contributors
borkaehw f3b8972
Add more content
borkaehw eb14ea0
Fix md
borkaehw 5dde9db
Test for all rules
borkaehw 37d6904
Fix junit test
borkaehw 7f0fa4e
Fix lint
borkaehw aa35cd3
Add more tests
borkaehw 5b6f4ae
Fix junit test
borkaehw a581634
Fix doc
borkaehw 4f3dd64
Change _test_ to _scalatest_
borkaehw c36cdd0
More doc on provider
borkaehw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
# 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 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 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: | ||
``` | ||
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 phases. Rules with `make_` prefix, like `make_scala_binary`, are customizable rules. `make_<RULE_NAME>`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) | ||
``` | ||
`make_<RULE_NAME>`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 [ | ||
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 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) | ||
``` | ||
- 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.<PHASE_NAME>.<FIELD_NAME>`. 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.<FIELD_NAME>`. | ||
|
||
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`: the API of executing and modifying the phase list | ||
- `scala/private/phases/phases.bzl`: re-expose phases for convenience | ||
- `scala/private/phases/phase_<PHASE_NAME>.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 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_<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 | ||
|
||
Function names in `phase_<PHASE_NAME>.bzl` | ||
- `phase_<RULE_NAME>_<PHASE_NAME>`: function with custom inputs of specific rule | ||
- `phase_common_<PHASE_NAME>`: function without custom inputs | ||
- `_phase_default_<PHASE_NAME>`: private function that takes `_args` for custom inputs | ||
- `_phase_<PHASE_NAME>`: private function with the actual logic | ||
|
||
See `phase_compile.bzl` for example. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
""" | ||
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 = { | ||
"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.", | ||
}, | ||
) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
""" | ||
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", | ||
) | ||
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
""" | ||
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 | ||
ittaiz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# 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 | ||
for (relation, peer_name, phase_name, phase_function) in adjustments: | ||
for idx, (needle, _) in enumerate(phases): | ||
if relation in ["^", "first"]: | ||
phases.insert(0, (phase_name, phase_function)) | ||
elif relation in ["$", "last"]: | ||
phases.append((phase_name, phase_function)) | ||
elif needle == peer_name: | ||
if relation in ["-", "before"]: | ||
ittaiz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
phases.insert(idx, (phase_name, phase_function)) | ||
elif relation in ["+", "after"]: | ||
phases.insert(idx + 1, (phase_name, phase_function)) | ||
elif relation in ["=", "replace"]: | ||
phases[idx] = (phase_name, phase_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, | ||
[ | ||
phase | ||
for phase_provider in phase_providers | ||
for phase in phase_provider.custom_phases | ||
], | ||
) | ||
|
||
# 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 | ||
borkaehw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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): | ||
borkaehw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return { | ||
"_phase_providers": attr.label_list( | ||
default = [ | ||
phase_provider | ||
for extra in extras | ||
for phase_provider in extra["phase_providers"] | ||
], | ||
providers = [_ScalaRulePhase], | ||
), | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.