[codex] default Windows ARM backend runtime to x64 #90
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||
| export const DESKTOP_TARGET_ARCH_ENV = 'ASTRBOT_DESKTOP_TARGET_ARCH'; | ||||||
| export const WINDOWS_ARM_BACKEND_ARCH_ENV = 'ASTRBOT_DESKTOP_WINDOWS_ARM_BACKEND_ARCH'; | ||||||
| export const BUNDLED_RUNTIME_ARCH_ENV = 'ASTRBOT_DESKTOP_BUNDLED_RUNTIME_ARCH'; | ||||||
|
|
||||||
| const PROCESS_ARCH_MAP = { | ||||||
| x64: 'amd64', | ||||||
| arm64: 'arm64', | ||||||
| }; | ||||||
|
|
||||||
| export const normalizeDesktopArch = (rawArch, sourceName) => { | ||||||
| const raw = String(rawArch ?? '').trim().toLowerCase(); | ||||||
| if (raw === 'amd64' || raw === 'x64') { | ||||||
| return 'amd64'; | ||||||
| } | ||||||
| if (raw === 'arm64' || raw === 'aarch64') { | ||||||
| return 'arm64'; | ||||||
| } | ||||||
| throw new Error( | ||||||
| `Invalid ${sourceName} value "${raw}". Expected one of: amd64, x64, arm64, aarch64.`, | ||||||
| ); | ||||||
| }; | ||||||
|
|
||||||
| export const resolveDesktopTargetArch = ({ arch = process.arch, env = process.env } = {}) => { | ||||||
| const overrideRaw = env[DESKTOP_TARGET_ARCH_ENV]; | ||||||
| if (overrideRaw !== undefined && String(overrideRaw).trim()) { | ||||||
| return normalizeDesktopArch(overrideRaw, DESKTOP_TARGET_ARCH_ENV); | ||||||
| } | ||||||
|
|
||||||
| const mappedArch = PROCESS_ARCH_MAP[arch]; | ||||||
| if (mappedArch) { | ||||||
| return mappedArch; | ||||||
| } | ||||||
|
|
||||||
| throw new Error(`Unsupported process.arch for desktop target resolution: ${arch}`); | ||||||
| }; | ||||||
|
|
||||||
| export const resolveBundledRuntimeArch = ({ | ||||||
| platform = process.platform, | ||||||
| arch = process.arch, | ||||||
| env = process.env, | ||||||
| } = {}) => { | ||||||
| const explicitBundledRuntimeArch = env[BUNDLED_RUNTIME_ARCH_ENV]; | ||||||
| if (explicitBundledRuntimeArch !== undefined && String(explicitBundledRuntimeArch).trim()) { | ||||||
|
There was a problem hiding this comment. Similar to the previous comment, this check is not robust against
Suggested change
|
||||||
| return normalizeDesktopArch(explicitBundledRuntimeArch, BUNDLED_RUNTIME_ARCH_ENV); | ||||||
| } | ||||||
|
|
||||||
| const targetArch = resolveDesktopTargetArch({ arch, env }); | ||||||
| if (platform !== 'win32' || targetArch !== 'arm64') { | ||||||
| return targetArch; | ||||||
| } | ||||||
|
|
||||||
| const windowsArmBackendArch = env[WINDOWS_ARM_BACKEND_ARCH_ENV]; | ||||||
| if (windowsArmBackendArch === undefined || !String(windowsArmBackendArch).trim()) { | ||||||
|
There was a problem hiding this comment. This check for
Suggested change
|
||||||
| return 'amd64'; | ||||||
| } | ||||||
|
|
||||||
| return normalizeDesktopArch(windowsArmBackendArch, WINDOWS_ARM_BACKEND_ARCH_ENV); | ||||||
| }; | ||||||
|
|
||||||
| export const isWindowsArm64BundledRuntime = ({ | ||||||
| platform = process.platform, | ||||||
| arch = process.arch, | ||||||
| env = process.env, | ||||||
| } = {}) => platform === 'win32' && resolveBundledRuntimeArch({ platform, arch, env }) === 'arm64'; | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { test } from 'node:test'; | ||
|
|
||
| import { | ||
| isWindowsArm64BundledRuntime, | ||
| resolveBundledRuntimeArch, | ||
| } from './runtime-arch-utils.mjs'; | ||
|
|
||
| test('resolveBundledRuntimeArch defaults Windows ARM64 backend runtime to x64', () => { | ||
| assert.equal( | ||
| resolveBundledRuntimeArch({ platform: 'win32', arch: 'arm64', env: {} }), | ||
| 'amd64', | ||
| ); | ||
| }); | ||
|
|
||
| test('resolveBundledRuntimeArch honors explicit target arch on emulated x64 Node', () => { | ||
| assert.equal( | ||
| resolveBundledRuntimeArch({ | ||
| platform: 'win32', | ||
| arch: 'x64', | ||
| env: { ASTRBOT_DESKTOP_TARGET_ARCH: 'arm64' }, | ||
| }), | ||
| 'amd64', | ||
| ); | ||
|
|
||
| assert.equal( | ||
| resolveBundledRuntimeArch({ | ||
| platform: 'win32', | ||
| arch: 'x64', | ||
| env: { | ||
| ASTRBOT_DESKTOP_TARGET_ARCH: 'arm64', | ||
| ASTRBOT_DESKTOP_WINDOWS_ARM_BACKEND_ARCH: 'arm64', | ||
| }, | ||
| }), | ||
| 'arm64', | ||
| ); | ||
| }); | ||
|
|
||
| test('isWindowsArm64BundledRuntime uses explicit bundled runtime arch handoff', () => { | ||
| assert.equal( | ||
| isWindowsArm64BundledRuntime({ | ||
| platform: 'win32', | ||
| arch: 'x64', | ||
| env: { ASTRBOT_DESKTOP_BUNDLED_RUNTIME_ARCH: 'arm64' }, | ||
| }), | ||
| true, | ||
| ); | ||
|
|
||
| assert.equal( | ||
| isWindowsArm64BundledRuntime({ | ||
| platform: 'win32', | ||
| arch: 'x64', | ||
| env: { ASTRBOT_DESKTOP_BUNDLED_RUNTIME_ARCH: 'amd64' }, | ||
| }), | ||
| false, | ||
| ); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { test } from 'node:test'; | ||
|
|
||
| import * as backendRuntime from './backend-runtime.mjs'; | ||
|
|
||
| const resolvePbsTarget = (options) => backendRuntime.resolvePbsTarget(options); | ||
|
|
||
| test('resolvePbsTarget defaults Windows ARM64 backend runtime to x64', () => { | ||
| assert.equal( | ||
| resolvePbsTarget({ platform: 'win32', arch: 'arm64', env: {} }), | ||
| 'x86_64-pc-windows-msvc', | ||
| ); | ||
| }); | ||
|
|
||
| test('resolvePbsTarget accepts explicit Windows ARM64 backend overrides', () => { | ||
| assert.equal( | ||
| resolvePbsTarget({ | ||
| platform: 'win32', | ||
| arch: 'arm64', | ||
| env: { ASTRBOT_DESKTOP_WINDOWS_ARM_BACKEND_ARCH: 'amd64' }, | ||
| }), | ||
| 'x86_64-pc-windows-msvc', | ||
| ); | ||
|
|
||
| assert.equal( | ||
| resolvePbsTarget({ | ||
| platform: 'win32', | ||
| arch: 'arm64', | ||
| env: { ASTRBOT_DESKTOP_WINDOWS_ARM_BACKEND_ARCH: 'x64' }, | ||
| }), | ||
| 'x86_64-pc-windows-msvc', | ||
| ); | ||
|
|
||
| assert.equal( | ||
| resolvePbsTarget({ | ||
| platform: 'win32', | ||
| arch: 'arm64', | ||
| env: { ASTRBOT_DESKTOP_WINDOWS_ARM_BACKEND_ARCH: 'arm64' }, | ||
| }), | ||
| 'aarch64-pc-windows-msvc', | ||
| ); | ||
|
|
||
| assert.equal( | ||
| resolvePbsTarget({ | ||
| platform: 'win32', | ||
| arch: 'arm64', | ||
| env: { ASTRBOT_DESKTOP_WINDOWS_ARM_BACKEND_ARCH: 'aarch64' }, | ||
| }), | ||
| 'aarch64-pc-windows-msvc', | ||
| ); | ||
| }); | ||
|
|
||
| test('resolvePbsTarget honors the explicit desktop target arch when process arch is emulated x64', () => { | ||
| assert.equal( | ||
| resolvePbsTarget({ | ||
| platform: 'win32', | ||
| arch: 'x64', | ||
| env: { ASTRBOT_DESKTOP_TARGET_ARCH: 'arm64' }, | ||
| }), | ||
| 'x86_64-pc-windows-msvc', | ||
| ); | ||
|
|
||
| assert.equal( | ||
| resolvePbsTarget({ | ||
| platform: 'win32', | ||
| arch: 'x64', | ||
| env: { | ||
| ASTRBOT_DESKTOP_TARGET_ARCH: 'arm64', | ||
| ASTRBOT_DESKTOP_WINDOWS_ARM_BACKEND_ARCH: 'arm64', | ||
| }, | ||
| }), | ||
| 'aarch64-pc-windows-msvc', | ||
| ); | ||
| }); | ||
|
|
||
| test('resolvePbsTarget rejects invalid Windows ARM64 backend override values', () => { | ||
| assert.throws( | ||
| () => | ||
| resolvePbsTarget({ | ||
| platform: 'win32', | ||
| arch: 'arm64', | ||
| env: { ASTRBOT_DESKTOP_WINDOWS_ARM_BACKEND_ARCH: 'wat' }, | ||
| }), | ||
| /ASTRBOT_DESKTOP_WINDOWS_ARM_BACKEND_ARCH[\s\S]*amd64[\s\S]*x64[\s\S]*arm64[\s\S]*aarch64/, | ||
| ); | ||
| }); | ||
|
|
||
| test('resolvePbsTarget rejects invalid explicit desktop target arch values', () => { | ||
| assert.throws( | ||
| () => | ||
| resolvePbsTarget({ | ||
| platform: 'win32', | ||
| arch: 'x64', | ||
| env: { ASTRBOT_DESKTOP_TARGET_ARCH: 'wat' }, | ||
| }), | ||
| /ASTRBOT_DESKTOP_TARGET_ARCH[\s\S]*amd64[\s\S]*x64[\s\S]*arm64[\s\S]*aarch64/, | ||
| ); | ||
| }); | ||
|
|
||
| test('resolvePbsTarget keeps same-arch mappings for other platform and arch combinations', () => { | ||
| const cases = [ | ||
| { platform: 'linux', arch: 'x64', expected: 'x86_64-unknown-linux-gnu' }, | ||
| { platform: 'linux', arch: 'arm64', expected: 'aarch64-unknown-linux-gnu' }, | ||
| { platform: 'darwin', arch: 'x64', expected: 'x86_64-apple-darwin' }, | ||
| { platform: 'darwin', arch: 'arm64', expected: 'aarch64-apple-darwin' }, | ||
| { platform: 'win32', arch: 'x64', expected: 'x86_64-pc-windows-msvc' }, | ||
| ]; | ||
|
|
||
| for (const { platform, arch, expected } of cases) { | ||
| assert.equal(resolvePbsTarget({ platform, arch, env: {} }), expected); | ||
| } | ||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The current check for
overrideRawis not robust againstnullvalues. Ifenv[DESKTOP_TARGET_ARCH_ENV]isnull,overrideRaw !== undefinedevaluates to true, andString(overrideRaw).trim()becomes'null', which is a truthy string. This would causenormalizeDesktopArchto be called withnull, leading to an error. Using the nullish coalescing operator (??) provides a more robust way to handle potentiallynullorundefinedvalues, ensuring they are treated as empty strings and correctly ignored.