From 0579dcaed6fbfa88765e5640c9262dd3bcc8801d Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 16 Jan 2019 10:26:59 -0800 Subject: [PATCH 1/4] Escalate the failure of protoc execution --- tools/distrib/python/grpcio_tools/grpc_tools/command.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/distrib/python/grpcio_tools/grpc_tools/command.py b/tools/distrib/python/grpcio_tools/grpc_tools/command.py index 7ede05f1404..93503c4cc36 100644 --- a/tools/distrib/python/grpcio_tools/grpc_tools/command.py +++ b/tools/distrib/python/grpcio_tools/grpc_tools/command.py @@ -42,7 +42,7 @@ def build_package_protos(package_root): '--grpc_python_out={}'.format(inclusion_root), ] + [proto_file] if protoc.main(command) != 0: - sys.stderr.write('warning: {} failed'.format(command)) + raise RuntimeError('error: {} failed'.format(command)) class BuildPackageProtos(setuptools.Command): From 2dbabc7a8f92d5fe6d3aef8a30297979e5073a9b Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 16 Jan 2019 12:07:50 -0800 Subject: [PATCH 2/4] Add new grpc_tools setuptools command BuildPackageProtosStrict --- .../python/grpcio_tools/grpc_tools/command.py | 100 ++++++++++++++++-- 1 file changed, 89 insertions(+), 11 deletions(-) diff --git a/tools/distrib/python/grpcio_tools/grpc_tools/command.py b/tools/distrib/python/grpcio_tools/grpc_tools/command.py index 93503c4cc36..ee311144225 100644 --- a/tools/distrib/python/grpcio_tools/grpc_tools/command.py +++ b/tools/distrib/python/grpcio_tools/grpc_tools/command.py @@ -15,11 +15,36 @@ import os import pkg_resources import sys +import tempfile import setuptools from grpc_tools import protoc +_WELL_KNOWN_PROTOS_INCLUDE = pkg_resources.resource_filename( + 'grpc_tools', '_proto') + + +def _compile_proto(proto_file, + include='', + python_out='', + grpc_python_out='', + strict=False): + command = [ + 'grpc_tools.protoc', + '--proto_path={}'.format(include), + '--proto_path={}'.format(_WELL_KNOWN_PROTOS_INCLUDE), + '--python_out={}'.format(python_out), + '--grpc_python_out={}'.format(grpc_python_out), + ] + [proto_file] + if protoc.main(command) != 0: + if strict: + sys.stderr.write('error: {} failed'.format(command)) + else: + sys.stderr.write('warning: {} failed'.format(command)) + return False + return True + def build_package_protos(package_root): proto_files = [] @@ -30,19 +55,49 @@ def build_package_protos(package_root): proto_files.append( os.path.abspath(os.path.join(root, filename))) - well_known_protos_include = pkg_resources.resource_filename( - 'grpc_tools', '_proto') + for proto_file in proto_files: + _compile_proto( + proto_file, + include=inclusion_root, + python_out=inclusion_root, + grpc_python_out=inclusion_root, + strict=False, + ) + +def build_package_protos_strict(package_root): + proto_files = [] + inclusion_root = os.path.abspath(package_root) + for root, _, files in os.walk(inclusion_root): + for filename in files: + if filename.endswith('.proto'): + proto_files.append( + os.path.abspath(os.path.join(root, filename))) + + tmp_out_directory = tempfile.mkdtemp() + compile_failed = False for proto_file in proto_files: - command = [ - 'grpc_tools.protoc', - '--proto_path={}'.format(inclusion_root), - '--proto_path={}'.format(well_known_protos_include), - '--python_out={}'.format(inclusion_root), - '--grpc_python_out={}'.format(inclusion_root), - ] + [proto_file] - if protoc.main(command) != 0: - raise RuntimeError('error: {} failed'.format(command)) + # Output all the errors across all the files instead of exiting on the + # first error proto file. + compile_failed |= not _compile_proto( + proto_file, + include=inclusion_root, + python_out=tmp_out_directory, + grpc_python_out=tmp_out_directory, + strict=True, + ) + + if compile_failed: + sys.exit(1) + + for proto_file in proto_files: + _compile_proto( + proto_file, + include=inclusion_root, + python_out=inclusion_root, + grpc_python_out=inclusion_root, + strict=False, + ) class BuildPackageProtos(setuptools.Command): @@ -63,3 +118,26 @@ class BuildPackageProtos(setuptools.Command): # to `self.distribution.package_dir` (and get a key error if it's not # there). build_package_protos(self.distribution.package_dir['']) + + +class BuildPackageProtosStrict(setuptools.Command): + """Command to strictly generate project *_pb2.py modules from proto files. + + The generation will abort if any of the proto files contains error. + """ + + description = 'strictly build grpc protobuf modules' + user_options = [] + + def initialize_options(self): + pass + + def finalize_options(self): + pass + + def run(self): + # due to limitations of the proto generator, we require that only *one* + # directory is provided as an 'include' directory. We assume it's the '' key + # to `self.distribution.package_dir` (and get a key error if it's not + # there). + build_package_protos_strict(self.distribution.package_dir['']) From 51ba492d6dcb73bd7a7b6f6cfdd6d29ca42393ce Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 21 Jan 2019 16:26:33 -0800 Subject: [PATCH 3/4] Minimize the change --- .../python/grpcio_tools/grpc_tools/command.py | 114 ++++-------------- 1 file changed, 22 insertions(+), 92 deletions(-) diff --git a/tools/distrib/python/grpcio_tools/grpc_tools/command.py b/tools/distrib/python/grpcio_tools/grpc_tools/command.py index ee311144225..85273a65aea 100644 --- a/tools/distrib/python/grpcio_tools/grpc_tools/command.py +++ b/tools/distrib/python/grpcio_tools/grpc_tools/command.py @@ -21,32 +21,8 @@ import setuptools from grpc_tools import protoc -_WELL_KNOWN_PROTOS_INCLUDE = pkg_resources.resource_filename( - 'grpc_tools', '_proto') - -def _compile_proto(proto_file, - include='', - python_out='', - grpc_python_out='', - strict=False): - command = [ - 'grpc_tools.protoc', - '--proto_path={}'.format(include), - '--proto_path={}'.format(_WELL_KNOWN_PROTOS_INCLUDE), - '--python_out={}'.format(python_out), - '--grpc_python_out={}'.format(grpc_python_out), - ] + [proto_file] - if protoc.main(command) != 0: - if strict: - sys.stderr.write('error: {} failed'.format(command)) - else: - sys.stderr.write('warning: {} failed'.format(command)) - return False - return True - - -def build_package_protos(package_root): +def build_package_protos(package_root, strict_mode=False): proto_files = [] inclusion_root = os.path.abspath(package_root) for root, _, files in os.walk(inclusion_root): @@ -55,82 +31,33 @@ def build_package_protos(package_root): proto_files.append( os.path.abspath(os.path.join(root, filename))) - for proto_file in proto_files: - _compile_proto( - proto_file, - include=inclusion_root, - python_out=inclusion_root, - grpc_python_out=inclusion_root, - strict=False, - ) - + well_known_protos_include = pkg_resources.resource_filename( + 'grpc_tools', '_proto') -def build_package_protos_strict(package_root): - proto_files = [] - inclusion_root = os.path.abspath(package_root) - for root, _, files in os.walk(inclusion_root): - for filename in files: - if filename.endswith('.proto'): - proto_files.append( - os.path.abspath(os.path.join(root, filename))) - - tmp_out_directory = tempfile.mkdtemp() - compile_failed = False for proto_file in proto_files: - # Output all the errors across all the files instead of exiting on the - # first error proto file. - compile_failed |= not _compile_proto( - proto_file, - include=inclusion_root, - python_out=tmp_out_directory, - grpc_python_out=tmp_out_directory, - strict=True, - ) - - if compile_failed: - sys.exit(1) - - for proto_file in proto_files: - _compile_proto( - proto_file, - include=inclusion_root, - python_out=inclusion_root, - grpc_python_out=inclusion_root, - strict=False, - ) + command = [ + 'grpc_tools.protoc', + '--proto_path={}'.format(inclusion_root), + '--proto_path={}'.format(well_known_protos_include), + '--python_out={}'.format(inclusion_root), + '--grpc_python_out={}'.format(inclusion_root), + ] + [proto_file] + if protoc.main(command) != 0: + if strict_mode: + raise Exception('error: {} failed'.format(command)) + else: + sys.stderr.write('warning: {} failed'.format(command)) class BuildPackageProtos(setuptools.Command): """Command to generate project *_pb2.py modules from proto files.""" description = 'build grpc protobuf modules' - user_options = [] + user_options = [('strict-mode', 's', + 'exit with non-zero value if the proto compiling fails.')] def initialize_options(self): - pass - - def finalize_options(self): - pass - - def run(self): - # due to limitations of the proto generator, we require that only *one* - # directory is provided as an 'include' directory. We assume it's the '' key - # to `self.distribution.package_dir` (and get a key error if it's not - # there). - build_package_protos(self.distribution.package_dir['']) - - -class BuildPackageProtosStrict(setuptools.Command): - """Command to strictly generate project *_pb2.py modules from proto files. - - The generation will abort if any of the proto files contains error. - """ - - description = 'strictly build grpc protobuf modules' - user_options = [] - - def initialize_options(self): - pass + self.strict_mode = False def finalize_options(self): pass @@ -140,4 +67,7 @@ class BuildPackageProtosStrict(setuptools.Command): # directory is provided as an 'include' directory. We assume it's the '' key # to `self.distribution.package_dir` (and get a key error if it's not # there). - build_package_protos_strict(self.distribution.package_dir['']) + if self.strict_mode: + self.announce('Building Package Protos in Strict Mode') + build_package_protos(self.distribution.package_dir[''], + self.strict_mode) From 31bce3b12720fc290761d82067795867b6b46b1f Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 21 Jan 2019 16:49:36 -0800 Subject: [PATCH 4/4] Remove redundent lines --- tools/distrib/python/grpcio_tools/grpc_tools/command.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/distrib/python/grpcio_tools/grpc_tools/command.py b/tools/distrib/python/grpcio_tools/grpc_tools/command.py index 85273a65aea..1e556f5fd66 100644 --- a/tools/distrib/python/grpcio_tools/grpc_tools/command.py +++ b/tools/distrib/python/grpcio_tools/grpc_tools/command.py @@ -15,7 +15,6 @@ import os import pkg_resources import sys -import tempfile import setuptools @@ -67,7 +66,5 @@ class BuildPackageProtos(setuptools.Command): # directory is provided as an 'include' directory. We assume it's the '' key # to `self.distribution.package_dir` (and get a key error if it's not # there). - if self.strict_mode: - self.announce('Building Package Protos in Strict Mode') build_package_protos(self.distribution.package_dir[''], self.strict_mode)