Starlark plugins/rules: allow registration via the yaml config file (#291)

* Starlark plugins/rules: allow registration via the yaml config file
* Implement missing out and options attributes.
* Fix tests
* Add to proto_repository_test

Author: pcj

.bazelrc

@@ -1,3 +1,3 @@
 build:bazelci --deleted_packages=docs
 # workaround for scala
-build --incompatible_java_common_parameters=false
+build --incompatible_java_common_parameters=false

example/golden/testdata/proto_repository/config.yaml

@@ -1,10 +1,15 @@
+starlarkPlugins:
+  # protoc-gen-java duplicates the functionality of the
+  # builtin 'java' one, we have it here to test that it can
+  # be loaded via the YAML config.
+  # The format is IMPLEMENTATION_FILENAME%PLUGIN_NAME.
+  - starlark/plugins.star%protoc-gen-java
 plugins:
   - name: protoc-gen-go
     implementation: golang:protobuf:protoc-gen-go
     deps:
       - "@org_golang_google_protobuf//reflect/protoreflect"
       - "@org_golang_google_protobuf//runtime/protoimpl"
-
   - name: protoc-gen-go-grpc
     implementation: grpc:grpc-go:protoc-gen-go-grpc
     deps:
@@ -14,13 +19,13 @@ plugins:
 
   - name: java
     implementation: builtin:java
-
+  - name: protoc-gen-java
+    implementation: starlark/plugins.star%protoc-gen-java
   - name: protoc-gen-grpc-java
     implementation: grpc:grpc-java:protoc-gen-grpc-java
 
   - name: python
     implementation: builtin:python
-
   - name: protoc-gen-grpc-python
     implementation: grpc:grpc:protoc-gen-grpc-python
 
@@ -44,7 +49,6 @@ rules:
       - "//visibility:public"
     deps:
       - "@com_google_protobuf//:protobuf_java"
-
   - name: grpc_java_library
     implementation: stackb:rules_proto:grpc_java_library
     visibility:
@@ -58,7 +62,6 @@ rules:
       - "//visibility:public"
     deps:
       - "@tms_py_deps_protobuf//:pkg"
-
   - name: grpc_py_library
     implementation: stackb:rules_proto:grpc_py_library
     visibility:
@@ -80,6 +83,7 @@ languages:
     enabled: false
     plugins:
       - java
+      - protoc-gen-java
       - protoc-gen-grpc-java
     rules:
       - proto_compile

example/golden/testdata/proto_repository/starlark/plugins.star

@@ -0,0 +1,28 @@
+"""starlark plugin definitions"""
+
+def _configure_protoc_gen_java(ctx):
+    """_configure_protoc_gen_java prepares the PluginConfiguration for a fictitious protoc java plugin.
+
+    Args:
+        ctx (protoc.PluginContext): The context object.
+    Returns:
+        config (PluginConfiguration): The configured PluginConfiguration object.
+    """
+
+    srcjar = ctx.proto_library.base_name + ".srcjar"
+    if ctx.rel:
+        srcjar = "/".join(ctx.rel, srcjar)
+
+    config = protoc.PluginConfiguration(
+        label = "@build_stack_rules_proto//plugin/builtin:java",
+        outputs = [srcjar],
+        out = srcjar,
+        options = ctx.plugin_config.options,
+    )
+
+    return config
+
+protoc.Plugin(
+    name = "protoc-gen-java",
+    configure = _configure_protoc_gen_java,
+)

example/golden/testdata/starlark_java/BUILD.out

@@ -30,6 +30,7 @@ java_library(
 
 proto_compile(
     name = "example_java_compile",
+    outs = {"@build_stack_rules_proto//plugin/builtin:java": "example.srcjar"},
     outputs = ["example.srcjar"],
     plugins = ["@build_stack_rules_proto//plugin/builtin:java"],
     proto = "example_proto",

pkg/language/protobuf/config.go

@@ -46,6 +46,18 @@ func (pl *protobufLang) CheckFlags(fs *flag.FlagSet, c *config.Config) error {
 	cfg := protoc.NewPackageConfig(c)
 	c.Exts[pl.name] = cfg
 
+	for _, starlarkPlugin := range pl.starlarkPlugins {
+		if err := protoc.RegisterStarlarkPlugin(c, starlarkPlugin); err != nil {
+			return err
+		}
+	}
+
+	for _, starlarkRule := range pl.starlarkRules {
+		if err := protoc.RegisterStarlarkRule(c, starlarkRule); err != nil {
+			return err
+		}
+	}
+
 	if pl.configFiles != "" {
 		for _, filename := range strings.Split(pl.configFiles, ",") {
 			if err := protoc.LoadYConfigFile(c, cfg, filename); err != nil {
@@ -62,18 +74,6 @@ func (pl *protobufLang) CheckFlags(fs *flag.FlagSet, c *config.Config) error {
 		}
 	}
 
-	for _, starlarkPlugin := range pl.starlarkPlugins {
-		if err := registerStarlarkPlugin(c, starlarkPlugin); err != nil {
-			return err
-		}
-	}
-
-	for _, starlarkRule := range pl.starlarkRules {
-		if err := registerStarlarkRule(c, starlarkRule); err != nil {
-			return err
-		}
-	}
-
 	return nil
 }
 
@@ -128,52 +128,6 @@ func (pl *protobufLang) getOrCreatePackageConfig(config *config.Config) *protoc.
 	return cfg
 }
 
-func registerStarlarkPlugin(c *config.Config, starlarkPlugin string) error {
-	parts := strings.Split(starlarkPlugin, "%")
-	if len(parts) != 2 {
-		return fmt.Errorf("invalid starlark plugin name %q", starlarkPlugin)
-	}
-	fileName := parts[0]
-	ruleName := parts[1]
-	var configureError error
-	impl, err := protoc.LoadStarlarkPluginFromFile(c.WorkDir, fileName, ruleName, func(msg string) {
-	}, func(err error) {
-		configureError = err
-	})
-	if err != nil {
-		return err
-	}
-	if configureError != nil {
-		return configureError
-	}
-	configureError = err
-	protoc.Plugins().RegisterPlugin(starlarkPlugin, impl)
-	return nil
-}
-
-func registerStarlarkRule(c *config.Config, starlarkRule string) error {
-	parts := strings.Split(starlarkRule, "%")
-	if len(parts) != 2 {
-		return fmt.Errorf("invalid starlark rule name %q", starlarkRule)
-	}
-	fileName := parts[0]
-	ruleName := parts[1]
-	var configureError error
-	impl, err := protoc.LoadStarlarkLanguageRuleFromFile(c.WorkDir, fileName, ruleName, func(msg string) {
-	}, func(err error) {
-		configureError = err
-	})
-	if err != nil {
-		return err
-	}
-	if configureError != nil {
-		return configureError
-	}
-	configureError = err
-	protoc.Rules().MustRegisterRule(starlarkRule, impl)
-	return nil
-}
-
 func registerWellKnownProtos(resolver protoc.ImportResolver) {
 	for k, v := range map[string]label.Label{
 		"google/protobuf/any.proto":             label.New("com_google_protobuf", "", "any_proto"),

pkg/protoc/package_config.go

@@ -2,6 +2,7 @@ package protoc
 
 import (
 	"fmt"
+	"log"
 	"sort"
 	"strings"
 
@@ -189,6 +190,16 @@ func (c *PackageConfig) configuredLangs() []*LanguageConfig {
 }
 
 func (c *PackageConfig) LoadYConfig(y *YConfig) error {
+	for _, starlarkPlugin := range y.StarlarkPlugin {
+		if err := c.loadYStarlarkPlugin(starlarkPlugin); err != nil {
+			return err
+		}
+	}
+	for _, starlarkRule := range y.StarlarkRule {
+		if err := c.loadYStarlarkRule(starlarkRule); err != nil {
+			return err
+		}
+	}
 	for _, plugin := range y.Plugin {
 		if err := c.loadYPlugin(plugin); err != nil {
 			return err
@@ -207,6 +218,14 @@ func (c *PackageConfig) LoadYConfig(y *YConfig) error {
 	return nil
 }
 
+func (c *PackageConfig) loadYStarlarkPlugin(y string) error {
+	return RegisterStarlarkPlugin(c.Config, y)
+}
+
+func (c *PackageConfig) loadYStarlarkRule(y string) error {
+	return RegisterStarlarkRule(c.Config, y)
+}
+
 func (c *PackageConfig) loadYPlugin(y *YPlugin) error {
 	if y.Name == "" {
 		return fmt.Errorf("yaml plugin name missing in: %+v", y)
@@ -240,3 +259,51 @@ func (c *PackageConfig) loadYLanguage(y *YLanguage) error {
 	}
 	return lang.fromYAML(y)
 }
+
+func RegisterStarlarkPlugin(c *config.Config, starlarkPlugin string) error {
+	parts := strings.Split(starlarkPlugin, "%")
+	if len(parts) != 2 {
+		return fmt.Errorf("invalid starlark plugin name %q", starlarkPlugin)
+	}
+	fileName := parts[0]
+	ruleName := parts[1]
+	var configureError error
+	impl, err := LoadStarlarkPluginFromFile(c.WorkDir, fileName, ruleName, func(msg string) {
+		log.Printf("%s: %v", starlarkPlugin, msg)
+	}, func(err error) {
+		configureError = err
+	})
+	if err != nil {
+		return err
+	}
+	if configureError != nil {
+		return configureError
+	}
+	Plugins().RegisterPlugin(starlarkPlugin, impl)
+	return nil
+}
+
+func RegisterStarlarkRule(c *config.Config, starlarkRule string) error {
+	parts := strings.Split(starlarkRule, "%")
+	if len(parts) != 2 {
+		return fmt.Errorf("invalid starlark rule name %q", starlarkRule)
+	}
+	fileName := parts[0]
+	ruleName := parts[1]
+
+	var configureError error
+	impl, err := LoadStarlarkLanguageRuleFromFile(c.WorkDir, fileName, ruleName, func(msg string) {
+	}, func(err error) {
+		configureError = err
+	})
+	if err != nil {
+		return err
+	}
+	if configureError != nil {
+		return configureError
+	}
+	configureError = err
+	Rules().MustRegisterRule(starlarkRule, impl)
+	log.Println("Registered rule:", starlarkRule, impl.Name())
+	return nil
+}

pkg/protoc/starlark_plugin.go

@@ -56,7 +56,7 @@ func loadStarlarkPlugin(name, filename string, src interface{}, reporter func(ms
 	}
 }
 
-// starlarkPlugin is a plugin implemented in starlark that implements the protoc
+// starlarkPlugin is an adapter for starlark code that implements the protoc
 // plugin interface.
 type starlarkPlugin struct {
 	name          string
@@ -85,7 +85,7 @@ func (p *starlarkPlugin) Configure(ctx *PluginContext) *PluginConfiguration {
 		newPluginContextStruct(ctx),
 	}, []starlark.Tuple{})
 	if err != nil {
-		p.errorReporter("plugin %q configure failed: %w", p.name, err)
+		p.errorReporter("plugin %q configure failed: %v", p.name, err)
 		return nil
 	}
 
@@ -116,9 +116,30 @@ func (p *starlarkPlugin) Configure(ctx *PluginContext) *PluginConfiguration {
 			outputs[i] = outputsList.Index(i).(starlark.String).GoString()
 		}
 
+		optionsValue, err := value.Attr("options")
+		if err != nil {
+			p.errorReporter("PluginConfiguration.options get value: %v", err)
+		}
+		optionsList := optionsValue.(*starlark.List)
+		options := make([]string, optionsList.Len())
+		for i := 0; i < optionsList.Len(); i++ {
+			options[i] = optionsList.Index(i).(starlark.String).GoString()
+		}
+
+		outValue, err := value.Attr("out")
+		if err != nil {
+			p.errorReporter("PluginConfiguration.out get value: %v", err)
+		}
+		var out string
+		if outString, ok := outValue.(starlark.String); ok {
+			out = outString.GoString()
+		}
+
 		result = &PluginConfiguration{
 			Label:   lbl,
 			Outputs: outputs,
+			Out:     out,
+			Options: options,
 		}
 	default:
 		p.errorReporter("plugin %q configure returned invalid type: %T", p.name, value)

pkg/protoc/starlark_plugin_test.go

@@ -66,6 +66,7 @@ protoc.Plugin(
 			want: &PluginConfiguration{
 				Label:   label.New("", "mypkg", "python_plugin"),
 				Outputs: []string{"foo.py", "bar.py"},
+				Options: []string{},
 			},
 			wantPrinted: `PluginContext(package_config = PackageConfig(config = Config(repo_name = "", repo_root = "", work_dir = "")), plugin_config = LanguagePluginConfig(deps = [], enabled = False, implementation = "", label = "", name = "", options = []), proto_library = ProtoLibrary(base_name = "", deps = [], files = [], imports = [], name = "", srcs = [], strip_import_prefix = ""), rel = "mypkg")` + "\n",
 		},

pkg/protoc/yconfig.go

@@ -13,9 +13,11 @@ import (
 // YConfig is used to configure a combined set of plugins, rules, and languages
 // in a single YAML file.  This is the format of the -proto_config flag.
 type YConfig struct {
-	Plugin   []*YPlugin   `yaml:"plugins"`
-	Rule     []*YRule     `yaml:"rules"`
-	Language []*YLanguage `yaml:"languages"`
+	Plugin         []*YPlugin   `yaml:"plugins"`
+	Rule           []*YRule     `yaml:"rules"`
+	Language       []*YLanguage `yaml:"languages"`
+	StarlarkPlugin []string     `yaml:"starlarkPlugins"`
+	StarlarkRule   []string     `yaml:"starlarkRules"`
 }
 
 // YPlugin represents a LanguagePluginConfig in YAML.