From 3ea1e310de1e3b7cf5f35c8cb628060b565fe07a Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Thu, 24 Aug 2023 13:12:48 -0600 Subject: [PATCH 1/3] feat: check for renamed yarn.lock during install --- src/plugins.ts | 23 ++++++++++++++++++++--- src/yarn.ts | 39 ++------------------------------------- 2 files changed, 22 insertions(+), 40 deletions(-) diff --git a/src/plugins.ts b/src/plugins.ts index 1da8b8b6..de576cdf 100644 --- a/src/plugins.ts +++ b/src/plugins.ts @@ -15,6 +15,15 @@ const initPJSON: Interfaces.PJSON.User = { dependencies: {}, } +async function fileExists(filePath: string): Promise { + try { + await fs.promises.access(filePath) + return true + } catch { + return false + } +} + export default class Plugins { verbose = false; @@ -124,15 +133,23 @@ export default class Plugins { root: string, {prod = true}: { prod?: boolean } = {}, ): Promise { - if (fs.existsSync(path.join(root, 'yarn.lock'))) { - this.debug(`yarn.lock exists at ${root}. Installing prod dependencies`) + const doRefresh = async () => { // use yarn.lock to fetch dependencies await this.yarn.exec(prod ? ['--prod'] : [], { cwd: root, verbose: this.verbose, }) + } + + if (await fileExists(path.join(root, 'deps.lock'))) { + this.debug(`deps.lock exists at ${root}. Installing prod dependencies`) + await fs.promises.rename(path.join(root, 'deps.lock'), path.join(root, 'yarn.lock')) + await doRefresh() + } else if (await fileExists(path.join(root, 'yarn.lock'))) { + this.debug(`yarn.lock exists at ${root}. Installing prod dependencies`) + await doRefresh() } else { - this.debug(`no yarn.lock exists at ${root}. Skipping dependency refresh`) + this.debug(`no yarn.lock or deps.lock exists at ${root}. Skipping dependency refresh`) } } diff --git a/src/yarn.ts b/src/yarn.ts index 7d1c22c0..2dad9d1b 100644 --- a/src/yarn.ts +++ b/src/yarn.ts @@ -1,5 +1,5 @@ import {Interfaces, ux} from '@oclif/core' -import {fork, spawn} from 'child_process' +import {fork} from 'child_process' import NpmRunPath from 'npm-run-path' import * as path from 'path' @@ -42,37 +42,6 @@ export default class Yarn { }) } - spawn(executable: string, args: string[] = [], options: any = {}): Promise { - return new Promise((resolve, reject) => { - const spawned = spawn(executable, args, {...options, shell: true}) - spawned.stderr.setEncoding('utf8') - spawned.stderr.on('data', (d: any) => { - debug('spawned yarn stderr:', d) - process.stderr.write(d) - }) - spawned.stdout.setEncoding('utf8') - spawned.stdout.on('data', (d: any) => { - debug('spawned yarn stdout:', d) - if (options.verbose) process.stdout.write(d) - else ux.action.status = d.replace(/\n$/, '').split('\n').pop() - }) - - spawned.on('error', reject) - spawned.on('exit', (code: number) => { - if (code === 0) { - resolve() - } else { - reject(new Error(`${executable} ${args.join(' ')} exited with code ${code}`)) - } - }) - - // Fix windows bug with node-gyp hanging for input forever - // if (this.config.windows) { - // forked.stdin.write('\n') - // } - }) - } - // eslint-disable-next-line default-param-last async exec(args: string[] = [], opts: {cwd: string; verbose: boolean}): Promise { const cwd = opts.cwd @@ -111,11 +80,7 @@ export default class Yarn { debug(`${cwd}: ${this.bin} ${args.join(' ')}`) try { - // TODO: always use spawn instead of fork once this has been thoroughly tested - this.config.scopedEnvVarTrue('PLUGINS_INSTALL_USE_SPAWN') ? - await this.spawn(this.bin, args, options) : - await this.fork(this.bin, args, options) - + await this.fork(this.bin, args, options) debug('yarn done') } catch (error: any) { debug('yarn error', error) From 269d6fbffe99275db85dc63eb42833804fd98a83 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 30 Aug 2023 16:00:48 -0600 Subject: [PATCH 2/3] chore: rename to oclif.lock --- src/plugins.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/plugins.ts b/src/plugins.ts index de576cdf..54f88631 100644 --- a/src/plugins.ts +++ b/src/plugins.ts @@ -141,15 +141,16 @@ export default class Plugins { }) } - if (await fileExists(path.join(root, 'deps.lock'))) { - this.debug(`deps.lock exists at ${root}. Installing prod dependencies`) - await fs.promises.rename(path.join(root, 'deps.lock'), path.join(root, 'yarn.lock')) + if (await fileExists(path.join(root, 'oclif.lock'))) { + this.debug(`oclif.lock exists at ${root}. Installing prod dependencies`) + await fs.promises.rename(path.join(root, 'oclif.lock'), path.join(root, 'yarn.lock')) await doRefresh() + await fs.promises.unlink(path.join(root, 'yarn.lock')) } else if (await fileExists(path.join(root, 'yarn.lock'))) { this.debug(`yarn.lock exists at ${root}. Installing prod dependencies`) await doRefresh() } else { - this.debug(`no yarn.lock or deps.lock exists at ${root}. Skipping dependency refresh`) + this.debug(`no yarn.lock or oclif.lock exists at ${root}. Skipping dependency refresh`) } } From f8dfe94a41a32fd9bc9ba05224290cf473de25b4 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Fri, 1 Sep 2023 09:35:17 -0600 Subject: [PATCH 3/3] chore: code review --- src/plugins.ts | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/plugins.ts b/src/plugins.ts index 54f88631..70b7de68 100644 --- a/src/plugins.ts +++ b/src/plugins.ts @@ -128,27 +128,41 @@ export default class Plugins { } } - // if yarn.lock exists, fetch locked dependencies + /** + * If a yarn.lock or oclif.lock exists at the root, refresh dependencies by + * rerunning yarn. If options.prod is true, only install production dependencies. + * + * As of v9 npm will always ignore the yarn.lock during `npm pack`] + * (see https://github.com/npm/cli/issues/6738). To get around this plugins can + * rename yarn.lock to oclif.lock before running `npm pack`. + * + * We still check for the existence of yarn.lock since it could be included if a plugin was + * packed using yarn or v8 of npm. Plugins installed directly from a git url will also + * have a yarn.lock. + * + * @param root string + * @param options {prod?: boolean} + * @returns Promise + */ async refresh( root: string, {prod = true}: { prod?: boolean } = {}, ): Promise { const doRefresh = async () => { - // use yarn.lock to fetch dependencies await this.yarn.exec(prod ? ['--prod'] : [], { cwd: root, verbose: this.verbose, }) } - if (await fileExists(path.join(root, 'oclif.lock'))) { + if (await fileExists(path.join(root, 'yarn.lock'))) { + this.debug(`yarn.lock exists at ${root}. Installing prod dependencies`) + await doRefresh() + } else if (await fileExists(path.join(root, 'oclif.lock'))) { this.debug(`oclif.lock exists at ${root}. Installing prod dependencies`) await fs.promises.rename(path.join(root, 'oclif.lock'), path.join(root, 'yarn.lock')) await doRefresh() await fs.promises.unlink(path.join(root, 'yarn.lock')) - } else if (await fileExists(path.join(root, 'yarn.lock'))) { - this.debug(`yarn.lock exists at ${root}. Installing prod dependencies`) - await doRefresh() } else { this.debug(`no yarn.lock or oclif.lock exists at ${root}. Skipping dependency refresh`) }