Skip to content

Remove wrap_memcpy#23385

Closed
veblush wants to merge 1 commit intogrpc:masterfrom
veblush:del-wrap-memcpy
Closed

Remove wrap_memcpy#23385
veblush wants to merge 1 commit intogrpc:masterfrom
veblush:del-wrap-memcpy

Conversation

@veblush
Copy link
Copy Markdown
Contributor

@veblush veblush commented Jul 4, 2020

gRPC has been having the wrap_memcpy workaround to handle the breaking change of memcpy on glibc 2.14 (ref1, ref2). This was necessary to build more portable artifacts which are expected to run on most linux distributions and we manged to have #5007 years ago for this.

Since we started to use manylinux2010-based docker images for artifacts, however, this became unnecessary since CentOS 6 has glibc 2.12 which is older than 2.14 so it doesn't have memcpy issue, which is another benefit of using manylinux2010.

This shouldn't be merged before #23344 trying to switch ruby's base image to manylinux2010 but it's been merged now.

grpc_build_artifacts_multiplatform: passed

@veblush veblush added kind/internal cleanup area/build release notes: no Indicates if PR should not be in release notes labels Jul 4, 2020
Copy link
Copy Markdown
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, but I'd also like an approval from @nicolasnoble since he has more context on this than I do.

@stale
Copy link
Copy Markdown

stale bot commented Dec 10, 2020

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 30 days. It will be closed automatically if no further update occurs in 7 day. Thank you for your contributions!

@jtattermusch
Copy link
Copy Markdown
Contributor

qq: note that we still have some docker images for building artifacts are still using debian jessie:
e.g.



but I wasn't able to find whether they are being used (e.g. in artifact_targets.py). If there are unused, I think we should remove them?

We'd definitely need a passing artifacts - packages - distribtest build with passing distribtests before merging this.

@jtattermusch
Copy link
Copy Markdown
Contributor

qq: note that we still have some docker images for building artifacts are still using debian jessie:
e.g.

but I wasn't able to find whether they are being used (e.g. in artifact_targets.py). If there are unused, I think we should remove them?
We'd definitely need a passing artifacts - packages - distribtest build with passing distribtests before merging this.

I created.#25542

@veblush
Copy link
Copy Markdown
Contributor Author

veblush commented Feb 6, 2024

Closing this in favor of #35826

@veblush veblush closed this Feb 6, 2024
copybara-service bot pushed a commit that referenced this pull request Feb 16, 2024
gRPC has been having the `wrap_memcpy` workaround to handle the breaking change of `memcpy` on glibc 2.14 ([ref1](https://www.win.tue.nl/~aeb/linux/misc/gcc-semibug.html), [ref2](https://stackoverflow.com/questions/35656696/explanation-of-memcpy-memmove-glibc-2-14-2-2-5)). This was necessary to build more portable artifacts which are expected to run on most linux distributions and we manged to have #5007 years ago for this.

Since we started to use manylinux2010-based docker images for artifacts, however, this became unnecessary since CentOS 6 has glibc 2.12 which is older than 2.14 so it doesn't have memcpy issue, which is another benefit of using manylinux2010.

Superseding #23385
Changes after the original PR was made; Ruby is using manylinux images (manylinux2014) enabling this change, no more old docker images in our test env.

Closes #35826

COPYBARA_INTEGRATE_REVIEW=#35826 from veblush:del-wrap-memcpy-2 71e6718
PiperOrigin-RevId: 607499060
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants