Skip to content

avoid c-ares dependency on libnsl (for v1.16.x)#17270

Merged
nicolasnoble merged 1 commit intogrpc:v1.16.xfrom
jtattermusch:csharp_avoid_nsl_1_16x
Nov 28, 2018
Merged

avoid c-ares dependency on libnsl (for v1.16.x)#17270
nicolasnoble merged 1 commit intogrpc:v1.16.xfrom
jtattermusch:csharp_avoid_nsl_1_16x

Conversation

@jtattermusch
Copy link
Copy Markdown
Contributor

@jtattermusch jtattermusch commented Nov 21, 2018

Backport of fix for #17255

@jtattermusch jtattermusch added lang/C# release notes: no Indicates if PR should not be in release notes labels Nov 21, 2018
@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Nov 21, 2018 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@jtattermusch jtattermusch mentioned this pull request Nov 21, 2018
Merged
@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Nov 21, 2018 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Nov 21, 2018 

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,967,222      Total (=)      1,967,222

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,889,385      Total (<)     10,889,388

 No significant differences in binary sizes

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Nov 21, 2018 

Corrupt JSON data (indicates timeout or crash): 
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.new: 10
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.old: 10


[microbenchmarks] No significant performance differences

@apolcyn
Copy link
Copy Markdown
Contributor

apolcyn commented Nov 21, 2018

Will this go into master as well?

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Nov 21, 2018

@apolcyn yes, i'll upmerge #17271

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Nov 22, 2018

@apolcyn a bit more explanation why this change should be safe:

  • the only place where we enable gRPC_BACKWARDS_COMPATIBILITY_MODE is when building the C# artifact (and the "hack" from this PR only gets activated when backward compatibility mode is enabled, it's off by default), so this change really shouldn't influence anyone's build.

    grpc/tools/run_tests/artifacts/build_artifact_csharp.sh

    Line 24 in e0d9692

    -DgRPC_BACKWARDS_COMPATIBILITY_MODE=ON \
  • no other wrapped languages are using cmake to build the C-core extension.
  • in Makefile, this change is not applicable as the reason why libnsl dependency gets added is the c-ares' own CMakeLists.txt. When building using Makefile, we are basically replicating c-ares build by pulling the list of source files (and we are not doing any special handling for libnsl).

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Nov 27, 2018

Merging to backport #17271.

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Nov 28, 2018

@nicolasnoble please merge.

@nicolasnoble nicolasnoble merged commit 2cd1dcf into grpc:v1.16.x Nov 28, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Feb 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@nicolasnoble nicolasnoble nicolasnoble approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@nicolasnoble nicolasnoble

@apolcyn apolcyn

Labels

lang/C# release notes: no Indicates if PR should not be in release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jtattermusch @grpc-testing @apolcyn @nicolasnoble