From 73d6b77614685e30575b485695c485cddf635649 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 21 Feb 2019 19:32:08 -0600 Subject: [PATCH] Externalize spdlog .node module --- build/tasks.ts | 5 ++++ packages/server/scripts/nexe.js | 7 ------ packages/server/src/cli.ts | 26 ++++----------------- packages/server/src/modules.ts | 3 +++ packages/server/src/vscode/bootstrapFork.ts | 9 ++++--- packages/server/src/vscode/sharedProcess.ts | 2 +- packages/server/webpack.config.js | 2 +- packages/vscode/webpack.config.bootstrap.js | 1 - scripts/webpack.general.config.js | 18 +++++++++++++- 9 files changed, 38 insertions(+), 35 deletions(-) diff --git a/build/tasks.ts b/build/tasks.ts index 9515000a..27b6d273 100644 --- a/build/tasks.ts +++ b/build/tasks.ts @@ -82,10 +82,14 @@ const buildServerBinaryCopy = register("build:server:binary:copy", async (runner const webOutputPath = path.join(pkgsPath, "web", "out"); const browserAppOutputPath = path.join(pkgsPath, "app", "browser", "out"); const nodePtyModule = path.join(pkgsPath, "protocol", "node_modules", "node-pty", "build", "Release", "pty.node"); + const spdlogModule = path.join(pkgsPath, "server", "node_modules", "spdlog", "build", "Release", "spdlog.node"); if (!fs.existsSync(nodePtyModule)) { throw new Error("Could not find pty.node. Ensure all packages have been installed"); } + if (!fs.existsSync(spdlogModule)) { + throw new Error("Could not find spdlog.node. Ensure all packages have been installed"); + } if (!fs.existsSync(webOutputPath)) { throw new Error("Web bundle must be built"); } @@ -114,6 +118,7 @@ const buildServerBinaryCopy = register("build:server:binary:copy", async (runner cpDir(browserAppOutputPath, "unauth", browserAppOutputPath); fse.mkdirpSync(path.join(cliBuildPath, "modules")); fse.copySync(nodePtyModule, path.join(cliBuildPath, "modules", "pty.node")); + fse.copySync(spdlogModule, path.join(cliBuildPath, "modules", "spdlog.node")); }); const buildServerBundle = register("build:server:bundle", async (runner) => { diff --git a/packages/server/scripts/nexe.js b/packages/server/scripts/nexe.js index 042955d8..ad960a34 100644 --- a/packages/server/scripts/nexe.js +++ b/packages/server/scripts/nexe.js @@ -9,13 +9,6 @@ nexe.compile({ input: path.join(__dirname, "../out/cli.js"), output: `cli-${process.env.TRAVIS_OS_NAME || os.platform()}`, targets: [os.platform()], - native: { - spdlog: { - additionalFiles: [ - 'spdlog.node' - ], - }, - }, /** * To include native extensions, do NOT install node_modules for each one. They * are not required as each extension is built using webpack. diff --git a/packages/server/src/cli.ts b/packages/server/src/cli.ts index dd3740e3..03951641 100644 --- a/packages/server/src/cli.ts +++ b/packages/server/src/cli.ts @@ -38,28 +38,15 @@ export class Entry extends Command { }]; public async run(): Promise { - try { - /** - * Suuuper janky - * Comes from - https://github.com/nexe/nexe/issues/524 - * Seems to cleanup by removing this path immediately - * If any native module is added its assumed this pathname - * will change. - */ - require("spdlog"); - const nodePath = path.join(process.cwd(), "e91a410b"); - fs.unlinkSync(path.join(nodePath, "spdlog.node")); - fs.rmdirSync(nodePath); - } catch (ex) { - logger.warn("Failed to remove extracted dependency.", field("dependency", "spdlog"), field("error", ex.message)); - } - if (isCli) { fillFs(); } const { args, flags } = this.parse(Entry); + const dataDir = flags["data-dir"] || path.join(os.homedir(), ".vscode-remote"); + const workingDir = args["workdir"]; + setupNativeModules(dataDir); const builtInExtensionsDir = path.join(buildDir || path.join(__dirname, ".."), "build/extensions"); if (flags["bootstrap-fork"]) { const modulePath = flags["bootstrap-fork"]; @@ -75,7 +62,7 @@ export class Entry extends Command { process.argv[i + 2] = arg; }); - return requireModule(modulePath, builtInExtensionsDir); + return requireModule(modulePath, dataDir, builtInExtensionsDir); } if (flags["fork"]) { @@ -84,9 +71,6 @@ export class Entry extends Command { return requireFork(modulePath, JSON.parse(flags.args!), builtInExtensionsDir); } - const dataDir = flags["data-dir"] || path.join(os.homedir(), ".vscode-remote"); - const workingDir = args["workdir"]; - if (buildDir && buildDir.startsWith(workingDir)) { logger.error("Cannot run binary inside of BUILD_DIR", field("build_dir", buildDir), field("cwd", process.cwd())); process.exit(1); @@ -95,7 +79,7 @@ export class Entry extends Command { if (!fs.existsSync(dataDir)) { fs.mkdirSync(dataDir); } - setupNativeModules(dataDir); + require("spdlog"); const logDir = path.join(dataDir, "logs", new Date().toISOString().replace(/[-:.TZ]/g, "")); process.env.VSCODE_LOGS = logDir; diff --git a/packages/server/src/modules.ts b/packages/server/src/modules.ts index 4f298754..fcb646d7 100644 --- a/packages/server/src/modules.ts +++ b/packages/server/src/modules.ts @@ -34,11 +34,14 @@ export const setup = (dataDirectory: string): void => { * for this is unknown ATM, but this patch works around it. */ unpackModule("pty"); + unpackModule("spdlog"); const nodePtyUtils = require("../../protocol/node_modules/node-pty/lib/utils") as typeof import("../../protocol/node_modules/node-pty/src/utils"); // tslint:disable-next-line:no-any nodePtyUtils.loadNative = (modName: string): any => { return __non_webpack_require__(path.join(dataDirectory, "modules", modName + ".node")); }; + // tslint:disable-next-line:no-any + (global).SPDLOG_LOCATION = path.join(dataDirectory, "modules", "spdlog.node"); // tslint:disable-next-line:no-unused-expression require("../../protocol/node_modules/node-pty/lib/index") as typeof import("../../protocol/node_modules/node-pty/src/index"); }; diff --git a/packages/server/src/vscode/bootstrapFork.ts b/packages/server/src/vscode/bootstrapFork.ts index 834bae00..666fd46f 100644 --- a/packages/server/src/vscode/bootstrapFork.ts +++ b/packages/server/src/vscode/bootstrapFork.ts @@ -71,7 +71,7 @@ export const requireFork = (modulePath: string, args: string[], builtInExtension } }; -export const requireModule = (modulePath: string, builtInExtensionsDir: string): void => { +export const requireModule = (modulePath: string, dataDir: string, builtInExtensionsDir: string): void => { process.env.AMD_ENTRYPOINT = modulePath; const xml = require("xhr2"); xml.XMLHttpRequest.prototype._restrictedHeaders["user-agent"] = false; @@ -96,7 +96,7 @@ export const requireModule = (modulePath: string, builtInExtensionsDir: string): */ // tslint:disable-next-line:no-any (cp).fork = (modulePath: string, args: ReadonlyArray = [], options?: cp.ForkOptions): cp.ChildProcess => { - return cp.spawn(process.execPath, ["--fork", modulePath, "--args", JSON.stringify(args)], { + return cp.spawn(process.execPath, ["--fork", modulePath, "--args", JSON.stringify(args), "--data-dir", dataDir], { ...options, stdio: [null, null, null, "ipc"], }); @@ -123,7 +123,7 @@ export const requireModule = (modulePath: string, builtInExtensionsDir: string): * cp.stderr.on("data", (data) => console.log(data.toString("utf8"))); * @param modulePath Path of the VS Code module to load. */ -export const forkModule = (modulePath: string, args: string[], options: cp.ForkOptions): cp.ChildProcess => { +export const forkModule = (modulePath: string, args: string[], options: cp.ForkOptions, dataDir?: string): cp.ChildProcess => { let proc: cp.ChildProcess; const forkArgs = ["--bootstrap-fork", modulePath]; if (args) { @@ -134,6 +134,9 @@ export const forkModule = (modulePath: string, args: string[], options: cp.ForkO delete options.env.ELECTRON_RUN_AS_NODE; forkArgs.push("--env", JSON.stringify(options.env)); } + if (dataDir) { + forkArgs.push("--data-dir", dataDir); + } const forkOptions: cp.ForkOptions = { stdio: [null, null, null, "ipc"], }; diff --git a/packages/server/src/vscode/sharedProcess.ts b/packages/server/src/vscode/sharedProcess.ts index ce0bedb6..94823061 100644 --- a/packages/server/src/vscode/sharedProcess.ts +++ b/packages/server/src/vscode/sharedProcess.ts @@ -84,7 +84,7 @@ export class SharedProcess { VSCODE_ALLOW_IO: "true", VSCODE_LOGS: process.env.VSCODE_LOGS, }, - }); + }, this.userDataDir); if (this.logger.level <= Level.Trace) { this.activeProcess.stdout.on("data", (data) => { this.logger.trace(() => ["stdout", field("data", data.toString())]); diff --git a/packages/server/webpack.config.js b/packages/server/webpack.config.js index 582b5688..dadbd316 100644 --- a/packages/server/webpack.config.js +++ b/packages/server/webpack.config.js @@ -48,7 +48,7 @@ module.exports = merge({ __dirname: false, setImmediate: false }, - externals: ["spdlog", "tslib", "trash"], + externals: ["tslib", "trash"], entry: "./packages/server/src/cli.ts", target: "node", plugins: [ diff --git a/packages/vscode/webpack.config.bootstrap.js b/packages/vscode/webpack.config.bootstrap.js index 77fee189..3f9902a6 100644 --- a/packages/vscode/webpack.config.bootstrap.js +++ b/packages/vscode/webpack.config.bootstrap.js @@ -16,7 +16,6 @@ module.exports = (env) => { entry: path.join(root, "lib/vscode/src/bootstrap-fork.js"), mode: "development", target: "node", - externals: ["spdlog"], output: { chunkFilename: "[name].bundle.js", path: path.resolve(__dirname, "./bin"), diff --git a/scripts/webpack.general.config.js b/scripts/webpack.general.config.js index d4494a32..7320d606 100644 --- a/scripts/webpack.general.config.js +++ b/scripts/webpack.general.config.js @@ -83,7 +83,23 @@ module.exports = (options = {}) => ({ }, { test: /\.wasm$/, type: "javascript/auto", - }], + }, { + /** + * Fixes spdlog + */ + test: /spdlog\/index\.js/, + loader: "string-replace-loader", + options: { + multiple: [{ + // These will be handled by file-loader. We need the location because + // they are parsed as URIs and will throw errors if not fully formed. + // The !! prefix causes it to ignore other loaders (doesn't work). + search: "const spdlog.*;", + replace: "const spdlog = __non_webpack_require__(global.SPDLOG_LOCATION);", + flags: "g", + }], + }, + },], noParse: /\/test\/|\.test\.jsx?|\.test\.tsx?|tsconfig.+\.json$/, }, resolve: {