Merge branch 'devel' into release-1.6.2

packages/oauth/oauth_server.js

@@ -1,4 +1,3 @@
-var Fiber = Npm.require('fibers');
 var url = Npm.require('url');
 
 OAuth = {};

packages/oauth/package.js

@@ -1,6 +1,6 @@
 Package.describe({
   summary: "Common code for OAuth-based services",
-  version: "1.2.2"
+  version: "1.2.3"
 });
 
 Package.onUse(function (api) {

tools/isobuild/builder.js

@@ -390,6 +390,56 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}`
     return generated;
   }
 
+  // A version of copyDirectory that works better for copying node_modules
+  // directories when symlinks are involved.
+  copyNodeModulesDirectory(options) {
+    assert.strictEqual(files.pathBasename(options.from), "node_modules");
+    assert.strictEqual(files.pathBasename(options.to), "node_modules");
+
+    if (options.symlink) {
+      // If we're going to use symlinks to speed up this copy, then we
+      // need to make sure we've reserved all directories that are not
+      // package directories, such as the node_modules directory itself,
+      // as well as node_modules/meteor and the parent directories of any
+      // scoped npm packages.
+      this._ensureAllNonPackageDirectories(
+        files.realpath(options.from),
+        options.to
+      );
+    }
+
+    // Call this._copyDirectory rather than this.copyDirectory so that the
+    // subBuilder hacks from Builder#enter won't apply a second time.
+    return this._copyDirectory(options);
+  }
+
+  _ensureAllNonPackageDirectories(absFromDir, relToDir) {
+    const dirStat = optimisticStatOrNull(absFromDir);
+    if (! (dirStat && dirStat.isDirectory())) {
+      return;
+    }
+
+    const absFromPackageJson =
+      files.pathJoin(absFromDir, "package.json");
+
+    const stat = optimisticStatOrNull(absFromPackageJson);
+    if (stat && stat.isFile()) {
+      // If the directory has a package.json file, it's a package
+      // directory, and we should not call this._ensureDirectory, so that
+      // the package directory can later be symlinked in copyDirectory.
+      return;
+    }
+
+    this._ensureDirectory(relToDir);
+
+    optimisticReaddir(absFromDir).forEach(item => {
+      this._ensureAllNonPackageDirectories(
+        files.pathJoin(absFromDir, item),
+        files.pathJoin(relToDir, item)
+      );
+    });
+  }
+
   // Recursively copy a directory and all of its contents into the
   // bundle. But if the symlink option was passed to the Builder
   // constructor, then make a symlink instead, if possible.
@@ -408,7 +458,13 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}`
   //   entries that end with a slash if it's a directory.
   // - specificFiles: just copy these paths (specified as relative to 'to').
   // - symlink: true if the directory should be symlinked instead of copying
-  copyDirectory({
+  copyDirectory(options) {
+    // TODO(benjamn) Remove this wrapper when Builder#enter is no longer
+    // implemented using ridiculous hacks.
+    return this._copyDirectory(options);
+  }
+
+  _copyDirectory({
     from, to,
     ignore,
     specificFiles,
@@ -608,21 +664,31 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}`
   // The sub-builder returned does not have all Builder methods (for
   // example, complete() wouldn't make sense) and you should not rely
   // on it being instanceof Builder.
+  //
+  // TODO(benjamn) This nonsense should be ripped out by any means
+  // necessary... whenever someone has the time.
   enter(relPath) {
-    const methods = ["write", "writeJson", "reserve", "generateFilename",
-                   "copyDirectory", "enter"];
     const subBuilder = {};
     const relPathWithSep = relPath + files.pathSep;
+    const methods = [
+      "write",
+      "writeJson",
+      "reserve",
+      "generateFilename",
+      "copyDirectory",
+      "copyNodeModulesDirectory",
+      "enter",
+    ];
 
     methods.forEach(method => {
       subBuilder[method] = (...args) => {
-        if (method !== "copyDirectory") {
-          // Normal method (relPath as first argument)
-          args[0] = files.pathJoin(relPath, args[0]);
-        } else {
-          // with copyDirectory the path we have to fix up is inside
-          // an options hash
+        if (method === "copyDirectory" ||
+            method === "copyNodeModulesDirectory") {
+          // The copy methods take their relative paths via options.to.
           args[0].to = files.pathJoin(relPath, args[0].to);
+        } else {
+          // Other methods have relPath as the first argument.
+          args[0] = files.pathJoin(relPath, args[0]);
         }
 
         let ret = this[method](...args);
@@ -763,7 +829,13 @@ function symlinkWithOverwrite(source, target) {
 
 // Wrap slow methods into Profiler calls
 const slowBuilderMethods = [
-  '_ensureDirectory', 'write', 'enter', 'copyDirectory', 'enter', 'complete'
+  "_ensureDirectory",
+  "write",
+  "enter",
+  "copyDirectory",
+  "copyNodeModulesDirectory",
+  "enter",
+  "complete",
 ];
 
 slowBuilderMethods.forEach(method => {

tools/isobuild/bundler.js

@@ -2284,7 +2284,7 @@ class JsImage {
           copyOptions.filter = prodPackagePredicate;
         }
 
-        builder.copyDirectory(copyOptions);
+        builder.copyNodeModulesDirectory(copyOptions);
       }
     });
 

tools/isobuild/isopack.js

@@ -1144,7 +1144,7 @@ _.extend(Isopack.prototype, {
 
       // If unibuilds included node_modules, copy them in.
       _.each(npmDirsToCopy, (bundlePath, sourcePath) => {
-        builder.copyDirectory({
+        builder.copyNodeModulesDirectory({
           from: sourcePath,
           to: bundlePath,
           npmDiscards: self.npmDiscards,

tools/isobuild/unibuild.js

@@ -255,10 +255,8 @@ export class Unibuild {
       const bundlePath = _.has(npmDirsToCopy, nmd.sourcePath)
       // We already have this npm directory from another unibuild.
         ? npmDirsToCopy[nmd.sourcePath]
-        : npmDirsToCopy[nmd.sourcePath] = builder.generateFilename(
-          nmd.getPreferredBundlePath("isopack"),
-          { directory: true }
-        );
+        : npmDirsToCopy[nmd.sourcePath] =
+            nmd.getPreferredBundlePath("isopack");
       node_modules[bundlePath] = nmd.toJSON();
     });
 

tools/tests/old/test-bundler-options.js

@@ -123,28 +123,55 @@ var runTest = function () {
 
   if (process.platform !== "win32") { // Windows doesn't have symlinks
     console.log("includeNodeModules");
-    assert.doesNotThrow(function () {
-      var tmpOutputDir = tmpDir();
-      var result = bundler.bundle({
-        projectContext: projectContext,
-        outputPath: tmpOutputDir,
-        includeNodeModules: 'symlink'
-      });
-      assert.strictEqual(result.errors, false);
 
-      // sanity check -- main.js has expected contents.
-      assert.strictEqual(files.readFile(files.pathJoin(tmpOutputDir, "main.js"), "utf8"),
-                         bundler._mainJsContents);
-      // node_modules directory exists and is a symlink
-      assert(files.lstat(files.pathJoin(tmpOutputDir, "programs", "server", "node_modules")).isSymbolicLink());
-      // node_modules contains fibers
-      assert(files.exists(files.pathJoin(tmpOutputDir, "programs", "server", "node_modules", "fibers")));
-      // package node_modules directory also a symlink
-      // XXX might be breaking this
-      assert(files.lstat(files.pathJoin(
-        tmpOutputDir, "programs", "server", "npm", "node_modules", "meteor", "ddp-server", "node_modules"))
-             .isSymbolicLink());
+    var tmpOutputDir = tmpDir();
+    var result = bundler.bundle({
+      projectContext: projectContext,
+      outputPath: tmpOutputDir,
+      includeNodeModules: 'symlink'
     });
+
+    console.log("after bundler.bundle");
+
+    assert.strictEqual(result.errors, false);
+
+    console.log("before bundler._mainJsContents");
+
+    // sanity check -- main.js has expected contents.
+    assert.strictEqual(
+      files.readFile(files.pathJoin(tmpOutputDir, "main.js"), "utf8"),
+      bundler._mainJsContents
+    );
+
+    console.log("before programs/server/node_modules check");
+
+    // node_modules directory exists and is a symlink
+    assert(files.lstat(files.pathJoin(
+      tmpOutputDir, "programs", "server", "node_modules"
+    )).isSymbolicLink());
+
+    console.log("before programs/server/node_modules/fibers check");
+
+    // node_modules contains fibers as a symlink
+    assert(files.lstat(files.pathJoin(
+      tmpOutputDir, "programs", "server", "node_modules", "fibers"
+    )).isDirectory());
+
+    console.log("before ddp-server/node_modules check")
+
+    // package node_modules directory also a directory
+    assert(files.lstat(files.pathJoin(
+      tmpOutputDir, "programs", "server", "npm", "node_modules",
+      "meteor", "ddp-server", "node_modules"
+    )).isDirectory());
+
+    console.log("before ddp-server/node_modules/sockjs check");
+
+    // ddp-server/node_modules/sockjs is a symlink
+    assert(files.lstat(files.pathJoin(
+      tmpOutputDir, "programs", "server", "npm", "node_modules",
+      "meteor", "ddp-server", "node_modules", "sockjs"
+    )).isSymbolicLink());
   }
 };