diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5e83cff1..6ca77837 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -168,6 +168,24 @@ Optionally, these steps may be automated by installing the npx npm-merge-driver install -g ``` +## `fast-boot` and `npm link` - modifying the `node_modules` folder + +During development or debugging, it is sometimes useful to temporarily modify the `node_modules` +folder (with or without making the respective changes to the `npm-shrinkwrap.json` file), +replacing dependencies with different versions. This can be achieved with the `npm link` +command, or by manually editing or copying files to the `node_modules` folder. + +Unexpected behavior may then be observed because of the CLI's use of the +[fast-boot2](https://www.npmjs.com/package/fast-boot2) package that caches module resolution. +`fast-boot2` is configured in `lib/fast-boot.ts` to automatically invalidate the cache if +changes are made to the `package.json` or `npm-shrinkwrap.json` files, but the cache won't +be automatically invalidated if `npm link` is used or if manual modifications are made to the +`node_modules` folder. In this situation: + +* Manually delete the module cache file (typically `~/.balena/cli-module-cache.json`), or +* Use the `bin/balena-dev` entry point (instead of `bin/balena`) as it does not activate + `fast-boot2`. + ## TypeScript and oclif The CLI currently contains a mix of plain JavaScript and diff --git a/bin/balena b/bin/balena index 4ee709ca..c34a4461 100755 --- a/bin/balena +++ b/bin/balena @@ -9,14 +9,15 @@ process.env.UV_THREADPOOL_SIZE = '64'; // Disable oclif registering ts-node process.env.OCLIF_TS_NODE = 0; -// Use fast-boot to cache require lookups, speeding up startup -require('fast-boot2').start({ - cacheScope: __dirname + '/..', - cacheFile: __dirname + '/.fast-boot.json', -}); +async function run() { + // Use fast-boot to cache require lookups, speeding up startup + await require('../build/fast-boot').start(); -// Set the desired es version for downstream modules that support it -require('@balena/es-version').set('es2018'); + // Set the desired es version for downstream modules that support it + require('@balena/es-version').set('es2018'); -// Run the CLI -require('../build/app').run(); + // Run the CLI + await require('../build/app').run(); +} + +run(); diff --git a/bin/balena-dev b/bin/balena-dev index 951cf394..a0435e58 100755 --- a/bin/balena-dev +++ b/bin/balena-dev @@ -11,6 +11,22 @@ // operations otherwise, if the pool runs out. process.env.UV_THREADPOOL_SIZE = '64'; +// Note on `fast-boot2`: We do not use `fast-boot2` with `balena-dev` because: +// * fast-boot2's cacheKiller option is configured to include the timestamps of +// the package.json and npm-shrinkwrap.json files, to avoid unexpected CLI +// behavior when changes are made to dependencies during development. This is +// generally a good thing, however, `balena-dev` (a few lines below) edits +// `package.json` to modify oclif paths, and this results in cache +// invalidation and a performance hit rather than speedup. +// * Even if the timestamps are removed from cacheKiller, so that there is no +// cache invalidation, fast-boot's speedup is barely noticeable when ts-node +// is used, e.g. 1.43s vs 1.4s when running `balena version`. +// * `fast-boot` causes unexpected behavior when used with `npm link` or +// when the `node_modules` folder is manually modified (affecting transitive +// dependencies) during development (e.g. bug investigations). A workaround +// is to use `balena-dev` without `fast-boot`. See also notes in +// `CONTRIBUTING.md`. + const path = require('path'); const rootDir = path.join(__dirname, '..'); @@ -31,12 +47,6 @@ process.on('SIGINT', function () { process.exit(); }); -// Use fast-boot to cache require lookups, speeding up startup -require('fast-boot2').start({ - cacheScope: __dirname + '/..', - cacheFile: '.fast-boot.json', -}); - // Set the desired es version for downstream modules that support it require('@balena/es-version').set('es2018'); diff --git a/lib/app.ts b/lib/app.ts index a5897bb2..d5c4292d 100644 --- a/lib/app.ts +++ b/lib/app.ts @@ -97,24 +97,38 @@ async function oclifRun( command: string[], options: import('./preparser').AppOptions, ) { - const { CustomMain } = await import('./utils/oclif-utils'); - const runPromise = CustomMain.run(command).then( - () => { - if (!options.noFlush) { - return require('@oclif/command/flush'); - } - }, - (error) => { - // oclif sometimes exits with ExitError code 0 (not an error) + const runPromise = (async function (shouldFlush: boolean) { + const { CustomMain } = await import('./utils/oclif-utils'); + let isEEXIT = false; + try { + await CustomMain.run(command); + } catch (error) { + // oclif sometimes exits with ExitError code EEXIT 0 (not an error), + // for example the `balena help` command. // (Avoid `error instanceof ExitError` here for the reasons explained // in the CONTRIBUTING.md file regarding the `instanceof` operator.) if (error.oclif?.exit === 0) { - return; + isEEXIT = true; } else { throw error; } - }, - ); + } + if (shouldFlush) { + await import('@oclif/command/flush'); + } + // TODO: figure out why we need to call fast-boot stop() here, in + // addition to calling it in the main `run()` function in this file. + // If it is not called here as well, there is a process exit delay of + // 1 second when the fast-boot2 cache is modified (1 second is the + // default cache saving timeout). Try for example `balena help`. + // I have found that, when oclif's `Error: EEXIT: 0` is caught in + // the try/catch block above, execution does not get past the + // Promise.all() call below, but I don't understand why. + if (isEEXIT) { + (await import('./fast-boot')).stop(); + } + })(!options.noFlush); + const { trackPromise } = await import('./hooks/prerun/track'); await Promise.all([trackPromise, runPromise]); } @@ -146,6 +160,13 @@ export async function run( } catch (err) { await (await import('./errors')).handleError(err); } finally { + try { + (await import('./fast-boot')).stop(); + } catch (e) { + if (process.env.DEBUG) { + console.error(`[debug] Stopping fast-boot: ${e}`); + } + } // Windows fix: reading from stdin prevents the process from exiting process.stdin.pause(); } diff --git a/lib/fast-boot.ts b/lib/fast-boot.ts new file mode 100644 index 00000000..d31febc3 --- /dev/null +++ b/lib/fast-boot.ts @@ -0,0 +1,119 @@ +/** + * @license + * Copyright 2021 Balena Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This module sets up the `fast-boot2` module, including testing whether + * we have permissions over the cache file before even attempting to load + * fast boot. + * DON'T IMPORT BALENA-CLI MODULES HERE, as this module is loaded directly + * from `bin/balena`, before the CLI's entrypoint in `lib/app.ts`. + */ + +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +// `@types/node` does not know about `options: { bigint?: boolean }` +type statT = ( + fPath: string, + options: { bigint?: boolean }, +) => fs.Stats | Promise; + +// async stat does not work with pkg's internal `/snapshot` filesystem +const stat: statT = process.pkg ? fs.statSync : fs.promises.stat; + +let fastBootStarted = false; + +export async function start() { + if (fastBootStarted) { + return; + } + try { + await $start(); + fastBootStarted = true; + } catch (e) { + if (process.env.DEBUG) { + console.error(`\ +[debug] Unable to start 'fast-boot2': +[debug] ${(e.message || '').split('\n').join('\n[debug] ')} +[debug] The CLI should still work, but it will run a bit slower.`); + } + } +} + +export function stop() { + if (fastBootStarted) { + require('fast-boot2').stop(); + } + fastBootStarted = false; +} + +async function $start() { + const dotBalena = process.platform === 'win32' ? '_balena' : '.balena'; + // TODO: take into account `~/.balenarc.yml` or `./balenarc.yml`, + // without hurting performance at this early loading stage. + const dataDir = path.normalize( + process.env.BALENARC_DATA_DIRECTORY || path.join(os.homedir(), dotBalena), + ); + // Consider that the CLI may be installed to a folder owned by root + // such as `/usr[/local]/lib/balena-cli`, while being executed by + // a regular user account. + const cacheFile = path.join(dataDir, 'cli-module-cache.json'); + const root = path.join(__dirname, '..'); + const [, pJson, pStat, nStat] = await Promise.all([ + ensureCanWrite(dataDir, cacheFile), + import('../package.json'), + stat(path.join(root, 'package.json'), { bigint: true }), + stat(path.join(root, 'npm-shrinkwrap.json'), { bigint: true }), + ]); + // Include timestamps to account for dev-time changes to node_modules + const cacheKiller = `${pJson.version}-${pStat.mtimeMs}-${nStat.mtimeMs}`; + require('fast-boot2').start({ + cacheFile, + cacheKiller, + cacheScope: root, + }); +} + +/** + * Check that `file` has write permission. If so, return straight away. + * Throw an error if: + * - `file` exists but does have write permissions. + * - `file` does not exist and `dir` exists, but `dir` does not have + * write permissions. + * - `file` does not exist and `dir` does not exist, and an attempt + * to create `dir` failed. + */ +async function ensureCanWrite(dir: string, file: string) { + const { access, mkdir } = fs.promises; + try { + try { + await access(file, fs.constants.W_OK); + return; + } catch (e) { + // OK if file does not exist + if (e.code !== 'ENOENT') { + throw e; + } + } + // file does not exist; ensure that the directory is writable + await mkdir(dir, { recursive: true, mode: 0o755 }); + await access(dir, fs.constants.W_OK); + } catch (e) { + throw new Error(`Unable to write file "${file}":\n${e.message}`); + } +} diff --git a/package.json b/package.json index 25d11840..0a55b15d 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "node_modules/open/xdg-open", "node_modules/windosu/*.bat", "node_modules/windosu/*.cmd", + "npm-shrinkwrap.json", "oclif.manifest.json" ] },