From 0d84a61b726b33f843514c6b4c8f58f389d8157c Mon Sep 17 00:00:00 2001 From: James Henry Date: Mon, 4 Oct 2021 15:42:45 +0000 Subject: [PATCH] fix(core): changes to the daemon in order to support windows (#7211) --- .../core/project-graph/daemon/exec/index.ts | 20 +- .../core/project-graph/daemon/server.spec.ts | 6 +- .../src/core/project-graph/daemon/server.ts | 282 +++++++++++------- .../src/core/project-graph/project-graph.ts | 2 +- 4 files changed, 188 insertions(+), 122 deletions(-) diff --git a/packages/workspace/src/core/project-graph/daemon/exec/index.ts b/packages/workspace/src/core/project-graph/daemon/exec/index.ts index b7f4ae55cc..6401df80d1 100644 --- a/packages/workspace/src/core/project-graph/daemon/exec/index.ts +++ b/packages/workspace/src/core/project-graph/daemon/exec/index.ts @@ -1,8 +1,8 @@ -import { logger } from '@nrwl/devkit'; +import { logger, normalizePath } from '@nrwl/devkit'; import { appRootPath } from '@nrwl/tao/src/utils/app-root'; import { spawn, spawnSync } from 'child_process'; import { ensureFileSync } from 'fs-extra'; -import { join, sep } from 'path'; +import { join } from 'path'; import { dirSync } from 'tmp'; import { DaemonJson, @@ -17,9 +17,9 @@ export async function startInBackground(): Promise { * starting the server, as well as providing a reference to where any subsequent * log files can be found. */ - const tmpDirPrefix = `nx-daemon--${appRootPath.replace( - // Replace the occurrences of / on unix systems, the \ on windows, with a - - new RegExp(escapeRegExp(sep), 'g'), + const tmpDirPrefix = `nx-daemon--${normalizePath(appRootPath).replace( + // Replace the occurrences of / in the unix-style normalized path with a - + new RegExp(escapeRegExp('/'), 'g'), '-' )}`; const serverLogOutputDir = dirSync({ @@ -43,7 +43,7 @@ export async function startInBackground(): Promise { try { const backgroundProcess = spawn( - 'node', + process.execPath, ['./start.js', serverLogOutputFile], { cwd: __dirname, @@ -64,8 +64,8 @@ export async function startInBackground(): Promise { * Ensure the server is actually available to connect to via IPC before resolving */ return new Promise((resolve) => { - const id = setInterval(() => { - if (isServerAvailable()) { + const id = setInterval(async () => { + if (await isServerAvailable()) { clearInterval(id); resolve(); } @@ -80,7 +80,7 @@ export async function startInBackground(): Promise { export function startInCurrentProcess(): void { logger.info(`NX Daemon Server - Starting in the current process...`); - spawnSync('node', ['./start.js'], { + spawnSync(process.execPath, ['./start.js'], { cwd: __dirname, stdio: 'inherit', }); @@ -89,7 +89,7 @@ export function startInCurrentProcess(): void { export function stop(): void { logger.info(`NX Daemon Server - Stopping...`); - spawnSync('node', ['./stop.js'], { + spawnSync(process.execPath, ['./stop.js'], { cwd: __dirname, stdio: 'inherit', }); diff --git a/packages/workspace/src/core/project-graph/daemon/server.spec.ts b/packages/workspace/src/core/project-graph/daemon/server.spec.ts index 8ea3316c6b..922a3aef51 100644 --- a/packages/workspace/src/core/project-graph/daemon/server.spec.ts +++ b/packages/workspace/src/core/project-graph/daemon/server.spec.ts @@ -59,13 +59,13 @@ describe('Daemon Server', () => { describe('isServerAvailable()', () => { it('should return true if the daemon server is available for connections', async () => { - expect(isServerAvailable()).toBe(false); + expect(await isServerAvailable()).toBe(false); await startServer({}); - expect(isServerAvailable()).toBe(true); + expect(await isServerAvailable()).toBe(true); await stopServer(); - expect(isServerAvailable()).toBe(false); + expect(await isServerAvailable()).toBe(false); }); }); diff --git a/packages/workspace/src/core/project-graph/daemon/server.ts b/packages/workspace/src/core/project-graph/daemon/server.ts index 1594bd12ae..bcd4bf752b 100644 --- a/packages/workspace/src/core/project-graph/daemon/server.ts +++ b/packages/workspace/src/core/project-graph/daemon/server.ts @@ -118,10 +118,38 @@ function createAndSerializeProjectGraph(): string { } /** - * For now we just invoke the existing `createProjectGraph()` utility and return the project - * graph upon connection to the server + * We need to make sure that we instantiate the PerformanceObserver only once, otherwise + * we will end up with duplicate entries in the server logs. */ let performanceObserver: PerformanceObserver | undefined; + +/** + * Create the server and register a connection callback. + * + * NOTE: It is important that we do not eagerly perform any work directly upon connection + * of a client. + * + * This is because there are two different scenarios for connection that we need to support: + * 1) Checking if the server is fundamentally available + * 2) Requesting the project graph to be built + * + * For case (1) we want the operation to be as fast as possible - we just need to know if the + * server exists to connect to, and so this is why we do not perform any work directly upon + * connection. + * + * For case (2) we have a simple known data payload which the client will send to the server + * in order to trigger the more expensive graph construction logic. + * + * The real reason we need have these two separate steps in which we decouple connection from + * the request to actually build the project graph is down to the fact that on Windows, the named + * pipe behaves subtly differently from Unix domain sockets in that when you check if it exists + * as if it were a file (e.g. using fs.existsSync() or fs.open() or fs.statSync() etc), the native + * code which runs behind the scenes on the OS will actually trigger a connection to the server. + * Therefore if we were to simply perform the graph creation upon connection we would end up + * repeating work and throwing `EPIPE` errors. + */ +const REQUEST_PROJECT_GRAPH_PAYLOAD = 'REQUEST_PROJECT_GRAPH_PAYLOAD'; + const server = createServer((socket) => { if (!performanceObserver) { performanceObserver = new PerformanceObserver((list) => { @@ -129,105 +157,116 @@ const server = createServer((socket) => { // Slight indentation to improve readability of the overall log file serverLog(` Time taken for '${entry.name}'`, `${entry.duration}ms`); }); + performanceObserver.observe({ entryTypes: ['measure'], buffered: false }); } - performanceObserver.observe({ entryTypes: ['measure'], buffered: false }); - performance.mark('server-connection'); - serverLog('Connection Received'); - - const currentGitHead = gitRevParseHead(appRootPath); - - let serializedProjectGraph: string | undefined; - - /** - * Cached HEAD has changed, we must perform full file-hashing initialization work and - * recompute the project graph - */ - if (currentGitHead !== cachedGitHead) { - serverLog( - ` [SERVER STATE]: Cached HEAD does not match current (${currentGitHead}), performing full file hash init and recomputing project graph...` - ); + socket.on('data', (data) => { /** - * Update the cached values for the HEAD and untracked and uncommitted state which was computed - * as part of full init() + * If anything other than the known project graph creation request payload is sent to + * the server, we throw an error. */ - const untrackedAndUncommittedFileHashes = defaultFileHasher.init(); - hashAndCacheUntrackedUncommittedState(untrackedAndUncommittedFileHashes); - cachedGitHead = currentGitHead; - serializedProjectGraph = createAndSerializeProjectGraph(); - } else { - /** - * We know at this point that the cached HEAD has not changed but we must still always use git - * to check for untracked and uncommitted changes (and we then create and cache a hash which - * represents their overall state). - * - * We cannot ever skip this particular git operation, but we can compare its result to our - * previously cached hash which represents the overall state for untracked and uncommitted changes - * and then potentially skip project graph creation altogether if it is unchanged and we have an - * existing cached graph. - */ - const previousUntrackedUncommittedState = cachedUntrackedUncommittedState; - const untrackedAndUncommittedFileHashes = - defaultFileHasher.incrementalUpdate(); - hashAndCacheUntrackedUncommittedState(untrackedAndUncommittedFileHashes); - - /** - * Skip project graph creation if the untracked and uncommitted state is unchanged and we have - * a cached version of the graph available in memory. - */ - if ( - previousUntrackedUncommittedState === cachedUntrackedUncommittedState && - cachedSerializedProjectGraph - ) { - serverLog( - ` [SERVER STATE]: State unchanged since last request, resolving in-memory cached project graph...` - ); - serializedProjectGraph = cachedSerializedProjectGraph; - } else { - serverLog( - ` [SERVER STATE]: Hashed untracked/uncommitted file state changed (now ${cachedUntrackedUncommittedState}), recomputing project graph...` - ); - serializedProjectGraph = createAndSerializeProjectGraph(); + const payload = data.toString(); + if (payload !== REQUEST_PROJECT_GRAPH_PAYLOAD) { + throw new Error(`Unsupported payload sent to daemon server: ${payload}`); } - } - /** - * Cache the latest version of the project graph in memory so that we can potentially skip a lot - * of expensive work on the next client request. - * - * For reference, on the very large test repo https://github.com/vsavkin/interstellar the project - * graph nxdeps.json file is about 24MB, so memory utilization should not be a huge concern. - */ - cachedSerializedProjectGraph = serializedProjectGraph; + performance.mark('server-connection'); + serverLog('Client Request for Project Graph Received'); - performance.mark('serialized-project-graph-ready'); - performance.measure( - 'total for creating and serializing project graph', - 'server-connection', - 'serialized-project-graph-ready' - ); + const currentGitHead = gitRevParseHead(appRootPath); + + let serializedProjectGraph: string | undefined; - socket.write(serializedProjectGraph, () => { - performance.mark('serialized-project-graph-written-to-client'); - performance.measure( - 'write project graph to socket', - 'serialized-project-graph-ready', - 'serialized-project-graph-written-to-client' - ); /** - * Close the connection once all data has been written to the socket so that the client - * knows when to read it. + * Cached HEAD has changed, we must perform full file-hashing initialization work and + * recompute the project graph */ - socket.end(); + if (currentGitHead !== cachedGitHead) { + serverLog( + ` [SERVER STATE]: Cached HEAD does not match current (${currentGitHead}), performing full file hash init and recomputing project graph...` + ); + /** + * Update the cached values for the HEAD and untracked and uncommitted state which was computed + * as part of full init() + */ + const untrackedAndUncommittedFileHashes = defaultFileHasher.init(); + hashAndCacheUntrackedUncommittedState(untrackedAndUncommittedFileHashes); + cachedGitHead = currentGitHead; + serializedProjectGraph = createAndSerializeProjectGraph(); + } else { + /** + * We know at this point that the cached HEAD has not changed but we must still always use git + * to check for untracked and uncommitted changes (and we then create and cache a hash which + * represents their overall state). + * + * We cannot ever skip this particular git operation, but we can compare its result to our + * previously cached hash which represents the overall state for untracked and uncommitted changes + * and then potentially skip project graph creation altogether if it is unchanged and we have an + * existing cached graph. + */ + const previousUntrackedUncommittedState = cachedUntrackedUncommittedState; + const untrackedAndUncommittedFileHashes = + defaultFileHasher.incrementalUpdate(); + hashAndCacheUntrackedUncommittedState(untrackedAndUncommittedFileHashes); + + /** + * Skip project graph creation if the untracked and uncommitted state is unchanged and we have + * a cached version of the graph available in memory. + */ + if ( + previousUntrackedUncommittedState === cachedUntrackedUncommittedState && + cachedSerializedProjectGraph + ) { + serverLog( + ` [SERVER STATE]: State unchanged since last request, resolving in-memory cached project graph...` + ); + serializedProjectGraph = cachedSerializedProjectGraph; + } else { + serverLog( + ` [SERVER STATE]: Hashed untracked/uncommitted file state changed (now ${cachedUntrackedUncommittedState}), recomputing project graph...` + ); + serializedProjectGraph = createAndSerializeProjectGraph(); + } + } + + /** + * Cache the latest version of the project graph in memory so that we can potentially skip a lot + * of expensive work on the next client request. + * + * For reference, on the very large test repo https://github.com/vsavkin/interstellar the project + * graph nxdeps.json file is about 32MB, so memory utilization should not be a huge concern. + */ + cachedSerializedProjectGraph = serializedProjectGraph; + + performance.mark('serialized-project-graph-ready'); performance.measure( - 'total for server response', + 'total for creating and serializing project graph', 'server-connection', - 'serialized-project-graph-written-to-client' - ); - const bytesWritten = Buffer.byteLength(serializedProjectGraph, 'utf-8'); - serverLog( - `Closed Connection to Client (${bytesWritten} bytes transferred)` + 'serialized-project-graph-ready' ); + + socket.write(serializedProjectGraph, () => { + performance.mark('serialized-project-graph-written-to-client'); + performance.measure( + 'write project graph to socket', + 'serialized-project-graph-ready', + 'serialized-project-graph-written-to-client' + ); + /** + * Close the connection once all data has been written to the socket so that the client + * knows when to read it. + */ + socket.end(); + performance.measure( + 'total for server response', + 'server-connection', + 'serialized-project-graph-written-to-client' + ); + const bytesWritten = Buffer.byteLength(serializedProjectGraph, 'utf-8'); + serverLog( + `Closed Connection to Client (${bytesWritten} bytes transferred)` + ); + }); }); }); @@ -246,7 +285,7 @@ export async function startServer({ } return new Promise((resolve) => { server.listen(fullOSSocketPath, () => { - serverLog(`Started`); + serverLog(`Started listening on: ${fullOSSocketPath}`); return resolve(server); }); }); @@ -283,12 +322,28 @@ export function killSocketOrPath(): void { } catch {} } -export function isServerAvailable(): boolean { +/** + * As noted in the comments above the createServer() call, in order to reliably (meaning it works + * cross-platform) check whether or not the server is availabe to request a project graph from we + * need to actually attempt connecting to it. + * + * Because of the behavior of named pipes on Windows, we cannot simply treat them as a file and + * check for their existence on disk (unlike with Unix Sockets). + */ +export async function isServerAvailable(): Promise { try { - statSync(fullOSSocketPath); - return true; + const socket = connect(fullOSSocketPath); + return new Promise((resolve) => { + socket.on('connect', () => { + socket.destroy(); + resolve(true); + }); + socket.on('error', () => { + resolve(false); + }); + }); } catch { - return false; + return Promise.resolve(false); } } @@ -319,22 +374,33 @@ export async function getProjectGraphFromServer(): Promise { return reject(new Error(errorMessage) || err); }); - let serializedProjectGraph = ''; - socket.on('data', (data) => { - serializedProjectGraph += data.toString(); - }); + /** + * Immediately after connecting to the server we send it the known project graph creation + * request payload. See the notes above createServer() for more context as to why we explicitly + * request the graph from the client like this. + */ + socket.on('connect', () => { + socket.write(REQUEST_PROJECT_GRAPH_PAYLOAD); - socket.on('end', () => { - try { - const projectGraph = JSON.parse(serializedProjectGraph) as ProjectGraph; - logger.info('NX Daemon Client - Resolved ProjectGraph'); - return resolve(projectGraph); - } catch { - logger.error( - 'NX Daemon Client - Error: Could not deserialize the ProjectGraph' - ); - return reject(); - } + let serializedProjectGraph = ''; + socket.on('data', (data) => { + serializedProjectGraph += data.toString(); + }); + + socket.on('end', () => { + try { + const projectGraph = JSON.parse( + serializedProjectGraph + ) as ProjectGraph; + logger.info('NX Daemon Client - Resolved ProjectGraph'); + return resolve(projectGraph); + } catch { + logger.error( + 'NX Daemon Client - Error: Could not deserialize the ProjectGraph' + ); + return reject(); + } + }); }); }); } diff --git a/packages/workspace/src/core/project-graph/project-graph.ts b/packages/workspace/src/core/project-graph/project-graph.ts index 2b06eb5f63..378d284aa0 100644 --- a/packages/workspace/src/core/project-graph/project-graph.ts +++ b/packages/workspace/src/core/project-graph/project-graph.ts @@ -175,7 +175,7 @@ export async function createProjectGraphAsync( ); } - if (!isServerAvailable()) { + if (!(await isServerAvailable())) { logger.warn( '\nWARNING: You set NX_DAEMON=true but the Daemon Server is not running. Starting Daemon Server in the background...' );