Move android protos from nano to lite |
||||||||||||
Issue descriptionLite is the new prefered proto runtime on Android, for two reasons: - It lends itself to better proguard optimizing - There is only one flavor, so there is no risk in including proto libraries with overlapping definitions that use different flavors We should add support for the lite runtime on Android, and then convert the existing nano proto libraries over to lite.
,
Nov 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cc12c7a9522169c079a0c0c6ad446088f6cbea1d commit cc12c7a9522169c079a0c0c6ad446088f6cbea1d Author: Florian Uunk <feuunk@google.com> Date: Fri Nov 17 03:11:07 2017 Enable using lite protos for Android. Lite is the new preferred proto runtime on Android, for two reasons: - It lends itself to better proguard optimizing - There is only one flavor, so there is no risk in including proto libraries with overlapping definitions that use different flavors This CL adds a generate_nano argument to the proto_java_library build rule. This argument is default false, so new proto libraries will use lite by default. However, existing libraries will be migrated in a follow-up CL, so this change sets generate_nano to true for those libraries. It also adds the android_library rule that contains the runtime library for lite protos. For an example conversion CL for a proto target, see: https://chromium-review.googlesource.com/c/chromium/src/+/757103 Bug: 782237 Change-Id: I8100e70c38d41add9068e493ca2a5822f7025213 Reviewed-on: https://chromium-review.googlesource.com/757134 Commit-Queue: agrieve <agrieve@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Reviewed-by: Doug Steedman <dougsteed@chromium.org> Cr-Commit-Position: refs/heads/master@{#517268} [modify] https://crrev.com/cc12c7a9522169c079a0c0c6ad446088f6cbea1d/build/config/android/rules.gni [modify] https://crrev.com/cc12c7a9522169c079a0c0c6ad446088f6cbea1d/build/protoc_java.py [modify] https://crrev.com/cc12c7a9522169c079a0c0c6ad446088f6cbea1d/third_party/protobuf/BUILD.gn
,
Dec 1 2017
We should also consider using the protoc invokation to force the lite flavor of the proto library to be generated. We could do this by using the protoc-gen-java-lite plugin: https://mvnrepository.com/artifact/com.google.protobuf/protoc-gen-javalite Which allows you to generate lite code, by running protoc --java_lite_out=... This could replace the current workaround where we force all java protos to include 'option optimize_for = LITE_RUNTIME' to generate lite.
,
Dec 1 2017
Would there be any downside to removing the optimize_for configuration? Or would documentation be the only thing? Now that we're generating two different types, it does feel natural to be able to remove it.
,
Dec 1 2017
Currently, the optimize_for configuration is what actually triggers the lite java (as opposed to full java) generation, which means that all java protos should have it. However, if we make the change described in #3, and switch the plugin and use --java_lite_out, there is no benefit (for protos only used from java) in including the optimize_for configuration anymore, and at that point we could just remove it.
,
Dec 4 2017
,
Dec 4 2017
,
Dec 5 2017
My plan: - I started figuring out how to do #3. This can take some time. - In the mean-time, I convert individual protobuf uses to lite (using the optimize_for) in several CLs. - When I figure out #3, I remove all the optimize_for options in one final CL.
,
Jan 8 2018
I've digged into removing the 'option optimize_for = LITE_RUNTIME', discussed in #3: - it would require us to switch to javalite branch of the protobuf library, - a owner of third_party/protobuf strongly discouraged me from switching to non-master branch, meaning it could take a significant amount of work, - there are plans to eventually merge this branch onto master, it will probably take some time (~ until a new major version). My proposal is to keep the options in the proto files until the master branch of protobuf is capable of producing lite using just a command-line flag. I'll annotate all the instances of the option with a crbug.
,
Jan 8 2018
I agree it would be great to wait until a new version that supports command line switch.
,
Jan 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3013ca1bafd34f776b14ec5595b8693bf4c562dc commit 3013ca1bafd34f776b14ec5595b8693bf4c562dc Author: Jan Krcal <jkrcal@chromium.org> Date: Wed Jan 17 18:06:16 2018 [protobuf] Remove cast_certification proto java targets As part of moving android chrome from proto nano to proto lite, this CL removes unused java proto (nano) build targets. Bug: 782237 Change-Id: I1be7b699f551daaa48b696c9e4859c1dd1c89222 Reviewed-on: https://chromium-review.googlesource.com/856936 Reviewed-by: Doug Steedman <dougsteed@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#529802} [modify] https://crrev.com/3013ca1bafd34f776b14ec5595b8693bf4c562dc/components/cast_certificate/proto/BUILD.gn
,
Mar 14 2018
Summarizing all the discussions in the mean-time. Reasons to switch: - lite has been already used by code in the internal repo - lite is used by //third_party/feed (which heavily relies on protobuf) - nano is used by //third_party/cacheinvalidation that is deprecated and should get removed later this year - switching the remaining java code to lite will allow us to remove the nano dependency Impact of the switch (overall code change can be seen in https://chromium-review.googlesource.com/c/chromium/src/+/878263): Metric Count Diff in % ----------------------------------------------------------------------------- Dex: methods 55139 entries 114 0.21% Dex: types 10839 entries 23 0.21% Dex: fields 26163 entries 56 0.21% Dex: strings 49479 entries 113 0.23% DexCache: DexCache 566480 bytes 1,224 0.22% ----------------------------------------------------------------------------- InstallSize: APK size 112654976 bytes 8,191 0.01% InstallSize: Estimated installed size 141815308 bytes 86,324 0.06% TransferSize: Transfer size (deflate) 52590779 bytes 13,200 0.03% Specifics: main dex size 7148988 bytes 19,340 0.27%
,
Mar 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/65ae1de21ba8e64e15ace4ab99028628e166b5c6 commit 65ae1de21ba8e64e15ace4ab99028628e166b5c6 Author: Jan Krcal <jkrcal@chromium.org> Date: Thu Mar 15 06:32:14 2018 [protobuf] Convert thumbnail_cache_entry from nano to lite This is the first CL in a sequence that converts all java protobuf uses from nano to lite. It disables one proguard optimization that breaks with lite-generated code. Bug: 782237 Change-Id: Ifed32f216dacad69c1f32e903d8d30070070404b Reviewed-on: https://chromium-review.googlesource.com/808684 Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#543317} [modify] https://crrev.com/65ae1de21ba8e64e15ace4ab99028628e166b5c6/build/android/gyp/proguard.py [modify] https://crrev.com/65ae1de21ba8e64e15ace4ab99028628e166b5c6/chrome/android/BUILD.gn [modify] https://crrev.com/65ae1de21ba8e64e15ace4ab99028628e166b5c6/chrome/android/java/src/org/chromium/chrome/browser/widget/ThumbnailDiskStorage.java [modify] https://crrev.com/65ae1de21ba8e64e15ace4ab99028628e166b5c6/chrome/android/java/src/org/chromium/chrome/browser/widget/thumbnail_cache_entry.proto
,
Mar 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe785bc5075e6c74c39ccdcb64b6e80b3f5d33f2 commit fe785bc5075e6c74c39ccdcb64b6e80b3f5d33f2 Author: Jan Krcal <jkrcal@chromium.org> Date: Thu Mar 15 06:48:27 2018 [protobuf] Convert document_tab_model_info from nano to lite Bug: 782237 Change-Id: Id89fbfdd30c769fc1dc272d81b1a2fffdb7903fd Reviewed-on: https://chromium-review.googlesource.com/809017 Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#543318} [modify] https://crrev.com/fe785bc5075e6c74c39ccdcb64b6e80b3f5d33f2/chrome/android/BUILD.gn [modify] https://crrev.com/fe785bc5075e6c74c39ccdcb64b6e80b3f5d33f2/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/StorageDelegate.java [modify] https://crrev.com/fe785bc5075e6c74c39ccdcb64b6e80b3f5d33f2/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/document_tab_model_info.proto
,
Mar 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3760b8c85d55161fc05626d395383d5bd97c28e6 commit 3760b8c85d55161fc05626d395383d5bd97c28e6 Author: Jan Krcal <jkrcal@chromium.org> Date: Thu Mar 15 06:51:40 2018 [protobuf] Convert partner_location_descriptor from nano to lite Bug: 782237 Change-Id: Ieb698518c15a84a5759d1c3cf99cff8a40f8f663 Reviewed-on: https://chromium-review.googlesource.com/854053 Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#543319} [modify] https://crrev.com/3760b8c85d55161fc05626d395383d5bd97c28e6/chrome/android/BUILD.gn [modify] https://crrev.com/3760b8c85d55161fc05626d395383d5bd97c28e6/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java [modify] https://crrev.com/3760b8c85d55161fc05626d395383d5bd97c28e6/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworks.java [modify] https://crrev.com/3760b8c85d55161fc05626d395383d5bd97c28e6/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/partner_location_descriptor.proto [modify] https://crrev.com/3760b8c85d55161fc05626d395383d5bd97c28e6/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeaderTest.java [modify] https://crrev.com/3760b8c85d55161fc05626d395383d5bd97c28e6/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworksTest.java
,
Mar 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2464913feaf48c65e38da101f5059380eacbe409 commit 2464913feaf48c65e38da101f5059380eacbe409 Author: Jan Krcal <jkrcal@chromium.org> Date: Thu Mar 15 07:14:37 2018 [protobuf] Convert serialized_invalidation from nano to lite Bug: 782237 Change-Id: I030322a93c71eec70fc4a53a4cb21458c63678ee Reviewed-on: https://chromium-review.googlesource.com/856796 Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#543322} [modify] https://crrev.com/2464913feaf48c65e38da101f5059380eacbe409/components/invalidation/impl/BUILD.gn [modify] https://crrev.com/2464913feaf48c65e38da101f5059380eacbe409/components/invalidation/impl/android/java/src/org/chromium/components/invalidation/PendingInvalidation.java [modify] https://crrev.com/2464913feaf48c65e38da101f5059380eacbe409/components/invalidation/impl/android/proto/serialized_invalidation.proto
,
Mar 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/15de70612a01d97219b3d07b5b4d7a16553d287d commit 15de70612a01d97219b3d07b5b4d7a16553d287d Author: Jan Krcal <jkrcal@chromium.org> Date: Mon Mar 19 16:55:27 2018 Revert "[protobuf] Convert serialized_invalidation from nano to lite" This reverts commit 2464913feaf48c65e38da101f5059380eacbe409. Reason for revert: crasher due to semantic changes between nano and lite and lacking unit-tests (crbug.com/822845) Original change's description: > [protobuf] Convert serialized_invalidation from nano to lite > > Bug: 782237 > Change-Id: I030322a93c71eec70fc4a53a4cb21458c63678ee > Reviewed-on: https://chromium-review.googlesource.com/856796 > Reviewed-by: Tommy Nyquist <nyquist@chromium.org> > Commit-Queue: Jan Krcal <jkrcal@chromium.org> > Cr-Commit-Position: refs/heads/master@{#543322} TBR=nyquist@chromium.org,jkrcal@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 782237 Change-Id: Id448b779470070a5e0e2f55708f1a1a194064d02 Reviewed-on: https://chromium-review.googlesource.com/968961 Reviewed-by: Jan Krcal <jkrcal@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#544064} [modify] https://crrev.com/15de70612a01d97219b3d07b5b4d7a16553d287d/components/invalidation/impl/BUILD.gn [modify] https://crrev.com/15de70612a01d97219b3d07b5b4d7a16553d287d/components/invalidation/impl/android/java/src/org/chromium/components/invalidation/PendingInvalidation.java [modify] https://crrev.com/15de70612a01d97219b3d07b5b4d7a16553d287d/components/invalidation/impl/android/proto/serialized_invalidation.proto
,
Mar 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d961d9dd1faa140b3e9fe6c1a4d46bb90ca0f590 commit d961d9dd1faa140b3e9fe6c1a4d46bb90ca0f590 Author: Jan Krcal <jkrcal@chromium.org> Date: Mon Mar 19 19:11:19 2018 [protobuf] Convert sync tests from nano to lite Bug: 782237 Change-Id: Ic1f84717cd457218cc91dc08f57af0c138d5b36b Reviewed-on: https://chromium-review.googlesource.com/859762 Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Reviewed-by: Mikel Astiz <mastiz@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#544101} [modify] https://crrev.com/d961d9dd1faa140b3e9fe6c1a4d46bb90ca0f590/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/AutofillTest.java [modify] https://crrev.com/d961d9dd1faa140b3e9fe6c1a4d46bb90ca0f590/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/BookmarksTest.java [modify] https://crrev.com/d961d9dd1faa140b3e9fe6c1a4d46bb90ca0f590/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java [modify] https://crrev.com/d961d9dd1faa140b3e9fe6c1a4d46bb90ca0f590/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/OpenTabsTest.java [modify] https://crrev.com/d961d9dd1faa140b3e9fe6c1a4d46bb90ca0f590/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/TypedUrlsTest.java [modify] https://crrev.com/d961d9dd1faa140b3e9fe6c1a4d46bb90ca0f590/components/sync/BUILD.gn
,
Apr 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b31b5e58fb517a568678974fc4a7965095c22c1e commit b31b5e58fb517a568678974fc4a7965095c22c1e Author: Jan Krcal <jkrcal@chromium.org> Date: Tue Apr 03 14:00:11 2018 Reland "[protobuf] Convert serialized_invalidation from nano to lite" Compared to the original CL, this one fixes (in the second patch set) a bug in PendingIvalidation.encodeToString() that caused frequent NullPointerException crashes on canary. A corresponding unit-test is added. Bug: 782237 Change-Id: Ia2eacd4bc71122e4b3ca721b44f2b4472530861a Reviewed-on: https://chromium-review.googlesource.com/970341 Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#547691} [modify] https://crrev.com/b31b5e58fb517a568678974fc4a7965095c22c1e/components/invalidation/impl/BUILD.gn [modify] https://crrev.com/b31b5e58fb517a568678974fc4a7965095c22c1e/components/invalidation/impl/android/java/src/org/chromium/components/invalidation/PendingInvalidation.java [modify] https://crrev.com/b31b5e58fb517a568678974fc4a7965095c22c1e/components/invalidation/impl/android/junit/src/org/chromium/components/invalidation/PendingInvalidationTest.java [modify] https://crrev.com/b31b5e58fb517a568678974fc4a7965095c22c1e/components/invalidation/impl/android/proto/serialized_invalidation.proto
,
Apr 3 2018
The only task that remain on this bug: Remove the "generate_lite = true" params from all the build files, making lite the default for java protobuf generated code. If needed, this includes adding "generate_nano = true" to the build files in //third_party/invalidation. Note that this third-party library should get removed later in 2018.
,
Apr 3 2018
It sounds like we should keep around generate_nano instead until we can update everything, but I agree that we should change the default around now.
,
Apr 6 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/clank/internal/apps/+/e704260a300290fca7f363b5a1d3c27f0b3de732 commit e704260a300290fca7f363b5a1d3c27f0b3de732 Author: Jan Krcal <jkrcal@google.com> Date: Fri Apr 06 12:23:22 2018
,
Apr 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dc072552bd919ccdbacdd8f0ca4373791712f10b commit dc072552bd919ccdbacdd8f0ca4373791712f10b Author: Jan Krcal <jkrcal@chromium.org> Date: Mon Apr 09 08:52:16 2018 [protobuf] Switch the default java protobuf generator from nano to lite This CL takes the (almost) last step in converting java code from protobuf nano to lite. The only remaining code with nano is //third_party/cacheinvalidation which is deprecated and should be replaced later this year. We still need to keep the generate_lite variable in rules.gni because it is used in the internal repo. After this CL, I will - remove instances of generate_lite from the internal repo and finally - remove generate_lite from rules.gni. Bug: 782237 Change-Id: I65f144c59d84e8c75fe3152543c6e1941e52a3dd Reviewed-on: https://chromium-review.googlesource.com/998033 Reviewed-by: Pavel Yatsuk <pavely@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#549129} [modify] https://crrev.com/dc072552bd919ccdbacdd8f0ca4373791712f10b/build/config/android/rules.gni [modify] https://crrev.com/dc072552bd919ccdbacdd8f0ca4373791712f10b/build/protoc_java.py [modify] https://crrev.com/dc072552bd919ccdbacdd8f0ca4373791712f10b/chrome/android/BUILD.gn [modify] https://crrev.com/dc072552bd919ccdbacdd8f0ca4373791712f10b/components/invalidation/impl/BUILD.gn [modify] https://crrev.com/dc072552bd919ccdbacdd8f0ca4373791712f10b/components/sync/BUILD.gn [modify] https://crrev.com/dc072552bd919ccdbacdd8f0ca4373791712f10b/third_party/cacheinvalidation/BUILD.gn [modify] https://crrev.com/dc072552bd919ccdbacdd8f0ca4373791712f10b/third_party/feed/BUILD.gn
,
Apr 10 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/clank/internal/apps/+/95f65a9e8343f1e29ac2850d7f8b84c7dd720359 commit 95f65a9e8343f1e29ac2850d7f8b84c7dd720359 Author: Jan Krcal <jkrcal@google.com> Date: Tue Apr 10 08:04:39 2018
,
Apr 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/de7c257bd6c2600d01fb1e8b7d4697f4dd4119bc commit de7c257bd6c2600d01fb1e8b7d4697f4dd4119bc Author: Jan Krcal <jkrcal@chromium.org> Date: Tue Apr 10 13:28:46 2018 [protobuf] Remove the 'generate_lite' variable from rules.gni This CL is part of the process of switching from protobuf nano to protobuf lite. Bug: 782237 Change-Id: Iffbdafb225d2ba2dadee14a5b17e7be88a452341 Reviewed-on: https://chromium-review.googlesource.com/1000698 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#549502} [modify] https://crrev.com/de7c257bd6c2600d01fb1e8b7d4697f4dd4119bc/build/config/android/rules.gni
,
Apr 10 2018
The most of this task is done now. The only thing that remains is to remove the generate_nano variable from build/config/android/rules.gni (and the nano option from build/protoc_java.py). This can only happen once we remove //third_party/cacheinvalidation, blocked on that task.
,
Aug 29
I get lots of these errors when building release monochrome_apk: [libprotobuf WARNING ../../third_party/protobuf/src/google/protobuf/compiler/java/java_file.cc:228] The optimize_for = LITE_RUNTIME option is no longer supported by protobuf Java code generator and may generate broken code. It will be ignored by protoc in the future and protoc will always generate full runtime code for Java. To use Java Lite runtime, users should use the Java Lite plugin instead. See: https://github.com/google/protobuf/blob/master/java/lite.md Is this a known issue?
,
Aug 29
,
Aug 29
protobuf just rolled 2 days ago: e684cf418f04a776b3ffe3074dd635cce18e1870 Likely new.
,
Aug 30
Issue 878938 has been merged into this issue.
,
Aug 30
Will take a look. I am wondering what is exactly the impact of this roll, I hope it still produces correct lite code (and not full protobuf code).
,
Aug 30
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/27a0ea6a6edc65e425b3b0eae20e56c5d0fffac5 commit 27a0ea6a6edc65e425b3b0eae20e56c5d0fffac5 Author: Jan Krcal <jkrcal@chromium.org> Date: Tue Sep 04 06:34:36 2018 [Protobuf] Remove stale dependencies from chrome/android/BUILD.gn This CL cleans up BUILD.gn for android to make clear no-one here is using the nano protobuf. The only remaining user is //third_party/cacheinvalidation/BUILD.gn (to be removed soon). Bug: 782237 Change-Id: Ic6e09a8eeb192971249666f1b34cd3fc0d7ee0d1 Reviewed-on: https://chromium-review.googlesource.com/1202182 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#588461} [modify] https://crrev.com/27a0ea6a6edc65e425b3b0eae20e56c5d0fffac5/chrome/android/BUILD.gn
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/27a0ea6a6edc65e425b3b0eae20e56c5d0fffac5 commit 27a0ea6a6edc65e425b3b0eae20e56c5d0fffac5 Author: Jan Krcal <jkrcal@chromium.org> Date: Tue Sep 04 06:34:36 2018 [Protobuf] Remove stale dependencies from chrome/android/BUILD.gn This CL cleans up BUILD.gn for android to make clear no-one here is using the nano protobuf. The only remaining user is //third_party/cacheinvalidation/BUILD.gn (to be removed soon). Bug: 782237 Change-Id: Ic6e09a8eeb192971249666f1b34cd3fc0d7ee0d1 Reviewed-on: https://chromium-review.googlesource.com/1202182 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#588461} [modify] https://crrev.com/27a0ea6a6edc65e425b3b0eae20e56c5d0fffac5/chrome/android/BUILD.gn
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/27a0ea6a6edc65e425b3b0eae20e56c5d0fffac5 commit 27a0ea6a6edc65e425b3b0eae20e56c5d0fffac5 Author: Jan Krcal <jkrcal@chromium.org> Date: Tue Sep 04 06:34:36 2018 [Protobuf] Remove stale dependencies from chrome/android/BUILD.gn This CL cleans up BUILD.gn for android to make clear no-one here is using the nano protobuf. The only remaining user is //third_party/cacheinvalidation/BUILD.gn (to be removed soon). Bug: 782237 Change-Id: Ic6e09a8eeb192971249666f1b34cd3fc0d7ee0d1 Reviewed-on: https://chromium-review.googlesource.com/1202182 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#588461} [modify] https://crrev.com/27a0ea6a6edc65e425b3b0eae20e56c5d0fffac5/chrome/android/BUILD.gn
,
Sep 6
Will the removal of nano protobuf usage in //third_party/cacheinvalidation/BUILD.gn result in the removal of these build messages? [libprotobuf WARNING ../../third_party/protobuf/src/google/protobuf/compiler/java/java_file.cc:228] The optimize_for = LITE_RUNTIME option is no longer supported by proto buf Java code generator and may generate broken code. It will be ignored by protoc in the future and protoc will always generate full runtime code for Java. To use Java L ite runtime, users should use the Java Lite plugin instead. See: https://github.com/google/protobuf/blob/master/java/lite.md Currently we get several screens' worth of these messages on every build.
,
Sep 10
Re 36#: no, this needs a separate fix. It is on my plate but I am booked for this week. Hope to look into it next week. If anyone is willing to help out, please see the discussion in https://groups.google.com/a/google.com/d/topic/protobuf-team/dBA6bjYMHmc/discussion
,
Oct 26
Any ETA on a fix for the compile warnings? If this is a known-issue and the fix is underway, we should at least silence them in the interim.
,
Oct 29
Unfortunately, no progress so far. I am sorry for not moving forward with this. Let me drop the bug to indicate that I am not really actively working on this. Does anyone in cc know of someone experienced with clank / maven that has some free cycles to fix this, please?
,
Oct 29
So, from the discussion, is the plan to use https://chromium.googlesource.com/chromium/src/+/master/tools/android/roll/android_deps/README.md and roll in https://github.com/protocolbuffers/protobuf/blob/master/java/lite.md using that tool?
,
Oct 29
Yes, this is how I understand the conclusion.
,
Nov 15
I'll look into this. I'm sufficiently miffed about the extra output of every debug build that I have to scroll past.
,
Nov 15
Reassigning to Mohamed who has already started working on it
,
Nov 16
Here is an update of the plan and where we are currently. The new way of doing things requires two things 1- Use the lite gen plugin for protoc to generate lite protos 2- Use the new separate java lite runtime instead For (1) I have a cl (crrev.com/c/1338207) to add the plugin executables to the repo for linux, osx and windows. I am not sure if building clank supports other platforms but that would no longer be the case after this. The cl is awaiting review by security and opensource licensing. For (2), I have a cl (crrev.com/c/1338574) to add the new java lite runtime library to the repo. This is going to replace //third_party/protobuf:protobuf_lite_javalib as the runtime library included in the apk. This is also awaiting review by security, eng and opensource licensing. Finally I have a third cl (crrev.com/c/1340522) to move us to the new way of doing things and finally silence the deprecation warning. I will send it for review once the previous CLs have been merged.
,
Nov 16
Re (1); I think it's quite common to have a single checkout on a Linux desktop computer and being able to build either Linux or Android. Are you saying that this specific developer workflow will break?
,
Nov 16
No, building on linux would work. I was more worried about building clank on other host OSs (ie if we supported building clank as a developer running a windows machine or a mac). Either way, it turns out that we only support compiling from a linux machine for clank so I will just support that and ignore other possible host OSs
,
Nov 19
Mentioning here for completeness (already commented in the CLs). For (1): No need for security and licensing, this is a Google binary that we are using during build, not being shipped. For (2): No need for security, licensing, or third_party/OWNERS. The code is simply split out of the existing third_party/protobuf (literally the same code, maybe a couple revisions behind) with the same LICENSE. This is effectively a rename and no need to review again.
,
Nov 19
Also, +1 for only supporting building on linux.
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7dc993cbc61a1519cbf449c8eb4d3e28aee97238 commit 7dc993cbc61a1519cbf449c8eb4d3e28aee97238 Author: Mohamed Heikal <mheikal@chromium.org> Date: Tue Nov 20 22:16:34 2018 [Android Deps] Add protobuf lite runtime to repo The Protobuf Java Lite runtime is now separate from the main java runtime. This cl adds the java lite runtime to the repo. In a follow up Bug: 782237 Change-Id: I9f4af1736006ab224dc4230d3a071290974135b8 Reviewed-on: https://chromium-review.googlesource.com/c/1338574 Reviewed-by: Grace Kloba <klobag@chromium.org> Reviewed-by: Peter Wen <wnwen@chromium.org> Reviewed-by: Oliver Chang <ochang@chromium.org> Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Cr-Commit-Position: refs/heads/master@{#609823} [modify] https://crrev.com/7dc993cbc61a1519cbf449c8eb4d3e28aee97238/DEPS [modify] https://crrev.com/7dc993cbc61a1519cbf449c8eb4d3e28aee97238/third_party/android_deps/BUILD.gn [modify] https://crrev.com/7dc993cbc61a1519cbf449c8eb4d3e28aee97238/third_party/android_deps/additional_readme_paths.json [add] https://crrev.com/7dc993cbc61a1519cbf449c8eb4d3e28aee97238/third_party/android_deps/libs/com_google_protobuf_protobuf_lite/LICENSE [add] https://crrev.com/7dc993cbc61a1519cbf449c8eb4d3e28aee97238/third_party/android_deps/libs/com_google_protobuf_protobuf_lite/OWNERS [add] https://crrev.com/7dc993cbc61a1519cbf449c8eb4d3e28aee97238/third_party/android_deps/libs/com_google_protobuf_protobuf_lite/README.chromium [add] https://crrev.com/7dc993cbc61a1519cbf449c8eb4d3e28aee97238/third_party/android_deps/libs/com_google_protobuf_protobuf_lite/cipd.yaml [modify] https://crrev.com/7dc993cbc61a1519cbf449c8eb4d3e28aee97238/tools/android/roll/android_deps/build.gradle
,
Nov 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c488598e6a9fa806467a1648762c4e8dc2cea30 commit 5c488598e6a9fa806467a1648762c4e8dc2cea30 Author: Mohamed Heikal <mheikal@chromium.org> Date: Mon Nov 26 23:32:15 2018 Add protoc javalite plugin to third_party/ New versions of protoc require a java lite plugin to generate lite protos. That will be added to the checkout inside third_party/protoc_javalite using cipd. Bug: 782237 Change-Id: I808b0acc14832c80e5efc4baacffb3323bc98653 Reviewed-on: https://chromium-review.googlesource.com/c/1338207 Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Reviewed-by: Grace Kloba <klobag@chromium.org> Reviewed-by: Peter Wen <wnwen@chromium.org> Reviewed-by: Oliver Chang <ochang@chromium.org> Cr-Commit-Position: refs/heads/master@{#610975} [modify] https://crrev.com/5c488598e6a9fa806467a1648762c4e8dc2cea30/DEPS [modify] https://crrev.com/5c488598e6a9fa806467a1648762c4e8dc2cea30/third_party/.gitignore [add] https://crrev.com/5c488598e6a9fa806467a1648762c4e8dc2cea30/third_party/protoc_javalite/OWNERS [add] https://crrev.com/5c488598e6a9fa806467a1648762c4e8dc2cea30/third_party/protoc_javalite/README.chromium [add] https://crrev.com/5c488598e6a9fa806467a1648762c4e8dc2cea30/third_party/protoc_javalite/cipd.yaml
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8aa669efad5cd46935c89fbd96dfe3d207d5ce9e commit 8aa669efad5cd46935c89fbd96dfe3d207d5ce9e Author: Mohamed Heikal <mheikal@chromium.org> Date: Fri Nov 30 19:23:11 2018 [protoc] Use protoc's lite java plugin and runtime protoc the protobuf compiler now requires using a separate plugin for compiling protos for the java lite runtime. The lite java runtime is now also separate from the main java runtime and is distributed separately. This cl switches java proto compilation for lite protos to this new mode. TBR=android_webview/BUILD.gn Bug: 782237, 800281 Change-Id: I31c2d073bed51109dffeea133495679deef3186b Reviewed-on: https://chromium-review.googlesource.com/c/1340522 Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Reviewed-by: Pavel Yatsuk <pavely@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Reviewed-by: Peter Wen <wnwen@chromium.org> Cr-Commit-Position: refs/heads/master@{#612720} [modify] https://crrev.com/8aa669efad5cd46935c89fbd96dfe3d207d5ce9e/android_webview/BUILD.gn [modify] https://crrev.com/8aa669efad5cd46935c89fbd96dfe3d207d5ce9e/build/config/android/rules.gni [modify] https://crrev.com/8aa669efad5cd46935c89fbd96dfe3d207d5ce9e/build/protoc_java.py [modify] https://crrev.com/8aa669efad5cd46935c89fbd96dfe3d207d5ce9e/chrome/android/BUILD.gn [modify] https://crrev.com/8aa669efad5cd46935c89fbd96dfe3d207d5ce9e/components/invalidation/impl/BUILD.gn [modify] https://crrev.com/8aa669efad5cd46935c89fbd96dfe3d207d5ce9e/third_party/feed/BUILD.gn [modify] https://crrev.com/8aa669efad5cd46935c89fbd96dfe3d207d5ce9e/third_party/protobuf/BUILD.gn
,
Nov 30
The following revision refers to this bug: https://chrome-internal.googlesource.com/clank/internal/apps/+/489a4c9fa52cb23e133e8c87138b98c3302c67c8 commit 489a4c9fa52cb23e133e8c87138b98c3302c67c8 Author: Mohamed Heikal <mheikal@chromium.org> Date: Fri Nov 30 20:31:42 2018
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8121da4a4506f3a590305cec0792e7d31dc1b8a9 commit 8121da4a4506f3a590305cec0792e7d31dc1b8a9 Author: oysteine <oysteine@chromium.org> Date: Fri Nov 30 22:49:19 2018 Revert "[protoc] Use protoc's lite java plugin and runtime" This reverts commit 8aa669efad5cd46935c89fbd96dfe3d207d5ce9e. Reason for revert: crbug.com/910759 Original change's description: > [protoc] Use protoc's lite java plugin and runtime > > protoc the protobuf compiler now requires using a separate plugin for > compiling protos for the java lite runtime. The lite java runtime is now > also separate from the main java runtime and is distributed separately. > > This cl switches java proto compilation for lite protos to this new > mode. > > TBR=android_webview/BUILD.gn > > Bug: 782237, 800281 > Change-Id: I31c2d073bed51109dffeea133495679deef3186b > Reviewed-on: https://chromium-review.googlesource.com/c/1340522 > Commit-Queue: Mohamed Heikal <mheikal@chromium.org> > Reviewed-by: Pavel Yatsuk <pavely@chromium.org> > Reviewed-by: Peter Kasting <pkasting@chromium.org> > Reviewed-by: agrieve <agrieve@chromium.org> > Reviewed-by: Peter Wen <wnwen@chromium.org> > Cr-Commit-Position: refs/heads/master@{#612720} TBR=pkasting@chromium.org,michaelbai@chromium.org,wnwen@chromium.org,pavely@chromium.org,agrieve@chromium.org,mheikal@chromium.org Change-Id: Ie2dce6e8c5ad03e6a75607a02e234c813f3072a5 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 782237, 800281 Reviewed-on: https://chromium-review.googlesource.com/c/1357571 Reviewed-by: oysteine <oysteine@chromium.org> Commit-Queue: oysteine <oysteine@chromium.org> Cr-Commit-Position: refs/heads/master@{#612826} [modify] https://crrev.com/8121da4a4506f3a590305cec0792e7d31dc1b8a9/android_webview/BUILD.gn [modify] https://crrev.com/8121da4a4506f3a590305cec0792e7d31dc1b8a9/build/config/android/rules.gni [modify] https://crrev.com/8121da4a4506f3a590305cec0792e7d31dc1b8a9/build/protoc_java.py [modify] https://crrev.com/8121da4a4506f3a590305cec0792e7d31dc1b8a9/chrome/android/BUILD.gn [modify] https://crrev.com/8121da4a4506f3a590305cec0792e7d31dc1b8a9/components/invalidation/impl/BUILD.gn [modify] https://crrev.com/8121da4a4506f3a590305cec0792e7d31dc1b8a9/third_party/feed/BUILD.gn [modify] https://crrev.com/8121da4a4506f3a590305cec0792e7d31dc1b8a9/third_party/protobuf/BUILD.gn
,
Dec 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cabc7f418640412ae496bd23fdd096efac1f7b33 commit cabc7f418640412ae496bd23fdd096efac1f7b33 Author: oysteine <oysteine@chromium.org> Date: Sat Dec 01 00:08:52 2018 Reland "[protoc] Use protoc's lite java plugin and runtime" This reverts commit 8121da4a4506f3a590305cec0792e7d31dc1b8a9. Reason for revert: Builder turned green while the revert was in flight, and the revert seems to have broken it. Original change's description: > Revert "[protoc] Use protoc's lite java plugin and runtime" > > This reverts commit 8aa669efad5cd46935c89fbd96dfe3d207d5ce9e. > > Reason for revert: crbug.com/910759 > > Original change's description: > > [protoc] Use protoc's lite java plugin and runtime > > > > protoc the protobuf compiler now requires using a separate plugin for > > compiling protos for the java lite runtime. The lite java runtime is now > > also separate from the main java runtime and is distributed separately. > > > > This cl switches java proto compilation for lite protos to this new > > mode. > > > > TBR=android_webview/BUILD.gn > > > > Bug: 782237, 800281 > > Change-Id: I31c2d073bed51109dffeea133495679deef3186b > > Reviewed-on: https://chromium-review.googlesource.com/c/1340522 > > Commit-Queue: Mohamed Heikal <mheikal@chromium.org> > > Reviewed-by: Pavel Yatsuk <pavely@chromium.org> > > Reviewed-by: Peter Kasting <pkasting@chromium.org> > > Reviewed-by: agrieve <agrieve@chromium.org> > > Reviewed-by: Peter Wen <wnwen@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#612720} > > TBR=pkasting@chromium.org,michaelbai@chromium.org,wnwen@chromium.org,pavely@chromium.org,agrieve@chromium.org,mheikal@chromium.org > > Change-Id: Ie2dce6e8c5ad03e6a75607a02e234c813f3072a5 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 782237, 800281 > Reviewed-on: https://chromium-review.googlesource.com/c/1357571 > Reviewed-by: oysteine <oysteine@chromium.org> > Commit-Queue: oysteine <oysteine@chromium.org> > Cr-Commit-Position: refs/heads/master@{#612826} TBR=pkasting@chromium.org,michaelbai@chromium.org,wnwen@chromium.org,oysteine@chromium.org,pavely@chromium.org,agrieve@chromium.org,mheikal@chromium.org Change-Id: Ia8b508fd7ab9c7c42cc976dc0827683da81a435f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 782237, 800281 Reviewed-on: https://chromium-review.googlesource.com/c/1357641 Reviewed-by: oysteine <oysteine@chromium.org> Commit-Queue: oysteine <oysteine@chromium.org> Cr-Commit-Position: refs/heads/master@{#612863} [modify] https://crrev.com/cabc7f418640412ae496bd23fdd096efac1f7b33/android_webview/BUILD.gn [modify] https://crrev.com/cabc7f418640412ae496bd23fdd096efac1f7b33/build/config/android/rules.gni [modify] https://crrev.com/cabc7f418640412ae496bd23fdd096efac1f7b33/build/protoc_java.py [modify] https://crrev.com/cabc7f418640412ae496bd23fdd096efac1f7b33/chrome/android/BUILD.gn [modify] https://crrev.com/cabc7f418640412ae496bd23fdd096efac1f7b33/components/invalidation/impl/BUILD.gn [modify] https://crrev.com/cabc7f418640412ae496bd23fdd096efac1f7b33/third_party/feed/BUILD.gn [modify] https://crrev.com/cabc7f418640412ae496bd23fdd096efac1f7b33/third_party/protobuf/BUILD.gn |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by jbudorick@chromium.org
, Nov 7 2017