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

protobuf-lite does not include well-known protos #1889

Closed
ejona86 opened this issue Jul 29, 2016 · 29 comments
Closed

protobuf-lite does not include well-known protos #1889

ejona86 opened this issue Jul 29, 2016 · 29 comments

Comments

@ejona86
Copy link
Contributor

ejona86 commented Jul 29, 2016

It does not include generated code nor .proto files. Users need to compile these themselves, which works okay short-term but also means they can't be used from libraries. We can either include the generated code in the protobuf-lite artifact, or create a new artifact to hold them.

The reasoning behind a new artifact is that, until recently, protobuf-lite was a subset of protobuf, which would no longer be possible if it included lite-generated code for wellknown protos. However, if lite isn't going to be a subset of protobuf again, then putting the generated classes in the same artifact would become easier for everyone.

ejona86 added a commit to ejona86/grpc-java that referenced this issue Jul 29, 2016
Protobuf-lite since beta-4 is now more of a fork than a subset of
protobuf-java, which may cause us problems later since lite API is not
stable. Also, lite-generated code may now depend on APIs only in
protobuf-lite, so our users must depend on the protobuf-lite runtime.
Having all our users explicitly override the dependency is bothersome to
them and can easily only expose problems only after we do a release.

So now we are doing the dependency overriding; most users should "just
work" and pick up the correct protobuf artifact. I've confirmed the
exclusion is listed in the grpc-protobuf pom and "gradle dependencies"
and "mvn dependency:tree" do not include protobuf-lite for the examples.
Vanilla protobuf users are most likely to experience any breakage, which
should detect problems more quickly since we use protobuf-java more
frequently than protobuf-lite during development.

