Fix Android build issues and gRPC LB policy

Fix Android build issues and gRPC LB policy

* New build.yaml changes and get rid of custom changes.
* By default excludes Census LB alone.
* Android fixes for logging purposes.
* Reduces overall binary/dep size because of the LB changes.

R=gcasto@chromium.org, xyzzyz@chromium.org

Committed: https://chromium.googlesource.com/external/github.com/grpc/grpc/+/7032ff02c405fc51cfe2a4b8e350547f046ee250

[perumaal, 2016-10-20 17:40:12]

perumaal@chromium.org changed reviewers:
+ garret@chromium.org, xyzzyz@chromium.org

[Garrett Casto, 2016-10-20 18:05:28]

Description was changed from

==========
Fix Android build issues and gRPC LB policy

* New build.yaml changes and get rid of custom changes.
* By default excludes Census LB alone.
* Android fixes for logging purposes.
* Reduces overall binary/dep size because of the LB changes.
==========

to

==========
Fix Android build issues and gRPC LB policy

* New build.yaml changes and get rid of custom changes.
* By default excludes Census LB alone.
* Android fixes for logging purposes.
* Reduces overall binary/dep size because of the LB changes.
==========

[Garrett Casto, 2016-10-20 18:05:28]

gcasto@chromium.org changed reviewers:
+ gcasto@chromium.org
- garret@chromium.org

[xyzzyz, 2016-10-20 18:11:40]

Can you remove files we don't care about from this commit, like BUILD,
CMakeLists.txt etc? They make code review more difficult, and add only more
merge conflicts when we merge with upstream,

[perumaal, 2016-10-20 23:58:36]

PTAL thanks!
Removed unnecessary files.

[xyzzyz, 2016-10-21 20:14:39]

https://codereview.chromium.org/2440763002/diff/40001/OWNERS
File OWNERS (right):

https://codereview.chromium.org/2440763002/diff/40001/OWNERS#newcode4
OWNERS:4: <<<<<<< HEAD
Remove merge conflict marks.

https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registr...
File src/core/plugin_registry/grpc_unsecure_plugin_registry.c (left):

