Allow ProtoPlugin to specify an executable artifact that does not need to be run by Java

[cheister]

Applicable Issues
I didn't open an issue for this before creating the PR

Description
I want to be able to use proto plugins that are executables and not jars. e.g. java-grpc https://github.com/grpc/grpc-java/blob/master/README.md

I couldn't get the combination of protocPlugins and pluginArtifact with custom-compile to run consistently.

Currently all protocPlugins are assumed to be jar classes that need to be run with java. java-grpc is a standalone executable plugin so I added the option for the following to work.

            <protocPlugin>
              <id>grpc-java</id>
              <groupId>io.grpc</groupId>
              <artifactId>protoc-gen-grpc-java</artifactId>
              <version>1.12.0</version>
              <type>exe</type>
              <classifier>${os.detected.classifier}</classifier>
            </protocPlugin>

If no mainClass is specified then it is assumed the protocPlugin is an executable artifact and is downloaded and run.

I also wonder if this change might also eliminate the need for the pluginArtifact option?

[sergei-ivanov]

This should be already working with compile-custom goal.
Have you seen the example on gRPC page that you linked? It has an example of running grpc-java native plugin, configured through pluginArtifact parameter.

[sergei-ivanov]

Common configuration mistakes are:

1. Not setting clearOutputDirectory to false. It's true by default for legacy reasons, but I've already changed the default to false in the upcoming release.
2. Not splitting goals into separate executions. It's usually a good practice to have a separate execution per generation goal, because you can have a more granular configuration element on the execution level.

[cheister]

It feels odd to have two ways to specify the protoc plugins.

This

<plugin>
    <groupId>org.xolstice.maven.plugins</groupId>
    <artifactId>protobuf-maven-plugin</artifactId>
    <executions>
        <execution>
            <id>custom-protoc-generate</id>
            <goals>
                <goal>compile</goal>
            </goals>
            <configuration>
                <protocPlugins>
                    <protocPlugin>
                        <id>minimal</id>
                        <groupId>org.xolstice.maven.plugins.protobuf.its</groupId>
                        <artifactId>test-protoc-plugin</artifactId>
                        <version>1.0.5</version>
                        <mainClass>org.xolstice.protobuf.plugin.minimal.MinimalPlugin</mainClass>
                    </protocPlugin>
                    <protocPlugin>
                        <id>prefix</id>
                        <groupId>org.xolstice.maven.plugins.protobuf.its</groupId>
                        <artifactId>test-protoc-plugin</artifactId>
                        <!-- Test that version ranges are correctly resolved too -->
                        <version>[1.0.0,1.1.0)</version>
                        <mainClass>org.xolstice.protobuf.plugin.minimal.MinimalPlugin</mainClass>
                        <args>
                            <arg>prefix-</arg>
                        </args>
                    </protocPlugin>
                    <protocPlugin>
                        <id>grpc-java</id>
                        <groupId>io.grpc</groupId>
                        <artifactId>protoc-gen-grpc-java</artifactId>
                        <version>1.12.0</version>
                        <type>exe</type>
                        <classifier>${os.detected.classifier}</classifier>
                    </protocPlugin>
                </protocPlugins>
            </configuration>
        </execution>
    </executions>
</plugin>

seems more consistent than

<plugin>
    <groupId>org.xolstice.maven.plugins</groupId>
    <artifactId>protobuf-maven-plugin</artifactId>
    <executions>
        <execution>
            <id>custom-protoc-generate</id>
            <goals>
                <goal>compile</goal>
                <goal>compile-custom</goal>
            </goals>
            <configuration>
                <protocPlugins>
                    <protocPlugin>
                        <id>minimal</id>
                        <groupId>org.xolstice.maven.plugins.protobuf.its</groupId>
                        <artifactId>test-protoc-plugin</artifactId>
                        <version>1.0.5</version>
                        <mainClass>org.xolstice.protobuf.plugin.minimal.MinimalPlugin</mainClass>
                    </protocPlugin>
                    <protocPlugin>
                        <id>prefix</id>
                        <groupId>org.xolstice.maven.plugins.protobuf.its</groupId>
                        <artifactId>test-protoc-plugin</artifactId>
                        <!-- Test that version ranges are correctly resolved too -->
                        <version>[1.0.0,1.1.0)</version>
                        <mainClass>org.xolstice.protobuf.plugin.minimal.MinimalPlugin</mainClass>
                        <args>
                            <arg>prefix-</arg>
                        </args>
                    </protocPlugin>
                </protocPlugins>
            </configuration>
        </execution>
    </executions>
    <configuration>
        <pluginId>grpc</pluginId>
        <pluginArtifact>
            io.grpc:protoc-gen-grpc-java:${grpcVersion}:exe:${os.detected.classifier}
        </pluginArtifact>
    </configuration>
</plugin>

since all of the protocPlugins are specified in one place. It's also more similar to how we run protoc which is specifying all of the plugins in one step.

This change still leaves the <pluginArtifact> method if people would prefer to use it and separate the protoc generation calls, we never do that though and prefer to run protoc with all the plugins all at once.

[sergei-ivanov]

I see what you mean. It's not that it's not working as expected, or that your goal is unachievable by the existing means. It's more about bringing better consistency to the configuration. You are right, a lot of these additional options were developed without much of a foresight. I recently started a major refactoring of the existing codebase, which is expected bring more simplicity and consistency to the way various protoc outputs are configured. I shall either release it for a preview soon or merge your patch in the interim, if my work takes too long.

[cheister]

@sergei-ivanov do mind taking another look at this?