diff --git a/packages/oauth/oauth_server.js b/packages/oauth/oauth_server.js index 0df5ec978a..b3cde82f33 100644 --- a/packages/oauth/oauth_server.js +++ b/packages/oauth/oauth_server.js @@ -1,4 +1,3 @@ -var Fiber = Npm.require('fibers'); var url = Npm.require('url'); OAuth = {}; diff --git a/packages/oauth/package.js b/packages/oauth/package.js index baae0d24ee..f96f76920e 100644 --- a/packages/oauth/package.js +++ b/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) { diff --git a/tools/isobuild/builder.js b/tools/isobuild/builder.js index b5d3f573cb..fdb5973fba 100644 --- a/tools/isobuild/builder.js +++ b/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 => { diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 564af1e15b..0debf70877 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -2284,7 +2284,7 @@ class JsImage { copyOptions.filter = prodPackagePredicate; } - builder.copyDirectory(copyOptions); + builder.copyNodeModulesDirectory(copyOptions); } }); diff --git a/tools/isobuild/isopack.js b/tools/isobuild/isopack.js index aa11726f06..1fce62d140 100644 --- a/tools/isobuild/isopack.js +++ b/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, diff --git a/tools/isobuild/unibuild.js b/tools/isobuild/unibuild.js index 58361331ac..d3bc27a1a4 100644 --- a/tools/isobuild/unibuild.js +++ b/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(); }); diff --git a/tools/tests/old/test-bundler-options.js b/tools/tests/old/test-bundler-options.js index 12b9a8c5e8..2482ca4999 100644 --- a/tools/tests/old/test-bundler-options.js +++ b/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()); } };