Stop logging the proc info when a command fails (#2716)

Stop logging the proc info which had no value and because rush decide it is so important to crop the logs and only show the end we never had meaningful info Before ![image](https://github.com/microsoft/typespec/assets/1031227/438962ed-cf83-4517-b8bd-2f1d86180add) Now ![image](https://github.com/microsoft/typespec/assets/1031227/5e738b2b-43e0-4a70-a4b5-c392803e2a94) --------- Co-authored-by: Brian Terlson <[email protected]>
microsoft · Jan 4, 2024 · 53aa383 · 53aa383
1 parent 8d55283
commit 53aa383
Show file tree

Hide file tree

Showing 14 changed files with 129 additions and 96 deletions.
diff --git a/common/changes/@typespec/internal-build-utils/internal-remove-proc-log_2023-12-04-17-14.json b/common/changes/@typespec/internal-build-utils/internal-remove-proc-log_2023-12-04-17-14.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@typespec/internal-build-utils",
+      "comment": "Add `runOrExit` helper to have a cleaner script runner without caring about the failure",
+      "type": "none"
+    }
+  ],
+  "packageName": "@typespec/internal-build-utils"
+}
diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml
diff --git a/e2e/e2e-tests.js b/e2e/e2e-tests.js
@@ -1,41 +1,42 @@
 // @ts-check
 import { existsSync, readdirSync, rmSync, writeFileSync } from "fs";
 import { join } from "path";
-import { repoRoot, run } from "../eng/scripts/helpers.js";
+import { repoRoot } from "../eng/scripts/helpers.js";
+import { runOrExit } from "../packages/internal-build-utils/dist/src/index.js";
 
 const e2eTestDir = join(repoRoot, "e2e");
 const npxCmd = process.platform === "win32" ? "npx.cmd" : "npx";
 
