From 822f8fa2da7f6b41bde6ec6d23283fcfec55ec21 Mon Sep 17 00:00:00 2001 From: ekatek Date: Wed, 25 Jun 2014 23:36:03 -0700 Subject: [PATCH] more error handling and reporting on package changes & downloads --- tools/bundler.js | 9 +++- tools/commands-packages.js | 103 ++++++++----------------------------- tools/project.js | 103 ++++++++++++++++++++++++++++++++++--- 3 files changed, 126 insertions(+), 89 deletions(-) diff --git a/tools/bundler.js b/tools/bundler.js index 202ffe67aa..c468032f11 100644 --- a/tools/bundler.js +++ b/tools/bundler.js @@ -1657,9 +1657,16 @@ exports.bundle = function (options) { var appDir = project.project.rootDir; var packageLoader = project.project.getPackageLoader(); - project.project._ensurePackagesExistOnDisk( + var downloaded = project.project._ensurePackagesExistOnDisk( project.project.dependencies, arch); + if (_.keys(downloaded).length !== + _.keys(project.project.dependencies).length) { + buildmessage.error("Unable to download package builds for this architecture."); + // Recover by returning. + return; + } + var releaseName = release.current.isCheckout() ? "none" : release.current.name; var builtBy = "Meteor" + (release.current.name ? diff --git a/tools/commands-packages.js b/tools/commands-packages.js index 5d6406400e..349947343b 100644 --- a/tools/commands-packages.js +++ b/tools/commands-packages.js @@ -53,7 +53,7 @@ var checkAuthorizedPackageMaintainer = function (record, action) { if (authorized == -1) { process.stderr.write('You are not an authorized maintainer of ' + record.name + ".\n"); process.stderr.write('Only authorized maintainers may ' + action + ".\n"); - process.exit(1); + return 1;; } }; @@ -85,73 +85,6 @@ var formatList = function (items) { }; -var showPackageChanges = function (versions, newVersions, options) { - // options.skipPackages - // options.ondiskPackages - - // Don't tell the user what all the operations were until we finish -- we - // don't want to give a false sense of completeness until everything is - // written to disk. - var messageLog = []; - var failed = false; - - // Remove the versions that don't exist - var removed = _.difference(_.keys(versions), _.keys(newVersions)); - _.each(removed, function(packageName) { - messageLog.push("removed dependency on " + packageName); - }); - - _.each(newVersions, function(version, packageName) { - if (failed) - return; - - if (_.has(versions, packageName) && - versions[packageName] === version) { - // Nothing changed. Skip this. - return; - } - - if (options.onDiskPackages && - (! options.onDiskPackages[packageName] || - options.onDiskPackages[packageName] !== version)) { - // XXX maybe we shouldn't be letting the constraint solver choose - // things that don't have the right arches? - process.stderr.write("Package " + packageName + - " has no compatible build for version " + - version + "\n"); - failed = true; - return; - } - - // Add a message to the update logs to show the user what we have done. - if ( _.contains(options.skipPackages, packageName)) { - // If we asked for this, we will log it later in more detail. - return; - } - - // If the previous versions file had this, then we are upgrading, if it did - // not, then we must be adding this package anew. - if (_.has(versions, packageName)) { - messageLog.push(" upgraded " + packageName + " from version " + - versions[packageName] + - " to version " + newVersions[packageName]); - } else { - messageLog.push(" added " + packageName + - " at version " + newVersions[packageName]); - }; - }); - - if (failed) - return 1; - - // Show the user the messageLog of packages we added. - _.each(messageLog, function (msg) { - process.stdout.write(msg + "\n"); - }); - return 0; -}; - - /////////////////////////////////////////////////////////////////////////////// // publish a package /////////////////////////////////////////////////////////////////////////////// @@ -685,7 +618,7 @@ main.registerCommand({ // If we fail to publish, just exit outright, something has gone wrong. if (pub > 0) { process.stderr.write("Failed to publish: " + name + "\n"); - process.exit(1); + return 1;; } }); @@ -805,7 +738,7 @@ main.registerCommand({ var lastVersion = versionRecords[versionRecords.length - 1]; if (!lastVersion && full.length > 1) { console.log("Unknown version of", name, ":", full[1]); - process.exit(1); + return 1;; } var unknown = "< unknown >"; _.each(versionRecords, function (v) { @@ -971,7 +904,7 @@ main.registerCommand({ // Some basic checks to make sure that this command is being used correctly. if (options["packages-only"] && options["patch"]) { process.stderr.write("There is no such thing as a patch update to packages."); - process.exit(1); + return 1;; } if (!options["packages-only"]) { @@ -1094,7 +1027,7 @@ main.registerCommand({ if (appRelease == null) { console.log( "Cannot patch update unless a release is set."); - process.exit(1); + return 1;; } var r = appRelease.split('@'); var record = catalog.official.getReleaseVersion(r[0], r[1]); @@ -1102,7 +1035,7 @@ main.registerCommand({ if (!updateTo) { console.log( "You are at the latest patch version."); - process.exit(1); + return 1; } releaseVersionsToTry = [updateTo]; } else if (release.forced) { @@ -1168,11 +1101,15 @@ main.registerCommand({ var upgraders = require('./upgraders.js'); var upgradersToRun = upgraders.upgradersToRun(); - // XXX did we have to change some package versions? we should probably - // mention that fact. - // Write the new versions to .meteor/packages and .meteor/versions. - project.setVersions(solutionPackageVersions); + var setV = project.setVersions(solutionPackageVersions); + self.showPackageChanges(previousVersions, newVersions, { + ondiskPackages: setV.downloaded + }); + if (!setV.success) { + process.stderr.write("Could not install all the requested packages."); + return 1; + } // Write the release to .meteor/release. project.writeMeteorReleaseVersion(solutionReleaseName); @@ -1228,12 +1165,12 @@ main.registerCommand({ } // Set our versions and download the new packages. - var downloaded = project.setVersions(newVersions, { + setV = project.setVersions(newVersions, { alwaysRecord : true }); // Display changes: what we have added/removed/upgraded. - showPackageChanges(versions, newVersions, { - ondiskPackages: downloaded}); + return project.showPackageChanges(versions, newVersions, { + ondiskPackages: setV.downloaded}); } return 0; }); @@ -1366,9 +1303,10 @@ main.registerCommand({ // reality. var downloaded = project.addPackages(constraints, newVersions); - showPackageChanges(versions, newVersions, { + var ret = project.showPackageChanges(versions, newVersions, { skipPackages: constraints, ondiskPackages: downloaded}); + if (ret !== 0) return ret; // Show the user the messageLog of the packages that they installed. process.stdout.write("Successfully added the following packages.\n"); @@ -1435,8 +1373,9 @@ main.registerCommand({ var newVersions = project.getVersions(); // Show what we did. (We removed some things) - showPackageChanges(versions, newVersions, { + var ret = project.showPackageChanges(versions, newVersions, { skipPackages: options.args }); + if (ret !== 0) return ret; // Log that we removed the constraints. It is possible that there are // constraints that we officially removed that the project still 'depends' on, diff --git a/tools/project.js b/tools/project.js index e958954522..7e6e0d5c95 100644 --- a/tools/project.js +++ b/tools/project.js @@ -167,17 +167,25 @@ _.extend(Project.prototype, { self.calculateCombinedConstraints(releasePackages); // Call the constraint solver, using the previous dependencies as the last - // solution. Remember to check 'ignoreProjectDeps', otherwise it will just - // try to look up the solution in our own dependencies and it will be a - // disaster. + // solution. It is useful to set ignoreProjectDeps, but not nessessary, + // since self.viableDepSource is false. var newVersions = catalog.complete.resolveConstraints( self.combinedConstraints, - { previousSolution: self.dependencies } + { previousSolution: self.dependencies }, + { ignoreProjectDeps: true } ); // Download packages to disk, and rewrite .meteor/versions if it has // changed. - self.setVersions(newVersions); + var oldVersions = self.versions; + var setV = self.setVersions(newVersions); + self.showPackageChanges(oldVersions, newVersions, { + ondiskPackages: setV.downloaded + }); + + if (!setV.success) { + throw new Error ("Could not install all the requested packages."); + } // Finally, initialize the package loader. self.packageLoader = new packageLoader.PackageLoader({ @@ -260,6 +268,75 @@ _.extend(Project.prototype, { return allDeps; }, + // Print out the changest hat we have made in the versions files. + // + // return 0 if everything went well, or 1 if we failed in some way. + showPackageChanges : function (versions, newVersions, options) { + // options.skipPackages + // options.ondiskPackages + + // Don't tell the user what all the operations were until we finish -- we + // don't want to give a false sense of completeness until everything is + // written to disk. + var messageLog = []; + var failed = false; + + // Remove the versions that don't exist + var removed = _.difference(_.keys(versions), _.keys(newVersions)); + _.each(removed, function(packageName) { + messageLog.push("removed dependency on " + packageName); + }); + + _.each(newVersions, function(version, packageName) { + if (failed) + return; + + if (_.has(versions, packageName) && + versions[packageName] === version) { + // Nothing changed. Skip this. + return; + } + + if (options.onDiskPackages && + (! options.onDiskPackages[packageName] || + options.onDiskPackages[packageName] !== version)) { + // XXX maybe we shouldn't be letting the constraint solver choose + // things that don't have the right arches? + process.stderr.write("Package " + packageName + + " has no compatible build for version " + + version + "\n"); + failed = true; + return; + } + + // Add a message to the update logs to show the user what we have done. + if ( _.contains(options.skipPackages, packageName)) { + // If we asked for this, we will log it later in more detail. + return; + } + + // If the previous versions file had this, then we are upgrading, if it did + // not, then we must be adding this package anew. + if (_.has(versions, packageName)) { + messageLog.push(" upgraded " + packageName + " from version " + + versions[packageName] + + " to version " + newVersions[packageName]); + } else { + messageLog.push(" added " + packageName + + " at version " + newVersions[packageName]); + }; + }); + + if (failed) + return 1; + + // Show the user the messageLog of packages we added. + _.each(messageLog, function (msg) { + process.stdout.write(msg + "\n"); + }); + return 0; + }, + // Accessor methods dealing with programs. // Gets the program directory for this project, as derived from the root @@ -482,9 +559,23 @@ _.extend(Project.prototype, { // // options: // alwaysRecord: record the versions file, even when we aren't supposed to. + // + // returns: + // success: true/false + // downloaded: package:version of packages that we have downloaded setVersions: function (newVersions, options) { var self = this; var downloaded = self._ensurePackagesExistOnDisk(newVersions); + var ret = { + success: true, + downloaded: downloaded + }; + + // We have failed to download the packages successfully! That's bad. + if (_.keys(downloaded).length !== _.keys(newVersions).length) { + ret.success = false; + return ret; + } // Skip the disk IO if the versions haven't changed. if (!_.isEqual(newVersions, self.dependencies)) { @@ -492,7 +583,7 @@ _.extend(Project.prototype, { self._recordVersions(options); } - return downloaded; + return ret; }, // Recalculates the project dependencies if needed and records them to disk.