Skip to content

Support CentOS 7 for gRPC.NET#21481

Merged
veblush merged 2 commits intogrpc:masterfrom
veblush:net-csharp
Dec 17, 2019
Merged

Support CentOS 7 for gRPC.NET#21481
veblush merged 2 commits intogrpc:masterfrom
veblush:net-csharp

Conversation

@veblush
Copy link
Contributor

@veblush veblush commented Dec 15, 2019

Added missing symbols for CentOS 7 which lacks symbols from GLIBCXX_3.4.20. These symbols can be used by C++ libraries and adding Abseil makes gRPC use these symbols.

This is a prerequisite to #20184.

@veblush veblush added lang/C# area/build release notes: no Indicates if PR should not be in release notes labels Dec 15, 2019
@veblush
Copy link
Contributor Author

veblush commented Dec 15, 2019

grpc_build_artifacts_multiplatform: passed

@veblush veblush marked this pull request as ready for review December 15, 2019 23:58
@veblush veblush requested a review from jtattermusch December 16, 2019 00:14
language: csharp
src:
- src/csharp/ext/grpc_csharp_ext.c
- src/csharp/ext/std++compat.cc
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this only required for C#?

Copy link
Contributor Author

@veblush veblush Dec 16, 2019

Choose a reason for hiding this comment

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

All languages having a binary distribution have this problem but they have their own solution. Python, for example, uses manylinux to build python binaries and Ruby uses a static linking.


namespace std {

// CentOS 7 (GLIBC_2.17 / GLIBCXX_3.4.19) doesn't have a following symbol
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the minimum GLIBCXX required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have version requirement for GLIBCXX in general. This happens to have this because .NET artifacts are built on Debian 8. If we're using Cent OS 6 or 7 to build .NET artifacts, then it has a lower version requirement for GLIBCXX, which is the same idea with the manylinux platform for Python.

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, we could build the .NET artifacts on an older distro if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't prevent users from building .NET artifacts from source as far as they have required build tools available.

Copy link
Contributor

@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

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

LGTM to avoid blocking the absl effort and because the tests pass.
My personal opinion is that hacks like this suck though.

Copy link
Contributor Author

@veblush veblush left a comment

Choose a reason for hiding this comment

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

Thank you for the review. Having more conservative build environment such as manylinux2010 would be better than this. I'll review that option, too.

@veblush veblush merged commit 4444ab9 into grpc:master Dec 17, 2019
@veblush veblush deleted the net-csharp branch December 17, 2019 16:25
@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants