From e6837b652c8b727180d244284bf5ba2bcb894f8d Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@meteor.com>
Date: Mon, 18 Mar 2013 11:53:59 -0700
Subject: [PATCH] factor out "untar atomically" code

---
 engine/files.js     | 26 +++++++++++++++++++++-----
 engine/warehouse.js | 25 ++++---------------------
 2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/engine/files.js b/engine/files.js
index cfffb29aa2..b5e1741f1a 100644
--- a/engine/files.js
+++ b/engine/files.js
@@ -365,9 +365,15 @@ var files = module.exports = {
     throw new Error("failed to make tempory directory in " + tmp_dir);
   },
 
-  // Takes a buffer containing `.tar.gz` data and extracts the archive into
-  // a destination directory.
+  // Takes a buffer containing `.tar.gz` data and extracts the archive into a
+  // destination directory. destPath should not exist yet, and the archive
+  // should contain a single top-level directory, which will be renamed
+  // atomically to destPath.
   extractTarGz: function (buffer, destPath) {
+    var parentDir = path.dirname(destPath);
+    var tempDir = path.join(parentDir, '.tmp' + files._randomToken());
+    files.mkdir_p(tempDir);
+
     var future, error;
 
     future = new Future;
@@ -380,7 +386,7 @@ var files = module.exports = {
       throw error;
 
     future = new Future;
-    var extractor = new tar.Extract({ path: destPath })
+    var extractor = new tar.Extract({ path: tempDir })
           .on('error', function (e) {
             error = e;
             future['return'](false);
@@ -396,7 +402,14 @@ var files = module.exports = {
     if (error)
       throw error;
 
-    // succeed! no return value.
+    // succeed!
+    var topLevelOfArchive = fs.readdirSync(tempDir);
+    if (topLevelOfArchive.length !== 1)
+      throw new Error(
+        "Extracted archive '" + tempDir + "' should only contain one entry");
+
+    fs.renameSync(path.join(tempDir, topLevelOfArchive[0]), destPath);
+    fs.rmdirSync(tempDir);
   },
 
   // Tar-gzips a directory, returning a stream that can then
@@ -413,6 +426,9 @@ var files = module.exports = {
     return request(urlOrOptions, function (error, response, body) {
       callback(error, body, response);
     });
-  }
+  },
 
+  _randomToken: function() {
+    return (Math.random() * 0x100000000 + 1).toString(36);
+  }
 };
diff --git a/engine/warehouse.js b/engine/warehouse.js
index 08809d35e5..01da74bb58 100644
--- a/engine/warehouse.js
+++ b/engine/warehouse.js
@@ -33,7 +33,7 @@ var PACKAGES_URLBASE = 'https://warehouse.meteor.com';
 // Like fs.symlinkSync, but creates a temporay link and renames it over the
 // file; this means it works even if the file already exists.
 var symlinkOverSync = function (linkText, file) {
-  var tmpSymlink = file + ".tmp" + warehouse._randomToken();
+  var tmpSymlink = file + ".tmp" + files._randomToken();
   fs.symlinkSync(linkText, tmpSymlink);
   fs.renameSync(tmpSymlink, file);
 };
@@ -226,13 +226,8 @@ var warehouse = module.exports = {
           url: PACKAGES_URLBASE + engineTarballPath,
           encoding: null
         }).wait();
-        var engineDir = warehouse.getEngineDir(engineVersion);
-        // use a temp dir to avoid getting a corrupt warehouse
-        var tmpEngineDir = warehouse.getEngineDir(
-          ".tmp" + warehouse._randomToken());
-        files.mkdir_p(tmpEngineDir);
-        files.extractTarGz(engineTarball, tmpEngineDir);
-        fs.renameSync(path.join(tmpEngineDir, releaseManifest.engine), engineDir);
+        files.extractTarGz(engineTarball,
+                           warehouse.getEngineDir(engineVersion));
       } catch (e) {
         if (!background)
           console.error("Failed to load engine for release " + releaseVersion);
@@ -318,15 +313,7 @@ var warehouse = module.exports = {
 
     _.each(futures, function (f) {
       var result = f.get();
-      // extract to a temporary directory and then rename, to ensure
-      // we don't end up with a corrupt warehouse
-      var tmpPackageDir = result.packageDir + ".tmp" + warehouse._randomToken();
-      files.mkdir_p(tmpPackageDir);
-      files.extractTarGz(result.buffer, tmpPackageDir);
-      // tmpPackageDir should contain precisely one entry: a directory named
-      // after the package.
-      fs.renameSync(path.join(tmpPackageDir, result.name), result.packageDir);
-      fs.rmdirSync(tmpPackageDir);
+      files.extractTarGz(result.buffer, result.packageDir);
 
       // fetch npm dependencies
       var packages = require(path.join(__dirname, "packages.js")); // load late to work around circular require
@@ -335,10 +322,6 @@ var warehouse = module.exports = {
     });
   },
 
-  _randomToken: function() {
-    return (Math.random() * 0x100000000 + 1).toString(36);
-  },
-
   _unameAndArch: function () {
     // Normalize from Node "os.arch()" to "uname -m".
     var arch = os.arch();