Merge pull request #1274 from murgatroid99/node_relative_requires

Fix CommonJS relative require generation, and test it

js/binary/proto_test.js

@@ -32,7 +32,7 @@
 
 goog.require('goog.testing.asserts');
 
-// CommonJS-LoadFromFile: testbinary_pb proto.jspb.test
+// CommonJS-LoadFromFile: ../testbinary_pb proto.jspb.test
 goog.require('proto.jspb.test.ExtendsWithMessage');
 goog.require('proto.jspb.test.ForeignEnum');
 goog.require('proto.jspb.test.ForeignMessage');

js/commonjs/import_test.js

+// Protocol Buffers - Google's data interchange format
+// Copyright 2016 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Test suite is written using Jasmine -- see http://jasmine.github.io/
+
+
+
+var googleProtobuf = require('google-protobuf');
+var asserts = require('closure_asserts_commonjs');
+var global = Function('return this')();
+
+// Bring asserts into the global namespace.
+googleProtobuf.object.extend(global, asserts);
+googleProtobuf.exportSymbol('jspb.Message', googleProtobuf.Message, global);
+
+var test7_pb = require('./test7/test7_pb');
+googleProtobuf.exportSymbol('proto.jspb.test.framing.FramingMessage', test7_pb.FramingMessage, global);
+
+describe('Import test suite', function() {
+  it('testImportedMessage', function() {
+    var framing1 = new proto.jspb.test.framing.FramingMessage([]);
+    var framing2 = new proto.jspb.test.framing.FramingMessage([]);
+    assertObjectEquals(framing1.toObject(), framing2.toObject());
+  });
+});

js/commonjs/rewrite_tests_for_commonjs.js

@@ -80,11 +80,12 @@ lineReader.on('line', function(line) {
       console.log("// Bring asserts into the global namespace.");
       console.log("googleProtobuf.object.extend(global, asserts);");
     }
-    module = camelCase(isLoadFromFile[1])
+    var module_path = isLoadFromFile[1].split('/');
+    module = camelCase(module_path[module_path.length - 1]);
     pkg = isLoadFromFile[2];
 
     if (module != "googleProtobuf") {  // We unconditionally require this in the header.
-      console.log("var " + module + " = require('" + isLoadFromFile[1] + "');");
+      console.log("var " + module + " = require('./" + isLoadFromFile[1] + "');");
     }
   } else if (!isSetTestOnly) {  // Remove goog.setTestOnly() lines.
     console.log(line);

js/commonjs/test6/test6.proto

+// Protocol Buffers - Google's data interchange format
+// Copyright 2016 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+syntax = "proto3";
+
+option java_package = "com.google.apps.jspb.proto";
+option java_multiple_files = true;
+
+package jspb.test.importing;
+
+message ImportedMessage {
+  string string_value = 1;
+}

js/commonjs/test7/test7.proto

+// Protocol Buffers - Google's data interchange format
+// Copyright 2016 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+syntax = "proto3";
+
+option java_package = "com.google.apps.jspb.proto";
+option java_multiple_files = true;
+
+package jspb.test.framing;
+
+import "test6/test6.proto";
+
+message FramingMessage {
+  jspb.test.importing.ImportedMessage imported_message = 1;
+}

js/gulpfile.js

@@ -5,7 +5,7 @@ var glob = require('glob');
 var protoc = process.env.PROTOC || '../src/protoc';
 
 gulp.task('genproto_closure', function (cb) {
-  exec(protoc + ' --js_out=library=testproto_libs,binary:. -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
+  exec(protoc + ' --js_out=library=testproto_libs,binary:.  -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -14,7 +14,7 @@ gulp.task('genproto_closure', function (cb) {
 });
 
 gulp.task('genproto_commonjs', function (cb) {
-  exec('mkdir -p commonjs_out && ' + protoc + ' --js_out=import_style=commonjs,binary:commonjs_out -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
+  exec('mkdir -p commonjs_out && ' + protoc + ' --js_out=import_style=commonjs,binary:commonjs_out -I ../src -I commonjs -I . *.proto commonjs/test*/*.proto ../src/google/protobuf/descriptor.proto',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -34,7 +34,7 @@ gulp.task('dist', function (cb) {
 });
 
 gulp.task('commonjs_asserts', function (cb) {
-  exec('mkdir -p commonjs_out && ./node_modules/google-closure-library/closure/bin/calcdeps.py -i commonjs/export_asserts.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > commonjs_out/closure_asserts_commonjs.js',
+  exec('mkdir -p commonjs_out/test_node_modules && ./node_modules/google-closure-library/closure/bin/calcdeps.py -i commonjs/export_asserts.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > commonjs_out/test_node_modules/closure_asserts_commonjs.js',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -45,7 +45,7 @@ gulp.task('commonjs_asserts', function (cb) {
 gulp.task('make_commonjs_out', ['dist', 'genproto_commonjs', 'commonjs_asserts'], function (cb) {
   // TODO(haberman): minify this more aggressively.
   // Will require proper externs/exports.
-  var cmd = "mkdir -p commonjs_out/binary && ";
+  var cmd = "mkdir -p commonjs_out/binary && mkdir -p commonjs_out/test_node_modules && ";
   function addTestFile(file) {
     cmd += 'node commonjs/rewrite_tests_for_commonjs.js < ' + file +
            ' > commonjs_out/' + file + '&& ';
@@ -56,7 +56,8 @@ gulp.task('make_commonjs_out', ['dist', 'genproto_commonjs', 'commonjs_asserts']
 
   exec(cmd +
        'cp commonjs/jasmine.json commonjs_out/jasmine.json && ' +
-       'cp google-protobuf.js commonjs_out',
+       'cp google-protobuf.js commonjs_out/test_node_modules && ' +
+       'cp commonjs/import_test.js commonjs_out/import_test.js',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -83,7 +84,7 @@ gulp.task('test_closure', ['genproto_closure', 'deps'], function (cb) {
 });
 
 gulp.task('test_commonjs', ['make_commonjs_out'], function (cb) {
-  exec('cd commonjs_out && JASMINE_CONFIG_PATH=jasmine.json NODE_PATH=. ../node_modules/.bin/jasmine',
+  exec('cd commonjs_out && JASMINE_CONFIG_PATH=jasmine.json NODE_PATH=test_node_modules ../node_modules/.bin/jasmine',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);

js/test.proto

@@ -216,4 +216,3 @@ message TestMessageWithOneof {
     int32 btwo = 13 [default = 1234];
   }
 }
-

src/google/protobuf/compiler/js/js_generator.cc

@@ -149,6 +149,20 @@ string GetJSFilename(const string& filename) {
   return StripSuffixString(filename, suffix) + "_pb.js";
 }
 
+// Given a filename like foo/bar/baz.proto, returns the root directory
+// path ../../
+string GetRootPath(const string& filename) {
+  size_t slashes = std::count(filename.begin(), filename.end(), '/');
+  if (slashes == 0) {
+    return "./";
+  }
+  string result = "";
+  for (size_t i = 0; i < slashes; i++) {
+    result += "../";
+  }
+  return result;
+}
+
 // Returns the alias we assign to the module of the given .proto filename
 // when importing.
 string ModuleAlias(const string& filename) {
@@ -2518,7 +2532,7 @@ void Generator::GenerateFile(const GeneratorOptions& options,
       printer->Print(
           "var $alias$ = require('$file$');\n",
           "alias", ModuleAlias(name),
-          "file", GetJSFilename(name));
+          "file", GetRootPath(file->name()) + GetJSFilename(name));
     }
   }