From 023876c07112bef39c0d9a844e4430d2db9d6672 Mon Sep 17 00:00:00 2001 From: Kun Zhang Date: Tue, 20 Feb 2018 17:46:59 -0800 Subject: [PATCH] Stop flattening protoc command line. (#214) Keep the command line as a list of arguments so that spaces in arguments will not be mistaken as argument delimiters. Resolves #212 --- .../protobuf/gradle/GenerateProtoTask.groovy | 35 ++++++++++++------- .../gradle/ProtobufJavaPluginTest.groovy | 16 ++++----- testProjectCustomProtoDir/build.gradle | 1 + .../src/main/java/Foo.java | 2 ++ .../main/protocol buffers/spaceinpath.proto | 5 +++ .../src/test/java/FooTest.java | 2 +- 6 files changed, 39 insertions(+), 22 deletions(-) create mode 100644 testProjectCustomProtoDir/src/main/protocol buffers/spaceinpath.proto diff --git a/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy b/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy index 751654b2..0127d65d 100644 --- a/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy +++ b/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy @@ -447,14 +447,14 @@ public class GenerateProtoTask extends DefaultTask { } } - List cmds = generateCmds(baseCmd.join(" "), protoFiles, getCmdLengthLimit()) - for (String cmd : cmds) { + List> cmds = generateCmds(baseCmd, protoFiles, getCmdLengthLimit()) + for (List cmd : cmds) { compileFiles(cmd) } } - private void compileFiles(String cmd) { - logger.log(LogLevel.INFO, cmd) + private void compileFiles(List cmd) { + logger.log(LogLevel.INFO, cmd.toString()) StringBuffer stdout = new StringBuffer() StringBuffer stderr = new StringBuffer() @@ -468,23 +468,32 @@ public class GenerateProtoTask extends DefaultTask { } } - static List generateCmds(String baseCmd, List protoFiles, int cmdLengthLimit) { - List cmds = [] + static List> generateCmds(List baseCmd, List protoFiles, int cmdLengthLimit) { + List> cmds = [] if (!protoFiles.isEmpty()) { - StringBuilder currentCmd = new StringBuilder(baseCmd) + // We count one additional space needed for each command line + // argument. The result will be one more than the actual + // commandline length, but we can tolerate it for the sake of + // simplicity. + int baseCmdLength = baseCmd.sum { it.length() + 1 } + List currentArgs = new ArrayList() + int currentArgsLength = 0 for (File proto: protoFiles) { String protoFileName = proto + int currentFileLength = protoFileName.length() + 1 // Check if appending the next proto string will overflow the cmd length limit - if (currentCmd.length() + protoFileName.length() + 1 > cmdLengthLimit) { + if (baseCmdLength + currentArgsLength + currentFileLength > cmdLengthLimit) { // Add the current cmd before overflow - cmds.add(currentCmd.toString()) - currentCmd.setLength(baseCmd.length()) + cmds.add(baseCmd + currentArgs) + currentArgs.clear() + currentArgsLength = 0 } - // Append the proto file to the command - currentCmd.append(" ").append(protoFileName) + // Append the proto file to the args + currentArgs.add(protoFileName) + currentArgsLength += currentFileLength } // Add the last cmd for execution - cmds.add(currentCmd.toString()) + cmds.add(baseCmd + currentArgs) } return cmds } diff --git a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy index d98f6283..46d521cd 100644 --- a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy +++ b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy @@ -256,43 +256,43 @@ class ProtobufJavaPluginTest extends Specification { void "test generateCmds should split commands when limit exceeded"() { given: "a cmd length limit and two proto files" - String baseCmd = "protoc" + List baseCmd = ["protoc"] List protoFiles = [ new File("short.proto"), new File("long_proto_name.proto") ] int cmdLengthLimit = 32 when: "the commands are generated" - List cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit) + List> cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit) then: "it splits appropriately" - cmds.size() == 2 && cmds[0] == "protoc short.proto" && cmds[1] == "protoc long_proto_name.proto" + cmds.size() == 2 && cmds[0] == ["protoc", "short.proto"] && cmds[1] == ["protoc", "long_proto_name.proto"] } void "test generateCmds should not split commands when under limit"() { given: "a cmd length limit and two proto files" - String baseCmd = "protoc" + List baseCmd = ["protoc"] List protoFiles = [ new File("short.proto"), new File("long_proto_name.proto") ] int cmdLengthLimit = 64 when: "the commands are generated" - List cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit) + List> cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit) then: "it splits appropriately" - cmds.size() == 1 && cmds[0] == "protoc short.proto long_proto_name.proto" + cmds.size() == 1 && cmds[0] == ["protoc", "short.proto", "long_proto_name.proto"] } void "test generateCmds should not return commands when no protos are given"() { given: "a cmd length limit and no proto files" - String baseCmd = "protoc" + List baseCmd = ["protoc"] List protoFiles = [] int cmdLengthLimit = 32 when: "the commands are generated" - List cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit) + List> cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit) then: "it returns no commands" cmds.isEmpty() diff --git a/testProjectCustomProtoDir/build.gradle b/testProjectCustomProtoDir/build.gradle index aa44c260..4493fd0f 100644 --- a/testProjectCustomProtoDir/build.gradle +++ b/testProjectCustomProtoDir/build.gradle @@ -14,6 +14,7 @@ sourceSets { // In addition to the default 'src/main/proto' srcDir 'src/main/protobuf' srcDir 'src/main/protocolbuffers' + srcDir 'src/main/protocol buffers' // In addition to '**/*.proto' (use with caution). // Using an extension other than 'proto' is NOT recommended, because when // proto files are published along with class files, we can only tell the diff --git a/testProjectCustomProtoDir/src/main/java/Foo.java b/testProjectCustomProtoDir/src/main/java/Foo.java index c5ef6132..d523291b 100644 --- a/testProjectCustomProtoDir/src/main/java/Foo.java +++ b/testProjectCustomProtoDir/src/main/java/Foo.java @@ -17,6 +17,8 @@ public static List getDefaultInstances() { // from src/main/protocolbuffers/more.proto list.add(More.MoreMsg.getDefaultInstance()); list.add(More.Foo.getDefaultInstance()); + // from "src/main/protocol buffers/spaceinpath.proto" + list.add(Spaceinpath.SpaceInPath.getDefaultInstance()); return list; } } diff --git a/testProjectCustomProtoDir/src/main/protocol buffers/spaceinpath.proto b/testProjectCustomProtoDir/src/main/protocol buffers/spaceinpath.proto new file mode 100644 index 00000000..04ec9284 --- /dev/null +++ b/testProjectCustomProtoDir/src/main/protocol buffers/spaceinpath.proto @@ -0,0 +1,5 @@ +syntax = "proto3"; + +message SpaceInPath { + string bar = 1; +} diff --git a/testProjectCustomProtoDir/src/test/java/FooTest.java b/testProjectCustomProtoDir/src/test/java/FooTest.java index 2b36c769..03062637 100644 --- a/testProjectCustomProtoDir/src/test/java/FooTest.java +++ b/testProjectCustomProtoDir/src/test/java/FooTest.java @@ -2,7 +2,7 @@ public class FooTest { @org.junit.Test public void testMainProtos() { - org.junit.Assert.assertEquals(8, Foo.getDefaultInstances().size()); + org.junit.Assert.assertEquals(9, Foo.getDefaultInstances().size()); } @org.junit.Test