From b69716d77856fb3473eb7ef54fbbec730b0edbca Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 17 Oct 2016 21:20:53 -0400 Subject: [PATCH] Use files.* methods instead of optimistic functions in meteor-npm.js. This fixes a bug that prevents fourseven:scss from properly rebuilding, because the new .../node_modules/node-sass/vendor//binding.node file is not found by Builder#copyDirectory, because the cached results of optimisticReaddir are the same as before the rebuild. Unfortunately, this change introduces a small performance regression (hundreds of milliseconds at worst), because these files.* methods are called many times. I think we can continue using optimistic functions here if we are more careful about invalidating their results, especially after calling meteorNpm.rebuildIfNonPortable, but I'll save that for a future commit. --- tools/isobuild/meteor-npm.js | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/tools/isobuild/meteor-npm.js b/tools/isobuild/meteor-npm.js index f67247f112..fbebefde5c 100644 --- a/tools/isobuild/meteor-npm.js +++ b/tools/isobuild/meteor-npm.js @@ -22,13 +22,6 @@ import { convert as convertColonsInPath } from "../utils/colon-converter.js"; -import { - optimisticLStat, - optimisticStatOrNull, - optimisticReadFile, - optimisticReaddir, -} from "../fs/optimistic.js"; - var meteorNpm = exports; // if a user exits meteor while we're trying to create a .npm @@ -140,11 +133,11 @@ export function getProdPackageNames(nodeModulesDir) { // Returns true iff dir is a package directory. function walk(dir) { const packageJsonPath = files.pathJoin(dir, "package.json"); - const packageJsonStat = optimisticStatOrNull(packageJsonPath); + const packageJsonStat = files.statOrNull(packageJsonPath); if (packageJsonStat && packageJsonStat.isFile()) { - const pkg = JSON.parse(optimisticReadFile(packageJsonPath)); + const pkg = JSON.parse(files.readFile(packageJsonPath)); const nodeModulesDir = files.pathJoin(dir, "node_modules"); nodeModulesDirStack.push(nodeModulesDir); @@ -194,7 +187,7 @@ export function getProdPackageNames(nodeModulesDir) { for (let i = nodeModulesDirStack.length - 1; i >= 0; --i) { const nodeModulesDir = nodeModulesDirStack[i]; const candidate = files.pathJoin(nodeModulesDir, name); - const stat = optimisticStatOrNull(candidate); + const stat = files.statOrNull(candidate); if (stat && stat.isDirectory()) { return candidate; } @@ -437,13 +430,13 @@ function copyNpmPackageWithSymlinkedNodeModules(fromPkgDir, toPkgDir) { } const isPortable = Profile("meteorNpm.isPortable", dir => { - const lstat = optimisticLStat(dir); + const lstat = files.lstat(dir); if (! lstat.isDirectory()) { // Non-directory files are portable unless they end with .node. return ! dir.endsWith(".node"); } - const pkgJsonStat = optimisticStatOrNull(files.pathJoin(dir, "package.json")); + const pkgJsonStat = files.statOrNull(files.pathJoin(dir, "package.json")); const canCache = pkgJsonStat && pkgJsonStat.isFile(); const portableFile = files.pathJoin(dir, ".meteor-portable"); @@ -455,7 +448,7 @@ const isPortable = Profile("meteorNpm.isPortable", dir => { // directories, so that they will get cleared away the next time those // packages are (re)installed. try { - return JSON.parse(optimisticReadFile(portableFile)); + return JSON.parse(files.readFile(portableFile)); } catch (e) { if (! (e instanceof SyntaxError || e.code === "ENOENT")) { @@ -468,7 +461,7 @@ const isPortable = Profile("meteorNpm.isPortable", dir => { fs.unlink(portableFile, error => {}); } - const result = optimisticReaddir(dir).every( + const result = files.readdir(dir).every( // Ignore files that start with a ".", such as .bin directories. itemName => itemName.startsWith(".") || isPortable(files.pathJoin(dir, itemName)));