From b72a7e2e83340ca23da0d24bde4f701331a3d5b4 Mon Sep 17 00:00:00 2001 From: Geoff Schmidt Date: Tue, 7 Jan 2014 23:19:32 -0800 Subject: [PATCH] Eliminate the process.exit in library.js --- tools/commands.js | 52 ++++++++++++++++++++++++++++++++++++----------- tools/library.js | 30 +++++++++++++-------------- tools/run-all.js | 4 ++-- 3 files changed, 56 insertions(+), 30 deletions(-) diff --git a/tools/commands.js b/tools/commands.js index d376cdb29d..abeb7a7f9b 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -40,6 +40,21 @@ var hostedWithGalaxy = function (site) { return !! require('./deploy-galaxy.js').discoverGalaxy(site); }; +// Get all packages available. Returns a map from the package name to +// a Package object. +// +// If problems happen while generating the list, print appropriate +// messages to stderr and return null. +var getPackages = function () { + var result = release.current.library.list(); + if (result.packages) + return result.packages; + + process.stderr.write("=> Errors while scanning packages:\n\n"); + process.stderr.write(result.messages.formatMessages()); + return null; +}; + /////////////////////////////////////////////////////////////////////////////// // options that act like commands /////////////////////////////////////////////////////////////////////////////// @@ -109,7 +124,12 @@ main.registerCommand({ if (! release.current) // This is a weird case. Fail silently. return 0; - _.each(release.current.library.list(), function (p) { + + var packages = getPackages(); + if (! packages) + return 1; // build failed + + _.each(packages, function (p) { p.preheat(); }); } @@ -486,7 +506,10 @@ main.registerCommand({ maxArgs: Infinity, requiresApp: true }, function (options) { - var all = release.current.library.list(); + var all = getPackages(); + if (! all) + return 1; + var using = {}; _.each(project.getPackages(options.appDir), function (name) { using[name] = true; @@ -536,7 +559,9 @@ main.registerCommand({ main.registerCommand({ name: 'list', - requiresApp: true, + requiresApp: function (options) { + return options.using; + }, options: { using: { type: Boolean } } @@ -559,7 +584,9 @@ main.registerCommand({ return; } - var list = release.current.library.list(); + var list = getPackages(); + if (! list) + return 1; var names = _.keys(list); names.sort(); var pkgs = []; @@ -956,15 +983,16 @@ main.registerCommand({ 'driver-package': { type: String } } }, function (options) { - var library = release.current.library; - var testPackages; if (options.args.length === 0) { - // XXX The call to list() here is unfortunate, because list() - // can fail (eg, a package has a parse error) and if it does - // we currently just exit! Which sucks because we don't get - // reloading. - testPackages = _.keys(library.list()); + var packageList = getPackages(); + if (! packageList) { + // Couldn't load the package list, probably because some package + // has a parse error. Bail out -- this kind of sucks; we would + // like to find a way to get reloading. + return 1; + } + testPackages = _.keys(packageList); } else { testPackages = _.map(options.args, function (p) { // If it's a package name, just pass it through. @@ -976,7 +1004,7 @@ main.registerCommand({ // have a trailing slash. var packageDir = path.resolve(p); var packageName = path.basename(packageDir); - library.override(packageName, packageDir); + release.current.library.override(packageName, packageDir); return packageName; }); } diff --git a/tools/library.js b/tools/library.js index 14e0a257d2..f57bd29480 100644 --- a/tools/library.js +++ b/tools/library.js @@ -322,18 +322,19 @@ _.extend(Library.prototype, { }); }, - // Get all packages available. Returns a map from the package name - // to a Package object. + // Get all packages available and their metadata. This can fail + // since it currently involves building packages. // - // XXX Hack: If errors occur while generating the list (which could - // easily happen, since it currently involves building packages) - // print them to the console and exit(1)! Certainly not ideal but is - // expedient since, eg, test-packages calls list() before it does - // anything else. + // On success, returns an object with keys: + // - packages: map from the package name to a Package object for all + // available packages + // + // On failure, returns an object with keys: + // - messages: a buildmessage.MessageSet with the errors list: function () { var self = this; var names = []; - var ret = {}; + var packages = {}; var messages = buildmessage.capture(function () { names = _.keys(self.overrides); @@ -349,17 +350,14 @@ _.extend(Library.prototype, { _.each(names, function (name) { var pkg = self.get(name, false); if (pkg) - ret[name] = pkg; + packages[name] = pkg; }); }); - if (messages.hasMessages()) { - process.stdout.write("=> Errors while scanning packages:\n\n"); - process.stdout.write(messages.formatMessages()); - process.exit(1); - } - - return ret; + if (messages.hasMessages()) + return { messages: messages }; + else + return { packages: packages }; }, // Rebuild all source packages in our search paths -- even including diff --git a/tools/run-all.js b/tools/run-all.js index 7b4344d369..884096de16 100644 --- a/tools/run-all.js +++ b/tools/run-all.js @@ -14,11 +14,11 @@ var Updater = require('./run-updater.js').Updater; /////////////////////////////////////////////////////////////////////////////// // XXX XXX NEXT (if you want to do more): // -// - add warnings to buildmessage, per slava // - make files.getSettings return errors instead of throwing (or eliminate) // - deal with XXX's in updater about it needing to go though runlog since // no more stdout redirection -// - kill process.exit everywhere +// - kill process.exit everywhere: library.js, unipackage.js, and finally +// deploy-galaxy.js // - deal with options last on command line without args being tolerated // ///////////////////////////////////////////////////////////////////////////////