google / protobuf-gradle-plugin

Protobuf Plugin for Gradle
Other
1.75k stars 273 forks source link

Stop defaulting to javanano for Android #84

Closed zhangkun83 closed 8 years ago

zhangkun83 commented 8 years ago

Protobuf-lite is replacing nano as the recommended protobuf for Android. The plugin currently generates for javanano by default for Android, we should stop doing that soon, and probably switch to generate lite by default. Protobuf-lite, instead of being a separate output type as nano does, works as an option of the java output:

java {
  option 'lite'
}
mxk commented 8 years ago

I assume that this is the change we should make:

generateProtoTasks {
    all().each { task ->
        task.builtins {
            java { option 'lite' }
        }
    }
}

When I do that, I get --java_out: rpc.proto: Unknown generator option: lite and a whole bunch of warnings about missing directories (src\main\proto, build\extracted-protos\main, build\extracted-include-protos\main, etc.). I don't know why it's even checking src\main\proto because my proto files are here:

android {
    ...
    sourceSets.main.proto { srcDir '../proto' }
    ...
}
mxk commented 8 years ago

Finally got it working. First, Windows Defender (MSE on Win7) identifies protoc-gen-javalite-3.0.0-windows-x86_64.exe as Trojan:Win32/Repjexi. This used to be the case for protoc 3.0.0 as well. You just need to allow it.

Second, protobuf release notes say that we should use --javalite_out instead of the lite option, but if you add javalite {} to task.builtins you'll get Tried to write the same file twice error.

The configuration below seems to work, but it generates warnings about missing directories (no idea why it's passing so many -I options to protoc or how to disable those), and the resulting .java files are reported to use unchecked or unsafe operations at PARSER = new DefaultInstanceBasedParser(DEFAULT_INSTANCE); calls. I haven't actually tested the code yet, but I'm assuming that these are safe to ignore.

buildscript {
    repositories { jcenter() }
    dependencies {
        classpath 'com.android.tools.build:gradle:2.1.2'
        classpath 'com.google.protobuf:protobuf-gradle-plugin:0.7.7'
    }
}

apply plugin: 'com.android.application'
apply plugin: 'com.google.protobuf'

android {
    ...
    sourceSets.main.proto { srcDir ... }
}

dependencies {
    ...
    compile 'com.google.protobuf:protobuf-lite:3.0.1'
}

protobuf {
    protoc {
        artifact = 'com.google.protobuf:protoc:3.0.0'
    }
    plugins {
        javalite {
            artifact = 'com.google.protobuf:protoc-gen-javalite:3.0.0'
        }
    }
    generateProtoTasks {
        all().each { task ->
            task.builtins {
                remove javanano
            }
            task.plugins {
                javalite {}
            }
        }
    }
}
ejona86 commented 8 years ago

That snippet looks about right, and resembles what we're doing for grpc.

zhangkun83 commented 8 years ago

@ejona86 now that javalite has become a plugin, I am wondering whether and how we should continue applying default outputs, especially for Android. I am consider two options:

  1. Reconstruct the protoc-gen-javalite artifact spec out of the protoc spec, but this is hacky and doesn't always work, e.g., if a local protoc is specified.
  2. Always generate full proto by default, even for Android. People will need to remove java and add javalite if they want. I don't know how bad the full proto means to Android.
zhangkun83 commented 8 years ago

Talked with @ejona86, we both agree that it's better to not generate for lite by default. The full proto is undesirable in most Android project too. We will just stop defaulting to any output for Android.