Refactor rules into configurable phases (#865)

* Add configurable phases * Refactor rules implementation into configurable phases * Customizable phases * Customizable phases tests * Break up init to more reasonable phases * Move final to non-configurable phase * Rename parameter builtin_customizable_phases * Fix ijar * Switch default for buildijar * Add TODOs * Rename provider * Move to advanced_usage * rename custom_phases * Make default phase private * Fix exports_jars * Adjusted_phases * Rename p to be more clear * Add in-line comments * Fix lint * Add doc for phases * Doc for consumers * Doc for contributors * Add more content * Fix md * Test for all rules * Fix junit test * Fix lint * Add more tests * Fix junit test * Fix doc * Change _test_ to _scalatest_ * More doc on provider
bazelbuild · Dec 25, 2019 · 937d469 · 937d469
1 parent cfff088
commit 937d469
Show file tree

Hide file tree

Showing 42 changed files with 1,866 additions and 619 deletions.
diff --git a/README.md b/README.md
@@ -189,6 +189,16 @@ 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 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.
+
 ## Building from source
 Test & Build:
 ```

diff --git 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/docs/customizable_phase.md b/docs/customizable_phase.md
@@ -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.
diff --git a/scala/advanced_usage/providers.bzl b/scala/advanced_usage/providers.bzl
@@ -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.",
+    },
+)
diff --git a/scala/advanced_usage/scala.bzl b/scala/advanced_usage/scala.bzl
@@ -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
diff --git a/scala/private/phases/api.bzl b/scala/private/phases/api.bzl
@@ -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
+    # 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"]:
+                    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
+            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(
+            default = [
+                phase_provider
+                for extra in extras
+                for phase_provider in extra["phase_providers"]
+            ],
+            providers = [_ScalaRulePhase],
+        ),
+    }
diff --git a/scala/private/phases/phase_collect_exports_jars.bzl 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)