From bd30a3cd1061189b213b3d3b29f4250a02aa4ac6 Mon Sep 17 00:00:00 2001 From: Webber Date: Sun, 21 Aug 2022 22:31:02 +0200 Subject: [PATCH] chore: remove all previous run/exec commands in favour of one cleaned up version System.run --- src/commands/command/unity/build-command.ts | 13 -- src/dependencies.ts | 3 +- src/logic/unity/platform-setup/setup-mac.ts | 21 ++-- .../unity/platform-setup/setup-windows.ts | 5 +- src/model/docker.ts | 98 ++++++++------- src/model/image-environment-factory.ts | 9 +- src/model/mac-builder.ts | 8 +- src/model/system.test.ts | 4 +- src/model/system.ts | 112 +++++------------- src/model/versioning.ts | 30 +++-- src/modules/actions/exec.ts | 61 ---------- src/modules/actions/index.ts | 1 - 12 files changed, 135 insertions(+), 230 deletions(-) delete mode 100644 src/modules/actions/exec.ts diff --git a/src/commands/command/unity/build-command.ts b/src/commands/command/unity/build-command.ts index 600257fa..06d46051 100644 --- a/src/commands/command/unity/build-command.ts +++ b/src/commands/command/unity/build-command.ts @@ -27,19 +27,6 @@ export class BuildCommand extends CommandBase implements CommandInterface { await Docker.run(baseImage, { workspace, actionFolder, ...parameters }); } - // const result = await exec('docker run -it unityci/editor:2020.3.15f2-base-1 /bin/bash -c "echo test"', { - // output: OutputMode.Capture, - // continueOnError: true, - // - // // verbose: true, - // }); - // - // log.info('result', result.output); - // const { success } = result.status; - // log.info('success', success); - // - // return success; - // Set output await Output.setBuildVersion(parameters.buildVersion); } catch (error) { diff --git a/src/dependencies.ts b/src/dependencies.ts index d115c9ab..398d81b0 100644 --- a/src/dependencies.ts +++ b/src/dependencies.ts @@ -23,7 +23,7 @@ import { config, configSync } from 'https://deno.land/std@0.151.0/dotenv/mod.ts' // Internally managed packages import waitUntil from './modules/wait-until.ts'; -import { core, exec } from './modules/actions/index.ts'; +import { core } from './modules/actions/index.ts'; import { dedent } from './modules/dedent.ts'; // Polyfill for https://github.com/tc39/proposal-string-dedent @@ -58,7 +58,6 @@ export { configSync, core, crypto, - exec, fs, fsSync, getUnityChangeSet, diff --git a/src/logic/unity/platform-setup/setup-mac.ts b/src/logic/unity/platform-setup/setup-mac.ts index ece1c076..353e9f3b 100644 --- a/src/logic/unity/platform-setup/setup-mac.ts +++ b/src/logic/unity/platform-setup/setup-mac.ts @@ -1,5 +1,6 @@ import { Parameters } from '../../../model/index.ts'; -import { fsSync as fs, exec, getUnityChangeSet } from '../../../dependencies.ts'; +import { fsSync as fs, getUnityChangeSet } from '../../../dependencies.ts'; +import System from '../../../model/system.ts'; class SetupMac { static unityHubPath = `"/Applications/Unity Hub.app/Contents/MacOS/Unity Hub"`; @@ -19,11 +20,10 @@ class SetupMac { private static async installUnityHub(silent = false) { const command = 'brew install unity-hub'; if (!fs.existsSync(this.unityHubPath)) { - // Ignoring return code because the log seems to overflow the internal buffer which triggers - // a false error - const { exitCode } = await exec(command, undefined, { silent, ignoreReturnCode: true }); - if (exitCode) { - throw new Error(`There was an error installing the Unity Editor. See logs above for details.`); + try { + await System.run(command, { silent, ignoreReturnCode: true }); + } catch (error) { + throw new Error(`There was an error installing the Unity Editor. See logs above for details. ${error}`); } } } @@ -36,11 +36,10 @@ class SetupMac { --module mac-il2cpp \ --childModules`; - // Ignoring return code because the log seems to overflow the internal buffer which triggers - // a false error - const { exitCode } = await exec(command, undefined, { silent, ignoreReturnCode: true }); - if (exitCode) { - throw new Error(`There was an error installing the Unity Editor. See logs above for details.`); + try { + await System.run(command, { silent, ignoreReturnCode: true }); + } catch (error) { + throw new Error(`There was an error installing the Unity Editor. See logs above for details. ${error}`); } } diff --git a/src/logic/unity/platform-setup/setup-windows.ts b/src/logic/unity/platform-setup/setup-windows.ts index 8e0592de..8bf50354 100644 --- a/src/logic/unity/platform-setup/setup-windows.ts +++ b/src/logic/unity/platform-setup/setup-windows.ts @@ -1,6 +1,7 @@ -import { fsSync as fs, exec } from '../../../dependencies.ts'; +import { fsSync as fs } from '../../../dependencies.ts'; import { Parameters } from '../../../model/index.ts'; import ValidateWindows from '../platform-validation/validate-windows.ts'; +import System from '../../../model/system.ts'; class SetupWindows { public static async setup(parameters: Parameters) { @@ -17,7 +18,7 @@ class SetupWindows { const copyWinSdkRegistryKeyCommand = `reg export "HKLM\\SOFTWARE\\WOW6432Node\\Microsoft\\Microsoft SDKs\\Windows\\v10.0" ${registryKeysPath}/winsdk.reg /y`; await fs.ensureDir(registryKeysPath); - await exec(copyWinSdkRegistryKeyCommand); + await System.run(copyWinSdkRegistryKeyCommand); } } diff --git a/src/model/docker.ts b/src/model/docker.ts index 172a9249..b9be14aa 100644 --- a/src/model/docker.ts +++ b/src/model/docker.ts @@ -1,67 +1,77 @@ import ImageEnvironmentFactory from './image-environment-factory.ts'; -import { path, exec, fs } from '../dependencies.ts'; +import { path, fsSync as fs } from '../dependencies.ts'; +import System from './system.ts'; class Docker { static async run(image, parameters, silent = false) { - let runCommand = ''; + log.warning('running docker process for', process.platform, silent); + let command = ''; switch (process.platform) { case 'linux': - runCommand = this.getLinuxCommand(image, parameters); + command = await this.getLinuxCommand(image, parameters); break; case 'win32': - runCommand = this.getWindowsCommand(image, parameters); + command = await this.getWindowsCommand(image, parameters); } - await exec(runCommand, undefined, { silent }); + + const test = await System.newRun(`docker`, command.replace(/\s\s+/, ' ').split(' '), { silent, verbose: true }); + log.error('test', test); } - static getLinuxCommand(image, parameters): string { + static async getLinuxCommand(image, parameters): string { const { workspace, actionFolder, runnerTempPath, sshAgent, gitPrivateToken } = parameters; const githubHome = path.join(runnerTempPath, '_github_home'); - if (!fs.existsSync(githubHome)) fs.mkdirSync(githubHome); + await fs.ensureDir(githubHome); const githubWorkflow = path.join(runnerTempPath, '_github_workflow'); - if (!fs.existsSync(githubWorkflow)) fs.mkdirSync(githubWorkflow); + await fs.ensureDir(githubWorkflow); - return `docker run \ - --workdir /github/workspace \ - --rm \ - ${ImageEnvironmentFactory.getEnvVarString(parameters)} \ - --env UNITY_SERIAL \ - --env GITHUB_WORKSPACE=/github/workspace \ - ${gitPrivateToken ? `--env GIT_PRIVATE_TOKEN="${gitPrivateToken}"` : ''} \ - ${sshAgent ? '--env SSH_AUTH_SOCK=/ssh-agent' : ''} \ - --volume "${githubHome}":"/root:z" \ - --volume "${githubWorkflow}":"/github/workflow:z" \ - --volume "${workspace}":"/github/workspace:z" \ - --volume "${actionFolder}/default-build-script:/UnityBuilderAction:z" \ - --volume "${actionFolder}/platforms/ubuntu/steps:/steps:z" \ - --volume "${actionFolder}/platforms/ubuntu/entrypoint.sh:/entrypoint.sh:z" \ - ${sshAgent ? `--volume ${sshAgent}:/ssh-agent` : ''} \ - ${sshAgent ? '--volume /home/runner/.ssh/known_hosts:/root/.ssh/known_hosts:ro' : ''} \ - ${image} \ - /bin/bash -c /entrypoint.sh`; + return String.dedent` + docker run \ + --workdir /github/workspace \ + --rm \ + ${ImageEnvironmentFactory.getEnvVarString(parameters)} \ + --env UNITY_SERIAL \ + --env GITHUB_WORKSPACE=/github/workspace \ + ${gitPrivateToken ? `--env GIT_PRIVATE_TOKEN="${gitPrivateToken}"` : ''} \ + ${sshAgent ? '--env SSH_AUTH_SOCK=/ssh-agent' : ''} \ + --volume "${githubHome}":"/root:z" \ + --volume "${githubWorkflow}":"/github/workflow:z" \ + --volume "${workspace}":"/github/workspace:z" \ + --volume "${actionFolder}/default-build-script:/UnityBuilderAction:z" \ + --volume "${actionFolder}/platforms/ubuntu/steps:/steps:z" \ + --volume "${actionFolder}/platforms/ubuntu/entrypoint.sh:/entrypoint.sh:z" \ + ${sshAgent ? `--volume ${sshAgent}:/ssh-agent` : ''} \ + ${sshAgent ? '--volume /home/runner/.ssh/known_hosts:/root/.ssh/known_hosts:ro' : ''} \ + ${image} \ + /bin/bash -c /entrypoint.sh + `; } - static getWindowsCommand(image: any, parameters: any): string { + static async getWindowsCommand(image: any, parameters: any): string { const { workspace, actionFolder, unitySerial, gitPrivateToken, cliStoragePath } = parameters; - return `docker run \ - --workdir /github/workspace \ - --rm \ - ${ImageEnvironmentFactory.getEnvVarString(parameters)} \ - --env UNITY_SERIAL="${unitySerial}" \ - --env GITHUB_WORKSPACE=c:/github/workspace \ - ${gitPrivateToken ? `--env GIT_PRIVATE_TOKEN="${gitPrivateToken}"` : ''} \ - --volume "${workspace}":"c:/github/workspace" \ - --volume "${cliStoragePath}/registry-keys":"c:/registry-keys" \ - --volume "C:/Program Files (x86)/Microsoft Visual Studio":"C:/Program Files (x86)/Microsoft Visual Studio" \ - --volume "C:/Program Files (x86)/Windows Kits":"C:/Program Files (x86)/Windows Kits" \ - --volume "C:/ProgramData/Microsoft/VisualStudio":"C:/ProgramData/Microsoft/VisualStudio" \ - --volume "${actionFolder}/default-build-script":"c:/UnityBuilderAction" \ - --volume "${actionFolder}/platforms/windows":"c:/steps" \ - --volume "${actionFolder}/BlankProject":"c:/BlankProject" \ - ${image} \ - powershell c:/steps/entrypoint.ps1`; + // Todo - get this to work on a non-github runner local machine + // return String.dedent`run ${image} powershell c:/steps/entrypoint.ps1`; + return String.dedent` + docker run \ + --workdir /github/workspace \ + --rm \ + ${ImageEnvironmentFactory.getEnvVarString(parameters)} \ + --env UNITY_SERIAL="${unitySerial}" \ + --env GITHUB_WORKSPACE=c:/github/workspace \ + ${gitPrivateToken ? `--env GIT_PRIVATE_TOKEN="${gitPrivateToken}"` : ''} \ + --volume "${workspace}":"c:/github/workspace" \ + --volume "${cliStoragePath}/registry-keys":"c:/registry-keys" \ + --volume "C:/Program Files (x86)/Microsoft Visual Studio":"C:/Program Files (x86)/Microsoft Visual Studio" \ + --volume "C:/Program Files (x86)/Windows Kits":"C:/Program Files (x86)/Windows Kits" \ + --volume "C:/ProgramData/Microsoft/VisualStudio":"C:/ProgramData/Microsoft/VisualStudio" \ + --volume "${actionFolder}/default-build-script":"c:/UnityBuilderAction" \ + --volume "${actionFolder}/platforms/windows":"c:/steps" \ + --volume "${actionFolder}/BlankProject":"c:/BlankProject" \ + ${image} \ + powershell c:/steps/entrypoint.ps1 + `; } } diff --git a/src/model/image-environment-factory.ts b/src/model/image-environment-factory.ts index 8a22f610..22736ed9 100644 --- a/src/model/image-environment-factory.ts +++ b/src/model/image-environment-factory.ts @@ -19,7 +19,14 @@ class ImageEnvironmentFactory { continue; } - string += `--env ${p.name}="${p.value}" `; + if (Deno.build.os === 'windows') { + // The ampersand (&) character is not allowed. The & operator is reserved for future use; wrap an ampersand in + // double quotation marks ("&") to pass it as part of a string. + const escapedValue = typeof p.value !== 'string' ? p.value : p.value?.replace(/&/, '\\"&\\"'); + string += `--env ${p.name}='${escapedValue}' `; + } else { + string += `--env ${p.name}="${p.value}" `; + } } return string; diff --git a/src/model/mac-builder.ts b/src/model/mac-builder.ts index 574a4599..4e75f3a3 100644 --- a/src/model/mac-builder.ts +++ b/src/model/mac-builder.ts @@ -1,10 +1,10 @@ -import { exec } from '../dependencies.ts'; import { Parameters } from './parameters.ts'; +import System from './system.ts'; class MacBuilder { - public static async run(actionFolder, workspace, buildParameters: BuildParameters, silent = false) { - await exec('bash', [`${actionFolder}/platforms/mac/entrypoint.sh`], { - silent, + public static async run(actionFolder) { + log.warning('running the process'); + await System.run(`bash ${actionFolder}/platforms/mac/entrypoint.sh`, { ignoreReturnCode: true, }); } diff --git a/src/model/system.test.ts b/src/model/system.test.ts index b0cb3e19..338a121b 100644 --- a/src/model/system.test.ts +++ b/src/model/system.test.ts @@ -1,11 +1,11 @@ -import { core, exec } from '../../dependencies.ts'; +import { core } from '../../dependencies.ts'; import System from './system.ts'; jest.spyOn(core, 'debug').mockImplementation(() => {}); const info = jest.spyOn(core, 'info').mockImplementation(() => {}); jest.spyOn(core, 'warning').mockImplementation(() => {}); jest.spyOn(core, 'error').mockImplementation(() => {}); -const execSpy = jest.spyOn(exec, 'exec').mockImplementation(async () => 0); +const execSpy = jest.spyOn(System, 'run').mockImplementation(async () => 0); afterEach(() => jest.clearAllMocks()); diff --git a/src/model/system.ts b/src/model/system.ts index bbf59840..409f8ecb 100644 --- a/src/model/system.ts +++ b/src/model/system.ts @@ -1,6 +1,4 @@ -import { exec } from '../dependencies.ts'; - -export interface ShellRunOptions { +export interface RunOptions { pwd: string; } @@ -11,22 +9,45 @@ class System { * * Intended to always be silent and capture the output. */ - static async shellRun(rawCommand: string, options: ShellRunOptions = {}) { + static async run(rawCommand: string, options: RunOptions = {}) { const { pwd } = options; let command = rawCommand; if (pwd) command = `cd ${pwd} ; ${command}`; + const isWindows = Deno.build.os === 'windows'; + const shellMethod = isWindows ? System.powershellRun : System.shellRun; + + if (log.isVeryVerbose) log.debug(`The following command is run using ${shellMethod.name}`); + + return shellMethod(command, options); + } + + static async shellRun(command: string, options: RunOptions = {}) { return System.newRun('sh', ['-c', command]); } + static async powershellRun(command: string, options: RunOptions = {}) { + return System.newRun('powershell', [command]); + } + /** - * Example: - * System.newRun(sh, ['-c', 'echo something']) + * Internal cross-platform run, that spawns a new process and captures its output. * - * private for now, but could become public if this happens to be a great replacement for the other run method. + * If any error is written to stderr, this method will throw them. + * ❌ new Error(stdoutErrors) + * + * In case of no errors, this will return an object similar to these examples + * ✔️ { status: { success: true, code: 0 }, output: 'output from the command' } + * ⚠️ { status: { success: false, code: 1~255 }, output: 'output from the command' } + * + * Example usage: + * System.newRun(sh, ['-c', 'echo something']) + * System.newRun(powershell, ['echo something']) + * + * @deprecated use System.run instead, this method will be private */ - private static async newRun(command, args: string | string[] = []) { + public static async newRun(command, args: string | string[] = []) { if (!Array.isArray(args)) args = [args]; const argsString = args.join(' '); @@ -42,8 +63,8 @@ class System { process.close(); - const output = new TextDecoder().decode(outputBuffer).replace(/\n+$/, ''); - const error = new TextDecoder().decode(errorBuffer).replace(/\n+$/, ''); + const output = new TextDecoder().decode(outputBuffer).replace(/[\n\r]+$/, ''); + const error = new TextDecoder().decode(errorBuffer).replace(/[\n\r]+$/, ''); const result = { status, output }; @@ -61,77 +82,6 @@ class System { return result; } - - /** - * @deprecated use more simplified `shellRun` if possible. - */ - static async run(command, arguments_: any = [], options = {}, shouldLog = true) { - let result = ''; - let error = ''; - let debug = ''; - - const listeners = { - stdout: (dataBuffer) => { - result += dataBuffer.toString(); - }, - stderr: (dataBuffer) => { - error += dataBuffer.toString(); - }, - debug: (dataString) => { - debug += dataString.toString(); - }, - }; - - const showOutput = () => { - if (debug !== '' && shouldLog) { - log.debug(debug); - } - - if (result !== '' && shouldLog) { - log.info(result); - } - - if (error !== '' && shouldLog) { - log.warning(error); - } - }; - - const throwContextualError = (message: string) => { - let commandAsString = command; - if (Array.isArray(arguments_)) { - commandAsString += ` ${arguments_.join(' ')}`; - } else if (typeof arguments_ === 'string') { - commandAsString += ` ${arguments_}`; - } - - throw new Error(`Failed to run "${commandAsString}".\n ${message}`); - }; - - try { - if (command.trim() === '') { - throw new Error(`Failed to execute empty command`); - } - - const { exitCode, success, output } = await exec(command, arguments_, { silent: true, listeners, ...options }); - showOutput(); - if (!success) { - throwContextualError(`Command returned non-zero exit code (${exitCode}).\nError: ${error}`); - } - - // Todo - remove this after verifying it works as expected - const trimmedResult = result.replace(/\n+$/, ''); - if (!output && trimmedResult) { - log.warning('returning result instead of output for backward compatibility'); - - return trimmedResult; - } - - return output; - } catch (inCommandError) { - showOutput(); - throwContextualError(`In-command error caught: ${inCommandError}`); - } - } } export default System; diff --git a/src/model/versioning.ts b/src/model/versioning.ts index 165ba9db..8cd943bc 100644 --- a/src/model/versioning.ts +++ b/src/model/versioning.ts @@ -2,6 +2,7 @@ import NotImplementedException from './error/not-implemented-exception.ts'; import ValidationError from './error/validation-error.ts'; import Input from './input.ts'; import System from './system.ts'; +import { Action } from './index.ts'; export default class Versioning { static get projectPath() { @@ -225,7 +226,7 @@ export default class Versioning { * Returns whether the repository is shallow. */ static async isShallow() { - const output = await this.git(['rev-parse', '--is-shallow-repository']); + const output = await this.git('rev-parse --is-shallow-repository'); return output !== 'false'; } @@ -239,10 +240,10 @@ export default class Versioning { */ static async fetch() { try { - await this.git(['fetch', '--unshallow']); + await this.git('fetch --unshallow'); } catch { log.warning(`fetch --unshallow did not work, falling back to regular fetch`); - await this.git(['fetch']); + await this.git('fetch'); } } @@ -255,14 +256,23 @@ export default class Versioning { * identifies the current commit. */ static async getVersionDescription() { - return this.git(['describe', '--long', '--tags', '--always', this.sha]); + let commitIsh = ''; + + // In CI the repo is checked out in detached head mode. + // We MUST specify the commitIsh that triggered the job. + // Todo - make this compatible with more CI systems + if (!Action.isRunningLocally) { + commitIsh = this.sha; + } + + return this.git(`describe --long --tags --always ${commitIsh}`); } /** * Returns whether there are uncommitted changes that are not ignored. */ static async isDirty() { - const output = await this.git(['status', '--porcelain']); + const output = await this.git('status --porcelain'); const isDirty = output !== ''; if (isDirty) { @@ -279,7 +289,7 @@ export default class Versioning { * Get the tag if there is one pointing at HEAD */ static async getTag() { - return await this.git(['tag', '--points-at', 'HEAD']); + return await this.git('tag --points-at HEAD'); } /** @@ -309,7 +319,7 @@ export default class Versioning { * Note: HEAD should not be used, as it may be detached, resulting in an additional count. */ static async getTotalNumberOfCommits() { - const numberOfCommitsAsString = await this.git(['rev-list', '--count', this.sha]); + const numberOfCommitsAsString = await this.git(`rev-list --count ${this.sha}`); return Number.parseInt(numberOfCommitsAsString, 10); } @@ -318,6 +328,10 @@ export default class Versioning { * Run git in the specified project path */ static async git(arguments_, options = {}) { - return System.run('git', arguments_, { cwd: this.projectPath, ...options }); + const result = await System.run(`git ${arguments_}`, { cwd: this.projectPath, ...options }); + + log.warning(result); + + return result.output; } } diff --git a/src/modules/actions/exec.ts b/src/modules/actions/exec.ts deleted file mode 100644 index 4b24cd4f..00000000 --- a/src/modules/actions/exec.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { exec as originalExec } from 'https://deno.land/x/exec@0.0.5/mod.ts'; - -export enum OutputMode { - None = 0, // no output, just run the command - StdOut, // dump the output to stdout - Capture, // capture the output and return it - Tee, // both dump and capture the output -} - -export interface ICommandResult { - status?: { - code: number; - success: boolean; - }; - output: string; -} - -export interface ISanitisedCommandResult { - exitCode: number; - success: boolean; - output: string; -} - -interface IOptions { - silent?: boolean; - ignoreReturnCode?: boolean; - output?: OutputMode; - verbose?: boolean; - continueOnError?: boolean; -} - -const exec = async ( - command: string, - args: string | string[] = [], - ghActionsOptions: IOptions = {}, -): Promise => { - const options = { - output: OutputMode.Tee, - verbose: false, - continueOnError: false, - }; - - // log.debug('Command:', command, args); - - const { silent = false, ignoreReturnCode } = ghActionsOptions; - if (silent) options.output = OutputMode.Capture; - if (ignoreReturnCode) options.continueOnError = true; - - const argsString = typeof args === 'string' ? args : args.join(' '); - const result: ICommandResult = await originalExec(`${command} ${argsString}`, options); - - const { status, output = '' } = result; - const { code: exitCode, success } = status; - - const symbol = success ? '✅' : '❗'; - log.debug('Command:', command, argsString, symbol, result); - - return { exitCode, success, output: output.replace(/\n+$/, '') }; -}; - -export { exec }; diff --git a/src/modules/actions/index.ts b/src/modules/actions/index.ts index e0202240..d5ec94af 100644 --- a/src/modules/actions/index.ts +++ b/src/modules/actions/index.ts @@ -3,4 +3,3 @@ * This substitutes the parts we use in a Deno-compatible way. */ export { core } from './core.ts'; -export { exec } from './exec.ts';