Version 0.8.4: Cannot compile on Windows when username has a space

[yeefan]

On my computer, my username is "Tan Yee Fan" and hence the path to the Protobuf compiler contains spaces:

C:\Users\Tan Yee Fan\.gradle\caches\modules-2\files-2.1\com.google.protobuf\protoc\3.5.1-1\416df7b7f577fada867e9edd1864d45184e3ff41\protoc-3.5.1-1-windows-x86_64.exe

When using version 0.8.4 of the plugin, the compilation failed with the following error message:

* What went wrong:
Execution failed for task ':acv-service-common:generateProto'.
> java.io.IOException: Cannot run program "C:\Users\Tan": CreateProcess error=193, %1 is not a valid Win32 application

Downgrading to version 0.8.3 of the plugin resolved the problem.

System information:

• Operating system: Windows 10
• Java: 1.8.0_161
• Gradle: 4.5.1
• Protobuf Plugin for Gradle: 0.8.4
• Protobuf compiler: 3.5.1-1

[yeefan]

On running gradle with --info, it appears that version 0.8.3 executes the following list:
[C:\Users\Tan Yee Fan\.gradle\caches\modules-2\files-2.1\com.google.protobuf\protoc\3.5.1-1\416df7b7f577fada867e9edd1864d45184e3ff41\protoc-3.5.1-1-windows-x86_64.exe, ...]

whereas version 0.8.4 executes the following string:
C:\Users\Tan Yee Fan\.gradle\caches\modules-2\files-2.1\com.google.protobuf\protoc\3.5.1-1\416df7b7f577fada867e9edd1864d45184e3ff41\protoc-3.5.1-1-windows-x86_64.exe ...

The following in GenerateProtoTask.groovy appears very suspect:

List<String> baseCmd = [ tools.protoc.path ]
...
List<String> cmds = generateCmds(baseCmd.join(" "), protoFiles, getCmdLengthLimit())

You cannot just perform string concatenation without at least quoting the protoc path, if the path contains a space. I didn't read the source codes very closely, but I suppose this also affects users of the gRPC plugin.

[zhangkun83]

@yeefan can you check out the head, install the plugin locally with ./gradlew install -x test, and test it if it the issue is fixed?

[yeefan]

@zhangkun83 I checked out the head, and this fixes the problem as far as I am concerned. Thanks a lot.

However, looking at your commit, I feel that the following line may not be entirely correct.
int currentFileLength = protoFileName.length() + 1

My understanding is that on Windows, Java ultimately calls the CreateProcess function to execute a command, and this command is a null-terminated C-string. Somewhere along the way, Java must convert the list of strings into the C-string, which means adding quotes whenever an argument contains a space, thus lengthening the command by at least 2. Yet, according to MSDN it may not be as simple as just adding 2 characters, because \" is treated as a literal quote.

I didn't check my claim, but I believe there might be a bug in the length computation code.

[zhangkun83]

The Windows length limits that the plugin currently assumes is 2047 and 8191, which only apply to commands typed in cmd.exe . When it comes to CreateProcess, the limit is 32,768.

@gegy1000, since you contributed #174, I assume you have a project large enough to trigger the limit issue. Can you try increasing the limits to 30000, which is over the cmd.exe limit but below CreateProcess limit, and see if it still works?

[zhangkun83]

I experimented with the following Gradle task on Windows Server 2012 trying to find out the Windows limit.

task('runcmd') {
  doLast {
    for (int length = 32768; length >= 0; length--) {
      println "Trying length ${length}"
      def cmd = []
      cmd.add('java')
      for (int i = 0; i < length; i++) {
      	cmd.add('X')
      }
      try {
        def p = cmd.execute()
        def stdout = new StringBuffer()
        def stderr = new StringBuffer()
        p.waitForProcessOutput(stdout, stderr)
        println "stdout: ${stdout}, stderr: ${stderr}"
	break
      } catch (Exception e){}
    }
  }
}

The command was accepted by Windows when length==16381. It looks like the 32768 limit applies here, and the spaces between argument are counted. When I changed 'X' to ' ' in the script, the result was 8190. Looks like two quotes are added per argument when necessary.

[zhangkun83]

This is now released as 0.8.5

[yeefan]

Thanks. But 0.8.5 not published on Maven Central?

[zhangkun83]

Hmm.. I don't know what happened. I just tried again now it shows up. Search index needs more time to be updated, but the artifact is already available.

[yeefan]

Can download the 0.8.5 artifacts now. Thanks again.