Skip to content

avoid c-ares dependency on libnsl (for v1.17.x)#17271

Merged
jtattermusch merged 1 commit intogrpc:v1.17.xfrom
jtattermusch:csharp_avoid_nsl_1_17x
Nov 27, 2018
Merged

avoid c-ares dependency on libnsl (for v1.17.x)#17271
jtattermusch merged 1 commit intogrpc:v1.17.xfrom
jtattermusch:csharp_avoid_nsl_1_17x

Conversation

@jtattermusch
Copy link
Copy Markdown
Contributor

@jtattermusch jtattermusch commented Nov 21, 2018

Fix for #17255.

Also see #17270

Still needs to be upmerged to master.

@jtattermusch jtattermusch added lang/C# release notes: no Indicates if PR should not be in release notes labels Nov 21, 2018
@jtattermusch jtattermusch requested a review from apolcyn November 21, 2018 10:20
@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  [ = ]

@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
 2,020,496      Total (=)      2,020,496

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
11,175,628      Total (>)     11,175,627

 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

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Nov 21, 2018

This nuget with this change passes with a makeshift C# distribtest on alpine: https://source.cloud.google.com/results/invocations/f3c0d70f-066f-4dee-8d17-cca44bf26194/targets/github%2Fgrpc/tests;query=alpine;failed=false;passed=true

@jtattermusch jtattermusch mentioned this pull request Nov 21, 2018
Merged
@jtattermusch jtattermusch added release notes: yes Indicates if PR needs to be in release notes priority/P0/RELEASE BLOCKER and removed release notes: no Indicates if PR should not be in release notes labels Nov 26, 2018
@apolcyn
Copy link
Copy Markdown
Contributor

apolcyn commented Nov 26, 2018

Saw #17272, but just for the idea I'm wondering if a set of C tests that run under BACKWARDS_COMPATIBILITY_MODE would be useful

apolcyn
apolcyn approved these changes Nov 26, 2018
View reviewed changes
cmake/cares.cmake
set(CARES_SHARED OFF CACHE BOOL "disable shared library")
set(CARES_STATIC ON CACHE BOOL "link cares statically")
if(gRPC_BACKWARDS_COMPATIBILITY_MODE)
# See https://github.com/grpc/grpc/issues/17255
Copy link
Copy Markdown
Contributor

@apolcyn apolcyn Nov 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a comment here that we expect the C# binary packages to be the only builds that use this?

Copy link
Copy Markdown
Contributor Author

@jtattermusch jtattermusch Nov 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the issue link already hints that this is related to C# packaging.

@jtattermusch jtattermusch merged commit 81c2243 into grpc:v1.17.x Nov 27, 2018
@jtattermusch jtattermusch mentioned this pull request Nov 28, 2018
Closed
@ghost ghost mentioned this pull request Dec 6, 2018
Closed
@lock lock bot locked as resolved and limited conversation to collaborators Feb 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@apolcyn apolcyn apolcyn approved these changes

@nicolasnoble nicolasnoble Awaiting requested review from nicolasnoble

Assignees

@nicolasnoble nicolasnoble

@apolcyn apolcyn

Labels

lang/C# priority/P0/RELEASE BLOCKER release notes: yes Indicates if PR needs to 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