allow passing the same PluginGenerator several times

[bjaglin]

Trying out support for bjaglin/sbt-protoc@53dbd6a#diff-bbd8a5163e4909c406d2fd2b12cca4345f91859fae3676fd8ee189d121e2c9f1R2-R10 so that https://github.com/scalapb/common-protos/blob/b55b43071974159f9ab05de8457d66e107be86c5/project/BuildHelpers.scala#L61-L65 could package both flat_package and regular stubs in the same JAR (unless we want to publish with a flat classifier?).

Utltimately, my goal is to make scalapb-validate works out of the box with akka-grpc, as I realized the instructions I added in akka/akka-grpc@19f1be0#diff-4780ac674b878a4a2a35e6740dc7efce31de8de2e342719c694de7a36a8e1f83R55-R61 are not enough:

object ValidateProto is not a member of package io.envoyproxy.pgv.validate
[error]     io.envoyproxy.pgv.validate.ValidateProto

[bjaglin]

I couldn't tell if this was a safeguard or something unsupported?

[thesamet]

Passing --protoc-gen-foo=path1 and --protoc-gen-foo=path2 is ambiguous, which one should protoc invoke for --foo_out=/dir ?

[bjaglin]

So I guess that's a limitation of the current implementation, as this PR handles it by giving unique names to each target

protoc-bridge/bridge/src/test/scala/protocbridge/ProtocBridgeSpec.scala

Lines 90 to 106 in c8799d8

	run(
	Seq(
	Target(gens.plugin("foo", "/path/to/plugin"), TmpPath),
	Target(gens.plugin("foo", "/otherpath/to/plugin"), TmpPath1),
	Target(gens.plugin("foo"), TmpPath2),
	Target(gens.plugin("bar"), TmpPath)
	)
	) must be(
	Seq(
	"--plugin=protoc-gen-foo_0=/path/to/plugin",
	"--plugin=protoc-gen-foo_1=/otherpath/to/plugin",
	s"--foo_0_out=$TmpPath",
	s"--foo_1_out=$TmpPath1",
	s"--foo_2_out=$TmpPath2",
	s"--bar_out=$TmpPath"
	)
	)

. In other words, the current implementation in this PR does not leverage the fact that the same plugin may have different outputs, in order to support different options for each output. We could continue using that feature as long as options are the same across outputs, but it would make the implementation harder, and I don't know if there is any gain to that?

[bjaglin]

I have added 8f2f6a9 75124d5 ede30d3 d37025e to showcase this

[bjaglin]

As discussed above, this would also work but would make the suffixing strategy (that the client currently needs to know about when the path is not defined as part of the target) even more complicated.

Suggested change

	"--plugin=protoc-gen-foo_0=/path/to/plugin",
	"--plugin=protoc-gen-foo_1=/path/to/plugin",
	"--plugin=protoc-gen-foo_2=/otherpath/to/plugin",
	"--plugin=protoc-gen-foo_3=/otherpath/to/plugin",
	s"--foo_0_out=$TmpPath",
	s"--foo_0_opt=w",
	s"--foo_1_out=$TmpPath",
	s"--foo_1_opt=x",
	s"--foo_2_out=$TmpPath1",
	s"--foo_2_opt=y",
	s"--foo_3_out=$TmpPath2",
	s"--foo_3_opt=y",
	s"--foo_4_out=$TmpPath2",
	s"--foo_4_opt=z",
	"--plugin=protoc-gen-foo_0=/path/to/plugin",
	"--plugin=protoc-gen-foo_1=/path/to/plugin",
	"--plugin=protoc-gen-foo_2=/otherpath/to/plugin",
	s"--foo_0_out=$TmpPath",
	s"--foo_0_opt=w",
	s"--foo_1_out=$TmpPath",
	s"--foo_1_opt=x",
	s"--foo_2_out=$TmpPath1",
	s"--foo_2_out=$TmpPath2",
	s"--foo_2_opt=y",
	s"--foo_3_out=$TmpPath2",
	s"--foo_3_opt=z",

[thesamet]

It seems that passing the same plugin multiple times with different options works prior to this change. I tried this on a sample project:

PB.targets in Compile := Seq(
  scalapb.gen(javaConversions=true) -> (sourceManaged in Compile).value / "scalapb",
  scalapb.gen(javaConversions=true, flatPackage=true) -> (sourceManaged in Compile).value / "scalapb-flat"
)

Looks like there's test coverage for this as well.

Is there a specific case related to running the same plugin twice that is not already supported?

I am not excited about packaging both flat and non-flat files in the same JAR nor publishing them separately (with a flat prefix). I believe it will work, but it will lead to confusion due to the duplicate classes available for the same messages. There are also a few gotchas around duplicate descriptors being around for the same message (some code assumes there's exactly one). I filed scalapb/scalapb-validate#70 so we can identify a solution for users of akka-grpc.

[bjaglin]

Is there a specific case related to running the same plugin twice that is not already supported?

Yes, this is about running native plugins (the PluginGenerator in the title is maybe not precise enough), not JVM ones, like in https://github.com/scalapb/common-protos/blob/b55b43071974159f9ab05de8457d66e107be86c5/project/BuildHelpers.scala#L60-L64.

[bjaglin]

By the way, I didn't open a PR but bjaglin/sbt-protoc@53dbd6a#diff-bbd8a5163e4909c406d2fd2b12cca4345f91859fae3676fd8ee189d121e2c9f1R2-R10 demonstrate this.