Skip to content

Revert "Revert "Adding xxhash as a subtree""#25645

Merged
donnadionne merged 1 commit intogrpc:masterfrom
donnadionne:xxhash_second
Mar 11, 2021
Merged

Revert "Revert "Adding xxhash as a subtree""#25645
donnadionne merged 1 commit intogrpc:masterfrom
donnadionne:xxhash_second

Conversation

@donnadionne
Copy link
Copy Markdown
Contributor

Adding xxhash as a subtree

Attempting to submit the same code after taking care of import issues.

@donnadionne donnadionne added the release notes: no Indicates if PR should not be in release notes label Mar 8, 2021
@donnadionne donnadionne marked this pull request as draft March 8, 2021 06:25
@jtattermusch
Copy link
Copy Markdown
Contributor

there's a bunch of failures and you'll also need a passing adhoc artifacts - packages - distribtest (distribtests were broken the last time) run before this can go in (see bullet point 2 here: https://github.com/grpc/grpc/tree/master/third_party#checklist-for-adding-a-new-third-party-dependency)

@donnadionne
Copy link
Copy Markdown
Contributor Author

there's a bunch of failures and you'll also need a passing adhoc artifacts - packages - distribtest (distribtests were broken the last time) run before this can go in (see bullet point 2 here: https://github.com/grpc/grpc/tree/master/third_party#checklist-for-adding-a-new-third-party-dependency)

started: https://fusion2.corp.google.com/invocations/8e76a837-d153-4677-987e-2a373e9df5ea/targets

@donnadionne donnadionne marked this pull request as ready for review March 9, 2021 06:13
@donnadionne donnadionne marked this pull request as draft March 9, 2021 06:15
@jtattermusch
Copy link
Copy Markdown
Contributor

@donnadionne
Copy link
Copy Markdown
Contributor Author

donnadionne commented Mar 9, 2021

@donnadionne
Copy link
Copy Markdown
Contributor Author

Known issues: #25660 (interop test failures)

@donnadionne
Copy link
Copy Markdown
Contributor Author

kokoro failure: due to unable to access the commitsha that is not yet submitted

@donnadionne donnadionne marked this pull request as ready for review March 10, 2021 05:04
@donnadionne
Copy link
Copy Markdown
Contributor Author

All Required tests pass; failed test have explainations.

This is really the same change as last week; just now it's ready to be imported.

PTAL thank you!

@jtattermusch
Copy link
Copy Markdown
Contributor

jtattermusch commented Mar 10, 2021

FTR, as discussed I think the issue with python-dev distribtests is that the python "sdist" archive is missing the newly added xxhash.h header:

https://pantheon.corp.google.com/storage/browser/_details/grpc-testing-kokoro-prod/test_result_public/prod/grpc/core/master/linux/grpc_build_artifacts/16172/20210309-131926/github/grpc/artifacts/python_manylinux2010_x86_cp38-cp38/grpcio-1.37.0.dev0.tar.gz (and indeed, if you open the archive you'll see that third_party/xxhash/xxhash.h is not there).

Made a one line change in setup.py (similar fix as #24450: to allow grpcio to build with xxhash), re-running adhoc to test it out:
https://fusion2.corp.google.com/invocations/a7a23ae2-39a6-4d02-9f14-ca4dcf8ef25d/targets

This was not the right fix; setup.py restored now

@donnadionne donnadionne force-pushed the xxhash_second branch 2 times, most recently from 07aa917 to 59c5010 Compare March 10, 2021 19:20
@donnadionne
Copy link
Copy Markdown
Contributor Author

donnadionne commented Mar 11, 2021

@donnadionne
Copy link
Copy Markdown
Contributor Author

Known issues: #25660 (interop test failures)

Ruby Failue is just a timeout, and this test has passed in previous runs before the 1 line change for Python.

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 (assuming you've tried the cherrypick into google3 and it worked fine).

@donnadionne donnadionne merged commit d3e97d9 into grpc:master Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants