From 33ca5c92cc1513ce77eea0fd8268b3e983aa329f Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Mon, 16 May 2022 17:16:49 -0700 Subject: [PATCH] fix(@angular/cli): skip prompt or warn when setting up autocompletion without a global CLI install If the user does not have a global install of the Angular CLI, the autocompletion prompt is skipped and `ng completion` emits a warning. The reasoning for this is that `source <(ng completion script)` won't work without `ng` on the `$PATH`, which is only really viable with a global install. Local executions like `git clone ... && npm install && npm start` or ephemeral executions like `npx @angular/cli` don't benefit from autocompletion and unnecessarily impede users. A global install of the Angular CLI is detected by running `which -a ng`, which appears to be a cross-platform means of listing all `ng` commands on the `$PATH`. We then look over all binaries in the list and exclude anything which is a directo child of a `node_modules/.bin/` directory. These include local executions and `npx`, so the only remaining locations should be global installs (`/usr/bin/ng`, NVM, etc.). The tests are a little awkward since `ng` is installed globally by helper functions before tests start. These tests uninstall the global CLI and install a local, project-specific version to verify behavior, before restoring the global version. Hypothetically this could be emulated by manipulating the `$PATH` variable, but `which` needs to be available (so we can't clobber the whole `$PATH`) and `node` exists in the same directory as the global `ng` command (so we can't remove that directory anyways). There's also no good way of testing the case where `which` fails to run. Closes #23135. --- .../cli/src/commands/completion/cli.ts | 18 +++-- .../angular/cli/src/utilities/completion.ts | 69 +++++++++++++++++++ .../e2e/tests/misc/completion-prompt.ts | 62 ++++++++++++++++- tests/legacy-cli/e2e/tests/misc/completion.ts | 55 ++++++++++++++- tests/legacy-cli/e2e/utils/process.ts | 27 ++++++-- 5 files changed, 218 insertions(+), 13 deletions(-) diff --git a/packages/angular/cli/src/commands/completion/cli.ts b/packages/angular/cli/src/commands/completion/cli.ts index 6879726592a9..2bfd5a7152e7 100644 --- a/packages/angular/cli/src/commands/completion/cli.ts +++ b/packages/angular/cli/src/commands/completion/cli.ts @@ -8,14 +8,10 @@ import { join } from 'path'; import yargs, { Argv } from 'yargs'; -import { - CommandModule, - CommandModuleImplementation, - CommandScope, -} from '../../command-builder/command-module'; +import { CommandModule, CommandModuleImplementation } from '../../command-builder/command-module'; import { addCommandModuleToYargs } from '../../command-builder/utilities/command'; import { colors } from '../../utilities/color'; -import { initializeAutocomplete } from '../../utilities/completion'; +import { hasGlobalCliInstall, initializeAutocomplete } from '../../utilities/completion'; export class CompletionCommandModule extends CommandModule implements CommandModuleImplementation { command = 'completion'; @@ -44,6 +40,16 @@ Appended \`source <(ng completion script)\` to \`${rcFile}\`. Restart your termi `.trim(), ); + if ((await hasGlobalCliInstall()) === false) { + this.context.logger.warn( + 'Setup completed successfully, but there does not seem to be a global install of the' + + ' Angular CLI. For autocompletion to work, the CLI will need to be on your `$PATH`, which' + + ' is typically done with the `-g` flag in `npm install -g @angular/cli`.' + + '\n\n' + + 'For more information, see https://angular.io/cli/completion#global-install', + ); + } + return 0; } } diff --git a/packages/angular/cli/src/utilities/completion.ts b/packages/angular/cli/src/utilities/completion.ts index 546f962543d4..d95d60c79b33 100644 --- a/packages/angular/cli/src/utilities/completion.ts +++ b/packages/angular/cli/src/utilities/completion.ts @@ -7,6 +7,7 @@ */ import { json, logging } from '@angular-devkit/core'; +import { execFile } from 'child_process'; import { promises as fs } from 'fs'; import * as path from 'path'; import { env } from 'process'; @@ -78,6 +79,16 @@ Appended \`source <(ng completion script)\` to \`${rcFile}\`. Restart your termi `.trim(), ); + if ((await hasGlobalCliInstall()) === false) { + logger.warn( + 'Setup completed successfully, but there does not seem to be a global install of the' + + ' Angular CLI. For autocompletion to work, the CLI will need to be on your `$PATH`, which' + + ' is typically done with the `-g` flag in `npm install -g @angular/cli`.' + + '\n\n' + + 'For more information, see https://angular.io/cli/completion#global-install', + ); + } + // Save configuration to remember that the user was prompted. await setCompletionConfig({ ...completionConfig, prompted: true }); @@ -147,6 +158,12 @@ async function shouldPromptForAutocompletionSetup( return false; // Unknown shell. } + // Don't prompt if the user is missing a global CLI install. Autocompletion won't work after setup + // anyway and could be annoying for users running one-off commands via `npx` or using `npm start`. + if ((await hasGlobalCliInstall()) === false) { + return false; + } + // Check each RC file if they already use `ng completion script` in any capacity and don't prompt. for (const rcFile of rcFiles) { const contents = await fs.readFile(rcFile, 'utf-8').catch(() => undefined); @@ -246,3 +263,55 @@ function getShellRunCommandCandidates(shell: string, home: string): string[] | u return undefined; } } + +/** + * Returns whether the user has a global CLI install or `undefined` if this can't be determined. + * Execution from `npx` is *not* considered a global CLI install. + * + * This does *not* mean the current execution is from a global CLI install, only that a global + * install exists on the system. + */ +export async function hasGlobalCliInstall(): Promise { + // List all binaries with the `ng` name on the user's `$PATH`. + const proc = execFile('which', ['-a', 'ng']); + let stdout = ''; + proc.stdout?.addListener('data', (content) => { + stdout += content; + }); + const exitCode = await new Promise((resolve) => { + proc.addListener('exit', (exitCode) => { + resolve(exitCode); + }); + }); + + switch (exitCode) { + case 0: + // Successfully listed all `ng` binaries on the `$PATH`. Look for at least one line which is a + // global install. We can't easily identify global installs, but local installs are typically + // placed in `node_modules/.bin` by NPM / Yarn. `npx` also currently caches files at + // `~/.npm/_npx/*/node_modules/.bin/`, so the same logic applies. + const lines = stdout.split('\n').filter((line) => line !== ''); + const hasGlobalInstall = lines.some((line) => { + // A binary is a local install if it is a direct child of a `node_modules/.bin/` directory. + const parent = path.parse(path.parse(line).dir); + const grandparent = path.parse(parent.dir); + const localInstall = grandparent.base === 'node_modules' && parent.base === '.bin'; + + return !localInstall; + }); + + return hasGlobalInstall; + case 1: + // No instances of `ng` on the user's `$PATH`. + return false; + case null: + // `which` was killed by a signal and did not exit gracefully. Maybe it hung or something else + // went very wrong, so treat this as inconclusive. + return undefined; + default: + // `which` returns exit code 2 if an invalid option is specified and `-a` doesn't appear to be + // supported on all systems. Other exit codes mean unknown errors occurred. Can't tell whether + // CLI is globally installed, so treat this as inconclusive. + return undefined; + } +} diff --git a/tests/legacy-cli/e2e/tests/misc/completion-prompt.ts b/tests/legacy-cli/e2e/tests/misc/completion-prompt.ts index 2fd74de20bcd..983fc55f87e4 100644 --- a/tests/legacy-cli/e2e/tests/misc/completion-prompt.ts +++ b/tests/legacy-cli/e2e/tests/misc/completion-prompt.ts @@ -2,7 +2,13 @@ import { promises as fs } from 'fs'; import * as os from 'os'; import * as path from 'path'; import { env } from 'process'; -import { execAndCaptureError, execWithEnv } from '../../utils/process'; +import { getGlobalVariable } from '../../utils/env'; +import { + execAndCaptureError, + execAndWaitForOutputToMatch, + execWithEnv, + silentNpm, +} from '../../utils/process'; const AUTOCOMPLETION_PROMPT = /Would you like to enable autocompletion\?/; const DEFAULT_ENV = Object.freeze({ @@ -18,6 +24,8 @@ const DEFAULT_ENV = Object.freeze({ NG_CLI_ANALYTICS: 'false', }); +const testRegistry = getGlobalVariable('package-registry'); + export default async function () { // Windows Cmd and Powershell do not support autocompletion. Run a different set of tests to // confirm autocompletion skips the prompt appropriately. @@ -368,6 +376,58 @@ source <(ng completion script) ); } }); + + // Prompts when a global CLI install is present on the system. + await mockHome(async (home) => { + const bashrc = path.join(home, '.bashrc'); + await fs.writeFile(bashrc, `# Other content...`); + + await execAndWaitForOutputToMatch('ng', ['version'], AUTOCOMPLETION_PROMPT, { + ...DEFAULT_ENV, + SHELL: '/bin/bash', + HOME: home, + }); + }); + + // Does *not* prompt when a global CLI install is missing from the system. + await mockHome(async (home) => { + try { + // Temporarily uninstall the global CLI binary from the system. + await silentNpm(['uninstall', '--global', '@angular/cli', `--registry=${testRegistry}`]); + + // Setup a fake project directory with a local install of the CLI. + const projectDir = path.join(home, 'project'); + await fs.mkdir(projectDir); + await silentNpm(['init', '-y', `--registry=${testRegistry}`], { cwd: projectDir }); + await silentNpm(['install', '@angular/cli', `--registry=${testRegistry}`], { + cwd: projectDir, + }); + + const bashrc = path.join(home, '.bashrc'); + await fs.writeFile(bashrc, `# Other content...`); + + const localCliDir = path.join(projectDir, 'node_modules', '.bin'); + const localCliBinary = path.join(localCliDir, 'ng'); + const pathDirs = process.env['PATH'].split(':'); + const pathEnvVar = [...pathDirs, localCliDir].join(':'); + const { stdout } = await execWithEnv(localCliBinary, ['version'], { + ...DEFAULT_ENV, + SHELL: '/bin/bash', + HOME: home, + PATH: pathEnvVar, + }); + + if (AUTOCOMPLETION_PROMPT.test(stdout)) { + throw new Error( + 'Execution without a global CLI install prompted for autocompletion setup but should' + + ' not have.', + ); + } + } finally { + // Reinstall global CLI for remainder of the tests. + await silentNpm(['install', '--global', '@angular/cli', `--registry=${testRegistry}`]); + } + }); } async function windowsTests(): Promise { diff --git a/tests/legacy-cli/e2e/tests/misc/completion.ts b/tests/legacy-cli/e2e/tests/misc/completion.ts index f7ce6243fa8e..90061319191a 100644 --- a/tests/legacy-cli/e2e/tests/misc/completion.ts +++ b/tests/legacy-cli/e2e/tests/misc/completion.ts @@ -1,7 +1,16 @@ +import { execFile } from 'child_process'; import { promises as fs } from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { execAndCaptureError, execAndWaitForOutputToMatch } from '../../utils/process'; +import { getGlobalVariable } from '../../utils/env'; +import { + execAndCaptureError, + execAndWaitForOutputToMatch, + execWithEnv, + silentNpm, +} from '../../utils/process'; + +const testRegistry = getGlobalVariable('package-registry'); export default async function () { // Windows Cmd and Powershell do not support autocompletion. Run a different set of tests to @@ -332,6 +341,50 @@ source <(ng completion script) throw new Error(`Expected unknown \`$SHELL\` error message, but got:\n\n${err.message}`); } }); + + // Does *not* warn when a global CLI install is present on the system. + await mockHome(async (home) => { + const { stdout } = await execWithEnv('ng', ['completion'], { + ...process.env, + 'SHELL': '/usr/bin/zsh', + 'HOME': home, + }); + + if (stdout.includes('there does not seem to be a global install of the Angular CLI')) { + throw new Error(`CLI warned about missing global install, but one should exist.`); + } + }); + + // Warns when a global CLI install is *not* present on the system. + await mockHome(async (home) => { + try { + // Temporarily uninstall the global CLI binary from the system. + await silentNpm(['uninstall', '--global', '@angular/cli', `--registry=${testRegistry}`]); + + // Setup a fake project directory with a local install of the CLI. + const projectDir = path.join(home, 'project'); + await fs.mkdir(projectDir); + await silentNpm(['init', '-y', `--registry=${testRegistry}`], { cwd: projectDir }); + await silentNpm(['install', '@angular/cli', `--registry=${testRegistry}`], { + cwd: projectDir, + }); + + // Invoke the local CLI binary. + const localCliBinary = path.join(projectDir, 'node_modules', '.bin', 'ng'); + const { stdout } = await execWithEnv(localCliBinary, ['completion'], { + ...process.env, + 'SHELL': '/usr/bin/zsh', + 'HOME': home, + }); + + if (stdout.includes('there does not seem to be a global install of the Angular CLI')) { + throw new Error(`CLI warned about missing global install, but one should exist.`); + } + } finally { + // Reinstall global CLI for remainder of the tests. + await silentNpm(['install', '--global', '@angular/cli', `--registry=${testRegistry}`]); + } + }); } async function windowsTests(): Promise { diff --git a/tests/legacy-cli/e2e/utils/process.ts b/tests/legacy-cli/e2e/utils/process.ts index 3db0dff78742..7a4100e44822 100644 --- a/tests/legacy-cli/e2e/utils/process.ts +++ b/tests/legacy-cli/e2e/utils/process.ts @@ -12,6 +12,7 @@ interface ExecOptions { waitForMatch?: RegExp; env?: { [varname: string]: string }; stdin?: string; + cwd?: string; } let _processes: child_process.ChildProcess[] = []; @@ -28,7 +29,7 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise { - err.message += `${error}...\n\nSTDOUT:\n${stdout}\n\nSTDERR:\n${stderr}\n`; + childProcess.on('error', (err) => { + err.message += `${err}...\n\nSTDOUT:\n${stdout}\n\nSTDERR:\n${stderr}\n`; reject(err); }); @@ -257,8 +258,24 @@ export function silentNg(...args: string[]) { return _exec({ silent: true }, 'ng', args); } -export function silentNpm(...args: string[]) { - return _exec({ silent: true }, 'npm', args); +export function silentNpm(...args: string[]): Promise; +export function silentNpm(args: string[], options?: { cwd?: string }): Promise; +export function silentNpm( + ...args: string[] | [args: string[], options?: { cwd?: string }] +): Promise { + if (Array.isArray(args[0])) { + const [params, options] = args; + return _exec( + { + silent: true, + cwd: (options as { cwd?: string } | undefined)?.cwd, + }, + 'npm', + params, + ); + } else { + return _exec({ silent: true }, 'npm', args as string[]); + } } export function silentYarn(...args: string[]) {