From 1755918f8edded7c4edd7e1091ae40dee0d16b68 Mon Sep 17 00:00:00 2001 From: "Brian J. Watson" Date: Wed, 7 Sep 2016 13:29:54 -0700 Subject: [PATCH 1/5] Only build common protos with --grpc option for LRO. --- config/common_protos.yml | 2 +- config/dependencies.yml | 2 +- lib/api_repo.js | 16 +++++++++++----- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/config/common_protos.yml b/config/common_protos.yml index 353c581..c845a03 100644 --- a/config/common_protos.yml +++ b/config/common_protos.yml @@ -33,4 +33,4 @@ packages: version: '' # semver is the semantic version of the common protos package. -semver: 1.3.2 +semver: 1.3.3 diff --git a/config/dependencies.yml b/config/dependencies.yml index 44ff431..8cb2cde 100644 --- a/config/dependencies.yml +++ b/config/dependencies.yml @@ -67,7 +67,7 @@ protobuf: googleapis_common_protos: python: - version: '1.3.2' + version: '1.3.3' ruby: version: '1.2.0' diff --git a/lib/api_repo.js b/lib/api_repo.js index f596626..3299fd5 100644 --- a/lib/api_repo.js +++ b/lib/api_repo.js @@ -56,6 +56,9 @@ var DEFAULT_PROTO_COMPILER = 'protoc'; // Python builds using grpcio-tools. var NO_PROTOC_PLUGIN = ['nodejs', 'java', 'python']; +// common protos for which we build gRPC service methods +var COMMON_GRPC_PROTOS = ['operations.proto']; + // the default include path, where install the protobuf runtime installs its // protos. var DEFAULT_INCLUDE_PATH = Object.freeze(['/usr/local/include']); @@ -1018,11 +1021,14 @@ ApiRepo.prototype._makeProtocFunc = function _makeProtocFunc(opts, language) { pluginOption = '--plugin='; } else { args.push('--' + language + '_out=' + outDir); - if (language === 'python') { - // required per https://github.com/grpc/grpc/issues/7857 - args.push('--grpc_python_out=' + outDir); - } else { - args.push('--grpc_out=' + outDir); + if (!opts.buildCommonProtos || + _.includes(COMMON_GRPC_PROTOS, protoPath)) { + if (language === 'python') { + // required per https://github.com/grpc/grpc/issues/7857 + args.push('--grpc_python_out=' + outDir); + } else { + args.push('--grpc_out=' + outDir); + } } } if (!_.includes(NO_PROTOC_PLUGIN, language)) { From 7c1ba995c41091d9c8b02be5f230dc4d92d6ed81 Mon Sep 17 00:00:00 2001 From: "Brian J. Watson" Date: Fri, 9 Sep 2016 16:43:13 -0700 Subject: [PATCH 2/5] Increase test coverage --- test/api_repo.js | 31 +++++++++++++++++++++---------- test/fixtures/fake_protoc | 2 +- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/test/api_repo.js b/test/api_repo.js index c794ce9..a6dd19b 100644 --- a/test/api_repo.js +++ b/test/api_repo.js @@ -133,17 +133,17 @@ describe('ApiRepo', function() { }); }); describe('configured for python', function() { - beforeEach(function() { - var testBins = ['protoc', 'grpc_python_plugin']; - fakes = addFakeBinsToPath.apply(null, testBins); - repo = new ApiRepo({ - env: {PATH: fakes.path}, - languages: ['python'], - templateRoot: path.join(__dirname, '..', 'templates') - }); - getsGoodZipFrom(repo.zipUrl); - }); describe('method `buildGaxPackages`', function() { + beforeEach(function() { + var testBins = ['protoc', 'grpc_python_plugin']; + fakes = addFakeBinsToPath.apply(null, testBins); + repo = new ApiRepo({ + env: {PATH: fakes.path}, + languages: ['python'], + templateRoot: path.join(__dirname, '..', 'templates') + }); + getsGoodZipFrom(repo.zipUrl); + }); it('should succeed with unrecognized apis', function(done) { repo.on('error', function(err) { expect(err).to.not.be.null(); @@ -164,6 +164,17 @@ describe('ApiRepo', function() { }); }); describe('method `buildCommonProtoPkgs`', function() { + beforeEach(function() { + var testBins = ['protoc', 'grpc_python_plugin']; + fakes = addFakeBinsToPath.apply(null, testBins); + repo = new ApiRepo({ + buildCommonProtos: true, + env: {PATH: fakes.path}, + languages: ['python'], + templateRoot: path.join(__dirname, '..', 'templates') + }); + getsGoodZipFrom(repo.zipUrl); + }); it('should pass', function(done) { repo.on('error', function() { throw new Error('should not be reached'); diff --git a/test/fixtures/fake_protoc b/test/fixtures/fake_protoc index b4d4da8..f1c095c 100755 --- a/test/fixtures/fake_protoc +++ b/test/fixtures/fake_protoc @@ -26,7 +26,7 @@ main() { # fix out_dir in case protoc was run for the go plugin local out_dir=$(echo "$out_dir" | sed 's/.*--go_out=plugins=grpc:\([^ ]*\).*/\1/') # fix out_dir in case protoc was run for the python plugin - local out_dir=$(echo "$out_dir" | sed 's/.*--grpc_python_out=\([^ ]*\).*/\1/') + local out_dir=$(echo "$out_dir" | sed 's/.*--python_out=\([^ ]*\).*/\1/') echo "out_dir is $out_dir" 1>&2; local out_proto_path=${out_dir}/$proto_path local out_parent=$(dirname $out_proto_path) From 6d0e0424058c76e2385997b2be229ca9b3eff1f1 Mon Sep 17 00:00:00 2001 From: "Brian J. Watson" Date: Fri, 9 Sep 2016 16:53:18 -0700 Subject: [PATCH 3/5] Latest released version of googleapis-common-protos is 1.3.4 --- config/common_protos.yml | 2 +- config/dependencies.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/common_protos.yml b/config/common_protos.yml index c845a03..42bdbb0 100644 --- a/config/common_protos.yml +++ b/config/common_protos.yml @@ -33,4 +33,4 @@ packages: version: '' # semver is the semantic version of the common protos package. -semver: 1.3.3 +semver: 1.3.4 diff --git a/config/dependencies.yml b/config/dependencies.yml index 8cb2cde..bd1dc3f 100644 --- a/config/dependencies.yml +++ b/config/dependencies.yml @@ -67,7 +67,7 @@ protobuf: googleapis_common_protos: python: - version: '1.3.3' + version: '1.3.4' ruby: version: '1.2.0' From b571542e1697884b15356751954536de00ef975d Mon Sep 17 00:00:00 2001 From: "Brian J. Watson" Date: Fri, 9 Sep 2016 16:54:03 -0700 Subject: [PATCH 4/5] Add bytestream.proto per request from @jmuk --- lib/api_repo.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api_repo.js b/lib/api_repo.js index 3299fd5..53bccd2 100644 --- a/lib/api_repo.js +++ b/lib/api_repo.js @@ -57,7 +57,7 @@ var DEFAULT_PROTO_COMPILER = 'protoc'; var NO_PROTOC_PLUGIN = ['nodejs', 'java', 'python']; // common protos for which we build gRPC service methods -var COMMON_GRPC_PROTOS = ['operations.proto']; +var COMMON_GRPC_PROTOS = ['operations.proto', 'bytestream.proto']; // the default include path, where install the protobuf runtime installs its // protos. From cc1f8bcdd7bad9151aa6bbd26a1d9c32c500b29e Mon Sep 17 00:00:00 2001 From: "Brian J. Watson" Date: Mon, 12 Sep 2016 13:57:31 -0700 Subject: [PATCH 5/5] Fix a nit --- test/fixtures/fake_protoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/fixtures/fake_protoc b/test/fixtures/fake_protoc index f1c095c..b470f19 100755 --- a/test/fixtures/fake_protoc +++ b/test/fixtures/fake_protoc @@ -24,9 +24,9 @@ main() { local proto_path=${!#} local out_dir=$(echo "$@" | sed 's/.*--grpc_out=\([^ ]*\).*/\1/') # fix out_dir in case protoc was run for the go plugin - local out_dir=$(echo "$out_dir" | sed 's/.*--go_out=plugins=grpc:\([^ ]*\).*/\1/') + out_dir=$(echo "$out_dir" | sed 's/.*--go_out=plugins=grpc:\([^ ]*\).*/\1/') # fix out_dir in case protoc was run for the python plugin - local out_dir=$(echo "$out_dir" | sed 's/.*--python_out=\([^ ]*\).*/\1/') + out_dir=$(echo "$out_dir" | sed 's/.*--python_out=\([^ ]*\).*/\1/') echo "out_dir is $out_dir" 1>&2; local out_proto_path=${out_dir}/$proto_path local out_parent=$(dirname $out_proto_path)