-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
fix: improve Windows Rust environment setup for build workflows #19960
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| import { spawnSync } from 'node:child_process' | ||
| import fs from 'node:fs' | ||
| import path from 'node:path' | ||
| import process from 'node:process' | ||
| import { fileURLToPath } from 'node:url' | ||
| import { | ||
| cargoBin, | ||
| commandExists, | ||
| hasDetectedCargoBin, | ||
| readInstalledRustTargets, | ||
| rustEnv, | ||
| } from './rust-env.mjs' | ||
|
|
||
| const __dirname = path.dirname(fileURLToPath(import.meta.url)) | ||
| const root = path.resolve(__dirname, '..') | ||
| const command = process.argv[2] ?? 'doctor' | ||
| const wasmTarget = 'wasm32-wasip1-threads' | ||
|
|
||
| const contexts = { | ||
| doctor: { | ||
| label: 'pnpm run check:env', | ||
| needsRust: true, | ||
| needsWasmTarget: true, | ||
| }, | ||
| build: { | ||
| label: 'pnpm build', | ||
| needsRust: true, | ||
| needsWasmTarget: true, | ||
| }, | ||
| dev: { | ||
| label: 'pnpm dev', | ||
| needsRust: true, | ||
| needsWasmTarget: true, | ||
| }, | ||
| test: { | ||
| label: 'pnpm test', | ||
| needsRust: true, | ||
| needsWasmTarget: false, | ||
| }, | ||
| 'test:integrations': { | ||
| label: 'pnpm test:integrations', | ||
| needsRust: true, | ||
| needsWasmTarget: true, | ||
| needsBuildArtifacts: true, | ||
| }, | ||
| 'test:ui': { | ||
| label: 'pnpm test:ui', | ||
| needsRust: true, | ||
| needsWasmTarget: true, | ||
| needsBuildArtifacts: true, | ||
| }, | ||
| vite: { | ||
| label: 'pnpm vite', | ||
| needsRust: true, | ||
| needsWasmTarget: true, | ||
| needsBuildArtifacts: true, | ||
| needsBun: true, | ||
| }, | ||
| nextjs: { | ||
| label: 'pnpm nextjs', | ||
| needsRust: true, | ||
| needsWasmTarget: true, | ||
| needsBuildArtifacts: true, | ||
| }, | ||
| } | ||
|
|
||
| const context = contexts[command] ?? contexts.doctor | ||
|
|
||
| const requiredArtifacts = [ | ||
| 'crates/node/index.js', | ||
| 'packages/tailwindcss/dist/lib.js', | ||
| ] | ||
|
|
||
| function run(command, args) { | ||
| return spawnSync(command, args, { | ||
| cwd: root, | ||
| encoding: 'utf8', | ||
| stdio: ['ignore', 'pipe', 'pipe'], | ||
| env: rustEnv, | ||
| }) | ||
| } | ||
|
|
||
| function formatList(items) { | ||
| return items.map((item) => `- ${item}`).join('\n') | ||
| } | ||
|
|
||
| function relativeArtifact(artifact) { | ||
| return artifact.replace(/\\/g, '/') | ||
| } | ||
|
|
||
| let errors = [] | ||
| let warnings = [] | ||
|
|
||
| if (context.needsRust) { | ||
| let missingRustTools = ['cargo', 'rustc'].filter((tool) => !commandExists(tool)) | ||
|
|
||
| if (missingRustTools.length > 0) { | ||
| errors.push({ | ||
| title: 'Missing required Rust tools', | ||
| body: [ | ||
| formatList(missingRustTools), | ||
| 'How to fix:', | ||
| '1. Install Rustup from https://rustup.rs/', | ||
| hasDetectedCargoBin() | ||
| ? `2. Rust appears to be installed in the detected Cargo bin directory (${cargoBin}), so rerun the command through the repo scripts such as \`pnpm build\` or \`pnpm run check:env\`.` | ||
| : '2. If you installed Rustup to a custom location, make sure `CARGO_HOME`, `RUSTUP_HOME`, or your PATH points at the installation.', | ||
| '3. Run `rustup default stable`.', | ||
| `4. Run \`rustup target add ${wasmTarget}\`.`, | ||
| ].join('\n'), | ||
| }) | ||
| } | ||
|
|
||
| if (!commandExists('rustup')) { | ||
| warnings.push( | ||
| 'rustup was not found, so the script could not verify the installed WASM targets.', | ||
| ) | ||
| } else if (context.needsWasmTarget) { | ||
| let installedTargets = readInstalledRustTargets() | ||
|
|
||
| if (installedTargets === null) { | ||
| warnings.push('Could not read the installed Rust targets from rustup.') | ||
| } else if (!installedTargets.has(wasmTarget)) { | ||
| errors.push({ | ||
| title: 'Missing required Rust target', | ||
| body: [`- ${wasmTarget}`, `Run \`rustup target add ${wasmTarget}\`.`].join('\n'), | ||
| }) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (context.needsBun && !commandExists('bun')) { | ||
| errors.push({ | ||
| title: 'Missing required tool for the Vite playground', | ||
| body: [ | ||
| '- bun', | ||
| 'The Vite playground uses `bun --bun vite`.', | ||
| 'Install Bun, or validate Vite changes with `pnpm build && pnpm test:integrations -- --run integrations/vite`.', | ||
| ].join('\n'), | ||
| }) | ||
| } | ||
|
|
||
| if (context.needsBuildArtifacts) { | ||
| let missingArtifacts = requiredArtifacts.filter((artifact) => { | ||
| return !fs.existsSync(path.join(root, artifact)) | ||
| }) | ||
|
|
||
| if (missingArtifacts.length > 0) { | ||
| errors.push({ | ||
| title: 'Missing build artifacts required by this command', | ||
| body: [ | ||
| formatList(missingArtifacts.map(relativeArtifact)), | ||
| 'Run `pnpm build` first, then retry this command.', | ||
| ].join('\n'), | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| if (errors.length > 0) { | ||
| console.error(`Environment preflight failed for ${context.label}.`) | ||
| console.error('') | ||
|
|
||
| for (let error of errors) { | ||
| console.error(`${error.title}:`) | ||
| console.error(error.body) | ||
| console.error('') | ||
| } | ||
|
|
||
| if (warnings.length > 0) { | ||
| console.error('Warnings:') | ||
| console.error(formatList(warnings)) | ||
| console.error('') | ||
| } | ||
|
|
||
| process.exit(1) | ||
| } | ||
|
|
||
| if (command === 'doctor') { | ||
| console.log(`Environment preflight passed for ${context.label}.`) | ||
|
|
||
| if (warnings.length > 0) { | ||
| console.log('') | ||
| console.log('Warnings:') | ||
| console.log(formatList(warnings)) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { spawnSync } from 'node:child_process' | ||
| import path from 'node:path' | ||
| import process from 'node:process' | ||
| import { fileURLToPath } from 'node:url' | ||
| import { isWindows, rustEnv } from './rust-env.mjs' | ||
|
|
||
| const __dirname = path.dirname(fileURLToPath(import.meta.url)) | ||
| const root = path.resolve(__dirname, '..') | ||
| const cwd = process.cwd() | ||
|
|
||
| const task = process.argv[2] | ||
| const passthrough = process.argv.slice(3) | ||
|
|
||
| const predefinedCommands = { | ||
| build: [['turbo', ['build', '--filter=!./playgrounds/*']]], | ||
| dev: [['turbo', ['dev', '--filter=!./playgrounds/*']]], | ||
| test: [ | ||
| ['cargo', ['test']], | ||
| ['vitest', ['run', '--hideSkippedTests']], | ||
| ], | ||
| 'test:integrations': [['vitest', ['--root=./integrations']]], | ||
| 'test:ui': [ | ||
| ['pnpm', ['run', '--filter=tailwindcss', 'test:ui']], | ||
| ['pnpm', ['run', '--filter=@tailwindcss/browser', 'test:ui']], | ||
| ], | ||
| vite: [['pnpm', ['run', '--filter=vite-playground', 'dev']]], | ||
| nextjs: [['pnpm', ['run', '--filter=nextjs-playground', 'dev']]], | ||
| } | ||
|
|
||
| if (!task || !predefinedCommands[task]) { | ||
| console.error(`Unknown task: ${task ?? '(missing)'}`) | ||
| process.exit(1) | ||
| } | ||
|
|
||
| let commands = predefinedCommands[task] | ||
|
|
||
| if (passthrough[0] === '--') { | ||
| if (!passthrough[1]) { | ||
| console.error('Missing command after `--`.') | ||
| process.exit(1) | ||
| } | ||
|
|
||
| commands = [[passthrough[1], passthrough.slice(2)]] | ||
| } else if (commands && passthrough.length > 0) { | ||
| commands = commands.map((entry, index) => { | ||
| if (index !== commands.length - 1) return entry | ||
| return [entry[0], [...entry[1], ...passthrough]] | ||
| }) | ||
| } | ||
|
|
||
| let preflight = spawnSync(process.execPath, [path.join(root, 'scripts', 'preflight.mjs'), task], { | ||
| cwd, | ||
| env: rustEnv, | ||
| stdio: 'inherit', | ||
| }) | ||
|
|
||
| if (preflight.status !== 0) { | ||
| process.exit(preflight.status ?? 1) | ||
| } | ||
|
|
||
| for (let [command, args] of commands) { | ||
| let result = spawnSync(command, args, { | ||
| cwd, | ||
| env: rustEnv, | ||
| stdio: 'inherit', | ||
| shell: isWindows, | ||
| }) | ||
|
Comment on lines
+61
to
+67
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: No, Node.js spawnSync with shell: true on Windows does not properly quote individual arguments containing spaces or special shell characters like '&'. Instead, it simply joins the command and all args array elements with spaces into a single string, which is then passed to cmd.exe /d /s /c. This raw concatenation means spaces split arguments unless the user has already included shell-quoting in the arg strings themselves, and special characters like '&' are interpreted by cmd.exe, potentially truncating or altering command execution. The official documentation warns against passing unsanitized input with shell: true due to this risk of command injection via shell metacharacters. When shell: true and the shell is CMD, windowsVerbatimArguments is automatically true, meaning no additional quoting/escaping is done by Node.js before passing to the shell. Citations:
🏁 Script executed: # First, find the file and inspect the relevant code sections
cd /tmp && find . -name "run-task.mjs" -type f 2>/dev/null | head -5Repository: tailwindlabs/tailwindcss Length of output: 50 🏁 Script executed: # Check if file exists at the expected path
if [ -f scripts/run-task.mjs ]; then
echo "File found at scripts/run-task.mjs"
wc -l scripts/run-task.mjs
fiRepository: tailwindlabs/tailwindcss Length of output: 126 🏁 Script executed: # Read the relevant sections of the file
if [ -f scripts/run-task.mjs ]; then
echo "=== Lines 40-70 (including context around the issue) ==="
sed -n '40,70p' scripts/run-task.mjs | cat -n
fiRepository: tailwindlabs/tailwindcss Length of output: 1070 🏁 Script executed: # Also check lines around line 43 to see the passthrough path
if [ -f scripts/run-task.mjs ]; then
echo "=== Lines 1-50 (to understand setup and line 43) ==="
sed -n '1,50p' scripts/run-task.mjs | cat -n
fiRepository: tailwindlabs/tailwindcss Length of output: 2012 Args may not be shell-escaped correctly when With Low impact for the current hard-coded call sites, but worth flagging since the override path is explicitly designed to accept arbitrary commands. 🤖 Prompt for AI Agents |
||
|
|
||
| if (result.status !== 0) { | ||
| process.exit(result.status ?? 1) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import fs from 'node:fs' | ||
| import path from 'node:path' | ||
| import process from 'node:process' | ||
| import { spawnSync } from 'node:child_process' | ||
|
|
||
| export const isWindows = process.platform === 'win32' | ||
|
|
||
| const home = process.env.USERPROFILE ?? process.env.HOME ?? process.env.HOMEPATH ?? '' | ||
|
|
||
| export const cargoHome = process.env.CARGO_HOME ?? (home ? path.join(home, '.cargo') : '') | ||
| export const rustupHome = process.env.RUSTUP_HOME ?? (home ? path.join(home, '.rustup') : '') | ||
| export const cargoBin = cargoHome ? path.join(cargoHome, 'bin') : '' | ||
|
|
||
| export const mergedPath = cargoBin | ||
| ? [cargoBin, process.env.PATH ?? ''].filter(Boolean).join(path.delimiter) | ||
| : process.env.PATH ?? '' | ||
|
|
||
| export const rustEnv = { | ||
| ...process.env, | ||
| ...(cargoHome ? { CARGO_HOME: cargoHome } : {}), | ||
| ...(rustupHome ? { RUSTUP_HOME: rustupHome } : {}), | ||
| PATH: mergedPath, | ||
| } | ||
|
|
||
| export function commandExists(command, { cwd = process.cwd() } = {}) { | ||
| let result = spawnSync(command, ['--version'], { | ||
| cwd, | ||
| env: rustEnv, | ||
| stdio: 'ignore', | ||
| shell: isWindows, | ||
| }) | ||
|
|
||
| return !result.error && result.status === 0 | ||
| } | ||
|
|
||
| export function readInstalledRustTargets({ cwd = process.cwd() } = {}) { | ||
| let result = spawnSync('rustup', ['target', 'list', '--installed'], { | ||
| cwd, | ||
| env: rustEnv, | ||
| encoding: 'utf8', | ||
| stdio: ['ignore', 'pipe', 'pipe'], | ||
| shell: isWindows, | ||
| }) | ||
|
|
||
| if (result.error || result.status !== 0) { | ||
| return null | ||
| } | ||
|
|
||
| return new Set(result.stdout.split(/\r?\n/).filter(Boolean)) | ||
| } | ||
|
|
||
| export function hasDetectedCargoBin() { | ||
| return Boolean(cargoBin) && fs.existsSync(cargoBin) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unknown context silently falls back to
doctor.contexts[command] ?? contexts.doctormeans an unrecognized task arg runs the doctor checks instead of erroring. In practicerun-task.mjsalready validates the task, so this is only reachable when preflight is invoked directly (e.g.node ./scripts/preflight.mjs typo), but the success message on line 178 still gates oncommand === 'doctor', so a typo'd context would run doctor-equivalent checks and print nothing on success, which is confusing. Consider erroring on unknown commands, or at least logging which context was selected.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents