Skip to content

Allow grpcio to be built against system re2#24450

Merged
gnossen merged 1 commit intogrpc:masterfrom
mtorromeo:patch-2
Oct 21, 2020
Merged

Allow grpcio to be built against system re2#24450
gnossen merged 1 commit intogrpc:masterfrom
mtorromeo:patch-2

Conversation

@mtorromeo
Copy link
Copy Markdown
Contributor

The main grpc library can be built with system re2 using the cmake flag -DgRPC_RE2_PROVIDER=package. This brings the grpcio package in line with grpc using similar checks and env variables used for BUILD_WITH_SYSTEM_CARES/OPENSSL/ZLIB.

@donnadionne

Copy link
Copy Markdown
Contributor

@gnossen gnossen left a comment

Choose a reason for hiding this comment

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

👍 Thanks for the fix! Keeping these sorts of build features working is tricky, so we depend on downstream users to make sure they keep on working. 🙂

Out of curiosity, since I've seen at least one other PR from you, are you vendoring gRPC for a distro or something?

@gnossen
Copy link
Copy Markdown
Contributor

gnossen commented Oct 20, 2020

CC @lidizheng for a second pair of eyes.

@gnossen
Copy link
Copy Markdown
Contributor

gnossen commented Oct 20, 2020

@mtorromeo The code formatter is complaining:

+ yapf_virtual_environment/bin/python -m yapf --diff --recursive --style=setup.cfg examples src test tools setup.py
--- setup.py	(original)
+++ setup.py	(reformatted)
@@ -136,8 +136,7 @@
 # Export this variable to use the system installation of re2. You need to
 # have the header files installed (in /usr/include/re2) and during
 # runtime, the shared library must be installed
-BUILD_WITH_SYSTEM_RE2 = os.environ.get('GRPC_PYTHON_BUILD_SYSTEM_RE2',
-                                        False)
+BUILD_WITH_SYSTEM_RE2 = os.environ.get('GRPC_PYTHON_BUILD_SYSTEM_RE2', False)

Can you please run tools/distrib/yapf_code.sh?

@mtorromeo
Copy link
Copy Markdown
Contributor Author

Out of curiosity, since I've seen at least one other PR from you, are you vendoring gRPC for a distro or something?

Yes, I am maintaining the Arch Linux package.

Can you please run tools/distrib/yapf_code.sh?

The script doesn't work as-is on Arch since python3 doesn't exist (python 3 is python while python 2 is python2) and also the virtualenv module is not present but the venv module should be used instead (built into python since 3.3) with the command python -m venv $VIRTUALENV.

Anyway I ran it after fixing it but it's also changing the format of src/python/grpcio_tests/tests_aio/benchmark/worker_servicer.py.

I force pushed the amended commit with the fixed setup.py and ignored the changes in other files.

@gnossen
Copy link
Copy Markdown
Contributor

gnossen commented Oct 21, 2020

Yes, I am maintaining the Arch Linux package.

Nice! I thought your name looked familiar. I use your package on my personal machine at home. I've commented in the past how the Arch package somehow gets published before we get our package up on PyPi.

@gnossen
Copy link
Copy Markdown
Contributor

gnossen commented Oct 21, 2020

Unrelated flake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build kind/bug lang/Python 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.

4 participants