From d734481abdc92035ef68d7c75ffde3b31dc1d50d Mon Sep 17 00:00:00 2001 From: zodern Date: Mon, 23 Dec 2019 10:31:03 -0600 Subject: [PATCH 1/6] Use optimisticRealpath when copying node_modules --- tools/fs/optimistic.ts | 19 +++++++++++++++++++ tools/isobuild/builder.js | 7 ++++--- tools/isobuild/bundler.js | 3 ++- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/tools/fs/optimistic.ts b/tools/fs/optimistic.ts index 86eb79ee1f..1fb57e81cd 100644 --- a/tools/fs/optimistic.ts +++ b/tools/fs/optimistic.ts @@ -16,6 +16,7 @@ import { readdir, dependOnPath, findAppDir, + realpath, } from "./files"; // When in doubt, the optimistic caching system can be completely disabled @@ -273,6 +274,24 @@ export const optimisticLStatOrNull = makeCheapPathFunction( }, ); +export const optimisticRealpath = makeOptimistic('realpath', realpath); +export const optimisticRealpathOrNull = makeOptimistic('realpathOrNull', ( + path: string, + options?: Parameters[1], +) => { + try { + return realpath(path, options) + } catch (e) { + if (e.code !== "ENOENT") { + throw e; + } + } + + dependOnParentDirectory(path); + + return null; +}); + export const optimisticReadFile = makeOptimistic("readFile", readFile); export const optimisticReaddir = makeOptimistic("readdir", readdir); export const optimisticHashOrNull = makeOptimistic("hashOrNull", ( diff --git a/tools/isobuild/builder.js b/tools/isobuild/builder.js index 04a23919ec..689645903b 100644 --- a/tools/isobuild/builder.js +++ b/tools/isobuild/builder.js @@ -11,6 +11,7 @@ import { optimisticStatOrNull, optimisticLStatOrNull, optimisticHashOrNull, + optimisticRealpath, } from "../fs/optimistic"; // Builder is in charge of writing "bundles" to disk, which are @@ -540,7 +541,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` // as well as node_modules/meteor and the parent directories of any // scoped npm packages. this._ensureAllNonPackageDirectories( - files.realpath(options.from), + optimisticRealpath(options.from), options.to ); } @@ -637,7 +638,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` }); } - const rootDir = files.realpath(from); + const rootDir = optimisticRealpath(from); const walk = (absFrom, relTo) => { if (symlink && ! (relTo in this.usedAsFile)) { @@ -671,7 +672,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` } try { - var real = files.realpath(thisAbsFrom); + var real = optimisticRealpath(thisAbsFrom); } catch (e) { if (e.code !== "ENOENT" && e.code !== "ELOOP") { diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index af7446bf0f..d6d34b0bda 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -173,6 +173,7 @@ import { loadIsopackage } from '../tool-env/isopackets.js'; import { CORDOVA_PLATFORM_VERSIONS } from '../cordova'; import { gzipSync } from "zlib"; import { PackageRegistry } from "../../packages/meteor/define-package.js"; +import { optimisticRealpathOrNull } from '../fs/optimistic'; const SOURCE_URL_PREFIX = "meteor://\u{1f4bb}app"; @@ -486,7 +487,7 @@ export class NodeModulesDirectory { return true; } - const real = files.realpathOrNull(path); + const real = optimisticRealpathOrNull(path); if (typeof real === "string" && real !== path) { // If node_modules/.bin/command is a symlink, determine the From 8c9251b9ac6ea380350d1fde71bcd48ee5376c8f Mon Sep 17 00:00:00 2001 From: zodern Date: Mon, 23 Dec 2019 11:42:02 -0600 Subject: [PATCH 2/6] Reduce realpath in SymlinkLoopChecker --- tools/isobuild/package-source.js | 34 ++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/tools/isobuild/package-source.js b/tools/isobuild/package-source.js index 1f85e89c16..2ba1ac25e3 100644 --- a/tools/isobuild/package-source.js +++ b/tools/isobuild/package-source.js @@ -213,14 +213,44 @@ var getExcerptFromReadme = function (text) { class SymlinkLoopChecker { constructor(sourceRoot) { this.sourceRoot = sourceRoot; + this._realSourceRoot = files.realpath(sourceRoot); this._seenPaths = {}; + this._cache = new Map(); } + // Avoids running realpath unless necessary + // since it is relatively slow on windows + _realpath = Profile('_realpath', function (relDir) { + const absPath = files.pathJoin(this._realSourceRoot, relDir); + + if (files.lstat(absPath).isSymbolicLink()) { + const result = files.realpath(absPath); + this._cache.set(relPath, result); + + return result; + } + + let result; + const parentDir = files.pathDirname(relDir); + const parentEntry = this._cache.get(parentDir); + if (parentDir === '.') { + result = absPath; + } else if (parentEntry) { + result = files.pathJoin(parentEntry, files.pathBasename(relDir)); + } else { + // The parent dir was never checked, which prevents us from + // skipping realpath + result = files.realpath(absPath); + } + + this._cache.set(relDir, result); + return result; + }) + check(relDir, quietly = true) { - const absPath = files.pathJoin(this.sourceRoot, relDir); try { - var realPath = files.realpath(absPath); + var realPath = this._realpath(relDir); } catch (e) { if (!e || e.code !== 'ELOOP') { throw e; From b7c7e329d088c340f45d2dfdac47ed207927f31d Mon Sep 17 00:00:00 2001 From: zodern Date: Mon, 24 Feb 2020 19:36:58 -0600 Subject: [PATCH 3/6] Fix variable name --- tools/isobuild/package-source.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/isobuild/package-source.js b/tools/isobuild/package-source.js index 2ba1ac25e3..c3c0bc9de8 100644 --- a/tools/isobuild/package-source.js +++ b/tools/isobuild/package-source.js @@ -225,7 +225,7 @@ class SymlinkLoopChecker { if (files.lstat(absPath).isSymbolicLink()) { const result = files.realpath(absPath); - this._cache.set(relPath, result); + this._cache.set(relDir, result); return result; } From a5fdd1a150d1ea88223bd226dbe87ca4f0a0b8b9 Mon Sep 17 00:00:00 2001 From: zodern Date: Mon, 24 Feb 2020 20:27:09 -0600 Subject: [PATCH 4/6] Reduce calls to realpath for bin files On Windows npm doesn't use symlinks for bin files, so the file's real path was never used. --- tools/isobuild/bundler.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index d6d34b0bda..ef553a1890 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -173,7 +173,7 @@ import { loadIsopackage } from '../tool-env/isopackets.js'; import { CORDOVA_PLATFORM_VERSIONS } from '../cordova'; import { gzipSync } from "zlib"; import { PackageRegistry } from "../../packages/meteor/define-package.js"; -import { optimisticRealpathOrNull } from '../fs/optimistic'; +import { optimisticLStatOrNull } from '../fs/optimistic'; const SOURCE_URL_PREFIX = "meteor://\u{1f4bb}app"; @@ -487,12 +487,11 @@ export class NodeModulesDirectory { return true; } - const real = optimisticRealpathOrNull(path); - if (typeof real === "string" && - real !== path) { + const fileStatus = optimisticLStatOrNull(path); + if (fileStatus && fileStatus.isSymbolicLink()) { // If node_modules/.bin/command is a symlink, determine the // answer by calling isWithinProdPackage(real). - return isWithinProdPackage(real); + return isWithinProdPackage(files.realpathOrNull(path)); } // If node_modules/.bin/command is not a symlink, then it's hard From 9421e76075e5d4decc38f2965101e4b8d2e0b336 Mon Sep 17 00:00:00 2001 From: zodern Date: Mon, 24 Feb 2020 22:03:18 -0600 Subject: [PATCH 5/6] Remove optimisticRealpath --- tools/fs/optimistic.ts | 18 ------------------ tools/isobuild/builder.js | 11 +++++------ 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/tools/fs/optimistic.ts b/tools/fs/optimistic.ts index 1fb57e81cd..ab9367c58b 100644 --- a/tools/fs/optimistic.ts +++ b/tools/fs/optimistic.ts @@ -274,24 +274,6 @@ export const optimisticLStatOrNull = makeCheapPathFunction( }, ); -export const optimisticRealpath = makeOptimistic('realpath', realpath); -export const optimisticRealpathOrNull = makeOptimistic('realpathOrNull', ( - path: string, - options?: Parameters[1], -) => { - try { - return realpath(path, options) - } catch (e) { - if (e.code !== "ENOENT") { - throw e; - } - } - - dependOnParentDirectory(path); - - return null; -}); - export const optimisticReadFile = makeOptimistic("readFile", readFile); export const optimisticReaddir = makeOptimistic("readdir", readdir); export const optimisticHashOrNull = makeOptimistic("hashOrNull", ( diff --git a/tools/isobuild/builder.js b/tools/isobuild/builder.js index 689645903b..797dca477c 100644 --- a/tools/isobuild/builder.js +++ b/tools/isobuild/builder.js @@ -1,7 +1,7 @@ import assert from "assert"; import {WatchSet, readAndWatchFile, sha1} from '../fs/watch'; import files, { - symlinkWithOverwrite, + symlinkWithOverwrite, realpath, } from '../fs/files'; import NpmDiscards from './npm-discards.js'; import {Profile} from '../tool-env/profile'; @@ -11,7 +11,6 @@ import { optimisticStatOrNull, optimisticLStatOrNull, optimisticHashOrNull, - optimisticRealpath, } from "../fs/optimistic"; // Builder is in charge of writing "bundles" to disk, which are @@ -541,7 +540,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` // as well as node_modules/meteor and the parent directories of any // scoped npm packages. this._ensureAllNonPackageDirectories( - optimisticRealpath(options.from), + realpath(options.from), options.to ); } @@ -638,7 +637,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` }); } - const rootDir = optimisticRealpath(from); + const rootDir = realpath(from); const walk = (absFrom, relTo) => { if (symlink && ! (relTo in this.usedAsFile)) { @@ -662,7 +661,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` return; } - // Returns files.realpath(thisAbsFrom), iff it is external to + // Returns files.realpath(thisAbsFrom), if it is external to // rootDir, using caching because this function might be called // more than once. let cachedExternalPath; @@ -672,7 +671,7 @@ Previous builder: ${previousBuilder.outputPath}, this builder: ${outputPath}` } try { - var real = optimisticRealpath(thisAbsFrom); + var real = realpath(thisAbsFrom); } catch (e) { if (e.code !== "ENOENT" && e.code !== "ELOOP") { From 7adace63ccdeb65edc20f50c2260fecb5686a4fd Mon Sep 17 00:00:00 2001 From: zodern Date: Mon, 24 Feb 2020 22:13:18 -0600 Subject: [PATCH 6/6] Remove unused import --- tools/fs/optimistic.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/fs/optimistic.ts b/tools/fs/optimistic.ts index ab9367c58b..86eb79ee1f 100644 --- a/tools/fs/optimistic.ts +++ b/tools/fs/optimistic.ts @@ -16,7 +16,6 @@ import { readdir, dependOnPath, findAppDir, - realpath, } from "./files"; // When in doubt, the optimistic caching system can be completely disabled