Add support for protoc --xxx_opt flag

[marcoferrer]

This PR resolves #210

[marcoferrer]

Im not happy about it but it seems that this was the only dependable way to get the current protoc compiler version configured. The artifact property of the protoc executable locator is overridden once the path is resolved.

[zhangkun83]

The protoc may be from local path and you won't get the version from configurations at all.
A more reliable way is to run the protoc with --version. It should be run only once after protoc is resolved and saved somewhere (probably in ToolsLocator).

[marcoferrer]

I like that idea a lot better

[marcoferrer]

Code narc is unhappy, will push fix tomorrow

[zhangkun83]

There are codenarc failures:

File: com/google/protobuf/gradle/GenerateProtoTask.groovy
    Violation: Rule=SpaceAfterIf P=3 Line=455 Msg=[The if keyword within class com.google.protobuf.gradle.GenerateProtoTask is not followed by a single space] Src=[if(Utils.compareProtocVersion(project, "3.2") >= 0 && pluginOutPrefix.length() > 1){]
    Violation: Rule=SpaceBeforeOpeningBrace P=3 Line=455 Msg=[The opening brace for the block in class com.google.protobuf.gradle.GenerateProtoTask is not preceded by a space or whitespace] Src=[if(Utils.compareProtocVersion(project, "3.2") >= 0 && pluginOutPrefix.length() > 1){]
    Violation: Rule=UnnecessarySubstring P=3 Line=456 Msg=[Violation in class com.google.protobuf.gradle.GenerateProtoTask. The String.substring(int, int) method can be replaced with the subscript operator] Src=[baseCmd += "--${name}_opt=${pluginOutPrefix.substring(0, pluginOutPrefix.length() - 1)}"]

File: com/google/protobuf/gradle/Utils.groovy
    Violation: Rule=SpaceAfterIf P=3 Line=119 Msg=[The if keyword within class com.google.protobuf.gradle.Utils is not followed by a single space] Src=[if(protocDep != null){]
    Violation: Rule=SpaceBeforeOpeningBrace P=3 Line=119 Msg=[The opening brace for the block in class com.google.protobuf.gradle.Utils is not preceded by a space or whitespace] Src=[if(protocDep != null){]
    Violation: Rule=SpaceAfterIf P=3 Line=122 Msg=[The if keyword within class com.google.protobuf.gradle.Utils is not followed by a single space] Src=[if(protocVersion == null || protocVersion == ""){]
    Violation: Rule=SpaceBeforeOpeningBrace P=3 Line=122 Msg=[The opening brace for the block in class com.google.protobuf.gradle.Utils is not preceded by a space or whitespace] Src=[if(protocVersion == null || protocVersion == ""){]
    Violation: Rule=PublicMethodsBeforeNonPublicMethods P=3 Line=145 Msg=[The public method isTest in class com.google.protobuf.gradle.Utils is declared after a non-public method] Src=[static boolean isTest(String sourceSetOrVariantName) {]
    Violation: Rule=PublicMethodsBeforeNonPublicMethods P=3 Line=155 Msg=[The public method addToIdeSources in class com.google.protobuf.gradle.Utils is declared after a non-public method] Src=[static void addToIdeSources(Project project, boolean isTest, File f) {]

It also needs to be tested. It's not trivial because the project's test-ability is not that great. I am thinking maybe adding a "dry-run" flag to GenerateProtoTask, that will print out protoc command lines to a file instead of executing them. Then we create a "test project", say testProjectDryRuns, that will run protoc with versions pre-3.2 and post-3.2, with non-existing plugins, with and without options. Something like this:

protobuf {
  protoc {
    // The protocVersion property is set from the test spec where it's parameterized
    artifact = 'com.google.protobuf:protoc:${protocVersion}'
  }
  generateProtoTasks {
    all().each { task ->
      task.plugins {
        // When referring plugins without specifying their paths or artifacts, their bare
        // names are passed to protoc and protoc will run them from system path
        plugin1 {
          option 'feature1'
        }
        plugin2 {}
      }
      task.setDryRun(true)
    }
  }
}

This test project would have a custom test task that depends on generateProto and verify the output from the dry-runs.

[zhangkun83]

For better readability, I would change makeOptionsPrefix() to assembleOptions(), which would leave out the trailing colon, then add the colon in the else branch.

[marcoferrer]

Happy to do that. That was my initial approach but I back tracked a bit. I was reluctant to introduce too many changes with out initial feedback.

[zhangkun83]

The protoc may be from local path and you won't get the version from configurations at all.
A more reliable way is to run the protoc with --version. It should be run only once after protoc is resolved and saved somewhere (probably in ToolsLocator).