From 830bd0c2a8b2f8d48995a8b487c9cb17bcca1eb2 Mon Sep 17 00:00:00 2001
From: Jun Mukai <mukai@google.com>
Date: Tue, 16 Aug 2016 17:22:30 -0700
Subject: [PATCH 1/5] Update NodeJS packaging patterns.

- retire use_pbjs flag -- it's not used anymore, and it does not
  fit with the new pattern of Gapic code.
- update the grpc package template: this fits with the proto packages
  used by gcloud-node.
- update gax package template: this will work well with the new
  pattern of Gapic code, as you can see in googleapis/toolkit#392
  or GoogleCloudPlatform/gcloud-node#1476
---
 bin/gen-api-package                           | 12 ----
 lib/api_repo.js                               | 61 +------------------
 lib/packager.js                               | 43 +++++++++----
 templates/gax/nodejs/index.js.mustache        | 38 +++++++++++-
 templates/gax/nodejs/package.json.mustache    |  2 +-
 templates/nodejs/index.js                     |  9 ++-
 templates/nodejs/package.json.mustache        | 10 +--
 templates/nodejs/service.js.mustache          | 26 --------
 test/api_repo.js                              |  2 -
 .../fixtures/gax/nodejs/{ => src/v2}/index.js | 29 ++++++---
 test/fixtures/nodejs/package.json             |  9 +--
 test/fixtures/proto-based/nodejs/README.md    |  6 --
 test/fixtures/proto-based/nodejs/package.json | 20 ------
 test/fixtures/proto-based/nodejs/service.js   | 27 --------
 test/packager.js                              | 38 +-----------
 15 files changed, 103 insertions(+), 229 deletions(-)
 delete mode 100644 templates/nodejs/service.js.mustache
 rename test/fixtures/gax/nodejs/{ => src/v2}/index.js (52%)
 delete mode 100644 test/fixtures/proto-based/nodejs/README.md
 delete mode 100644 test/fixtures/proto-based/nodejs/package.json
 delete mode 100644 test/fixtures/proto-based/nodejs/service.js

diff --git a/bin/gen-api-package b/bin/gen-api-package
index 18b8e5c..8215855 100755
--- a/bin/gen-api-package
+++ b/bin/gen-api-package
@@ -197,18 +197,6 @@ var parseArgs = function parseArgs() {
       dest: 'altJava'
     }
   );
-  cli.addArgument(
-    [ '--nodejs_use_pbjs' ],
-    {
-      defaultValue: false,
-      action: 'storeTrue',
-      help: 'When set, the nodejs grpc client is generated with the json data\n'
-            + ' generated from pbjs. Otherwise the client bundles .proto\n'
-            + ' files. Note that pbjs-based client does not work currently\n'
-            + ' (see https://github.com/googleapis/packman/issues/35).',
-      dest: 'nodejsUsePbjs'
-    }
-  );
   cli.addArgument(
     [ '--override_plugins' ],
     {
diff --git a/lib/api_repo.js b/lib/api_repo.js
index f596626..1b27716 100644
--- a/lib/api_repo.js
+++ b/lib/api_repo.js
@@ -24,14 +24,11 @@ var fs = require('fs-extra');
 var glob = require('glob');
 var packager = require('./packager');
 var path = require('path');
-var pbjs = require('protobufjs/cli/pbjs');
-var pbjsUtil = require('protobufjs/cli/pbjs/util');
 var readline = require('readline');
 var request = require('request');
 var tmp = require('tmp');
 
 var EventEmitter = require('events').EventEmitter;
-var ProtoBuf = require('protobufjs');
 var StreamZip = require('node-stream-zip');
 
 exports.ApiRepo = ApiRepo;
@@ -219,7 +216,7 @@ ApiRepo.prototype.buildPackages =
               packageInfo: that.packageInfo,
               generated: generated
             }, templateInfo[l]);
