Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redeclaration error with identically-named messages in different proto packages that share the same java package #179

Open
pddkhoa-fossil opened this issue Nov 10, 2021 · 3 comments
Labels
bug Something isn't working component-codegen help wanted Extra attention is needed platform-android platform-jvm

Comments

@pddkhoa-fossil
Copy link

pddkhoa-fossil commented Nov 10, 2021

I try define few proto files have same name Message with different package name. But when I generate kotlin file, They have same package name(com.example.protobuf) make error Redeclaration: Test

Example:

a.proto:

syntax = "proto3";
option java_package = "com.example.protobuf";
package A;

message Test {
  uint32 percentage = 1;
}

b.proto:

syntax = "proto3";
option java_package = "com.example.protobuf";
package B;

message Test {
  uint32 percentage = 1;
}

protoc config:

protobuf {
    generatedFilesBaseDir = "$projectDir/src"
    protoc {
        artifact = "com.google.protobuf:protoc:$protobufVersion"
    }
    plugins {
        id("protobuftest") {
            artifact = "pro.streem.pbandk:protoc-gen-pbandk-jvm:$pbandkVersion:jvm8@jar"
        }
    }
    generateProtoTasks {
        ofSourceSet("main").forEach { task ->
            task.builtins {
                remove("java")
            }
            task.plugins {
                id("protobuftest") {
                    option("log=debug")
                }
            }
        }
    }
}

I run: ./gradlew :myapp:installDist --stacktrace

@garyp
Copy link
Collaborator

garyp commented Nov 11, 2021

That's expected protobuf behavior since you're defining the same java_package in both files. I think you would get a similar error even with the official protobuf-java library.

Typical protobuf style is to have a 1:1 relationship between the value of package and java_package in every .proto file.

@pddkhoa-fossil
Copy link
Author

pddkhoa-fossil commented Nov 11, 2021

@garyp with official protobuf-java library support using the wrapper class as an outer class when I use same java_package in both files.
There are generated two files as below:

final class A {
  static final class Test {
      ...
  }
}
final class  B {
  static final class Test {
     ...
  }
}

Do you think we should support this feature in the library?

@garyp
Copy link
Collaborator

garyp commented Nov 11, 2021

Oh interesting. I think I've always used the java_package option together with option java_multiple_files = true, which doesn't nest definitions within an outer class. But I think you're right that the default behavior (with java_multiple_files = false) would nest the definitions inside separate outer classes and avoid the conflict you're seeing in pbandk.

This feels like an edge case that I'm not sure is worth supporting in pbandk. It's definitely not a setup that I would encourage. That said, I also don't like that pbandk generates errors for a .proto file which is handled successfully by protobuf-java since that would make it harder for users with existing protobuf files to easily migrate to pbandk.

I'll leave this issue open and try to think of a way to handle this gracefully in pbandk. I can't promise when we'd have time to implement this, but a PR to handle this situation is welcome.

@garyp garyp added bug Something isn't working help wanted Extra attention is needed labels Nov 11, 2021
@garyp garyp changed the title Cannot generate kotlin file by define package in proto file Redeclaration error with identically-named messages in different proto packages that share the same java package Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component-codegen help wanted Extra attention is needed platform-android platform-jvm
Projects
None yet
Development

No branches or pull requests

2 participants