protobuf-lite does not include pre-generated code for the well-known
protos, so users will need to generate them themselves for the moment
(protocolbuffers/protobuf#1889).

Note that today changing deps does not noticeably reduce the method code
for our users, since ProGuard already is stripping most classes. The
difference in output is only a reduction of 3 classes and 6 methods for
the android example.
ejona86 added a commit to ejona86/grpc-java that referenced this issue Aug 1, 2016
Protobuf-lite since beta-4 is now more of a fork than a subset of
protobuf-java, which may cause us problems later since lite API is not
stable. Also, lite-generated code may now depend on APIs only in
protobuf-lite, so our users must depend on the protobuf-lite runtime.
Having all our users explicitly override the dependency is bothersome to
them and can easily only expose problems only after we do a release.

So now we are doing the dependency overriding; most users should "just
work" and pick up the correct protobuf artifact. I've confirmed the
exclusion is listed in the grpc-protobuf pom and "gradle dependencies"
and "mvn dependency:tree" do not include protobuf-lite for the examples.
Vanilla protobuf users are most likely to experience any breakage, which
should detect problems more quickly since we use protobuf-java more
frequently than protobuf-lite during development.

protobuf-lite does not include pre-generated code for the well-known
protos, so users will need to generate them themselves for the moment
(protocolbuffers/protobuf#1889).

Note that today changing deps does not noticeably reduce the method code
for our users, since ProGuard already is stripping most classes. The
difference in output is only a reduction of 3 classes and 6 methods for
the android example.
ejona86 added a commit to ejona86/grpc-java that referenced this issue Aug 2, 2016
Protobuf-lite since beta-4 is now more of a fork than a subset of
protobuf-java, which may cause us problems later since lite API is not
stable. Also, lite-generated code may now depend on APIs only in
protobuf-lite, so our users must depend on the protobuf-lite runtime.
Having all our users explicitly override the dependency is bothersome to
them and can easily only expose problems only after we do a release.

So now we are doing the dependency overriding; most users should "just
work" and pick up the correct protobuf artifact. I've confirmed the
exclusion is listed in the grpc-protobuf pom and "gradle dependencies"
and "mvn dependency:tree" do not include protobuf-lite for the examples.
Vanilla protobuf users are most likely to experience any breakage, which
should detect problems more quickly since we use protobuf-java more
frequently than protobuf-lite during development.

protobuf-lite does not include pre-generated code for the well-known
protos, so users will need to generate them themselves for the moment
(protocolbuffers/protobuf#1889).

Note that today changing deps does not noticeably reduce the method code
for our users, since ProGuard already is stripping most classes. The
difference in output is only a reduction of 3 classes and 6 methods for
the android example.
@Cloudef
Copy link

Cloudef commented Aug 12, 2016

This is not only relevant to java but other languages too such as C++. There is also a huge problem when you compile the well-known protos for lite with type compatibility to full runtime (that is same typename in specification, meaning they reside in similar directory structure), that protobuf ships with pre-generated include files for full runtime in system path, and you can't simply workaround this by include path ordering as some compilers such as Xcode don't seem to respect that fully. So basically the runtime specific files provided in system path should be moved to own directory, and tell developers to add that to their include path if they decide to use full runtime (you could also now provide similar directory for lite, if you want to ship pre-generated well-known types).

@xfxyjwf xfxyjwf added the lite label Aug 17, 2016
@Cloudef
Copy link

Cloudef commented Aug 29, 2016

The generator also seems to generate PackFrom to UnpackTo methods which depend on symbol only included in full runtime.

@estsauver
Copy link

I am still seeing this issue, are other folks experiencing this still?

@xfxyjwf xfxyjwf added the bug label Oct 27, 2016
@xfxyjwf
Copy link
Contributor

xfxyjwf commented Oct 27, 2016

Right now well-known types doesn't work with lite runtime.

@ejona86
Copy link
Contributor Author

ejona86 commented Nov 19, 2016

For people experiencing this, there is an okay short-term workaround for applications (sorry, no good for libraries) which I alluded to it in my original comment. If using Gradle, you just add a "protobuf" dependency to your build. The normal protobuf jar includes the .protos, so this will extract those to let import work and will generate the code for them.

dependencies {
  protobuf 'com.google.protobuf:protobuf-java:3.0.2'
}

I think it is harder in Maven, but still possible to some degree. I've not tried it though. The above Gradle snippet appears to work in #2321.

@kamalmarhubi
Copy link

@pherl @xfxyjwf are you expecting to work in this in the near term?

@anandolee
Copy link
Contributor

@xfxyjwf any update ?

@m-sasha
Copy link

m-sasha commented Apr 30, 2018

ejona86's workaround doesn't help if you have two Android modules, both of which need the well-known-types, because you then end up with dex failing due to duplicate classes.

@ejona86
Copy link
Contributor Author

ejona86 commented Apr 30, 2018

@m-sasha, yes. Android modules like you describe are "libraries".

@kamalmarhubi
Copy link

Our solution is to shade the well-known protos we use. This is not ideal as it can result in unnecessary duplication, but it beats failing because of conflicts as @m-sasha points out.

@m-sasha
Copy link

m-sasha commented Apr 30, 2018

@kamalmarhubi, can you explain what you mean by that?

@xfxyjwf xfxyjwf assigned xfxyjwf and unassigned liujisi Apr 30, 2018
@xfxyjwf xfxyjwf added this to the Java Lite release milestone Apr 30, 2018
@ryanrhee
Copy link

Any updates?

@TeBoring TeBoring added this to To Do in Weekly Fixit via automation Jun 18, 2018
@xfxyjwf xfxyjwf removed the lite label Jun 22, 2018
@xfxyjwf
Copy link
Contributor

xfxyjwf commented Jul 9, 2018

I'll make a Java lite release this quarter (Q3), and for well-known types I plan to create a separate Java artifact.

@rok-povsic
Copy link

@kamalmarhubi I'd be interested in hearing more about this as well.

@xfxyjwf Is there still a plan for creating a separate well-known types artifact?

@jombie
Copy link

jombie commented Dec 4, 2018

@xfxyjwf Any update on these. For now I am copying the proto files I am using into my src manually.

@AaronTriplett
Copy link

Any updates on this? Would prefer not to shade the dependencies unless we absolutely have to.

@kamalmarhubi
Copy link

@xfxyjwf @BSBandme an update on this? An official javalite artifact would be great, and seems like not too much effort. Fellow googlers over in Firebase land are already doing this. :-)

@asan855
Copy link

asan855 commented Aug 19, 2019

Any updates on this?

@ejona86
Copy link
Contributor Author

ejona86 commented Nov 13, 2019

I just tested with 3.10.0 and this seems fixed. The protobuf-javalite artifact contains Any.class and Duration.class. It also includes some of the .protos themselves. It's not clear why duration.proto is missing, for example. But the generated code is there, so closing.

@ejona86 ejona86 closed this as completed Nov 13, 2019
@junteken
Copy link

I just tested with 3.10.0 and this seems fixed. The protobuf-javalite artifact contains Any.class and Duration.class. It also includes some of the .protos themselves. It's not clear why duration.proto is missing, for example. But the generated code is there, so closing.

com.google.protobuf.Any class don't have unpack method. when this problem will be solved?

@ericgribkoff
Copy link

I think closing this might have been premature. For example, the inclusion of Duration.class without the proto doesn't (obviously, at least to me) help protobuf-javalite users, since if their proto depends on duration.proto, they will also need the proto file itself. But blindly copy-and-pasting duration.proto into the local codebase then results in duplicate class errors, as you end up with the Duration.class generated from the copied proto file and the same class included with the artifact.

@ejona86
Copy link
Contributor Author

ejona86 commented Jan 6, 2020

@ericgribkoff, we can discuss some on Wednesday when it can be in-person, but I don't think it should work like that (you can also discuss with @voidzyc). Android users use Gradle and should use the Protobuf Gradle Plugin. The plugin allows you to include a dependency on a .jar file that contains generated code as well as the .proto files themselves. When you do so, the .proto files are extracted and used with -I to protoc which makes them available for import but it won't regenerate code for them. Basically, it all "just works" and no copy/pasting is necessary.

There are some details where things do break down, like I don't remember if the .proto files are included in the lite jar, you don't actually want the .proto files to be shipped in your app, and a few similar issues. But I do think this is a step in the right direction and there can be some ways to resolve the other issues.

@kaniundungu
Copy link

Here's a great workaround for those who can't wait for an official fix:

#7094 (comment)

@tokohone
Copy link

tokohone commented Jul 2, 2020

Just tried to use the Android Gradle integration with com.google.protobuf:protobuf-javalite:3.8.0 and com.google.protobuf:protoc:3.8.0.

I think the generated code is now missing protobuf.Any.pack() and unpack(). Could not see an issue reported about this yet and this issue in general fits the symptoms.

So will pack() and unpack() be added?

@tmaxted
Copy link

tmaxted commented Aug 6, 2020

I think the generated code is now missing protobuf.Any.pack() and unpack(). Could not see an issue reported about this yet and this issue in general fits the symptoms.

Same issue as @tokohone here. Tried using protobuf-javalite:3.8.0/3.10.0/3.12.2 and same result

@Maragues
Copy link

@ejona86 This is still an issue for other well-known protos, such as datetime.proto

For those of us shipping libraries in 2023, what's the recommended way forward?

I get runtime exceptions when unmarshalling grpc responses that include DateTime, for example.

I can fix that with protobuf-gradle-plugin + protobuf("com.google.api.grpc:proto-google-common-protos:x.y.z") in the app/build.gradle of the applications that we own. But, I can't ask clients to do that in their apps.

And, I don't want to ship a library that includes a com.google.type.DateTime, because it'll cause DuplicateClassException.

From my naive perspective, I don't understand the lack of a proto-google-common-protos-lite artifact.

@ejona86
Copy link
Contributor Author

ejona86 commented Nov 13, 2023

datetime.proto is not a "well-known proto." That is part of the googleapis project, not protobuf.

There's no proto-google-common-protos-lite because there is no API/ABI stability guarantees for lite codegen. You need to generate all lite protos with the same version of the generator, so pre-built libraries doesn't make much sense. You can't ship a library that has pre-built lite protos, unless you can control the version of protobuf the app uses (which could be the case within a single company, but not for anything that goes on Maven Central). If you use protobuf only internally to your library, you can shade protobuf, but obviously that bloats its size so isn't really much of a help.

@Maragues
Copy link

Thank you very much for the fast response and clear explanation 🙇🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Weekly Fixit
  
To Do
Development

No branches or pull requests