-            if (l === 'nodejs' && !that.opts.nodejsUsePbjs) {
+            if (l === 'nodejs') {
               opts.packageInfo.protoFiles = _.filter(generated, function(file) {
                 return file.match(/\.proto$/);
               });
@@ -615,22 +612,7 @@ ApiRepo.prototype._buildProtos =
           var fullPathProtos = [];
           function makeModule() {
             var includePath = _.union(that.includePath, that.repoDirs);
-            if (!that.opts.nodejsUsePbjs) {
-              makeProtoBasedNodeModule(fullPathProtos, includePath);
-              return;
-            }
-            var opts = {
-              root: that.repoDirs,
-              source: 'proto',
-              path: includePath
-            };
-
-            var builder = loadProtos(fullPathProtos, opts);
-            var outDir = path.join(that.outDir, language);
-            fs.mkdirsSync(outDir);
-            var servicePath = path.join(outDir, 'service.js');
-            var commonJS = pbjs.targets.commonjs(builder, opts);
-            fs.writeFile(servicePath, commonJS, findOutputs);
+            makeProtoBasedNodeModule(fullPathProtos, includePath);
           }
 
           this._findProtos(
@@ -640,7 +622,6 @@ ApiRepo.prototype._buildProtos =
                 next();
               }
           );
-          makeModule();
         }.bind(this);
         this._findProtos(name, version, makeNodeModule);
       } else {
@@ -1058,41 +1039,3 @@ ApiRepo.prototype._makeProtocFunc = function _makeProtocFunc(opts, language) {
 
   return callProtoc;
 };
-
-/**
- * Helps construct a JSON representation each proto file for the nodejs build.
- *
- * @param {string[]} filenames - The list of files.
- * @param {object} opts - provides configuration info
- * @param {object} opts.root - a virtual root folder that contains all the protos
- * @param {object} opts.path - an array of folders where other protos reside
- *
- * @return {object} a ProtoBuf.Builder containing loaded representations of the
- * protos
- */
-function loadProtos(filenames, opts) {
-  opts = opts || [];
-  var builder = ProtoBuf.newBuilder();
-  var loaded = [];
-  builder.importRoot = opts.root;
-  filenames.forEach(function(filename) {
-    var data = pbjs.sources.proto.load(filename, opts, loaded);
-    builder.import(data, filename);
-  });
-  builder.resolveAll();
-  return builder;
-}
-
-/**
- * Replace isDescriptor with a version that always returns false.
- *
- * pbjs/util.isDescriptor excludes imports that in google/protobuf.
- *
- * However, the nodejs packages need to be self-contained, so we actually want
- * to include these.
- * @param {string} name - The name of the message.
- * @return {Boolean} Whether it is a descriptor or not.
- */
-pbjsUtil.isDescriptor = function(name) {
-  return false;
-};
diff --git a/lib/packager.js b/lib/packager.js
index df65e16..54e138a 100644
--- a/lib/packager.js
+++ b/lib/packager.js
@@ -176,6 +176,12 @@ function removeMustacheExt(filePath) {
   return filePath.slice(0, extIndex);
 }
 
+function underscoreToCamelCase(str) {
+  return _.map(str.split('_'), function(segment) {
+    return segment.charAt(0).toUpperCase() + segment.slice(1);
+  }).join('');
+}
+
 /**
  * makePythonPackage creates a new python package.
  *
@@ -360,9 +366,6 @@ function makeNodejsPackage(opts, done) {
   opts = _.merge({}, settings.nodejs, opts);
   var tasks = [];
 
-  if (!opts.packageInfo.nodejsUsePbjs) {
-    opts.templates.push('service.js.mustache');
-  }
   // Move copyable files to the top-level dir.
   opts.copyables.forEach(function(f) {
     var src = path.join(opts.templateDir, f);
@@ -378,17 +381,38 @@ function makeNodejsPackage(opts, done) {
     var dstBase = removeMustacheExt(f);
     var tmpl = path.join(opts.templateDir, f);
     var dst = path.join(opts.top, dstBase);
-    tasks.push(expand(tmpl, dst, opts.packageInfo));
+    // index.js should reside among other .js files for GAX packages.
+    if (dstBase !== 'index.js') {
+      tasks.push(expand(tmpl, dst, opts.packageInfo));
+    }
   });
 
   function runTask(err, jsFiles) {
+    function underscoreToLowerCamelCase(str) {
+      var camelCase = underscoreToCamelCase(str);
+      return camelCase.charAt(0).toLowerCase() + camelCase.slice(1);
+    }
     if (err) {
       done(err);
       return;
     }
     opts.packageInfo.apiFiles = _.map(jsFiles, function(jsFile) {
-      return jsFile.replace(/\.js$/, '');
+      var filePath = path.basename(jsFile, '.js');
+      return {
+        name: underscoreToLowerCamelCase(filePath),
+        filePath: filePath
+      };
     });
+    if (opts.packageInfo.apiFiles.length > 0) {
+      _.last(opts.packageInfo.apiFiles).last = true;
+      opts.packageInfo.serviceAddressName = opts.packageInfo.apiFiles[0].name;
+      // Creating a task to bring index.js into the same directory where
+      // other generated .js files exist.
+      tasks.push(expand(path.join(opts.templateDir, 'index.js.mustache'),
+                        path.join(path.dirname(jsFiles[0]), 'index.js'),
+                        opts.packageInfo));
+    }
+    opts.packageInfo.singleFile = (opts.packageInfo.apiFiles.length === 1);
     async.series(tasks, function(err) {
       if (!err && opts.copyables.indexOf('PUBLISHING.md') !== -1) {
         console.log('The nodejs package', pkgName(opts.packageInfo),
@@ -403,7 +427,9 @@ function makeNodejsPackage(opts, done) {
   // find lib/*.js files for gax API packages to generate index.js properly.
   if (opts.mockApiFilesForTest) {
     // For tests, mock data is passed instead of scanning the filesystem.
-    runTask(null, opts.mockApiFilesForTest);
+    runTask(null, _.map(opts.mockApiFilesForTest, function(file) {
+      return path.join(opts.top, 'src', file);
+    }));
   } else {
     glob.glob('*.js', {cwd: path.join(opts.top, 'src')}, runTask);
   }
@@ -468,11 +494,6 @@ function makeRubyPackage(opts, done) {
    * @param {function(?Error, Object[])} done - The callback.
    */
   function collectApiClasses(done) {
-    function underscoreToCamelCase(str) {
-      return _.map(str.split('_'), function(segment) {
-        return segment.charAt(0).toUpperCase() + segment.slice(1);
-      }).join('');
-    }
     function rbFileToData(rbFile) {
       var pathComponents = _.map(
           rbFile.replace(/\.rb$/, '').split('/'), underscoreToCamelCase);
diff --git a/templates/gax/nodejs/index.js.mustache b/templates/gax/nodejs/index.js.mustache
index 7637610..86db335 100644
--- a/templates/gax/nodejs/index.js.mustache
+++ b/templates/gax/nodejs/index.js.mustache
@@ -13,10 +13,42 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+'use strict';
 
 {{#apiFiles}}
-var apiClient = require('./src/{{{.}}}');
-for (var key in apiClient) {
-  exports[key] = apiClient[key];
+var {{{name}}} = require('./{{{filePath}}}');
+{{/apiFiles}}
+var gax = require('google-gax');
+var extend = require('extend');
+{{^singleFile}}
+var lodash = require('lodash');
+{{/singleFile}}
+
+function {{{api.version}}}(options) {
+  options = extend({
+    scopes: {{{api.version}}}.ALL_SCOPES
+  }, options);
+  var gaxGrpc = gax.grpc(options);
+  {{#singleFile}}
+  return {{{apiFiles[0].name}}}(gaxGrpc);
+  {{/singleFile}}
+  {{^singleFile}}
+  var result = {};
+  {{#apiFiles}}
+  extend(result, {{{name}}}(gaxGrpc));
+  {{/apiFiles}}
+  return result;
+  {{/singleFile}}
 }
+{{{api.version}}}.SERVICE_ADDRESS = {{{serviceAddressName}}}.SERVICE_ADDRESS;
+{{#singleFile}}
+{{{api.version}}}.ALL_SCOPES = {{{serviceAddressName}}}.ALL_SCOPES;
+{{/singleFile}}
+{{^singleFile}}
+{{{api.version}}}.ALL_SCOPES = lodash.union(
+{{#apiFiles}}
+  {{{name}}}.ALL_SCOPES{{^last}},{{/last}}
 {{/apiFiles}}
+);
+{{/singleFile}}
+module.exports = {{{api.version}}};
diff --git a/templates/gax/nodejs/package.json.mustache b/templates/gax/nodejs/package.json.mustache
index d9e3b93..d96304f 100644
--- a/templates/gax/nodejs/package.json.mustache
+++ b/templates/gax/nodejs/package.json.mustache
@@ -11,5 +11,5 @@
     "grpc": "^{{{dependencies.grpc.nodejs.version}}}",
     "{{{api.dependsOn}}}-{{{api.version}}}": "^{{{api.semantic_version}}}"
   },
-  "main": "index.js"
+  "main": "src/index.js"
 }
diff --git a/templates/nodejs/index.js b/templates/nodejs/index.js
index 4a49055..aef6b31 100644
--- a/templates/nodejs/index.js
+++ b/templates/nodejs/index.js
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
-var grpc = require('grpc');
-exports.client = grpc.loadObject(require('./service'));
-exports.auth = require('google-auth-library');
+var path = require('path');
+
+module.exports = function(relative) {
+  return path.join(__dirname, 'proto', relative);
+};
+l
diff --git a/templates/nodejs/package.json.mustache b/templates/nodejs/package.json.mustache
index ba05f74..4067c5d 100644
--- a/templates/nodejs/package.json.mustache
+++ b/templates/nodejs/package.json.mustache
@@ -5,18 +5,10 @@
   "author": "Google Inc.",
   "description": "GRPC library for service {{{api.name}}}-{{{api.version}}}",
   "license": "{{{api.license}}}",
-  "dependencies": {
-    "grpc": "^{{{dependencies.grpc.nodejs.version}}}",
-    "protobufjs": "^{{{dependencies.protobuf.nodejs.version}}}",
-    "google-auth-library": "^{{{dependencies.auth.nodejs.version}}}"
-  },
   "main": "index.js",
   "files": [
     "LICENSE",
-    "index.js",
-{{#nodejsUseProtos}}
     "proto/**/*",
-{{/nodejsUseProtos}}
-    "service.js"
+    "index.js"
   ]
 }
diff --git a/templates/nodejs/service.js.mustache b/templates/nodejs/service.js.mustache
deleted file mode 100644
index c90ab63..0000000
--- a/templates/nodejs/service.js.mustache
+++ /dev/null
@@ -1,26 +0,0 @@
-/**
- * Copyright 2016 Google Inc. All Rights Reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-var protobufjs = require('protobufjs');
-var path = require('path');
-var builder = protobufjs.newBuilder({});
-builder.importRoot = path.join(__dirname, 'proto');
-{{#protoFiles}}
-protobufjs.loadProtoFile(
-    path.join(__dirname, '{{{.}}}'),
-    builder);
-{{/protoFiles}}
-module.exports = builder.ns;
diff --git a/test/api_repo.js b/test/api_repo.js
index c794ce9..54c85fb 100644
--- a/test/api_repo.js
+++ b/test/api_repo.js
@@ -100,10 +100,8 @@ describe('ApiRepo', function() {
   describe('on the test fixture repo with no plugins', function() {
     var fakes, repo; // eslint-disable-line
     describe('configured for nodejs', function() {
-      // TODO: add a test case for nodejsUsePbjs: false.
       beforeEach(function() {
         repo = new ApiRepo({
-          nodejsUsePbjs: true,
           includePath: [path.join(__dirname, 'fixtures', 'include')],
           languages: ['nodejs'],
           templateRoot: path.join(__dirname, '..', 'templates')
diff --git a/test/fixtures/gax/nodejs/index.js b/test/fixtures/gax/nodejs/src/v2/index.js
similarity index 52%
rename from test/fixtures/gax/nodejs/index.js
rename to test/fixtures/gax/nodejs/src/v2/index.js
index 099d724..97746f1 100644
--- a/test/fixtures/gax/nodejs/index.js
+++ b/test/fixtures/gax/nodejs/src/v2/index.js
@@ -13,12 +13,27 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+'use strict';
 
-var apiClient = require('./src/foo_api');
-for (var key in apiClient) {
-  exports[key] = apiClient[key];
-}
-var apiClient = require('./src/bar_api');
-for (var key in apiClient) {
-  exports[key] = apiClient[key];
+var fooApi = require('./foo_api');
+var barApi = require('./bar_api');
+var gax = require('google-gax');
+var extend = require('extend');
+var lodash = require('lodash');
+
+function v2(options) {
+  options = extend({
+    scopes: v2.ALL_SCOPES
+  }, options);
+  var gaxGrpc = gax.grpc(options);
+  var result = {};
+  extend(result, fooApi(gaxGrpc));
+  extend(result, barApi(gaxGrpc));
+  return result;
 }
+v2.SERVICE_ADDRESS = fooApi.SERVICE_ADDRESS;
+v2.ALL_SCOPES = lodash.union(
+  fooApi.ALL_SCOPES,
+  barApi.ALL_SCOPES
+);
+module.exports = v2;
diff --git a/test/fixtures/nodejs/package.json b/test/fixtures/nodejs/package.json
index 3af3756..701c788 100644
--- a/test/fixtures/nodejs/package.json
+++ b/test/fixtures/nodejs/package.json
@@ -5,15 +5,10 @@
   "author": "Google Inc.",
   "description": "GRPC library for service packager-unittest-v2",
   "license": "BSD-3-Clause",
-  "dependencies": {
-    "grpc": "^0.10.0",
-    "protobufjs": "^5.0.1",
-    "google-auth-library": "^0.9.2"
-  },
   "main": "index.js",
   "files": [
     "LICENSE",
-    "index.js",
-    "service.js"
+    "proto/**/*",
+    "index.js"
   ]
 }
diff --git a/test/fixtures/proto-based/nodejs/README.md b/test/fixtures/proto-based/nodejs/README.md
deleted file mode 100644
index 303083d..0000000
--- a/test/fixtures/proto-based/nodejs/README.md
+++ /dev/null
@@ -1,6 +0,0 @@
-# gRPC library for the packager-v2 service
-
-packager-unittest-v2 contains the IDL-generated [grpc][] library for the service: `packager-v2` in the [googleapis][] repository.
-
-[googleapis]:https://github.com/google/googleapis
-[grpc]:http://www.grpc.io/docs/tutorials/basic/node.html
diff --git a/test/fixtures/proto-based/nodejs/package.json b/test/fixtures/proto-based/nodejs/package.json
deleted file mode 100644
index d7db9ef..0000000
--- a/test/fixtures/proto-based/nodejs/package.json
+++ /dev/null
@@ -1,20 +0,0 @@
-{
-  "url": "https://github.com/google/googleapis",
-  "name": "packager-unittest-v2",
-  "version": "1.0.0",
-  "author": "Google Inc.",
-  "description": "GRPC library for service packager-unittest-v2",
-  "license": "BSD-3-Clause",
-  "dependencies": {
-    "grpc": "^0.10.0",
-    "protobufjs": "^5.0.1",
-    "google-auth-library": "^0.9.2"
-  },
-  "main": "index.js",
-  "files": [
-    "LICENSE",
-    "index.js",
-    "proto/**/*",
-    "service.js"
-  ]
-}
diff --git a/test/fixtures/proto-based/nodejs/service.js b/test/fixtures/proto-based/nodejs/service.js
deleted file mode 100644
index da5b244..0000000
--- a/test/fixtures/proto-based/nodejs/service.js
+++ /dev/null
@@ -1,27 +0,0 @@
-/**
- * Copyright 2016 Google Inc. All Rights Reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-var protobufjs = require('protobufjs');
-var path = require('path');
-var builder = protobufjs.newBuilder({});
-builder.importRoot = path.join(__dirname, 'proto');
-protobufjs.loadProtoFile(
-    path.join(__dirname, 'proto/foo.proto'),
-    builder);
-protobufjs.loadProtoFile(
-    path.join(__dirname, 'proto/bar/baz.proto'),
-    builder);
-module.exports = builder.ns;
diff --git a/test/packager.js b/test/packager.js
index 6e753ad..828a58f 100644
--- a/test/packager.js
+++ b/test/packager.js
@@ -366,48 +366,14 @@ describe('the nodejs package builder', function() {
     ], done);
   });
 
-  it('should construct a proto-based nodejs package', function(done) {
-    var opts = _.merge({
-      packageInfo: testPackageInfo,
-      nodejsUsePbjs: false,
-      top: path.join(top, 'nodejs')
-    }, templateDirs.nodejs);
-    opts.packageInfo.nodejsUseProtos = true;
-    opts.packageInfo.protoFiles = ['proto/foo.proto', 'proto/bar/baz.proto'];
-    var copies = [
-      'nodejs/index.js',
-      'nodejs/PUBLISHING.md'
-    ];
-    var checkCopies = function checkCopies(next) {
-      var checkACopy = genCopyCompareFunc(top);
-      var copyTasks = _.map(copies, checkACopy);
-      async.parallel(copyTasks, next);
-    };
-    var expanded = [
-      'nodejs/package.json',
-      'nodejs/service.js',
-      'nodejs/README.md'
-    ];
-    var compareWithFixture = genFixtureCompareFunc(top, ['proto-based']);
-    var checkExpanded = function checkExpanded(next) {
-      var expandTasks = _.map(expanded, compareWithFixture);
-      async.parallel(expandTasks, next);
-    };
-    async.series([
-      packager.nodejs.bind(null, opts),
-      checkCopies,
-      checkExpanded
-    ], done);
-  });
-
   it('should construct a gax package', function(done) {
     var opts = _.merge({
       packageInfo: testPackageInfo,
       top: path.join(top, 'nodejs')
     }, {templateDir: path.join(__dirname, '..', 'templates', 'gax', 'nodejs')});
-    opts.mockApiFilesForTest = ['foo_api.js', 'bar_api.js'];
+    opts.mockApiFilesForTest = ['v2/foo_api.js', 'v2/bar_api.js'];
     var expanded = [
-      'nodejs/index.js'
+      'nodejs/src/v2/index.js'
     ];
     var compareWithFixture = genFixtureCompareFunc(top, ['gax']);
     var checkExpanded = function checkExpanded(next) {

From 32540f36ba7ec7238de927779b9fb1543412e74f Mon Sep 17 00:00:00 2001
From: Jun Mukai <mukai@google.com>
Date: Fri, 19 Aug 2016 11:50:52 -0700
Subject: [PATCH 2/5] Fixes

- remove a typo
- add fake_protoc to pass nodejs test cases
---
 templates/nodejs/index.js | 1 -
 test/api_repo.js          | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/templates/nodejs/index.js b/templates/nodejs/index.js
index aef6b31..0e8cfce 100644
--- a/templates/nodejs/index.js
+++ b/templates/nodejs/index.js
@@ -19,4 +19,3 @@ var path = require('path');
 module.exports = function(relative) {
   return path.join(__dirname, 'proto', relative);
 };
-l
diff --git a/test/api_repo.js b/test/api_repo.js
index 54c85fb..d19b5dd 100644
--- a/test/api_repo.js
+++ b/test/api_repo.js
@@ -101,7 +101,9 @@ describe('ApiRepo', function() {
     var fakes, repo; // eslint-disable-line
     describe('configured for nodejs', function() {
       beforeEach(function() {
+        fakes = addFakeBinsToPath.apply(null, ['protoc']);
         repo = new ApiRepo({
+          env: {PATH: fakes.path},
           includePath: [path.join(__dirname, 'fixtures', 'include')],
           languages: ['nodejs'],
           templateRoot: path.join(__dirname, '..', 'templates')

From 91a36f55c8e8d93df7c5aa113f74b025d83fd785 Mon Sep 17 00:00:00 2001
From: Jun Mukai <mukai@google.com>
Date: Fri, 19 Aug 2016 14:18:31 -0700
Subject: [PATCH 3/5] Fix test failures.

- add 'env' parameter to dependency_out protoc
- modify fake_protoc not to fail on this test
---
 lib/api_repo.js           | 13 ++++---------
 test/api_repo.js          |  2 +-
 test/fixtures/fake_protoc |  3 +++
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/lib/api_repo.js b/lib/api_repo.js
index 1b27716..07e1b9b 100644
--- a/lib/api_repo.js
+++ b/lib/api_repo.js
@@ -434,11 +434,12 @@ ApiRepo.prototype.buildCommonProtoPkgs =
  * called with the list of file name within the includePath when
  * the task has finished.
  * @param {string[]} includePath - The list of include path of protoc.
+ * @param {Object} env - The environment variable mapping.
  * @param {string} protoFile - The target .proto file.
  * @param {function(?Error, string[])} done - The callback.
  */
 ApiRepo.prototype._collectProtoDeps = function _collectProtoDeps(
-    includePath, protoFile, done) {
+    includePath, env, protoFile, done) {
   var deps = tmp.fileSync();
   var desc = tmp.fileSync();
   var args = this.protoCompilerArgs.concat(
@@ -451,7 +452,7 @@ ApiRepo.prototype._collectProtoDeps = function _collectProtoDeps(
   args.push(protoFile);
   // Invokes protoc with --dependency_out to generate the dependency
   // proto files.
-  execFile(this.protoCompiler, args, {}, function(err) {
+  execFile(this.protoCompiler, args, {env: env}, function(err) {
     if (err) {
       done(err);
     }
@@ -579,7 +580,7 @@ ApiRepo.prototype._buildProtos =
               var outDir = path.join(langTopDir, 'proto');
               async.map(
                   fullPathProtos,
-                  this._collectProtoDeps.bind(null, includePath),
+                  this._collectProtoDeps.bind(null, includePath, this.opts.env),
                   function(err, fileLists) {
                     if (err) {
                       findOutputs(err);
@@ -656,12 +657,6 @@ ApiRepo.prototype._getPluginName = function _getPluginName(
  * @param {function(?Error)} done - The callback.
  */
 ApiRepo.prototype._checkDeps = function _checkDeps(opts, done) {
-  // If nodejs is the only language, there are no dependencies.
-  if (this.languages.length === 1 && this.languages.indexOf('nodejs') !== -1) {
-    done(null);
-    return;
-  }
-
   // If gaxDir is set, are no dependencies, as protoc is not run
   if (this.gaxDir) {
     done(null);
diff --git a/test/api_repo.js b/test/api_repo.js
index d19b5dd..bd57650 100644
--- a/test/api_repo.js
+++ b/test/api_repo.js
@@ -101,7 +101,7 @@ describe('ApiRepo', function() {
     var fakes, repo; // eslint-disable-line
     describe('configured for nodejs', function() {
       beforeEach(function() {
-        fakes = addFakeBinsToPath.apply(null, ['protoc']);
+        fakes = addFakeBinsToPath('protoc');
         repo = new ApiRepo({
           env: {PATH: fakes.path},
           includePath: [path.join(__dirname, 'fixtures', 'include')],
diff --git a/test/fixtures/fake_protoc b/test/fixtures/fake_protoc
index b4d4da8..b2adcb4 100755
--- a/test/fixtures/fake_protoc
+++ b/test/fixtures/fake_protoc
@@ -20,6 +20,9 @@
 
 main() {
     echo $@  # first echo, the protoc tests need that.
+    if [ -z `echo "$@" | grep -o 'grpc_out'` ]; then
+      return
+    fi
 
     local proto_path=${!#}
     local out_dir=$(echo "$@" | sed 's/.*--grpc_out=\([^ ]*\).*/\1/')

From 9dab7cef4439812d0ca077c3ea5448af76487ba3 Mon Sep 17 00:00:00 2001
From: Jun Mukai <mukai@google.com>
Date: Wed, 7 Sep 2016 16:53:41 -0700
Subject: [PATCH 4/5] Fix rebase failures

---
 lib/api_repo.js           | 10 +++++-----
 test/api_repo.js          |  3 ++-
 test/fixtures/fake_protoc |  3 ---
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/api_repo.js b/lib/api_repo.js
index 07e1b9b..90f0468 100644
--- a/lib/api_repo.js
+++ b/lib/api_repo.js
@@ -439,11 +439,11 @@ ApiRepo.prototype.buildCommonProtoPkgs =
  * @param {function(?Error, string[])} done - The callback.
  */
 ApiRepo.prototype._collectProtoDeps = function _collectProtoDeps(
-    includePath, env, protoFile, done) {
+    includePath, protoFile, done) {
   var deps = tmp.fileSync();
   var desc = tmp.fileSync();
-  var args = this.protoCompilerArgs.concat(
-      ['--dependency_out=' + deps.name, '-o', desc.name]);
+  var args = this.protoCompilerArgs ? this.protoCompilerArgs.split(' ') : [];
+  args.push('--dependency_out=' + deps.name, '-o', desc.name);
   if (includePath) {
     includePath.forEach(function(ipath) {
       args.push('-I', ipath);
@@ -452,7 +452,7 @@ ApiRepo.prototype._collectProtoDeps = function _collectProtoDeps(
   args.push(protoFile);
   // Invokes protoc with --dependency_out to generate the dependency
   // proto files.
-  execFile(this.protoCompiler, args, {env: env}, function(err) {
+  execFile(this.protoCompiler, args, {env: this.env}, function(err) {
     if (err) {
       done(err);
     }
@@ -580,7 +580,7 @@ ApiRepo.prototype._buildProtos =
               var outDir = path.join(langTopDir, 'proto');
               async.map(
                   fullPathProtos,
-                  this._collectProtoDeps.bind(null, includePath, this.opts.env),
+                  this._collectProtoDeps.bind(this, includePath),
                   function(err, fileLists) {
                     if (err) {
                       findOutputs(err);
diff --git a/test/api_repo.js b/test/api_repo.js
index bd57650..3d14e0c 100644
--- a/test/api_repo.js
+++ b/test/api_repo.js
@@ -106,7 +106,8 @@ describe('ApiRepo', function() {
           env: {PATH: fakes.path},
           includePath: [path.join(__dirname, 'fixtures', 'include')],
           languages: ['nodejs'],
-          templateRoot: path.join(__dirname, '..', 'templates')
+          templateRoot: path.join(__dirname, '..', 'templates'),
+          protoCompiler: 'echo'
         });
         getsGoodZipFrom(repo.zipUrl);
       });
diff --git a/test/fixtures/fake_protoc b/test/fixtures/fake_protoc
index b2adcb4..b4d4da8 100755
--- a/test/fixtures/fake_protoc
+++ b/test/fixtures/fake_protoc
@@ -20,9 +20,6 @@
 
 main() {
     echo $@  # first echo, the protoc tests need that.
-    if [ -z `echo "$@" | grep -o 'grpc_out'` ]; then
-      return
-    fi
 
     local proto_path=${!#}
     local out_dir=$(echo "$@" | sed 's/.*--grpc_out=\([^ ]*\).*/\1/')

From 1d7c2b8287be1217699c5d3c6622ebd5d2c6a2b8 Mon Sep 17 00:00:00 2001
From: Jun Mukai <mukai@google.com>
Date: Wed, 7 Sep 2016 17:03:38 -0700
Subject: [PATCH 5/5] Fix the lint error.

---
 lib/api_repo.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/api_repo.js b/lib/api_repo.js
index 90f0468..d72a98a 100644
--- a/lib/api_repo.js
+++ b/lib/api_repo.js
@@ -434,7 +434,6 @@ ApiRepo.prototype.buildCommonProtoPkgs =
  * called with the list of file name within the includePath when
  * the task has finished.
  * @param {string[]} includePath - The list of include path of protoc.
- * @param {Object} env - The environment variable mapping.
  * @param {string} protoFile - The target .proto file.
  * @param {function(?Error, string[])} done - The callback.
  */