mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(dev): keep the shared Electron dist writable for the dev app
pn dev crashes on macOS in any worktree that adopted the shared Electron dist. publishSharedElectronDist marks the cache entry read-only, which hardlink sharing needs, but clonefile preserves mode -- so the dist lands 0555, the dev runner copies it into out/electron-dev unchanged, and the first plutil -replace on Info.plist fails with a permission error. The shipped zip has that file at 0644; on disk it is 0555, so the mode is ours, not upstream's. copyPrivateTree now restores write permission. Its contract is a private tree the caller goes on to patch, and its one production caller is the dev runner. The test that should have caught this ran the wrapper with stdio: 'ignore', so a hard crash presented as a bare 20s timeout. It now captures the wrapper's output into the failure message, and waits long enough for the two synchronous swiftc builds and a codesign --deep over ~280MB that precede the assertion.
This commit is contained in:
@@ -110,32 +110,60 @@ export function makeTreeReadOnly(targetPath, chmod = chmodSync) {
|
||||
chmod(targetPath, 0o755)
|
||||
}
|
||||
|
||||
/**
|
||||
* Restore owner write permission across a private copy.
|
||||
*
|
||||
* Counterpart to `makeTreeReadOnly`: clonefile, reflink and `cpSync` all carry the source's mode
|
||||
* across, so a tree copied from the write-protected shared cache lands read-only and every patch
|
||||
* the caller then makes -- `plutil -replace`, `codesign` -- fails with EACCES. Only the owner bit
|
||||
* comes back; group and other stay as the source left them.
|
||||
*/
|
||||
export function makeTreeWritable(targetPath, chmod = chmodSync) {
|
||||
for (const entry of readdirSync(targetPath, { withFileTypes: true })) {
|
||||
const entryPath = join(targetPath, entry.name)
|
||||
if (entry.isDirectory()) {
|
||||
makeTreeWritable(entryPath, chmod)
|
||||
} else if (!entry.isSymbolicLink()) {
|
||||
const mode = statSync(entryPath, { throwIfNoEntry: false })?.mode
|
||||
chmod(entryPath, mode === undefined ? 0o644 : mode | 0o200)
|
||||
}
|
||||
}
|
||||
chmod(targetPath, 0o755)
|
||||
}
|
||||
|
||||
/**
|
||||
* Share storage when possible, otherwise copy the bytes.
|
||||
*
|
||||
* Never hardlinks: this is for trees the caller goes on to patch, where shared inodes would write
|
||||
* through into the source.
|
||||
* through into the source. The copy is unprotected on the way out for the same reason -- a private
|
||||
* tree the caller cannot write to is useless to it.
|
||||
*/
|
||||
export function copyPrivateTree(sourcePath, destinationPath, options = {}) {
|
||||
const platform = options.platform ?? process.platform
|
||||
const copy = options.copy ?? copyTreeVerbatim
|
||||
const unprotect = options.unprotect ?? makeTreeWritable
|
||||
const privateMechanisms = new Set(['clone', 'reflink'])
|
||||
let result = { mechanism: null, copyError: null }
|
||||
if (getShareMechanisms(platform).some((mechanism) => privateMechanisms.has(mechanism))) {
|
||||
try {
|
||||
const mechanism = shareTree(sourcePath, destinationPath, {
|
||||
...options,
|
||||
hardlink: () => {
|
||||
throw new Error('hardlinks would not be private')
|
||||
}
|
||||
})
|
||||
return { mechanism, copyError: null }
|
||||
result = {
|
||||
mechanism: shareTree(sourcePath, destinationPath, {
|
||||
...options,
|
||||
hardlink: () => {
|
||||
throw new Error('hardlinks would not be private')
|
||||
}
|
||||
}),
|
||||
copyError: null
|
||||
}
|
||||
} catch (copyError) {
|
||||
copy(sourcePath, destinationPath)
|
||||
return { mechanism: null, copyError }
|
||||
result = { mechanism: null, copyError }
|
||||
}
|
||||
} else {
|
||||
copy(sourcePath, destinationPath)
|
||||
}
|
||||
copy(sourcePath, destinationPath)
|
||||
return { mechanism: null, copyError: null }
|
||||
unprotect(destinationPath)
|
||||
return result
|
||||
}
|
||||
|
||||
function copyTreeVerbatim(sourcePath, destinationPath) {
|
||||
|
||||
@@ -19,6 +19,7 @@ import {
|
||||
copyPrivateTree,
|
||||
hardlinkTree,
|
||||
makeTreeReadOnly,
|
||||
makeTreeWritable,
|
||||
shareTree
|
||||
} from './space-sharing-copy.mjs'
|
||||
|
||||
@@ -170,7 +171,41 @@ describe('makeTreeReadOnly', () => {
|
||||
)
|
||||
})
|
||||
|
||||
describe('makeTreeWritable', () => {
|
||||
it.runIf(process.platform !== 'win32')('undoes makeTreeReadOnly for the owner', () => {
|
||||
const { source } = makeTree()
|
||||
makeTreeReadOnly(source)
|
||||
makeTreeWritable(source)
|
||||
const file = path.join(source, 'nested', 'file')
|
||||
expect(statSync(file).mode & 0o200).toBe(0o200)
|
||||
expect(() => writeFileSync(file, 'mutated')).not.toThrow()
|
||||
})
|
||||
|
||||
it.runIf(process.platform !== 'win32')('adds no write permission beyond the owner', () => {
|
||||
const { source } = makeTree()
|
||||
const executable = path.join(source, 'electron')
|
||||
writeFileSync(executable, 'binary')
|
||||
chmodSync(executable, 0o555)
|
||||
makeTreeWritable(source)
|
||||
expect(statSync(executable).mode & 0o777).toBe(0o755)
|
||||
})
|
||||
})
|
||||
|
||||
describe('copyPrivateTree', () => {
|
||||
it.runIf(process.platform !== 'win32')(
|
||||
'hands back a tree the caller can patch, even from a write-protected source',
|
||||
() => {
|
||||
const { root, source } = makeTree()
|
||||
const destination = path.join(root, 'private')
|
||||
makeTreeReadOnly(source)
|
||||
copyPrivateTree(source, destination)
|
||||
// The regression this guards: the shared Electron dist is read-only, clonefile/reflink/cpSync
|
||||
// all carry that across, and `pn dev` then died patching the copied bundle's Info.plist.
|
||||
expect(() => writeFileSync(path.join(destination, 'nested', 'file'), 'patched')).not.toThrow()
|
||||
expect(readFileSync(path.join(source, 'nested', 'file'), 'utf8')).toBe('contents')
|
||||
}
|
||||
)
|
||||
|
||||
it('never hardlinks, because the caller patches what it gets back', () => {
|
||||
const { root, source } = makeTree()
|
||||
const destination = path.join(root, 'private')
|
||||
|
||||
@@ -105,6 +105,56 @@ function devWrapperTestEnv(extra: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
|
||||
return { ...env, ...extra }
|
||||
}
|
||||
|
||||
/**
|
||||
* What the two cases below wait on: a ~280MB clone of Electron.app, two swiftc
|
||||
* helper builds, and `codesign --deep` over the result. Six seconds on an idle
|
||||
* machine; the swiftc builds alone pass fifteen when this file runs inside the
|
||||
* full suite and every core is taken. The generous ceiling only costs time on a
|
||||
* run that is already failing.
|
||||
*/
|
||||
const PREPARE_TIMEOUT_MS = 90_000
|
||||
|
||||
/**
|
||||
* Spawns the wrapper with its output retained.
|
||||
*
|
||||
* Why retained: the wrapper reports its own failures on stderr, and discarding
|
||||
* them turned a crash in prepare into a bare "Timed out waiting for condition"
|
||||
* with nothing to act on.
|
||||
*/
|
||||
function spawnDevWrapper(
|
||||
args: string[],
|
||||
env: NodeJS.ProcessEnv
|
||||
): { wrapper: ChildProcess; readOutput: () => string } {
|
||||
const wrapper = spawn(process.execPath, args, {
|
||||
cwd: resolve('.'),
|
||||
env,
|
||||
stdio: ['ignore', 'pipe', 'pipe']
|
||||
})
|
||||
let output = ''
|
||||
const collect = (chunk: Buffer): void => {
|
||||
output += chunk.toString()
|
||||
}
|
||||
wrapper.stdout?.on('data', collect)
|
||||
wrapper.stderr?.on('data', collect)
|
||||
return { wrapper, readOutput: () => output }
|
||||
}
|
||||
|
||||
async function waitForEnvFile(envFile: string, readOutput: () => string): Promise<void> {
|
||||
try {
|
||||
await waitFor(() => {
|
||||
try {
|
||||
return readFileSync(envFile, 'utf8').trim().length > 0
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}, PREPARE_TIMEOUT_MS)
|
||||
} catch (error) {
|
||||
throw new Error(
|
||||
`${(error as Error).message}: the dev wrapper never wrote ${envFile}. Wrapper output:\n${readOutput() || '(none)'}`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
describe('run-electron-vite-dev', () => {
|
||||
afterEach(async () => {
|
||||
for (const pid of processesToCleanUp) {
|
||||
@@ -351,26 +401,19 @@ describe('run-electron-vite-dev', () => {
|
||||
async function runWrapper(runId: string): Promise<{ electronExecPath: string }> {
|
||||
const pidFile = join(tempDir, `${runId}.pid`)
|
||||
const envFile = join(tempDir, `${runId}.json`)
|
||||
const wrapper = spawn(process.execPath, [wrapperPath, '--remote-debugging-port=9448'], {
|
||||
cwd: resolve('.'),
|
||||
env: {
|
||||
const { wrapper, readOutput } = spawnDevWrapper(
|
||||
[wrapperPath, '--remote-debugging-port=9448'],
|
||||
{
|
||||
...baseEnv,
|
||||
ORCA_DEV_WRAPPER_TEST_PID_FILE: pidFile,
|
||||
ORCA_DEV_WRAPPER_TEST_ENV_FILE: envFile
|
||||
},
|
||||
stdio: 'ignore'
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
expect(wrapper.pid).toBeTypeOf('number')
|
||||
processesToCleanUp.add(wrapper.pid!)
|
||||
|
||||
await waitFor(() => {
|
||||
try {
|
||||
return readFileSync(envFile, 'utf8').trim().length > 0
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}, 20000)
|
||||
await waitForEnvFile(envFile, readOutput)
|
||||
|
||||
const trackedPids = trackPidFile(pidFile)
|
||||
|
||||
@@ -409,7 +452,8 @@ describe('run-electron-vite-dev', () => {
|
||||
}
|
||||
}
|
||||
},
|
||||
30000
|
||||
// Two full prepares, each budgeted at PREPARE_TIMEOUT_MS.
|
||||
PREPARE_TIMEOUT_MS * 2 + 30_000
|
||||
)
|
||||
|
||||
it.skipIf(process.platform !== 'darwin')(
|
||||
@@ -421,9 +465,9 @@ describe('run-electron-vite-dev', () => {
|
||||
const wrapperPath = resolve('config/scripts/run-electron-vite-dev.mjs')
|
||||
const fakeCliPath = resolve('src/main/startup/__fixtures__/fake-electron-vite-dev-cli.mjs')
|
||||
|
||||
const wrapper = spawn(process.execPath, [wrapperPath, '--remote-debugging-port=9448'], {
|
||||
cwd: resolve('.'),
|
||||
env: devWrapperTestEnv({
|
||||
const { wrapper, readOutput } = spawnDevWrapper(
|
||||
[wrapperPath, '--remote-debugging-port=9448'],
|
||||
devWrapperTestEnv({
|
||||
ORCA_ELECTRON_VITE_CLI: fakeCliPath,
|
||||
ORCA_SKIP_DEV_CLI_PREPARE: '1',
|
||||
ORCA_SKIP_DEV_WEB_PREPARE: '1',
|
||||
@@ -431,20 +475,13 @@ describe('run-electron-vite-dev', () => {
|
||||
ORCA_DEV_WRAPPER_TEST_ENV_FILE: envFile,
|
||||
ORCA_DEV_BRANCH: 'feature/framework-symlinks',
|
||||
ORCA_DEV_WORKTREE_NAME: 'symlink-ui'
|
||||
}),
|
||||
stdio: 'ignore'
|
||||
})
|
||||
})
|
||||
)
|
||||
|
||||
expect(wrapper.pid).toBeTypeOf('number')
|
||||
processesToCleanUp.add(wrapper.pid!)
|
||||
|
||||
await waitFor(() => {
|
||||
try {
|
||||
return readFileSync(envFile, 'utf8').trim().length > 0
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}, 20000)
|
||||
await waitForEnvFile(envFile, readOutput)
|
||||
|
||||
const trackedPids = trackPidFile(pidFile)
|
||||
|
||||
@@ -464,6 +501,6 @@ describe('run-electron-vite-dev', () => {
|
||||
|
||||
await stopWrapperAndTrackedPids(wrapper, trackedPids)
|
||||
},
|
||||
30000
|
||||
PREPARE_TIMEOUT_MS + 30_000
|
||||
)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user