From 3069788e018c22ae8f873eb0f8baac945519eecc Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Thu, 2 Oct 2014 17:06:04 -0700 Subject: [PATCH] Rework --mobile-server defaults. * --port now requires a port ('meteor run --port example.com' isn't valid). * --mobile-server defaults to your detected IP address and the port from --port. * If you provide a value for --mobile-server, we default to http:// as the protocol. A host is required for --mobile-server if you don't omit the option entirely. Similar for the --server argument to 'meteor build'. This commit includes the 'netroute' npm module as a core package (which has binary dependencies) for IP detection. It would be nice to put it in packages/non-core, but I think it has to be a core package in order to uniload it. --- History.md | 5 ++ packages/netroute/.gitignore | 1 + packages/netroute/.npm/package/.gitignore | 1 + packages/netroute/.npm/package/README | 7 ++ .../netroute/.npm/package/npm-shrinkwrap.json | 15 ++++ packages/netroute/package.js | 15 ++++ packages/netroute/wrapper.js | 1 + tools/commands-cordova.js | 13 +-- tools/commands.js | 86 ++++++++---------- tools/tests/cordova-builds.js | 27 ++++-- tools/tests/run.js | 18 ++++ tools/tests/utils-tests.js | 47 ++++++++++ tools/uniload.js | 1 + tools/utils.js | 90 +++++++++++++++++-- 14 files changed, 259 insertions(+), 68 deletions(-) create mode 100644 packages/netroute/.gitignore create mode 100644 packages/netroute/.npm/package/.gitignore create mode 100644 packages/netroute/.npm/package/README create mode 100644 packages/netroute/.npm/package/npm-shrinkwrap.json create mode 100644 packages/netroute/package.js create mode 100644 packages/netroute/wrapper.js diff --git a/History.md b/History.md index d806fdef3f..c4d482e6b8 100644 --- a/History.md +++ b/History.md @@ -17,6 +17,11 @@ * Fix source maps when using a ROOT_URL with a path. #2627 +* --port now requires a port (e.g. `meteor run --port example.com` is + not valid). XXX --mobile-port deprecated in favor of --mobile-server + option for 'meteor run' and '--server' for 'meteor build'. --server + is required for meteor build. describe defaults for --mobile-server. + ## v0.9.3.1 diff --git a/packages/netroute/.gitignore b/packages/netroute/.gitignore new file mode 100644 index 0000000000..677a6fc263 --- /dev/null +++ b/packages/netroute/.gitignore @@ -0,0 +1 @@ +.build* diff --git a/packages/netroute/.npm/package/.gitignore b/packages/netroute/.npm/package/.gitignore new file mode 100644 index 0000000000..3c3629e647 --- /dev/null +++ b/packages/netroute/.npm/package/.gitignore @@ -0,0 +1 @@ +node_modules diff --git a/packages/netroute/.npm/package/README b/packages/netroute/.npm/package/README new file mode 100644 index 0000000000..3d492553a4 --- /dev/null +++ b/packages/netroute/.npm/package/README @@ -0,0 +1,7 @@ +This directory and the files immediately inside it are automatically generated +when you change this package's NPM dependencies. Commit the files in this +directory (npm-shrinkwrap.json, .gitignore, and this README) to source control +so that others run the same versions of sub-dependencies. + +You should NOT check in the node_modules directory that Meteor automatically +creates; if you are using git, the .gitignore file tells git to ignore it. diff --git a/packages/netroute/.npm/package/npm-shrinkwrap.json b/packages/netroute/.npm/package/npm-shrinkwrap.json new file mode 100644 index 0000000000..07ef3acb2b --- /dev/null +++ b/packages/netroute/.npm/package/npm-shrinkwrap.json @@ -0,0 +1,15 @@ +{ + "dependencies": { + "netroute": { + "version": "0.2.5", + "dependencies": { + "bindings": { + "version": "1.0.0" + }, + "nan": { + "version": "1.3.0" + } + } + } + } +} diff --git a/packages/netroute/package.js b/packages/netroute/package.js new file mode 100644 index 0000000000..33108ad690 --- /dev/null +++ b/packages/netroute/package.js @@ -0,0 +1,15 @@ +// This is a core package instead of in packages/non-core because it +// needs to be uniloaded from tool. +Package.describe({ + summary: "Wrapper for npm netroute module", + version: "0.2.5" +}); + +Npm.depends({ + netroute: "0.2.5" +}); + +Package.on_use(function (api) { + api.export("NpmModuleNetroute", "server"); + api.addFiles("wrapper.js", "server"); +}); diff --git a/packages/netroute/wrapper.js b/packages/netroute/wrapper.js new file mode 100644 index 0000000000..e1568ef594 --- /dev/null +++ b/packages/netroute/wrapper.js @@ -0,0 +1 @@ +NpmModuleNetroute = Npm.require("netroute"); diff --git a/tools/commands-cordova.js b/tools/commands-cordova.js index eac22ea3ac..c9cc2f2034 100644 --- a/tools/commands-cordova.js +++ b/tools/commands-cordova.js @@ -265,13 +265,17 @@ var generateCordovaBoilerplate = function (clientDir, options) { // XXX partially copied from autoupdate package var version = process.env.AUTOUPDATE_VERSION || calculatedHash; + var mobileServer = options.protocol + options.host; + if (options.port) { + mobileServer = mobileServer + ":" + options.port; + } + var runtimeConfig = { meteorRelease: meteorRelease, - ROOT_URL: options.protocol + options.host + ':' + options.port + '/', + ROOT_URL: mobileServer + "/", // XXX propagate it from options? ROOT_URL_PATH_PREFIX: '', - DDP_DEFAULT_CONNECTION_URL: options.protocol + - options.host + ':' + options.port, + DDP_DEFAULT_CONNECTION_URL: mobileServer, autoupdateVersionCordova: version, cleanCache: options.clean, httpProxyPort: options.httpProxyPort, @@ -1228,7 +1232,7 @@ var consumeControlFile = function (controlFilePath, cordovaPath) { * @param {Object} launchScreens A dictionary where keys are different * devices, screen sizes, and orientations, and the values are image paths * relative to the project root directory. - * + * * For Android, launch screen images should * be special "Nine-patch" image files that specify how they should be * stretched. See the [Android docs](https://developer.android.com/guide/topics/graphics/2d-graphics.html#nine-patch). @@ -2105,4 +2109,3 @@ main.registerCommand({ return 0; }); - diff --git a/tools/commands.js b/tools/commands.js index e2fa107544..808b59a392 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -2,7 +2,6 @@ var main = require('./main.js'); var path = require('path'); var _ = require('underscore'); var fs = require('fs'); -var ip = require('ip'); var files = require('./files.js'); var deploy = require('./deploy.js'); var buildmessage = require('./buildmessage.js'); @@ -31,11 +30,6 @@ var Console = require('./console.js').Console; // by 'meteor deploy'. var DEPLOY_ARCH = 'os.linux.x86_64'; -// The default host to use when building apps. (Specifically, for mobile -// builds, we need a host to use for DDP_DEFAULT_CONNECTION_URL if the -// user doesn't specify one with -p or --mobile-server). -var DEFAULT_BUILD_HOST = "localhost"; - // The default port that the development server listens on. var DEFAULT_PORT = 3000; @@ -192,9 +186,7 @@ function doRunCommand (options) { cordova.verboseLog('Parsing the --port option'); try { - var parsedUrl = utils.parseUrl(options.port, { - port: DEFAULT_PORT - }); + var parsedUrl = utils.parseUrl(options.port); } catch (err) { if (options.verbose) { Console.stderr.write('Error while parsing --port option: ' @@ -205,24 +197,19 @@ function doRunCommand (options) { return 1; } - // XXX COMPAT WITH 0.9.2.2 -- the 'mobile-port' option is deprecated - var mobileServer = options["mobile-server"] || - options["mobile-port"] || options.port; + if (! parsedUrl.port) { + Console.stderr.write("--port must include a port.\n"); + return 1; + } + try { - var parsedMobileServer = utils.parseUrl( - mobileServer, - { - host: DEFAULT_BUILD_HOST, - port: DEFAULT_PORT, - protocol: "http://" - } - ); + var parsedMobileServer = utils.mobileServerForRun(options); } catch (err) { if (options.verbose) { - process.stderr.write('Error while parsing --mobile-port option: ' + Console.stderr.write('Error while parsing --mobile-server option: ' + err.stack + '\n'); } else { - process.stderr.write(err.message + '\n'); + Console.stderr.write(err.message + '\n'); } return 1; } @@ -271,17 +258,21 @@ function doRunCommand (options) { // If we are targeting the remote devices, warn about ports and same network if (_.intersection(options.args, ['ios-device', 'android-device']).length) { cordova.verboseLog('A run on a device requested'); - Console.stderr.write([ + var warning = [ "WARNING: You are testing your app on a remote device.", " For the mobile app to be able to connect to the local server, make", " sure your device is on the same network, and that the network", " configuration allows clients to talk to each other", -" (no client isolation).", -"", +" (no client isolation)."]; + + if (! options["mobile-server"]) { + warning = warning.concat(["", " You can pass the host and the port in the --mobile-server argument.", " For example: --mobile-server " + - ip.address() + ":" + parsedUrl.port + "\n\n" -].join("\n")); + utils.ipAddress() + ":" + parsedUrl.port + "\n\n"]); + } + + Console.stderr.write(warning.join("\n")); } @@ -659,28 +650,26 @@ var buildCommand = function (options) { if (! _.isEmpty(mobilePlatforms)) { - // XXX COMPAT WITH 0.9.2.2 -- the mobile-port is deprecated - var mobileServer = options.server || - options["mobile-port"]; + // XXX COMPAT WITH 0.9.2.2 -- the --mobile-port option is deprecated + var mobileServer = options.server || options["mobile-port"]; if (mobileServer) { try { var parsedMobileServer = utils.parseUrl( - mobileServer, - { - host: DEFAULT_BUILD_HOST, - port: DEFAULT_PORT, - protocol: "http://" - } - ); + mobileServer, { protocol: "http://" }); } catch (err) { Console.stderr.write(err.message); return 1; } + + if (! parsedMobileServer.host) { + Console.stderr.write("--server must include a hostname.\n"); + return 1; + } } else { // For Cordova builds, require '--server'. // XXX better error message? - process.stderr.write( + Console.stderr.write( "Supply the server hostname and port in the --server option\n" + "for mobile app builds.\n"); return 1; @@ -1146,8 +1135,7 @@ main.registerCommand({ name: 'test-packages', maxArgs: Infinity, options: { - port: { type: String, short: "p", default: - DEFAULT_BUILD_HOST + ":" + DEFAULT_PORT }, + port: { type: String, short: "p", default: DEFAULT_PORT }, 'http-proxy-port': { type: String }, 'mobile-server': { type: String }, // XXX COMPAT WITH 0.9.2.2 @@ -1183,23 +1171,19 @@ main.registerCommand({ } }, function (options) { try { - var parsedUrl = utils.parseUrl( - options.port, { port: DEFAULT_PORT }); + var parsedUrl = utils.parseUrl(options.port); } catch (err) { Console.stderr.write(err.message); return 1; } + if (! parsedUrl.port) { + Console.stderr.write("--port must include a port.\n"); + return 1; + } + try { - var parsedMobileServer = utils.parseUrl( - // XXX COMPAT WITH 0.9.2.2 - options["mobile-server"] || options["mobile-port"] || options.port, - { - host: DEFAULT_BUILD_HOST, - port: DEFAULT_PORT, - protocol: "http://" - } - ); + var parsedMobileServer = utils.mobileServerForRun(options); } catch (err) { Console.stderr.write(err.message); return 1; diff --git a/tools/tests/cordova-builds.js b/tools/tests/cordova-builds.js index 3f68dc3ea5..2d9210e027 100644 --- a/tools/tests/cordova-builds.js +++ b/tools/tests/cordova-builds.js @@ -6,14 +6,16 @@ var files = require('../files.js'); var selftest = require('../selftest.js'); var Sandbox = selftest.Sandbox; -var checkMobileServer = function (s, expected) { +var checkMobileServer = selftest.markStack(function (s, expected) { var output = s.read("android/assets/www/application/index.html"); if (! output.match(new RegExp( '"DDP_DEFAULT_CONNECTION_URL":"' + expected + '"'))) { selftest.fail( - "Wrong DDP_DEFAULT_CONNECTION_URL; expected " + expected); + "Wrong DDP_DEFAULT_CONNECTION_URL; expected " + expected + ".\n" + + "Application index.html:\n" + + output); } -}; +}); var cleanUpBuild = function (s) { files.rm_recursive(path.join(s.cwd, "android")); @@ -41,12 +43,11 @@ selftest.define("cordova builds with server options", ["slow"], function () { run = s.run("build", ".", "--server", "5000"); run.waitSecs(90); - run.expectExit(0); - checkMobileServer(s, "http://localhost:5000"); - cleanUpBuild(s); + run.matchErr("--server must include a hostname"); + run.expectExit(1); run = s.run("build", ".", "--server", "https://example.com:5000"); - run.waitSecs(90); + run.waitSecs(300); run.expectExit(0); checkMobileServer(s, "https://example.com:5000"); cleanUpBuild(s); @@ -57,6 +58,18 @@ selftest.define("cordova builds with server options", ["slow"], function () { checkMobileServer(s, "http://example.com:5000"); cleanUpBuild(s); + run = s.run("build", ".", "--server", "example.com"); + run.waitSecs(90); + run.expectExit(0); + checkMobileServer(s, "http://example.com"); + cleanUpBuild(s); + + run = s.run("build", ".", "--server", "https://example.com"); + run.waitSecs(90); + run.expectExit(0); + checkMobileServer(s, "https://example.com"); + cleanUpBuild(s); + // XXX COMPAT WITH 0.9.2.2 run = s.run("build", ".", "--mobile-port", "example.com:5000"); run.waitSecs(90); diff --git a/tools/tests/run.js b/tools/tests/run.js index 4571c2bb24..a37370c761 100644 --- a/tools/tests/run.js +++ b/tools/tests/run.js @@ -390,3 +390,21 @@ selftest.define("run and SIGKILL parent process", function () { run.match("Your application is crashing"); run.stop(); }); + +selftest.define("'meteor run --port' requires a port", function () { + var s = new Sandbox(); + var run; + + s.createApp("myapp", "app-prints-pid"); + s.cd("myapp"); + + run = s.run("run", "--port", "example.com"); + run.waitSecs(30); + run.matchErr("--port must include a port"); + run.expectExit(1); + + run = s.run("run", "--port", "http://example.com"); + run.waitSecs(30); + run.matchErr("--port must include a port"); + run.expectExit(1); +}); diff --git a/tools/tests/utils-tests.js b/tools/tests/utils-tests.js index a4e4be674b..fa58768467 100644 --- a/tools/tests/utils-tests.js +++ b/tools/tests/utils-tests.js @@ -125,3 +125,50 @@ selftest.define("parse url", function () { protocol: "https://" }); }); + +selftest.define('get mobile server argument for meteor run', function () { + // meteor run -p 3000 => mobile server should be :3000 + selftest.expectEqual(utils.mobileServerForRun({ + port: "3000" + }), { host: utils.ipAddress(), port: "3000", protocol: "http://" }); + + // meteor run -p example.com:3000 => mobile server should be :3000 + selftest.expectEqual(utils.mobileServerForRun({ + port: "example.com:3000" + }), { host: utils.ipAddress(), port: "3000", protocol: "http://" }); + + // meteor run -p example.com:3000 --mobile-server 4000 => error, mobile + // server must specify a hostname + var error; + try { + utils.mobileServerForRun({ + port: "example.com:3000", + "mobile-server": "4000" + }); + } catch (e) { + error = e; + } + selftest.expectEqual( + error && error.message, "--mobile-server must specify a hostname."); + + // meteor run -p example.com:3000 --mobile-server example.com => + // mobile server should be example.com + selftest.expectEqual(utils.mobileServerForRun({ + port: "example.com:3000", + "mobile-server": "example.com" + }), { protocol: "http://", host: "example.com", port: undefined }); + + // meteor run -p example.com:3000 --mobile-server https://example.com => + // mobile server should be https://example.com + selftest.expectEqual(utils.mobileServerForRun({ + port: "example.com:3000", + "mobile-server": "https://example.com" + }), { host: "example.com", protocol: "https://", port: undefined }); + + // meteor run -p example.com:3000 --mobile-server http://example.com:4000 => + // mobile server should be http://example.com:4000 + selftest.expectEqual(utils.mobileServerForRun({ + port: "example.com:3000", + "mobile-server": "http://example.com:4000" + }), { host: "example.com", port: "4000", protocol: "http://" }); +}); diff --git a/tools/uniload.js b/tools/uniload.js index 303ac47d67..703b3a8ec6 100644 --- a/tools/uniload.js +++ b/tools/uniload.js @@ -20,6 +20,7 @@ var ROOT_PACKAGES = [ 'minifiers', 'minimongo', 'mongo', + 'netroute', 'package-version-parser', 'boilerplate-generator', 'webapp-hashing', diff --git a/tools/utils.js b/tools/utils.js index b63497fe81..e8abe731d7 100644 --- a/tools/utils.js +++ b/tools/utils.js @@ -13,10 +13,6 @@ var child_process = require('child_process'); var utils = exports; -exports.hasScheme = function (str) { - return !! str.match(/^[A-Za-z][A-Za-z0-9+-\.]*\:\/\//); -}; - // Parses ://: into an object { protocol: *, host: // *, port: * }. The input can also be of the form : or just // . We're not simply using 'url.parse' because we want '3000' to @@ -25,7 +21,7 @@ exports.hasScheme = function (str) { // undefined} or something like that. // // 'defaults' is an optional object with 'host', 'port', and 'protocol' keys. -exports.parseUrl = function (str, defaults) { +var parseUrl = function (str, defaults) { // XXX factor this out into a {type: host/port}? defaults = defaults || {}; @@ -57,6 +53,50 @@ exports.parseUrl = function (str, defaults) { }; }; +var ipAddress = function () { + var uniload = require("./uniload.js"); + var netroute = uniload.load({ packages: ["netroute"] }). + netroute.NpmModuleNetroute; + var info = netroute.getInfo(); + var defaultRoute = _.findWhere(info.IPv4 || [], { destination: "0.0.0.0" }); + if (! defaultRoute) { + return null; + } + + var iface = defaultRoute["interface"]; + + var getAddress = function (iface) { + var interfaces = os.networkInterfaces(); + return _.findWhere(interfaces[iface], { family: "IPv4" }); + }; + + var address = getAddress(iface); + if (! address) { + // Retry after a couple seconds in case the user is connecting or + // disconnecting from the Internet. + utils.sleepMs(2000); + address = getAddress(iface); + if (! address) { + throw new Error( +"Interface '" + iface + "' not found in interface list, or\n" + +"does not have an IPv4 address."); + } + } + return address.address; +}; + +exports.hasScheme = function (str) { + return !! str.match(/^[A-Za-z][A-Za-z0-9+-\.]*\:\/\//); +}; + +exports.parseUrl = parseUrl; + +exports.ipAddress = ipAddress; + +exports.hasScheme = function (str) { + return !! str.match(/^[A-Za-z][A-Za-z0-9+-\.]*\:\/\//); +}; + // Returns a pretty list suitable for showing to the user. Input is an // array of objects with keys 'name' and 'description'. exports.formatList = function (unsortedItems) { @@ -650,5 +690,45 @@ _.extend(exports.Patience.prototype, { } }); +// Given the options for a 'meteor run' command, returns a parsed URL ({ +// host: *, protocol: *, port: * }. The rules for --mobile-server are: +// * If you don't specify anything for --mobile-server, then it +// defaults to :. +// * If you specify something for --mobile-server, we use that, +// defaulting to http:// as the protocol and 80 or 443 as the port. +exports.mobileServerForRun = function (options) { + var parsedUrl = parseUrl(options.port); + if (! parsedUrl.port) { + throw new Error("--port must include a port."); + } + // XXX COMPAT WITH 0.9.2.2 -- the 'mobile-port' option is deprecated + var mobileServer = options["mobile-server"] || options["mobile-port"]; + var parsedMobileServer; + if (! mobileServer) { + var myIp = ipAddress(); + if (! myIp) { + throw new Error( +"Error detecting IP address for mobile app to connect to.\n" + +"Please specify the address that the mobile app should connect\n" + +"to with --mobile-server."); + } + + parsedMobileServer = { + host: myIp, + port: parsedUrl.port, + protocol: "http://" + }; + } else { + parsedMobileServer = parseUrl(mobileServer, { + protocol: "http://" + }); + + if (! parsedMobileServer.host) { + throw new Error("--mobile-server must specify a hostname."); + } + } + + return parsedMobileServer; +};