Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ e2e:
extends:
- .base-configuration
- .test-allowed-branches
- .resource-allocation-4-cpus
interruptible: true
artifacts:
when: always
Expand Down
2 changes: 1 addition & 1 deletion eslint-local-rules/secureCommandExecution.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function isCommandExecuted(node) {
currentMethodCall = currentMethodCall.parent.parent
}

return methodCallNames.includes('run')
return methodCallNames.includes('run') || methodCallNames.includes('runAsync')
}

function isCommandContainsShellCharacters(node) {
Expand Down
51 changes: 30 additions & 21 deletions scripts/build/build-test-apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const APPS: AppConfig[] = [
// React Router apps
{ name: 'react-router-v6-app' },
{ name: 'tanstack-router-app' },
{ name: 'react-router-v7-app', builderFn: buildReactRouterv7App },
{ name: 'react-router-v7-app', builderFn: buildReactRouterv7App, deps: ['react-router-v6-app'] },

// browser extensions
{ name: 'base-extension' },
Expand Down Expand Up @@ -79,22 +79,29 @@ runMain(async () => {
printLog('Packing packages...')
command`yarn run pack`.run()

const built = new Set<string>()
for (const app of appsToBuild) {
for (const dep of app.deps ?? []) {
if (!built.has(dep)) {
buildApp(dep)
built.add(dep)
}
const buildPromises = new Map<string, Promise<void>>()

function ensureBuild(app: AppConfig): Promise<void> {
let promise = buildPromises.get(app.name)
if (!promise) {
promise = (async () => {
// Ensure all dependencies are built
const dependenciesToBuild = (app.deps ?? []).map((name) => APPS.find((a) => a.name === name)!)
await Promise.all(dependenciesToBuild.map(ensureBuild))

if ('builderFn' in app) {
await app.builderFn(app.name, app.options)
} else {
await buildApp(app.name)
}
})()
buildPromises.set(app.name, promise)
}
if ('builderFn' in app) {
await app.builderFn(app.name, app.options)
} else {
buildApp(app.name)
}
built.add(app.name)
return promise
}

await Promise.all(appsToBuild.map(ensureBuild))

printLog('Test apps and extensions built successfully.')
})

Expand All @@ -112,10 +119,10 @@ function showHelpAndExit() {
process.exit(0)
}

function buildApp(appName: string) {
async function buildApp(appName: string) {
const appPath = `test/apps/${appName}`
printLog(`Building app at ${appPath}...`)
command`yarn install --no-immutable`.withCurrentWorkingDirectory(appPath).run()
await command`yarn install --no-immutable`.withCurrentWorkingDirectory(appPath).runAsync()

// install peer dependencies if any
// intent: renovate does not allow to generate local packages before install
Expand All @@ -126,16 +133,18 @@ function buildApp(appName: string) {
for (const [name] of Object.entries(packageJson.peerDependencies)) {
const resolution = packageJson.resolutions?.[name]
const specifier = resolution ? `${name}@${resolution}` : name
command`yarn add -D ${specifier}`.withCurrentWorkingDirectory(appPath).run()
await command`yarn add -D ${specifier}`.withCurrentWorkingDirectory(appPath).runAsync()
}
// revert package.json & yarn.lock changes if they are versioned
const areFilesVersioned = command`git ls-files package.json yarn.lock`.withCurrentWorkingDirectory(appPath).run()
const areFilesVersioned = await command`git ls-files package.json yarn.lock`
.withCurrentWorkingDirectory(appPath)
.runAsync()
if (areFilesVersioned) {
command`git checkout package.json yarn.lock`.withCurrentWorkingDirectory(appPath).run()
await command`git checkout package.json yarn.lock`.withCurrentWorkingDirectory(appPath).runAsync()
}
}

command`yarn build`.withCurrentWorkingDirectory(appPath).run()
await command`yarn build`.withCurrentWorkingDirectory(appPath).runAsync()
}

async function buildReactRouterv7App() {
Expand Down Expand Up @@ -163,7 +172,7 @@ async function buildReactRouterv7App() {
.replace('react-router-v6-app.js', 'react-router-v7-app.js')
)

buildApp('react-router-v7-app')
await buildApp('react-router-v7-app')
}

async function buildExtension(appName: string, options?: { runAt?: string }): Promise<void> {
Expand Down
98 changes: 86 additions & 12 deletions scripts/lib/command.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import childProcess from 'node:child_process'
import { printDebug, printError } from './executionUtils.ts'
import { printDebug, printError, printWarning } from './executionUtils.ts'

// Git operations share a single index lock, so they must not run concurrently.
let gitMutex = Promise.resolve<unknown>(undefined)

interface CommandOptions {
cwd?: string
Expand All @@ -12,6 +15,7 @@ interface CommandBuilder {
withCurrentWorkingDirectory(newCurrentWorkingDirectory: string): CommandBuilder
withLogs(): CommandBuilder
run(): string
runAsync(): Promise<string>
}

/**
Expand All @@ -31,6 +35,7 @@ interface CommandBuilder {
export function command(...templateArguments: [TemplateStringsArray, ...any[]]): CommandBuilder {
const [commandName, ...commandArguments] = parseCommandTemplateArguments(...templateArguments)

const formattedCommand = `${commandName} ${commandArguments.join(' ')}`
let input = ''
let env: Record<string, string> | undefined
const extraOptions: CommandOptions = {}
Expand All @@ -57,7 +62,6 @@ export function command(...templateArguments: [TemplateStringsArray, ...any[]]):
},

run(): string {
const formattedCommand = `${commandName} ${commandArguments.join(' ')}`
printDebug(`Running command: ${formattedCommand}`)

const commandResult = childProcess.spawnSync(commandName, commandArguments, {
Expand All @@ -68,16 +72,9 @@ export function command(...templateArguments: [TemplateStringsArray, ...any[]]):
})

if (commandResult.status !== 0) {
const formattedStderr = commandResult.stderr ? `\n---- stderr: ----\n${commandResult.stderr}\n----` : ''
const formattedStdout = commandResult.stdout ? `\n---- stdout: ----\n${commandResult.stdout}\n----` : ''
const exitCause =
commandResult.signal !== null
? ` due to signal ${commandResult.signal}`
: commandResult.status !== null
? ` with exit status ${commandResult.status}`
: ''
throw new Error(`Command failed${exitCause}: ${formattedCommand}${formattedStderr}${formattedStdout}`, {
cause: commandResult.error,
throw buildCommandError({
formattedCommand,
...commandResult,
})
}

Expand All @@ -87,9 +84,86 @@ export function command(...templateArguments: [TemplateStringsArray, ...any[]]):

return commandResult.stdout
},

runAsync(): Promise<string> {
if (extraOptions.stdio === 'inherit') {
printWarning(
`runAsync() ignores withLogs() for command: ${formattedCommand} (running multiple commands concurrently would produce interleaved output)`
)
}
printDebug(`Running command: ${formattedCommand}`)

const run = () =>
new Promise<string>((resolve, reject) => {
const child = childProcess.spawn(commandName, commandArguments, {
env: { ...process.env, ...env },
...extraOptions,
stdio: 'pipe',
})
Comment on lines +98 to +102
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor withInput() when running asynchronously

When callers use the new runAsync() path with withInput() (for example the existing gh auth login --with-token/ssh-add - patterns if they are converted to async), the configured input is never written to the spawned process. Because spawn() creates a stdin pipe here but the parent neither writes nor closes it, commands that read from stdin can hang indefinitely or fail with missing input, unlike run() which passes input to spawnSync().

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but we'll fix when we need it.


let stdout = ''
let stderr = ''

child.stdout.on('data', (chunk: Buffer) => {
stdout += chunk.toString()
})
child.stderr.on('data', (chunk: Buffer) => {
stderr += chunk.toString()
})

child.on('error', (error) => {
reject(buildCommandError({ formattedCommand, status: null, signal: null, stdout, stderr, error }))
})
child.on('close', (status, signal) => {
if (status !== 0) {
reject(buildCommandError({ formattedCommand, status, signal, stdout, stderr }))
} else {
resolve(stdout)
}
})
})

if (commandName === 'git') {
const result = gitMutex.then(run)
gitMutex = result.then(
() => {
// ignore result
},
() => {
// ignore exception
}
)
return result
}

return run()
},
}
}

function buildCommandError({
formattedCommand,
status,
signal,
stdout,
stderr,
error,
}: {
formattedCommand: string
status: number | null
signal: NodeJS.Signals | null
stdout: string
stderr: string
error?: Error
}): Error {
const formattedStderr = stderr ? `\n---- stderr: ----\n${stderr}\n----` : ''
const formattedStdout = stdout ? `\n---- stdout: ----\n${stdout}\n----` : ''
const exitCause = signal !== null ? ` due to signal ${signal}` : status !== null ? ` with exit status ${status}` : ''
return new Error(`Command failed${exitCause}: ${formattedCommand}${formattedStderr}${formattedStdout}`, {
cause: error,
})
}

/**
* Parse template values passed to the `command` template tag, and return a list of arguments to run
* the command.
Expand Down
Loading