Skip to content

Commit 08c8c62

Browse files
committed
refactor(hooks): optimize command execution and path resolution
- Refactor `getExecCommand` to use `docker exec` directly instead of `ddev exec` for better performance. - Implement robust UID/GID handling for DDEV containers (host UID on Linux, 1000 on macOS/Windows). - Centralize tool path resolution in `workflow.ts` to automatically prepend `vendor/bin/` or `node_modules/.bin/`. - Simplify `tools.ts` configuration by removing hardcoded paths. - Update CI workflow to generate distribution lockfile on dependency updates.
1 parent 58059f5 commit 08c8c62

16 files changed

+4850
-1755
lines changed

.github/workflows/update-booster-dependencies.yml

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ jobs:
123123
echo "Updated @types/node to version ^$NODE_MAJOR"
124124
125125
- name: Update NPM dependencies if needed
126-
if: env.updated == 'true' && steps.npm-updates.outputs.has_updates == 'true'
126+
if: steps.npm-updates.outputs.has_updates == 'true'
127127
working-directory: booster
128128
run: |
129129
echo "Updating NPM packages to latest versions..."
@@ -147,6 +147,18 @@ jobs:
147147
148148
echo "NPM package versions updated in package.json"
149149
echo "Note: pnpm-lock.yaml NOT updated - run 'pnpm install' locally to test"
150+
echo "updated=true" >> $GITHUB_ENV
151+
152+
- name: Update pnpm-lock.dist.yaml
153+
if: steps.npm-updates.outputs.has_updates == 'true'
154+
working-directory: booster
155+
run: |
156+
echo "Installing pnpm..."
157+
npm install -g pnpm
158+
echo "Updating pnpm-lock.dist.yaml..."
159+
chmod +x generate_dist_lockfile.sh
160+
./generate_dist_lockfile.sh
161+
echo "pnpm-lock.dist.yaml updated"
150162
151163
- name: Check PHP package updates
152164
id: php-updates
@@ -283,6 +295,11 @@ jobs:
283295
if [ "$NPM_CHANGED" = true ] || [ "$NODE_CHANGED" = true ]; then
284296
echo "- \`booster/package.json\`" >> update-summary.md
285297
fi
298+
if [ "$NPM_CHANGED" = true ]; then
299+
if ! git diff --quiet HEAD booster/pnpm-lock.dist.yaml; then
300+
echo "- \`booster/pnpm-lock.dist.yaml\`" >> update-summary.md
301+
fi
302+
fi
286303
if [ "${{ steps.php-updates.outputs.has_updates }}" == "true" ]; then
287304
if ! git diff --quiet HEAD booster/composer.json; then
288305
echo "- \`booster/composer.json\`" >> update-summary.md
@@ -317,6 +334,7 @@ jobs:
317334
booster/mise.toml
318335
booster/package.json
319336
booster/composer.json
337+
booster/pnpm-lock.dist.yaml
320338
321339
- name: No updates needed
322340
if: env.updated != 'true'

.prettierignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,6 @@
33
# Un-ignore booster directory
44
!booster/
55
!booster/**
6+
7+
# Ignore pnpm lock file in booster directory
8+
booster/pnpm-lock.yaml

booster/.husky/commit-msg.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ interface ProcessedConfig {
4242
/**
4343
* Load and parse validate-branch-name configuration
4444
*/
45-
function loadConfig(): BranchConfig {
45+
export function loadConfig(): BranchConfig {
4646
const config = validateBranchNameConfig.config
4747

4848
// Validate required properties and apply defaults
@@ -60,15 +60,15 @@ function loadConfig(): BranchConfig {
6060
/**
6161
* Check if the current branch should skip validation
6262
*/
63-
function isBranchSkipped(branchName: string, config: BranchConfig): boolean {
63+
export function isBranchSkipped(branchName: string, config: BranchConfig): boolean {
6464
const skipped = config.skipped || []
6565
return skipped.includes(branchName)
6666
}
6767

6868
/**
6969
* Process configuration and determine ticket requirements
7070
*/
71-
function processConfig(config: BranchConfig): ProcessedConfig {
71+
export function processConfig(config: BranchConfig): ProcessedConfig {
7272
// Use explicit requireTickets flag, but validate that patterns exist if tickets are required
7373
const needTicket =
7474
config.requireTickets && !!(config.ticketIdPrefix && config.ticketNumberPattern)
@@ -94,7 +94,7 @@ function processConfig(config: BranchConfig): ProcessedConfig {
9494
/**
9595
* Extract ticket ID from branch name
9696
*/
97-
function extractTicketId(branchName: string, config: BranchConfig): string | null {
97+
export function extractTicketId(branchName: string, config: BranchConfig): string | null {
9898
if (!config.ticketIdPrefix || !config.ticketNumberPattern) {
9999
return null
100100
}
@@ -155,7 +155,7 @@ async function lintCommitMessage(commitFile: string): Promise<boolean> {
155155
/**
156156
* Append ticket footer to commit message if needed
157157
*/
158-
async function appendTicketFooter(commitFile: string): Promise<boolean> {
158+
export async function appendTicketFooter(commitFile: string): Promise<boolean> {
159159
try {
160160
// Load and process configuration
161161
const config = loadConfig()

booster/.husky/pre-push.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
const SKIP_COMMIT_MSG = 'chore: update API documentation'
2323
const SKIP_DEPTRAC_MSG = 'chore: update deptrac image'
2424

25-
async function shouldSkip(): Promise<boolean> {
25+
export async function shouldSkip(): Promise<boolean> {
2626
const lastCommitMsg = (await $`git log -1 --pretty=%B`).stdout.trim()
2727
if (lastCommitMsg.includes(SKIP_COMMIT_MSG) || lastCommitMsg.includes(SKIP_DEPTRAC_MSG)) {
2828
log.info('Skipping pre-push hook for auto-generated commit.')
@@ -31,7 +31,7 @@ async function shouldSkip(): Promise<boolean> {
3131
return false
3232
}
3333

34-
async function runTests(): Promise<boolean> {
34+
export async function runTests(): Promise<boolean> {
3535
if (await fs.pathExists('vendor/bin/pest')) {
3636
log.tool('Pest', 'Running tests...')
3737
try {
@@ -45,7 +45,7 @@ async function runTests(): Promise<boolean> {
4545
return true
4646
}
4747

48-
async function handleApiDocs(): Promise<boolean> {
48+
export async function handleApiDocs(): Promise<boolean> {
4949
// Allow skipping API docs generation explicitly via env var
5050
if (isSkipped('api_docs')) {
5151
log.info('Skipping API docs generation (SKIP_API_DOCS environment variable set)')

booster/.husky/shared/core.ts

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ export async function isDdevProject(): Promise<boolean> {
150150
*/
151151
async function getDdevProjectName(): Promise<string | null> {
152152
try {
153-
const configPath = '.ddev/config.yaml'
154-
const configContent = await fs.readFile(configPath, 'utf-8')
153+
// isDdevProject() already confirmed .ddev/config.yaml exists in CWD
154+
const configContent = await fs.readFile('.ddev/config.yaml', 'utf-8')
155155
const match = configContent.match(/^name:\s*(.+)$/m)
156156
return match ? match[1].trim() : null
157157
} catch {
@@ -163,7 +163,6 @@ async function getDdevProjectName(): Promise<string | null> {
163163
* Get the command to execute, wrapping in ddev exec if necessary
164164
* @param command The command parts (e.g. ['php', '-v'])
165165
* @param type The tool type ('node', 'php', 'system')
166-
* @throws Error if DDEV project name cannot be determined
167166
* @returns The modified command array ready for execution
168167
*/
169168
async function getExecCommand(command: string[], type: string): Promise<string[]> {
@@ -180,31 +179,34 @@ async function getExecCommand(command: string[], type: string): Promise<string[]
180179
throw new Error('Could not determine DDEV project name from .ddev/config.yaml')
181180
}
182181

183-
// Use docker exec for performance instead of ddev exec
184-
// Run as current user to avoid permission issues with generated files
185-
const uid = process.getuid ? process.getuid() : 1000
186-
const gid = process.getgid ? process.getgid() : 1000
187-
const containerName = `ddev-${projectName}-web`
182+
// Since isDdevProject() confirms we are at the project root (where .ddev exists),
183+
// we map directly to the container's web root.
184+
const containerPath = '/var/www/html'
185+
186+
// Determine User ID
187+
// On Linux, we use the host UID so file permissions match.
188+
// On macOS/Windows, DDEV handles permissions via filesystem mounts (VirtioFS/Mutagen),
189+
// so we use the default container user (1000) to ensure tools like Composer work correctly.
190+
let uid = 1000
191+
let gid = 1000
188192

189-
// Adjust command path for docker exec
190-
// If command is not 'php' or 'composer' and doesn't start with '/', assume it's in vendor/bin
191-
let cmd = command[0]
192-
if (cmd !== 'php' && cmd !== 'composer' && !cmd.startsWith('/') && !cmd.startsWith('./')) {
193-
cmd = `vendor/bin/${cmd}`
193+
if (process.platform === 'linux') {
194+
uid = process.getuid ? process.getuid() : 1000
195+
gid = process.getgid ? process.getgid() : 1000
194196
}
195197

196-
const newCommand = [cmd, ...command.slice(1)]
198+
const containerName = `ddev-${projectName}-web`
197199

198200
return [
199201
'docker',
200202
'exec',
201-
'-t',
203+
'-t', // Allocate pseudo-TTY
202204
'-u',
203205
`${uid}:${gid}`,
204206
'-w',
205-
process.cwd(),
207+
containerPath,
206208
containerName,
207-
...newCommand,
209+
...command,
208210
]
209211
}
210212

booster/.husky/shared/workflow.ts

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { $, which } from 'zx'
1+
import { $, which, fs } from 'zx'
22
import {
33
ensureMutagenSync,
44
exec,
@@ -48,13 +48,60 @@ export async function runStep(
4848
}
4949
}
5050

51+
/**
52+
* Resolve the full path to a tool command based on its type
53+
*/
54+
function resolveCommandPath(tool: ToolConfig): string {
55+
// Don't modify absolute paths or paths starting with ./
56+
if (tool.command.startsWith('/') || tool.command.startsWith('./')) {
57+
return tool.command
58+
}
59+
60+
// If the command already contains a path separator, assume it's a relative path
61+
// (e.g. "bin/console" or "vendor/bin/rector") and don't modify it
62+
if (tool.command.includes('/')) {
63+
return tool.command
64+
}
65+
66+
// Don't modify standard system commands
67+
if (['php', 'composer', 'git', 'npm', 'pnpm', 'yarn', 'node'].includes(tool.command)) {
68+
return tool.command
69+
}
70+
71+
if (tool.type === 'php') {
72+
return `vendor/bin/${tool.command}`
73+
}
74+
75+
if (tool.type === 'node') {
76+
return `node_modules/.bin/${tool.command}`
77+
}
78+
79+
return tool.command
80+
}
81+
5182
/**
5283
* Check if a tool is available to run
5384
*/
5485
async function isToolAvailable(tool: ToolConfig): Promise<boolean> {
86+
const commandPath = resolveCommandPath(tool)
87+
5588
if (tool.type === 'php' && (await isDdevProject())) {
89+
// For standard PHP tools (php, composer), assume they exist in the container
90+
if (tool.command === 'php' || tool.command === 'composer') {
91+
return true
92+
}
93+
// For other tools (phpstan, ecs, rector), check if they exist
94+
// This prevents trying to run tools that aren't installed
95+
return await fs.pathExists(commandPath)
96+
}
97+
98+
// For node tools or system tools, check if the command exists as a file
99+
// (e.g. node_modules/.bin/eslint)
100+
if (await fs.pathExists(commandPath)) {
56101
return true
57102
}
103+
104+
// Fallback to checking PATH (for system tools like git)
58105
return await which(tool.command)
59106
.then(() => true)
60107
.catch(() => false)
@@ -65,6 +112,7 @@ async function isToolAvailable(tool: ToolConfig): Promise<boolean> {
65112
*/
66113
async function execTool(tool: ToolConfig, files: string[]): Promise<void> {
67114
const args = [...(tool.args || [])]
115+
const command = resolveCommandPath(tool)
68116

69117
if (tool.runForEachFile) {
70118
// Run command for each file individually with concurrency limit
@@ -77,7 +125,7 @@ async function execTool(tool: ToolConfig, files: string[]): Promise<void> {
77125
for (const chunk of chunks) {
78126
await Promise.all(
79127
chunk.map((file) =>
80-
exec([tool.command, ...args, file], {
128+
exec([command, ...args, file], {
81129
quiet: true,
82130
type: tool.type,
83131
}),
@@ -90,7 +138,7 @@ async function execTool(tool: ToolConfig, files: string[]): Promise<void> {
90138
if (tool.passFiles !== false) {
91139
cmdArgs.push(...files)
92140
}
93-
await exec([tool.command, ...cmdArgs], { type: tool.type })
141+
await exec([command, ...cmdArgs], { type: tool.type })
94142
}
95143
}
96144

0 commit comments

Comments
 (0)