Skip to content

Commit

Permalink
fix(@angular/cli): skip prompt or warn when setting up autocompletion…
Browse files Browse the repository at this point in the history
… 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.

(cherry picked from commit b79b0f0)
  • Loading branch information
dgp1130 committed May 18, 2022
1 parent 69b42dc commit e5bdada
Show file tree
Hide file tree
Showing 5 changed files with 218 additions and 13 deletions.
18 changes: 12 additions & 6 deletions packages/angular/cli/src/commands/completion/cli.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
}
Expand Down
69 changes: 69 additions & 0 deletions packages/angular/cli/src/utilities/completion.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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 });

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<boolean | undefined> {
// 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<number | null>((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;
}
}
62 changes: 61 additions & 1 deletion tests/legacy-cli/e2e/tests/misc/completion-prompt.ts
Expand Up @@ -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({
Expand All @@ -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.
Expand Down Expand Up @@ -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<void> {
Expand Down
55 changes: 54 additions & 1 deletion 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
Expand Down Expand Up @@ -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<void> {
Expand Down
27 changes: 22 additions & 5 deletions tests/legacy-cli/e2e/utils/process.ts
Expand Up @@ -12,6 +12,7 @@ interface ExecOptions {
waitForMatch?: RegExp;
env?: { [varname: string]: string };
stdin?: string;
cwd?: string;
}

let _processes: child_process.ChildProcess[] = [];
Expand All @@ -28,7 +29,7 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<Proce

let stdout = '';
let stderr = '';
const cwd = process.cwd();
const cwd = options.cwd ?? process.cwd();
const env = options.env;
console.log(
`==========================================================================================`,
Expand Down Expand Up @@ -108,8 +109,8 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<Proce
),
);
});
childProcess.on('error', (error) => {
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);
});

Expand Down Expand Up @@ -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<ProcessOutput>;
export function silentNpm(args: string[], options?: { cwd?: string }): Promise<ProcessOutput>;
export function silentNpm(
...args: string[] | [args: string[], options?: { cwd?: string }]
): Promise<ProcessOutput> {
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[]) {
Expand Down

0 comments on commit e5bdada

Please sign in to comment.