From a862b6b77f49527bf1f6e1da54265d82256c4bf1 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 24 Feb 2016 13:44:57 -0800 Subject: [PATCH 1/2] Fix CommonJS relative require generation, and test it --- js/binary/proto_test.js | 2 +- js/commonjs/rewrite_tests_for_commonjs.js | 5 +- js/gulpfile.js | 12 ++--- js/import_test.js | 50 +++++++++++++++++++ js/test.proto | 1 - js/test6/test6.proto | 40 +++++++++++++++ js/test7/test7.proto | 42 ++++++++++++++++ .../protobuf/compiler/js/js_generator.cc | 16 +++++- 8 files changed, 157 insertions(+), 11 deletions(-) create mode 100644 js/import_test.js create mode 100644 js/test6/test6.proto create mode 100644 js/test7/test7.proto diff --git a/js/binary/proto_test.js b/js/binary/proto_test.js index 817f8a79f..3b4aa17fe 100644 --- a/js/binary/proto_test.js +++ b/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'); diff --git a/js/commonjs/rewrite_tests_for_commonjs.js b/js/commonjs/rewrite_tests_for_commonjs.js index dc5effec5..ffa877220 100644 --- a/js/commonjs/rewrite_tests_for_commonjs.js +++ b/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); diff --git a/js/gulpfile.js b/js/gulpfile.js index b0faed06a..e8028b4b8 100644 --- a/js/gulpfile.js +++ b/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 test*/*.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 . *.proto 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,7 @@ 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', function (err, stdout, stderr) { console.log(stdout); console.log(stderr); @@ -83,7 +83,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); diff --git a/js/import_test.js b/js/import_test.js new file mode 100644 index 000000000..6655685f4 --- /dev/null +++ b/js/import_test.js @@ -0,0 +1,50 @@ +// 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/ + +goog.setTestOnly(); + +goog.require('goog.json'); +goog.require('goog.testing.asserts'); + +// CommonJS-LoadFromFile: google-protobuf jspb +goog.require('jspb.Message'); + +// CommonJS-LoadFromFile: test7/test7_pb proto.jspb.test.framing +goog.require('proto.jspb.test.framing.FramingMessage'); + +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()); + }); +}); diff --git a/js/test.proto b/js/test.proto index 3cea5f374..14418ac93 100644 --- a/js/test.proto +++ b/js/test.proto @@ -216,4 +216,3 @@ message TestMessageWithOneof { int32 btwo = 13 [default = 1234]; } } - diff --git a/js/test6/test6.proto b/js/test6/test6.proto new file mode 100644 index 000000000..60c26f8aa --- /dev/null +++ b/js/test6/test6.proto @@ -0,0 +1,40 @@ +// 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; +} \ No newline at end of file diff --git a/js/test7/test7.proto b/js/test7/test7.proto new file mode 100644 index 000000000..19d563dd9 --- /dev/null +++ b/js/test7/test7.proto @@ -0,0 +1,42 @@ +// 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; +} \ No newline at end of file diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc index 351c39660..0de7e2c6f 100755 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/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)); } } From c9f8a1b00607be071d8be9e2bdc42ffcf97442aa Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 24 Feb 2016 14:23:08 -0800 Subject: [PATCH 2/2] Moved CommonJS-specific files into commonjs directory --- js/{ => commonjs}/import_test.js | 16 +++++++++------- js/{ => commonjs}/test6/test6.proto | 2 +- js/{ => commonjs}/test7/test7.proto | 2 +- js/gulpfile.js | 7 ++++--- 4 files changed, 15 insertions(+), 12 deletions(-) rename js/{ => commonjs}/import_test.js (81%) rename js/{ => commonjs}/test6/test6.proto (99%) rename js/{ => commonjs}/test7/test7.proto (99%) diff --git a/js/import_test.js b/js/commonjs/import_test.js similarity index 81% rename from js/import_test.js rename to js/commonjs/import_test.js index 6655685f4..ffa34fea6 100644 --- a/js/import_test.js +++ b/js/commonjs/import_test.js @@ -30,16 +30,18 @@ // Test suite is written using Jasmine -- see http://jasmine.github.io/ -goog.setTestOnly(); -goog.require('goog.json'); -goog.require('goog.testing.asserts'); -// CommonJS-LoadFromFile: google-protobuf jspb -goog.require('jspb.Message'); +var googleProtobuf = require('google-protobuf'); +var asserts = require('closure_asserts_commonjs'); +var global = Function('return this')(); -// CommonJS-LoadFromFile: test7/test7_pb proto.jspb.test.framing -goog.require('proto.jspb.test.framing.FramingMessage'); +// 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() { diff --git a/js/test6/test6.proto b/js/commonjs/test6/test6.proto similarity index 99% rename from js/test6/test6.proto rename to js/commonjs/test6/test6.proto index 60c26f8aa..a060925f2 100644 --- a/js/test6/test6.proto +++ b/js/commonjs/test6/test6.proto @@ -37,4 +37,4 @@ package jspb.test.importing; message ImportedMessage { string string_value = 1; -} \ No newline at end of file +} diff --git a/js/test7/test7.proto b/js/commonjs/test7/test7.proto similarity index 99% rename from js/test7/test7.proto rename to js/commonjs/test7/test7.proto index 19d563dd9..f5574a3dd 100644 --- a/js/test7/test7.proto +++ b/js/commonjs/test7/test7.proto @@ -39,4 +39,4 @@ import "test6/test6.proto"; message FramingMessage { jspb.test.importing.ImportedMessage imported_message = 1; -} \ No newline at end of file +} diff --git a/js/gulpfile.js b/js/gulpfile.js index e8028b4b8..36fd9fda0 100644 --- a/js/gulpfile.js +++ b/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 test*/*.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 test*/*.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); @@ -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/test_node_modules', + '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);