-function main() {
+async function main() {
   printInfo();
-  cleanE2EDirectory();
-  const packages = packPackages();
+  await cleanE2EDirectory();
+  const packages = await packPackages();
 
   console.log("Check packages exists");
-  run("ls", [`${repoRoot}/common/temp/artifacts/packages`]);
+  await runOrExit("ls", [`${repoRoot}/common/temp/artifacts/packages`]);
 
   console.log("Check cli is working");
-  runTypeSpec(packages["@typespec/compiler"], ["--help"], { cwd: e2eTestDir });
+  await runTypeSpec(packages["@typespec/compiler"], ["--help"], { cwd: e2eTestDir });
   console.log("Cli is working");
 
-  testBasicLatest(packages);
-  testBasicCurrentTgz(packages);
+  await testBasicLatest(packages);
+  await testBasicCurrentTgz(packages);
 }
-main();
+await main();
 
 function printInfo() {
   console.log("-".repeat(100));
   console.log("Npm Version: ");
-  run("npm", ["-v"]);
+  runOrExit("npm", ["-v"]);
   console.log("-".repeat(100));
 }
 
-function cleanE2EDirectory() {
-  run("git", ["clean", "-xfd"], { cwd: e2eTestDir });
+async function cleanE2EDirectory() {
+  await runOrExit("git", ["clean", "-xfd"], { cwd: e2eTestDir });
 }
 
-function packPackages() {
-  run("rush", ["publish", "--publish", "--pack", "--include-all"]);
+async function packPackages() {
+  await runOrExit("rush", ["publish", "--publish", "--pack", "--include-all"]);
   const outputFolder = join(repoRoot, "common/temp/artifacts/packages");
   const files = readdirSync(outputFolder);
   console.log("Built packages:", files);
@@ -58,31 +59,35 @@ function packPackages() {
   };
 }
 
-function runTypeSpec(compilerTgz, args, options) {
-  run(npxCmd, ["-y", "-p", compilerTgz, "tsp", ...args], { ...options });
+async function runTypeSpec(compilerTgz, args, options) {
+  await runOrExit(npxCmd, ["-y", "-p", compilerTgz, "tsp", ...args], { ...options });
 }
 
-function testBasicLatest(packages) {
+async function testBasicLatest(packages) {
   const basicLatestDir = join(e2eTestDir, "basic-latest");
   const outputDir = join(basicLatestDir, "tsp-output");
   console.log("Clearing basic-latest output");
   rmSync(outputDir, { recursive: true, force: true });
   console.log("Cleared basic-latest output");
 
   console.log("Installing basic-latest dependencies");
-  runTypeSpec(packages["@typespec/compiler"], ["install"], { cwd: basicLatestDir });
+  await runTypeSpec(packages["@typespec/compiler"], ["install"], { cwd: basicLatestDir });
   console.log("Installed basic-latest dependencies");
 
   console.log("Running tsp compile .");
-  runTypeSpec(packages["@typespec/compiler"], ["compile", ".", "--emit", "@typespec/openapi3"], {
-    cwd: basicLatestDir,
-  });
+  await runTypeSpec(
+    packages["@typespec/compiler"],
+    ["compile", ".", "--emit", "@typespec/openapi3"],
+    {
+      cwd: basicLatestDir,
+    }
+  );
   console.log("Completed tsp compile .");
 
   expectOpenApiOutput(outputDir);
 }
 
-function testBasicCurrentTgz(packages) {
+async function testBasicCurrentTgz(packages) {
   const basicCurrentDir = join(e2eTestDir, "basic-current");
   const outputDir = join(basicCurrentDir, "tsp-output");
   console.log("Clearing basic-current");
@@ -106,13 +111,17 @@ function testBasicCurrentTgz(packages) {
   console.log("Generated package.json for basic-current");
 
   console.log("Installing basic-current dependencies");
-  runTypeSpec(packages["@typespec/compiler"], ["install"], { cwd: basicCurrentDir });
+  await runTypeSpec(packages["@typespec/compiler"], ["install"], { cwd: basicCurrentDir });
   console.log("Installed basic-current dependencies");
 
   console.log(`Running tsp compile . in "${basicCurrentDir}"`);
-  runTypeSpec(packages["@typespec/compiler"], ["compile", ".", "--emit", "@typespec/openapi3"], {
-    cwd: basicCurrentDir,
-  });
+  await runTypeSpec(
+    packages["@typespec/compiler"],
+    ["compile", ".", "--emit", "@typespec/openapi3"],
+    {
+      cwd: basicCurrentDir,
+    }
+  );
   console.log("Completed tsp compile .");
 
   expectOpenApiOutput(outputDir);

diff --git a/eng/scripts/check-for-changed-files.js b/eng/scripts/check-for-changed-files.js
@@ -1,6 +1,7 @@
-import { run } from "./helpers.js";
+// @ts-check
+import { runOrExit } from "../../packages/internal-build-utils/dist/src/common.js";
 
-const proc = run("git", ["status", "--porcelain"], {
+const proc = await runOrExit("git", ["status", "--porcelain"], {
   encoding: "utf-8",
   stdio: [null, "pipe", "pipe"],
 });

diff --git a/eng/scripts/check-format.js b/eng/scripts/check-format.js
@@ -1,20 +1,21 @@
 // @ts-check
 import {
+  CommandFailedError,
   ensureDotnetVersion,
   runDotnetFormat,
 } from "../../packages/internal-build-utils/dist/src/index.js";
-import { CommandFailedError, runPrettier } from "./helpers.js";
+import { runPrettier } from "./helpers.js";
 
 try {
-  runPrettier("--list-different");
-  ensureDotnetVersion({ exitWithSuccessInDevBuilds: true });
-  runDotnetFormat("--verify-no-changes");
+  await runPrettier("--list-different");
+  await ensureDotnetVersion({ exitWithSuccessInDevBuilds: true });
+  await runDotnetFormat("--verify-no-changes");
 } catch (err) {
   if (err instanceof CommandFailedError) {
     console.error(
       "\nERROR: Files above are not formatted correctly. Run `rush format` and commit the changes."
     );
-    process.exit(err.proc?.status ?? 1);
+    process.exit(err.proc?.exitCode ?? 1);
   }
   throw err;
 }
diff --git a/eng/scripts/cspell.js b/eng/scripts/cspell.js
@@ -1,12 +1,12 @@
 // @ts-check
 import { resolve } from "path";
-import { run, xplatCmd } from "../../packages/internal-build-utils/dist/src/index.js";
+import { runOrExit, xplatCmd } from "../../packages/internal-build-utils/dist/src/index.js";
 import { repoRoot } from "./helpers.js";
 export const cspell = xplatCmd(
   resolve(repoRoot, "packages/internal-build-utils/node_modules/.bin/cspell")
 );
 
-await run(
+await runOrExit(
   cspell,
   [
     "--no-progress",

diff --git a/eng/scripts/dogfood.js b/eng/scripts/dogfood.js
@@ -1,4 +1,6 @@
-import { npmForEach, run } from "./helpers.js";
-run("rush", ["update"]);
-run("rush", ["build"]);
+// @ts-check
+import { runOrExit } from "../../packages/internal-build-utils/dist/src/common.js";
+import { npmForEach } from "./helpers.js";
+await runOrExit("rush", ["update"]);
+await runOrExit("rush", ["build"]);
 npmForEach("dogfood");
diff --git a/eng/scripts/format.js b/eng/scripts/format.js
@@ -1,10 +1,12 @@
 // @ts-check
 import {
   ensureDotnetVersion,
+  exitOnFailedCommand,
   runDotnetFormat,
 } from "../../packages/internal-build-utils/dist/src/index.js";
 import { runPrettier } from "./helpers.js";
-runPrettier("--write");
+
+await exitOnFailedCommand(() => runPrettier("--write"));
 
 await ensureDotnetVersion({ exitWithSuccessInDevBuilds: true });
 await runDotnetFormat();
diff --git a/eng/scripts/helpers.js b/eng/scripts/helpers.js
@@ -1,7 +1,8 @@
-import { spawn, spawnSync } from "child_process";
+// @ts-check
 import { readFileSync } from "fs";
 import { dirname, join, resolve } from "path";
 import { fileURLToPath } from "url";
+import { run, runOrExit } from "../../packages/internal-build-utils/dist/src/index.js";
 
 function read(filename) {
   const txt = readFileSync(filename, "utf-8")
@@ -41,7 +42,7 @@ export function npmForEachDependency(cmd, projectDir, options) {
   forEachProject((name, location, project) => {
     if (project.scripts[cmd] || cmd === "pack") {
       const args = cmd === "pack" ? [cmd] : ["run", cmd];
-      run("npm", args, { cwd: location, ...options });
+      runOrExit("npm", args, { cwd: location, ...options });
     }
   }, deps);
 }
@@ -55,57 +56,13 @@ export function npmForEach(cmd, options) {
 
     if (project.scripts[cmd] || cmd === "pack") {
       const args = cmd === "pack" ? [cmd] : ["run", cmd];
-      run("npm", args, { cwd: location, ...options });
+      runOrExit("npm", args, { cwd: location, ...options });
     }
   });
 }
 
-// We could use { shell: true } to let Windows find .cmd, but that causes other issues.
-// It breaks ENOENT checking for command-not-found and also handles command/args with spaces
-// poorly.
-const isCmdOnWindows = ["rush", "npm", "code", "code-insiders", "docusaurus", tsc, prettier];
-
-export class CommandFailedError extends Error {
-  constructor(msg, proc) {
-    super(msg);
-    this.proc = proc;
-  }
-}
-
-export function run(command, args, options) {
-  console.log();
-  console.log(`> ${command} ${args.join(" ")}`);
-
-  options = {
-    stdio: "inherit",
-    sync: true,
-    throwOnNonZeroExit: true,
-    ...options,
-  };
-
-  if (process.platform === "win32" && isCmdOnWindows.includes(command)) {
-    command += ".cmd";
-  }
-
-  const proc = (options.sync ? spawnSync : spawn)(command, args, options);
-  if (proc.error) {
-    if (options.ignoreCommandNotFound && proc.error.code === "ENOENT") {
-      console.log(`Skipped: Command \`${command}\` not found.`);
-    } else {
-      throw proc.error;
-    }
-  } else if (options.throwOnNonZeroExit && proc.status !== undefined && proc.status !== 0) {
-    throw new CommandFailedError(
-      `Command \`${command} ${args.join(" ")}\` failed with exit code ${proc.status}`,
-      proc
-    );
-  }
-
-  return proc;
-}
-
-export function runPrettier(...args) {
-  run(
+export async function runPrettier(...args) {
+  await run(
     prettier,
     [
       ...args,

diff --git a/eng/scripts/merge-coverage.js b/eng/scripts/merge-coverage.js
@@ -1,6 +1,8 @@
+// @ts-check
 import { copyFileSync, existsSync, mkdirSync, readdirSync } from "fs";
 import { join } from "path";
-import { forEachProject, repoRoot, run } from "./helpers.js";
+import { runOrExit } from "../../packages/internal-build-utils/dist/src/index.js";
+import { forEachProject, repoRoot } from "./helpers.js";
 
 // Create folder to collect all coverage files
 const rootCoverageTmp = join(repoRoot, "coverage", "tmp");
@@ -18,7 +20,7 @@ forEachProject((name, location, project) => {
 });
 
 // Generate merged report
-run(
+await runOrExit(
   "npm",
   [
     "exec",

diff --git a/eng/scripts/watch.js b/eng/scripts/watch.js
@@ -1,2 +1,4 @@
-import { repoRoot, run, tsc } from "./helpers.js";
+import { run } from "../../packages/internal-build-utils/dist/src/index.js";
+import { repoRoot, tsc } from "./helpers.js";
+
 run(tsc, ["--build", "./tsconfig.ws.json", "--watch"], { cwd: repoRoot, sync: false });