https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registr...
src/core/plugin_registry/grpc_unsecure_plugin_registry.c:68: //
grpc_register_plugin(grpc_lb_policy_grpclb_init,
Did these lines disappear from the upstream gRPC, or are you just removing what
I commented out? If the latter, please leave the commented out lines as they
are, so that in future we have some memory of what happened here.

https://codereview.chromium.org/2440763002/diff/40001/templates/BUILD.gn.temp...
File templates/BUILD.gn.template (right):

https://codereview.chromium.org/2440763002/diff/40001/templates/BUILD.gn.temp...
templates/BUILD.gn.template:31: "-Wimplicit-function-declaration",
Why do we need -Wimplicit-function-declaration?

[Garrett Casto, 2016-10-21 20:18:25]

The comment implies that removing Census from the build means we can carry less
local patches, however it doesn't look like you've reverted them here. Are you
planning on doing that in a future CL?

Also, you say that Census is disabled by default, which implies that you can
selectively re-enable it, but that doesn't seem true.

https://codereview.chromium.org/2440763002/diff/40001/OWNERS
File OWNERS (right):

https://codereview.chromium.org/2440763002/diff/40001/OWNERS#newcode7
OWNERS:7: >>>>>>> 37667a0807047c1c7ca4e1ebc9ce5c5cc1faa7e8
Need to fix this :P

https://codereview.chromium.org/2440763002/diff/40001/templates/BUILD.gn.temp...
File templates/BUILD.gn.template (right):

https://codereview.chromium.org/2440763002/diff/40001/templates/BUILD.gn.temp...
templates/BUILD.gn.template:31: "-Wimplicit-function-declaration",
Nit: whitespace.

[perumaal, 2016-10-21 20:23:04]

Garrett:
build.yaml now generates the two plugin files automatically.
There are two more commented out lines in context_cc and another file that
touches census but this requires an actual upstream commit.
Idea is to use build.yaml primarily for auto-generation and get rid of any
nanopb issues.

[perumaal, 2016-10-21 20:25:38]

https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registr...
File src/core/plugin_registry/grpc_unsecure_plugin_registry.c (left):

https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registr...
src/core/plugin_registry/grpc_unsecure_plugin_registry.c:68: //
grpc_register_plugin(grpc_lb_policy_grpclb_init,
On 2016/10/21 at 20:14:38, xyzzyz wrote:
> Did these lines disappear from the upstream gRPC, or are you just removing
what I commented out? If the latter, please leave the commented out lines as
they are, so that in future we have some memory of what happened here.

So this is by changing build.yaml and running 
tools/buildgen/generate_projects.sh.
It automatically generates this file.

[perumaal, 2016-10-21 20:26:23]

>
https://codereview.chromium.org/2440763002/diff/40001/templates/BUILD.gn.temp...
> templates/BUILD.gn.template:31: "-Wimplicit-function-declaration",
> Why do we need -Wimplicit-function-declaration?

Oh, I forgot exactly why (been three weeks since I did this sorry :() but I
believe this is because of how gRPC calls memset.

[perumaal, 2016-10-21 20:28:56]

More details on why we don't need to manually edit the plugin files:

plugin code is auto-generated based on this template:
https://cs.chromium.org/chromium/src/third_party/grpc/templates/src/core/plug...

This goes around reading the list of plugins from build.yaml:
https://cs.chromium.org/search/?q=plugin+f:build.yaml+file:%5Esrc/third_party...

[xyzzyz, 2016-10-21 20:40:26]

On 2016/10/21 20:25:38, perumaal wrote:
>
https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registr...
> File src/core/plugin_registry/grpc_unsecure_plugin_registry.c (left):
> 
>
https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registr...
> src/core/plugin_registry/grpc_unsecure_plugin_registry.c:68: //
> grpc_register_plugin(grpc_lb_policy_grpclb_init,
> On 2016/10/21 at 20:14:38, xyzzyz wrote:
> > Did these lines disappear from the upstream gRPC, or are you just removing
> what I commented out? If the latter, please leave the commented out lines as
> they are, so that in future we have some memory of what happened here.
> 
> So this is by changing build.yaml and running 
> tools/buildgen/generate_projects.sh.
> It automatically generates this file.

Ah, cool!

[xyzzyz, 2016-10-21 20:42:12]

On 2016/10/21 20:26:23, perumaal wrote:
> >
>
https://codereview.chromium.org/2440763002/diff/40001/templates/BUILD.gn.temp...
> > templates/BUILD.gn.template:31: "-Wimplicit-function-declaration",
> > Why do we need -Wimplicit-function-declaration?
> 
> Oh, I forgot exactly why (been three weeks since I did this sorry :() but I
> believe this is because of how gRPC calls memset.

But then why is it actually necessary? To my understanding it just enables some
extra warnings.

[xyzzyz, 2016-10-21 20:42:49]

https://codereview.chromium.org/2440763002/diff/80001/templates/BUILD.gn.temp...
File templates/BUILD.gn.template (right):

https://codereview.chromium.org/2440763002/diff/80001/templates/BUILD.gn.temp...
templates/BUILD.gn.template:77: or "census" in f
Can we remove these now?

[perumaal, 2016-10-21 20:58:20]

On 2016/10/21 at 20:42:49, xyzzyz wrote:
>
https://codereview.chromium.org/2440763002/diff/80001/templates/BUILD.gn.temp...
> File templates/BUILD.gn.template (right):
> 
>
https://codereview.chromium.org/2440763002/diff/80001/templates/BUILD.gn.temp...
> templates/BUILD.gn.template:77: or "census" in f
> Can we remove these now?

Unfortunately not yet - the folders themselves are still present :(

[perumaal, 2016-10-21 21:00:40]

On 2016/10/21 at 20:42:12, xyzzyz wrote:
> On 2016/10/21 20:26:23, perumaal wrote:
> > >
> >
https://codereview.chromium.org/2440763002/diff/40001/templates/BUILD.gn.temp...
> > > templates/BUILD.gn.template:31: "-Wimplicit-function-declaration",
> > > Why do we need -Wimplicit-function-declaration?
> > 
> > Oh, I forgot exactly why (been three weeks since I did this sorry :() but I
> > believe this is because of how gRPC calls memset.
> 
> But then why is it actually necessary? To my understanding it just enables
some extra warnings.

Must be an artifact of my trying 10 things and having one of them stick. I might
have forgotten to remove this actually.
Thanks!

[xyzzyz, 2016-10-21 22:41:05]

lgtm

https://codereview.chromium.org/2440763002/diff/100001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2440763002/diff/100001/BUILD.gn#newcode29
BUILD.gn:29: "-Wimplicit-function-declaration",
Please regenerate the BUILD.gn file -- you removed the -Wimplicit-... from the
template, but it seems to remain here.

[perumaal, 2016-10-21 22:53:49]

On 2016/10/21 at 22:41:05, xyzzyz wrote:
> lgtm
> 
> https://codereview.chromium.org/2440763002/diff/100001/BUILD.gn
> File BUILD.gn (right):
> 
> https://codereview.chromium.org/2440763002/diff/100001/BUILD.gn#newcode29
> BUILD.gn:29: "-Wimplicit-function-declaration",
> Please regenerate the BUILD.gn file -- you removed the -Wimplicit-... from the
template, but it seems to remain here.

Done.

[Garrett Casto, 2016-10-21 22:59:32]

https://codereview.chromium.org/2440763002/diff/120001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2440763002/diff/120001/BUILD.gn#newcode539
BUILD.gn:539: "include/grpc/impl/codegen/gpr_types.h",
Why are there new includes added after this change? I can see why some would be
removed, but it isn't obvious why anything should be added.

[perumaal, 2016-10-21 23:04:39]

On 2016/10/21 at 22:59:32, gcasto wrote:
> https://codereview.chromium.org/2440763002/diff/120001/BUILD.gn
> File BUILD.gn (right):
> 
> https://codereview.chromium.org/2440763002/diff/120001/BUILD.gn#newcode539
> BUILD.gn:539: "include/grpc/impl/codegen/gpr_types.h",
> Why are there new includes added after this change? I can see why some would
be removed, but it isn't obvious why anything should be added.

The script / template syncs up with github's updates. i.e.
origin/chromium-deps/2016-10-18 has new files that need to be added to BUILD.gn
first.

[Garrett Casto, 2016-10-21 23:35:09]

lgtm

[perumaal, 2016-10-21 23:35:35]

The patchset sent to the CQ was uploaded after l-g-t-m from xyzzyz@chromium.org
Link to the patchset: https://codereview.chromium.org/2440763002/#ps120001
(title: "Sync BUILD.gn")

[perumaal, 2016-10-22 00:10:30]

Description was changed from

==========
Fix Android build issues and gRPC LB policy

* New build.yaml changes and get rid of custom changes.
* By default excludes Census LB alone.
* Android fixes for logging purposes.
* Reduces overall binary/dep size because of the LB changes.
==========

to

==========
Fix Android build issues and gRPC LB policy

* New build.yaml changes and get rid of custom changes.
* By default excludes Census LB alone.
* Android fixes for logging purposes.
* Reduces overall binary/dep size because of the LB changes.

R=gcasto@chromium.org, xyzzyz@chromium.org

Committed:
https://chromium.googlesource.com/external/github.com/grpc/grpc/+/7032ff02c40...
==========

[perumaal, 2016-10-22 00:10:31]

Committed patchset #8 (id:120001) manually as
7032ff02c405fc51cfe2a4b8e350547f046ee250 (presubmit successful).