Skip to content

Commit

Permalink
fix(core): set windowsHide: true wherever possible (#28073)
Browse files Browse the repository at this point in the history
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #
  • Loading branch information
MaxKless authored Sep 24, 2024
1 parent 8290969 commit b73f1e0
Show file tree
Hide file tree
Showing 81 changed files with 366 additions and 107 deletions.
1 change: 1 addition & 0 deletions e2e/utils/command-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ export function runCommandUntil(
...opts.env,
FORCE_COLOR: 'false',
},
windowsHide: true,
});
return new Promise((res, rej) => {
let output = '';
Expand Down
16 changes: 11 additions & 5 deletions e2e/utils/global-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,17 @@ export default async function (globalConfig: Config.ConfigGlobals) {
function getPublishedVersion(): Promise<string | undefined> {
return new Promise((resolve) => {
// Resolve the published nx version from verdaccio
exec('npm view nx@latest version', (error, stdout, stderr) => {
if (error) {
return resolve(undefined);
exec(
'npm view nx@latest version',
{
windowsHide: true,
},
(error, stdout, stderr) => {
if (error) {
return resolve(undefined);
}
return resolve(stdout.trim());
}
return resolve(stdout.trim());
});
);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function execAndWait(command: string, cwd: string) {
return new Promise<{ code: number; stdout: string }>((res, rej) => {
exec(
command,
{ cwd, env: { ...process.env, NX_DAEMON: 'false' } },
{ cwd, env: { ...process.env, NX_DAEMON: 'false' }, windowsHide: true },
(error, stdout, stderr) => {
if (error) {
const logFile = join(cwd, 'error.log');
Expand Down
1 change: 1 addition & 0 deletions packages/create-nx-workspace/src/utils/git/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export async function initializeGitRepo(
}
: {}),
},
windowsHide: true,
};
return new Promise<void>((resolve, reject) => {
spawn('git', args, spawnOptions).on('close', (code) => {
Expand Down
5 changes: 4 additions & 1 deletion packages/cypress/plugins/cypress-preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,15 @@ function startWebServer(webServerCommand: string) {
// Windows is fine so we leave it attached to this process
detached: process.platform !== 'win32',
stdio: 'inherit',
windowsHide: true,
});

return () => {
if (process.platform === 'win32') {
try {
execSync('taskkill /pid ' + serverProcess.pid + ' /T /F');
execSync('taskkill /pid ' + serverProcess.pid + ' /T /F', {
windowsHide: true,
});
} catch (e) {
if (process.env.NX_VERBOSE_LOGGING === 'true') {
console.error(e);
Expand Down
1 change: 1 addition & 0 deletions packages/devkit/src/tasks/install-packages-task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export function installPackagesTask(
const execSyncOptions: ExecSyncOptions = {
cwd: join(tree.root, cwd),
stdio: process.env.NX_GENERATE_QUIET === 'true' ? 'ignore' : 'inherit',
windowsHide: true,
};
// ensure local registry from process is not interfering with the install
// when we start the process from temp folder the local registry would override the custom registry
Expand Down
2 changes: 2 additions & 0 deletions packages/devkit/src/utils/package-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ export function ensurePackage<T extends any = any>(
execSync(preInstallCommand, {
cwd: tempDir,
stdio: isVerbose ? 'inherit' : 'ignore',
windowsHide: true,
});
}
let addCommand = getPackageManagerCommand(packageManager).addDev;
Expand All @@ -507,6 +508,7 @@ export function ensurePackage<T extends any = any>(
execSync(`${addCommand} ${pkg}@${requiredVersion}`, {
cwd: tempDir,
stdio: isVerbose ? 'inherit' : 'ignore',
windowsHide: true,
});

addToNodePath(join(workspaceRoot, 'node_modules'));
Expand Down
2 changes: 2 additions & 0 deletions packages/expo/src/utils/pod-install-task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export function podInstall(
execSync('touch .xcode.env', {
cwd: iosDirectory,
stdio: 'inherit',
windowsHide: true,
});
}
execSync(
Expand All @@ -77,6 +78,7 @@ export function podInstall(
{
cwd: iosDirectory,
stdio: 'inherit',
windowsHide: true,
}
);
} catch (e) {
Expand Down
4 changes: 2 additions & 2 deletions packages/expo/src/utils/resolve-eas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ export function resolveEas(workspaceRoot: string): string {

let npmGlobalPath: string, yarnGlobalPath: string;
try {
npmGlobalPath = execSync('npm root -g')
npmGlobalPath = execSync('npm root -g', { windowsHide: true })
?.toString()
?.trim()
?.replace('\u001b[2K\u001b[1G', ''); // strip out ansi codes
} catch {}
try {
yarnGlobalPath = execSync('yarn global dir')
yarnGlobalPath = execSync('yarn global dir', { windowsHide: true })
?.toString()
?.trim()
?.replace('\u001b[2K\u001b[1G', ''); // strip out ansi codes
Expand Down
34 changes: 21 additions & 13 deletions packages/js/src/executors/node/lib/kill-tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,27 @@ export async function killTree(pid: number, signal: NodeJS.Signals) {

switch (process.platform) {
case 'win32':
exec('taskkill /pid ' + pid + ' /T /F', (error) => {
// Ignore Fatal errors (128) because it might be due to the process already being killed.
// On Linux/Mac we can check ESRCH (no such process), but on Windows we can't.
callback(error?.code !== 128 ? error : null);
});
exec(
'taskkill /pid ' + pid + ' /T /F',
{
windowsHide: true,
},
(error) => {
// Ignore Fatal errors (128) because it might be due to the process already being killed.
// On Linux/Mac we can check ESRCH (no such process), but on Windows we can't.
callback(error?.code !== 128 ? error : null);
}
);
break;
case 'darwin':
buildProcessTree(
pid,
tree,
pidsToProcess,
function (parentPid) {
return spawn('pgrep', ['-P', parentPid]);
return spawn('pgrep', ['-P', parentPid], {
windowsHide: true,
});
},
function () {
killAll(tree, signal, callback);
Expand All @@ -43,13 +51,13 @@ export async function killTree(pid: number, signal: NodeJS.Signals) {
tree,
pidsToProcess,
function (parentPid) {
return spawn('ps', [
'-o',
'pid',
'--no-headers',
'--ppid',
parentPid,
]);
return spawn(
'ps',
['-o', 'pid', '--no-headers', '--ppid', parentPid],
{
windowsHide: true,
}
);
},
function () {
killAll(tree, signal, callback);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ Please update the local dependency on "${depName}" to be a valid semantic versio
env: processEnv(true),
cwd: context.root,
stdio: ['ignore', 'pipe', 'pipe'],
windowsHide: true,
});

const resultJson = JSON.parse(result.toString());
Expand All @@ -153,6 +154,7 @@ Please update the local dependency on "${depName}" to be a valid semantic versio
env: processEnv(true),
cwd: context.root,
stdio: 'ignore',
windowsHide: true,
});
console.log(
`Added the dist-tag ${tag} to v${currentVersion} for registry ${registry}.\n`
Expand Down Expand Up @@ -267,6 +269,7 @@ Please update the local dependency on "${depName}" to be a valid semantic versio
env: processEnv(true),
cwd: context.root,
stdio: ['ignore', 'pipe', 'pipe'],
windowsHide: true,
});

/**
Expand Down
37 changes: 23 additions & 14 deletions packages/js/src/executors/verdaccio/verdaccio.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ function createVerdaccioOptions(

function setupNpm(options: VerdaccioExecutorSchema) {
try {
execSync('npm --version', { env });
execSync('npm --version', { env, windowsHide: true });
} catch (e) {
return () => {};
}
Expand All @@ -151,20 +151,20 @@ function setupNpm(options: VerdaccioExecutorSchema) {
npmRegistryPaths.push(
execSync(
`npm config get ${registryName} --location ${options.location}`,
{ env }
{ env, windowsHide: true }
)
?.toString()
?.trim()
?.replace('\u001b[2K\u001b[1G', '') // strip out ansi codes
);
execSync(
`npm config set ${registryName} http://localhost:${options.port}/ --location ${options.location}`,
{ env }
{ env, windowsHide: true }
);

execSync(
`npm config set //localhost:${options.port}/:_authToken="secretVerdaccioToken" --location ${options.location}`,
{ env }
{ env, windowsHide: true }
);

logger.info(
Expand All @@ -181,7 +181,7 @@ function setupNpm(options: VerdaccioExecutorSchema) {
try {
const currentNpmRegistryPath = execSync(
`npm config get registry --location ${options.location}`,
{ env }
{ env, windowsHide: true }
)
?.toString()
?.trim()
Expand All @@ -194,7 +194,7 @@ function setupNpm(options: VerdaccioExecutorSchema) {
) {
execSync(
`npm config set ${registryName} ${npmRegistryPaths[index]} --location ${options.location}`,
{ env }
{ env, windowsHide: true }
);
logger.info(
`Reset npm ${registryName} to ${npmRegistryPaths[index]}`
Expand All @@ -204,14 +204,15 @@ function setupNpm(options: VerdaccioExecutorSchema) {
`npm config delete ${registryName} --location ${options.location}`,
{
env,
windowsHide: true,
}
);
logger.info('Cleared custom npm registry');
}
});
execSync(
`npm config delete //localhost:${options.port}/:_authToken --location ${options.location}`,
{ env }
{ env, windowsHide: true }
);
} catch (e) {
throw new Error(`Failed to reset npm registry: ${e.message}`);
Expand All @@ -230,6 +231,7 @@ function getYarnUnsafeHttpWhitelist(isYarnV1: boolean) {
JSON.parse(
execSync(`yarn config get unsafeHttpWhitelist --json`, {
env,
windowsHide: true,
}).toString()
)
)
Expand All @@ -245,13 +247,13 @@ function setYarnUnsafeHttpWhitelist(
`yarn config set unsafeHttpWhitelist --json '${JSON.stringify(
Array.from(currentWhitelist)
)}'` + (options.location === 'user' ? ' --home' : ''),
{ env }
{ env, windowsHide: true }
);
} else {
execSync(
`yarn config unset unsafeHttpWhitelist` +
(options.location === 'user' ? ' --home' : ''),
{ env }
{ env, windowsHide: true }
);
}
}
Expand All @@ -263,7 +265,9 @@ function setupYarn(options: VerdaccioExecutorSchema) {

try {
isYarnV1 =
major(execSync('yarn --version', { env }).toString().trim()) === 1;
major(
execSync('yarn --version', { env, windowsHide: true }).toString().trim()
) === 1;
} catch {
// This would fail if yarn is not installed which is okay
return () => {};
Expand All @@ -277,6 +281,7 @@ function setupYarn(options: VerdaccioExecutorSchema) {
yarnRegistryPaths.push(
execSync(`yarn config get ${scopeName}${registryConfigName}`, {
env,
windowsHide: true,
})
?.toString()
?.trim()
Expand All @@ -286,7 +291,7 @@ function setupYarn(options: VerdaccioExecutorSchema) {
execSync(
`yarn config set ${scopeName}${registryConfigName} http://localhost:${options.port}/` +
(options.location === 'user' ? ' --home' : ''),
{ env }
{ env, windowsHide: true }
);

logger.info(
Expand All @@ -313,7 +318,7 @@ function setupYarn(options: VerdaccioExecutorSchema) {
try {
const currentYarnRegistryPath = execSync(
`yarn config get ${registryConfigName}`,
{ env }
{ env, windowsHide: true }
)
?.toString()
?.trim()
Expand All @@ -331,7 +336,11 @@ function setupYarn(options: VerdaccioExecutorSchema) {
execSync(
`yarn config set ${registryName} ${yarnRegistryPaths[index]}` +
(options.location === 'user' ? ' --home' : ''),
{ env }
{
env,

windowsHide: true,
}
);
logger.info(
`Reset yarn ${registryName} to ${yarnRegistryPaths[index]}`
Expand All @@ -340,7 +349,7 @@ function setupYarn(options: VerdaccioExecutorSchema) {
execSync(
`yarn config ${isYarnV1 ? 'delete' : 'unset'} ${registryName}` +
(options.location === 'user' ? ' --home' : ''),
{ env }
{ env, windowsHide: true }
);
logger.info(`Cleared custom yarn ${registryConfigName}`);
}
Expand Down
3 changes: 3 additions & 0 deletions packages/js/src/generators/release-version/release-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ To fix this you will either need to add a package.json file at that location, or
currentVersion = await new Promise<string>((resolve, reject) => {
exec(
`npm view ${packageName} version --"${registryConfigKey}=${registry}" --tag=${tag}`,
{
windowsHide: true,
},
(error, stdout, stderr) => {
if (error) {
return reject(error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ function execLockFileUpdate(
...process.env,
...env,
},
windowsHide: true,
});
} catch (e) {
output.error({
Expand Down
7 changes: 5 additions & 2 deletions packages/js/src/generators/setup-verdaccio/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ export async function setupVerdaccio(
if (!tree.exists('.verdaccio/config.yml')) {
generateFiles(tree, path.join(__dirname, 'files'), '.verdaccio', {
npmUplinkRegistry:
execSync('npm config get registry')?.toString()?.trim() ??
'https://registry.npmjs.org',
execSync('npm config get registry', {
windowsHide: true,
})
?.toString()
?.trim() ?? 'https://registry.npmjs.org',
});
}

Expand Down
Loading

0 comments on commit b73f1e0

Please sign in to comment.