Skip to content

Commit

Permalink
Stop flattening protoc command line. (google#214)
Browse files Browse the repository at this point in the history
Keep the command line as a list of arguments so that spaces in
arguments will not be mistaken as argument delimiters.

Resolves google#212
  • Loading branch information
zhangkun83 committed Nov 7, 2018
1 parent 356e24c commit 023876c
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 22 deletions.
35 changes: 22 additions & 13 deletions src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -447,14 +447,14 @@ public class GenerateProtoTask extends DefaultTask {
}
}

List<String> cmds = generateCmds(baseCmd.join(" "), protoFiles, getCmdLengthLimit())
for (String cmd : cmds) {
List<List<String>> cmds = generateCmds(baseCmd, protoFiles, getCmdLengthLimit())
for (List<String> cmd : cmds) {
compileFiles(cmd)
}
}

private void compileFiles(String cmd) {
logger.log(LogLevel.INFO, cmd)
private void compileFiles(List<String> cmd) {
logger.log(LogLevel.INFO, cmd.toString())

StringBuffer stdout = new StringBuffer()
StringBuffer stderr = new StringBuffer()
Expand All @@ -468,23 +468,32 @@ public class GenerateProtoTask extends DefaultTask {
}
}

static List<String> generateCmds(String baseCmd, List<File> protoFiles, int cmdLengthLimit) {
List<String> cmds = []
static List<List<String>> generateCmds(List<String> baseCmd, List<File> protoFiles, int cmdLengthLimit) {
List<List<String>> 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<String> currentArgs = new ArrayList<String>()
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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> baseCmd = ["protoc"]
List<File> protoFiles = [ new File("short.proto"), new File("long_proto_name.proto") ]
int cmdLengthLimit = 32
when: "the commands are generated"
List<String> cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit)
List<List<String>> 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<String> baseCmd = ["protoc"]
List<File> protoFiles = [ new File("short.proto"), new File("long_proto_name.proto") ]
int cmdLengthLimit = 64
when: "the commands are generated"
List<String> cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit)
List<List<String>> 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<String> baseCmd = ["protoc"]
List<File> protoFiles = []
int cmdLengthLimit = 32
when: "the commands are generated"
List<String> cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit)
List<List<String>> cmds = GenerateProtoTask.generateCmds(baseCmd, protoFiles, cmdLengthLimit)
then: "it returns no commands"
cmds.isEmpty()
Expand Down
1 change: 1 addition & 0 deletions testProjectCustomProtoDir/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions testProjectCustomProtoDir/src/main/java/Foo.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public static List<MessageLite> 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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
syntax = "proto3";

message SpaceInPath {
string bar = 1;
}
2 changes: 1 addition & 1 deletion testProjectCustomProtoDir/src/test/java/FooTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 023876c

Please sign in to comment.