Fix meteor add cordova:* (but not add packages)

tools/commands-cordova.js

@@ -167,16 +167,6 @@ cordova.filterPackages = function (packages) {
   return ret;
 };
 
-// used by packages commands
-cordova.checkIsValidPlugin = function (name) {
-  var pluginHash = {};
-  pluginHash[name.split('@')[0]] = name.split('@')[1];
-
-  // check that every plugin is specifying either an exact constraint or a
-  // tarball url with sha
-  utils.ensureOnlyExactVersions(pluginHash);
-};
-
 // --- helpers ---
 
 var localCordova = path.join(files.getCurrentToolsDir(),

tools/commands-packages.js

@@ -1946,40 +1946,46 @@ main.registerCommand({
   pretty: true,
   catalogRefresh: new catalog.Refresh.OnceAtStart({ ignoreErrors: true })
 }, function (options) {
+  var projectContext = new projectContextModule.ProjectContext({
+    projectDir: options.appDir
+  });
+  main.captureAndExit("=> Errors while initializing project:", function () {
+    // We're just reading metadata here --- we're not going to resolve
+    // constraints until after we've made our changes.
+    projectContext.readProjectMetadata();
+  });
 
-  // Special case on reserved package namespaces, such as 'cordova'
-  var cordovaPlugins;
-  try {
-    var filteredPackages = cordova.filterPackages(options.args);
-    cordovaPlugins = filteredPackages.plugins;
+  var exitCode = 0;
 
-    _.each(cordovaPlugins, function (plugin) {
-      cordova.checkIsValidPlugin(plugin);
+  var filteredPackages = cordova.filterPackages(options.args);
+  var pluginsToAdd = filteredPackages.plugins;
+
+  if (pluginsToAdd.length) {
+    var plugins = projectContext.cordovaPluginsFile.getPluginVersions();
+    var changed = false;
+    _.each(pluginsToAdd, function (pluginSpec) {
+      var parts = pluginSpec.split('@');
+      if (parts.length !== 2) {
+        Console.error(
+          pluginSpec + ': exact version or tarball url is required');
+        exitCode = 1;
+      } else if (! utils.isExactVersion(parts[1])) {
+        Console.error(
+          "Must declare exact version of dependency: " + pluginSpec);
+        exitCode = 1;
+      } else {
+        plugins[parts[0]] = parts[1];
+        changed = true;
+      }
     });
-  } catch (err) {
-    Console.error(err.message + '');
-    return 1;
+    changed && projectContext.cordovaPluginsFile.write(plugins);
   }
 
-  var oldPlugins = project.getCordovaPlugins();
-
-  var pluginsDict = {};
-  _.each(cordovaPlugins, function (s) {
-    var splt = s.split('@');
-    if (splt.length !== 2)
-      throw new Error(s + ': exact version or tarball url is required');
-    pluginsDict[splt[0]] = splt[1];
-  });
-  project.addCordovaPlugins(pluginsDict);
-
-  _.each(cordovaPlugins, function (plugin) {
-    Console.info("added cordova plugin " + plugin);
-  });
-
   var args = filteredPackages.rest;
 
   if (_.isEmpty(args))
-    return 0;
+    return exitCode;
+  return 123
 
   // For every package name specified, add it to our list of package
   // constraints. Don't run the constraint solver until you have added all of
@@ -2207,7 +2213,6 @@ main.registerCommand({
   var projectContext = new projectContextModule.ProjectContext({
     projectDir: options.appDir
   });
-
   main.captureAndExit("=> Errors while initializing project:", function () {
     // We're just reading metadata here --- we're not going to resolve
     // constraints until after we've made our changes.
@@ -2225,7 +2230,10 @@ main.registerCommand({
     var plugins = projectContext.cordovaPluginsFile.getPluginVersions();
     var changed = false;
     _.each(pluginsToRemove, function (pluginName) {
-      if (_.has(plugins, pluginName)) {
+      if (/@/.test(pluginName)) {
+        Console.error(pluginName + ": do not specify version constraints.");
+        exitCode = 1;
+      } else if (_.has(plugins, pluginName)) {
         delete plugins[pluginName];
         Console.info("removed cordova plugin " + pluginName);
         changed = true;

tools/project.js

@@ -266,49 +266,3 @@
   //   var lines = files.getLinesOrEmpty(self._finishedUpgradersFile());
   //   return _.filter(_.map(lines, files.trimLine), _.identity);
   // },
-
-  // // Adds the passed plugins to the cordovaPlugins list. If the plugin was
-  // // already in the list, just updates it in-place.
-  // // newPlugins is an object with a mapping from the Cordova plugin identifier
-  // // to an semver string or a tarball url with a sha.
-  // addCordovaPlugins: function (newPlugins) {
-  //   var self = this;
-  //   self.cordovaPlugins = _.extend(self.cordovaPlugins, newPlugins);
-
-  //   var plugins = self._getCordovaPluginsFile();
-  //   var lines = [];
-  //   _.each(self.cordovaPlugins, function (versionString, plugin) {
-  //     if (versionString)
-  //       lines.push(plugin + '@' + versionString);
-  //     else
-  //       lines.push(plugin);
-  //   });
-  //   lines.push('\n');
-  //   fs.writeFileSync(plugins, lines.join('\n'), 'utf8');
-  // },
-
-  // // Removes the plugins from the cordova-plugins file if they existed.
-  // // pluginsToRemove - array of Cordova plugin identifiers
-  // //
-  // // Returns an array of plugin identifiers that were actually removed.
-  // removeCordovaPlugins: function (pluginsToRemove) {
-  //   var self = this;
-
-  //   var removed = _.intersection(_.keys(self.cordovaPlugins), pluginsToRemove);
-  //   self.cordovaPlugins =
-  //     _.omit.apply(null, [self.cordovaPlugins].concat(pluginsToRemove));
-
-  //   var plugins = self._getCordovaPluginsFile();
-  //   var lines = [];
-
-  //   _.each(self.cordovaPlugins, function (versionString, plugin) {
-  //     if (versionString)
-  //       lines.push(plugin + '@' + versionString);
-  //     else
-  //       lines.push(plugin);
-  //   });
-  //   lines.push('\n');
-  //   fs.writeFileSync(plugins, lines.join('\n'), 'utf8');
-
-  //   return removed;
-  // },

tools/utils.js

@@ -468,7 +468,7 @@ exports.isUrlWithSha = function (x) {
 // human-readable message that is suitable for showing to the user.
 // dependencies may be falsey or empty.
 //
-// This is talking about NPM versions specifically, not Meteor versions.
+// This is talking about NPM/Cordova versions specifically, not Meteor versions.
 // It does not support the wrap number syntax.
 exports.ensureOnlyExactVersions = function (dependencies) {
   _.each(dependencies, function (version, name) {
@@ -476,12 +476,16 @@ exports.ensureOnlyExactVersions = function (dependencies) {
     // .npm/npm-shrinkwrap.json) to pin down its dependencies precisely, so we
     // don't want anything too vague. For now, we support semvers and urls that
     // name a specific commit by SHA.
-    if (!semver.valid(version) && ! exports.isUrlWithSha(version))
+    if (! exports.isExactVersion(version)) {
       throw new Error(
-        "Must declare exact version of dependency: " +
-          name + '@' + version);
+        "Must declare exact version of dependency: " + name + '@' + version);
+    }
   });
 };
+exports.isExactVersion = function (version) {
+  return semver.valid(version) || exports.isUrlWithSha(version);
+};
+
 
 exports.execFileSync = function (file, args, opts) {
   var future = new Future;