rules_closure icon indicating copy to clipboard operation
rules_closure copied to clipboard

RFC: Add "modern" version of closure_template_library

Open Yannic opened this issue 6 years ago • 3 comments

Current situation

closure_template_{java,js}_library has been part of rules_closure for some time. Since its addition in 2016(?), a lot has changed, both in rules_closure and Bazel, and I think it's time to create something new. Something that looks and feels more like other "modern" Bazel-rules.

The current versions of closure_template_{java,js}_library are implemented as thin (independent) wrappers around genrule. This means that adding dependencies to or removing them from templates requires changing multiple targets, which is rather error-prone. Also, using closure templates from a new language (e.g. from Java if you only used them in JS) requires adding targets for all transitive dependencies.

closure_template_java_library(
    name = "a_java_soy",
    srcs = [
        "a.soy",
    ],
)

closure_template_js_library(
    name = "a_js_soy",
    srcs = [
        "a.soy",
    ],
)

closure_template_java_library(
    name = "b_java_soy",
    srcs = [
        "b.soy",
    ],
    deps = [
        ":a_java_soy",
    ],
)

closure_template_js_library(
    name = "b_js_soy",
    srcs = [
        "b.soy",
    ],
    deps = [
        ":a_js_soy",
    ],
)

New rule

The style of the new rules is inspired by proto_library. Instead of having per-language targets, we will create a dedicated closure_template_library rule that defines the dependencies between individual templates, and multiple closure_template_{language}_library-rules that generate language bindings for the templates.

closure_template_library(
    name = "a_soy",
    srcs = [
        "a.soy",
    ],
)

closure_template_library(
    name = "a_soy",
    srcs = [
        "b.soy",
    ],
    deps = [
        ":a_soy",
    ],
)

closure_template_java_library(
    name = "b_java_soy",
    deps = [
        ":b_soy",
    ],
)

closure_template_js_library(
    name = "b_js_soy",
    deps = [
        ":b_soy",
    ],
)

Internally, the language rules will use aspects to generate the language bindings for transitive dependencies.

Other design goals are:

  1. figure out how integration with closure_proto_library should work (e.g. #314, #388), and
  2. integrate the template generation into ClosureWorker (it currently isn't).

Yannic avatar Jun 07 '19 18:06 Yannic

this would be really awesome in terms of maintainability, at least the way we structure templates when using rules_closure. obviously i'm only commenting on the API surface here and not what is needed to land this feature, but this is really cool and we'd be happy to test, or help with PRs.

sgammon avatar Jun 07 '19 19:06 sgammon

Is there any particular reason that you want to use aspects here? They are complicated and only useful if you like to overlay on top of other rules that you don't have control on (for example closure_js_proto overlaying on top of existing proto rules).

I haven't dealt with closure templates but for this particular case, all you you need seems like a macro:

def closure_template_library(name, deps, ...): js_deps = [ d + "_js" for d in deps] closure_template_js_library(name + "_js", js_deps, ...) java_deps = [ d + "_java" for d in deps] closure_template_java_library(name + "_java", java_deps, ...)

Then your particular target java or js target depends on foo_java or foo_js.

gkdn avatar Jun 08 '19 02:06 gkdn

The reason why I thought of aspects to implement this is so users don't have to pay the overhead for languages they don't use during analysis, and that it might be slightly easier to add new languages (there is an official generator for python that rules_closure doesn't support). That said, I'm open to implementing this without aspects if they turn out to not be worth the extra effort.

The main reason why I'm proposing this new rule is to fix the limitations of the current genrule-based implementation. They have some issues when crossing workspace-boundaries, (especially the java version) have rather strong opinions on the layout of the workspace, and, in the case of Java, only support the "old" Tofu style.

Yannic avatar Jun 13 '19 17:06 Yannic