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

Kotlin package name conflicting #230

Closed
Dogacel opened this issue Sep 29, 2022 · 4 comments
Closed

Kotlin package name conflicting #230

Dogacel opened this issue Sep 29, 2022 · 4 comments
Labels
component-codegen enhancement New feature or request
Milestone

Comments

@Dogacel
Copy link
Contributor

Dogacel commented Sep 29, 2022

When using the Gradle plugin, I need to add a prefix to my package name (or change it) because it overlaps with kotlin {} builtin and without it, I can't use grpckt.

when I try option("kotlin_package=pbandk.example") it gives the error,

pbandk/example/types.kt: Tried to write the same file twice.
pbandk/example/enums.kt: Tried to write the same file twice.

seems like kotlin_package just overrides the proto package. But I want to add a suffix to porto package so it is something different from the default protoc generated packages. Otherwise imports are mixing up, grpc code tries to import from pbandk so it gives errors.

Is it possible to support something like package prefix?

@Dogacel
Copy link
Contributor Author

Dogacel commented Sep 29, 2022

Something like this:

@garyp

@garyp
Copy link
Collaborator

garyp commented Sep 30, 2022

There is an undocumented kotlin_package_mapping option to protoc-gen-pbandk that allows defining a custom mapping from proto package names to kotlin package names. E.g.:

option("kotlin_package_mapping=google.api->pbandk.google.api;google.rpc->pbandk.google.rpc")

Would that satisfy your use case? If not, your PR to add a prefix seems like a reasonable approach too. I just don't want to add another option if the existing option will work for you.

The implementation is here if you're curious:

// Support option kotlin_package_mapping=from.package1->to.package1;from.package2->to.package2

@garyp garyp added enhancement New feature or request component-codegen labels Sep 30, 2022
@Dogacel
Copy link
Contributor Author

Dogacel commented Oct 3, 2022

Would that satisfy your use case?

There are hundreds of different packages because we use monorepo structure for our protos. So there are 20-30 different microservices, each having a different package name, and they are divided into multiple sub-packages.

From reading the code, what I understand is package mapping does not work for prefixes, e.g. I have com.example.v1, com.example.v2 packages and I want to map com.example to org.example. I should add 2 mappings containing the exact same package name such as

option("kotlin_package_mapping=com.example.v1->org.example.v1;com.example.v2->org.example.v2")

And I can't use

option("kotlin_package_mapping=com.example->org.example")

If I was able to use the latter, (which is similar to prefixing), I would be satisfied but maintaining the first solution is very tough in our case. Maybe we can extend it to support some wildcard matching, what do you think?

Example (Dots might cause some issues but I think if we only parse regex inside parentheses, we will be safe)

option("kotlin_package_mapping=com.example(.*)->org.example($0)")

@Dogacel
Copy link
Contributor Author

Dogacel commented Oct 6, 2022

@garyp

Something like this would help a lot. I tried to keep syntax and implementation simple.

@garyp garyp closed this as completed in 2ca65c2 Nov 12, 2022
@garyp garyp added this to the 0.14.2 milestone Nov 14, 2022
garyp pushed a commit that referenced this issue Mar 7, 2023
1. Release `kotlin_package_mapping` option and document it.
2. Update package mapping logic to include wildcards.

Fixes #230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-codegen enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants