Eliminate the process.exit in library.js

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;
     });
   }

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

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
 //
 ///////////////////////////////////////////////////////////////////////////////