Fix wkt dupes

[perezd]

We already pre-compile the well known types into the runtimes so they shouldn't be re-compiled. #8925.

[elharo]

compiler and descriptor are missing here but present above.

[perezd]

They aren't available in lite.

[acozzette]

I'm going to close this since it appears to be unnecessary now that #8925 has been resolved, but please let me know if I am mistaken.

[eikemeier]

I'm going to close this since it appears to be unnecessary now that #8925 has been resolved, but please let me know if I am mistaken.

@acozzette It's valid. It's a minor issue, since the well known types don't take up much space and are probably identical anyways, but you get duplicates which waste space and introduce uncertainty which version is used.

That said, it's a long standing bug and probably not easy to fix, so it's not high priority. But IMHO the issue shouldn't be closed.

[eikemeier]

See also the demonstration in brach 8942 : https://github.com/eikemeier/proto-java/tree/8942

$ bazel run //:main    
...
Duplicate resource in .../bazel-out/darwin-fastbuild/bin/external/com_google_protobuf/libtimestamp_proto-speed.jar:
 -> com/google/protobuf/Timestamp$1.class
 -> com/google/protobuf/Timestamp$Builder.class
 -> com/google/protobuf/Timestamp.class
 -> com/google/protobuf/TimestampOrBuilder.class
 -> com/google/protobuf/TimestampProto.class
Duplicate resource in .../bazel-out/darwin-fastbuild/bin/external/com_google_protobuf/java/core/libcore.jar:
 -> com/google/protobuf/Timestamp$1.class
 -> com/google/protobuf/Timestamp$Builder.class
 -> com/google/protobuf/Timestamp.class
 -> com/google/protobuf/TimestampOrBuilder.class
 -> com/google/protobuf/TimestampProto.class
Check finished

[acozzette]

@eikemeier Thanks for letting me know. Do you know if this pull request solves this problem? Based on the comments on #8925, it sounded like we were just waiting on a Bazel release.

[eikemeier]

@eikemeier Thanks for letting me know. Do you know if this pull request solves this problem? Based on the comments on #8925, it sounded like we were just waiting on a Bazel release.

The issue is still present in Bazel 5.0rc1, Protobuf 3.19.1, see #9192. Should I try the latest HEAD?

[perezd]

I'm not sure what release that bazel patch came in through, can you check with them if it's present in that release? Try head as well.

…

On Fri, Nov 5, 2021 at 8:06 AM Oliver Eikemeier ***@***.***> wrote: @eikemeier <https://github.com/eikemeier> Thanks for letting me know. Do you know if this pull request solves this problem? Based on the comments on #8925 <#8925>, it sounded like we were just waiting on a Bazel release. The issue is still present in Bazel 5.0rc1, Protobuf 3.19.1, see #9192 <#9192>. Should I try the latest HEAD? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#8942 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFA5VDP4GIQOJBRJHPZB3UKPXH3ANCNFSM5DFEXVGQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[eikemeier]

Try head as well.

With HEAD (+PR #9195) and Bazel 5.0rc1 I see no duplicates any more, see the latest version of https://github.com/eikemeier/proto-java2

Fine so far, is there any documentation that you're supposed to do

maven_install(
    artifacts = [
...
    ] + PROTOBUF_MAVEN_ARTIFACTS,
    override_targets = {
        "com.google.protobuf:protobuf-java": "@com_google_protobuf//:protobuf_java",
        "com.google.protobuf:protobuf-java-util": "@com_google_protobuf//:protobuf_java_util",
        "com.google.protobuf:protobuf-javalite": "@com_google_protobuf//:protobuf_javalite",
    },
...
)

or should I open a PR for that, too?

[perezd]

good catch, please do!