refactor: simplify baseline version check, add native windows arm64 build#163
Closed
kane50613 wants to merge 4 commits intooven-sh:mainfrom
Closed
refactor: simplify baseline version check, add native windows arm64 build#163kane50613 wants to merge 4 commits intooven-sh:mainfrom
kane50613 wants to merge 4 commits intooven-sh:mainfrom
Conversation
WalkthroughAdds Windows ARM64 handling: new version check/warning and fallback to x64+AVX2 disabled for unsupported Windows aarch64 tags; refactors architecture/AVX2 helpers to accept only architecture; updates download URL assembly and expands tests for Windows ARM64 behavior and utilities. Changes
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/download-url.ts`:
- Around line 67-70: The current filename construction (around getArchitecture
and getAvx2) always yields the windows-aarch64 asset for Windows ARM64; change
the download-resolution logic that builds the asset name (the function that
composes the filename using getArchitecture(...) and getAvx2(...)) so that after
composing the windows-aarch64 filename you check whether that asset actually
exists for the requested tag (e.g., with a HEAD or by checking the release
assets list), and if it does not exist, fall back to the windows-x64-baseline
filename variant before returning the URL; update any helper that computes
eavx2/getAvx2-based suffixes accordingly so the fallback uses the x64-baseline
naming.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/download-url.tssrc/utils.tstests/utils.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/download-url.ts`:
- Around line 72-78: The code currently uses the URL-encoded variable eos for
platform branching which is brittle; introduce a raw osValue = os ??
getPlatform(), set eos = encodeURIComponent(osValue) for URL composition, and
change platform checks (e.g., the if that does eos === "windows" and calls
getArchitecture(archValue) and isWindowsArm64Supported(tag ?? version)) to use
osValue instead of eos; keep archValue and avx2Value logic unchanged and only
use eos when building the download URL.
In `@src/utils.ts`:
- Around line 65-71: The function isWindowsArm64Supported currently uses a
hardcoded cutoff "1.3.10"; extract that literal into a clearly named constant
(for example MIN_WINDOWS_ARM64_VERSION) at the top of the module and replace the
string in isWindowsArm64Supported with that constant so the policy boundary is
centralized and easier to update; keep the same validation and compareVersions
logic but refer to MIN_WINDOWS_ARM64_VERSION instead of the inline "1.3.10".
- Around line 76-79: The four warning strings in src/utils.ts currently read as
a global Windows ARM64 limitation; update them to be version-scoped so the log
makes clear this applies only to unsupported Bun/OS versions (e.g., change
phrasing to "On this OS/version, Bun does not provide native arm64 builds for
Windows" and similarly qualify the following lines to reference "on this
runner/version" or "on affected versions"); modify the string literals (the four
warning message entries) so they explicitly state the limitation is for the
detected/unsupported version rather than a general global statement and keep
existing contextual guidance (x64 fallback, performance caveats, suggestion to
use x64 runners). Ensure the updated messages are used wherever the original
message array/constant in src/utils.ts is referenced.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
dist/setup/index.jsis excluded by!**/dist/**
📒 Files selected for processing (3)
src/download-url.tssrc/utils.tstests/download-url.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils.ts`:
- Around line 8-10: The file contains an extra blank line after the
MIN_WINDOWS_ARM64_VERSION constant declaration; remove the redundant empty line
following the export of MIN_WINDOWS_ARM64_VERSION so the file has a single
newline at EOF and no double blank line after the constant.
In `@tests/download-url.spec.ts`:
- Around line 163-205: Add a test case exercising getDownloadUrl for version
"latest" on Windows/arm64 to assert it returns the aarch64 URL (same behavior as
canary and as dictated by isWindowsArm64Supported) and that no fallback warning
is emitted; locate the Windows ARM64 test suite (the describe block "Windows
ARM64 Fallback") and add an it block similar to the "canary" test but with
version: "latest" and expect the URL to be
"https://github.com/oven-sh/bun/releases/download/latest/bun-windows-aarch64.zip"
and do not expect core.warning to have been called.
In `@tests/utils.spec.ts`:
- Around line 29-34: Remove the unused variable declaration warningSpy from the
getArchitecture test suite: delete the line "let warningSpy: ReturnType<typeof
spyOn>;" and keep the existing afterEach block (remove any references to
warningSpy inside afterEach if present) so the suite no longer contains an
unused variable; verify getArchitecture tests still run and no other tests
reference warningSpy.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
dist/setup/index.jsis excluded by!**/dist/**
📒 Files selected for processing (4)
src/download-url.tssrc/utils.tstests/download-url.spec.tstests/utils.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils.ts`:
- Around line 67-75: Add a unit test that verifies Windows ARM64 handling when a
semver range is passed through the download flow: specifically create a test
that calls the download URL resolution path so that isWindowsArm64Supported(tag
?? version) in download-url.ts receives a semver range (e.g., "^1.0.0") instead
of a concrete version, and assert the boolean result matches the expected
behavior; target the isWindowsArm64Supported function and the code path in
download-url.ts that computes tag/version (mock or supply the resolved version
as needed) to ensure semver ranges are correctly evaluated against
MIN_WINDOWS_ARM64_VERSION and special tags ("canary"/"latest") are still
handled.
In `@tests/download-url.spec.ts`:
- Around line 165-181: The test creates a spy via spyOn(core, "warning")
(warningSpy) but calls warningSpy.mockRestore() only at the end which can be
skipped if an assertion throws; wrap the test assertions and expectations in a
try/finally (or otherwise ensure cleanup) so that warningSpy.mockRestore() runs
in the finally block to prevent the spy from leaking into other tests; apply the
same change to the other similar test block (the one around lines 207-220) that
also uses spyOn(core, "warning") and warningSpy.mockRestore().
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/utils.tstests/download-url.spec.tstests/utils.spec.ts
Comment on lines
+67
to
+75
| export function isWindowsArm64Supported(tag: string): boolean { | ||
| if (tag === "canary" || tag === "latest") return true; | ||
|
|
||
| const version = tag.startsWith("bun-v") ? tag.slice(5) : tag; | ||
|
|
||
| return ( | ||
| validate(version) && | ||
| compareVersions(version, MIN_WINDOWS_ARM64_VERSION) >= 0 | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Find where Windows ARM64 support is gated ==="
rg -n -C4 '\bisWindowsArm64Supported\s*\(' src/download-url.ts
echo ""
echo "=== 2) Inspect version resolution flow around dynamic inputs ==="
rg -n -C4 '\b(latest|canary|satisfies|maxSatisfying|resolved|version)\b' src/download-url.ts
echo ""
echo "=== 3) Check current tests for Windows ARM64 + semver range coverage ==="
rg -n -C3 'Windows ARM64|\^1\.0\.0|arch:\s*"arm64"' tests/download-url.spec.ts tests/utils.spec.ts 2>/dev/null || echo "Test files or patterns not found"
echo ""
echo "=== 4) Check if validate function is available and its behavior ==="
rg -n 'validate\(' src/utils.ts | head -20
echo ""
echo "=== 5) Find all imports in utils.ts to understand dependencies ==="
head -20 src/utils.tsRepository: oven-sh/setup-bun
Length of output: 5652
Add test coverage for Windows ARM64 with semver range inputs.
The version resolution flow (lines 30–69) should provide a concrete version to isWindowsArm64Supported(tag ?? version) at line 79 of download-url.ts. However, there is no test verifying that Windows ARM64 correctly handles semver ranges like "^1.0.0". The existing Windows ARM64 tests only use concrete versions ("1.1.0", MIN_WINDOWS_ARM64_VERSION) and special tags ("canary", "latest"). The fallback path tag ?? version is untested, and while the code flow suggests tag should be defined, an explicit test for Windows ARM64 + semver range would validate the assumption and ensure the function returns the expected value when a range is passed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils.ts` around lines 67 - 75, Add a unit test that verifies Windows
ARM64 handling when a semver range is passed through the download flow:
specifically create a test that calls the download URL resolution path so that
isWindowsArm64Supported(tag ?? version) in download-url.ts receives a semver
range (e.g., "^1.0.0") instead of a concrete version, and assert the boolean
result matches the expected behavior; target the isWindowsArm64Supported
function and the code path in download-url.ts that computes tag/version (mock or
supply the resolved version as needed) to ensure semver ranges are correctly
evaluated against MIN_WINDOWS_ARM64_VERSION and special tags ("canary"/"latest")
are still handled.
Comment on lines
+165
to
+181
| it("should fallback to x64-baseline for versions older than MIN_WINDOWS_ARM64_VERSION and log a warning", async () => { | ||
| const warningSpy = spyOn(core, "warning"); | ||
| const url = await getDownloadUrl({ | ||
| version: "1.1.0", | ||
| os: "windows", | ||
| arch: "arm64", | ||
| }); | ||
|
|
||
| expect(url).toBe( | ||
| "https://github.com/oven-sh/bun/releases/download/bun-v1.1.0/bun-windows-x64-baseline.zip", | ||
| ); | ||
| expect(warningSpy).toHaveBeenCalled(); | ||
| expect(warningSpy.mock.calls[0][0]).toContain( | ||
| "On this OS/version, Bun does not provide native arm64 builds for Windows", | ||
| ); | ||
| warningSpy.mockRestore(); | ||
| }); |
There was a problem hiding this comment.
Restore warning spies in finally to prevent cross-test leakage on failure.
If an assertion throws before mockRestore(), the spy can bleed into later tests and create noisy failures.
♻️ Suggested hardening
it("should fallback to x64-baseline for versions older than MIN_WINDOWS_ARM64_VERSION and log a warning", async () => {
const warningSpy = spyOn(core, "warning");
- const url = await getDownloadUrl({
- version: "1.1.0",
- os: "windows",
- arch: "arm64",
- });
-
- expect(url).toBe(
- "https://github.com/oven-sh/bun/releases/download/bun-v1.1.0/bun-windows-x64-baseline.zip",
- );
- expect(warningSpy).toHaveBeenCalled();
- expect(warningSpy.mock.calls[0][0]).toContain(
- "On this OS/version, Bun does not provide native arm64 builds for Windows",
- );
- warningSpy.mockRestore();
+ try {
+ const url = await getDownloadUrl({
+ version: "1.1.0",
+ os: "windows",
+ arch: "arm64",
+ });
+
+ expect(url).toBe(
+ "https://github.com/oven-sh/bun/releases/download/bun-v1.1.0/bun-windows-x64-baseline.zip",
+ );
+ expect(warningSpy).toHaveBeenCalled();
+ expect(warningSpy.mock.calls[0][0]).toContain(
+ "On this OS/version, Bun does not provide native arm64 builds for Windows",
+ );
+ } finally {
+ warningSpy.mockRestore();
+ }
});
@@
it("should use aarch64 for latest", async () => {
const warningSpy = spyOn(core, "warning");
- const url = await getDownloadUrl({
- version: "latest",
- os: "windows",
- arch: "arm64",
- });
-
- expect(url).toBe(
- "https://github.com/oven-sh/bun/releases/download/bun-v1.3.10/bun-windows-aarch64.zip",
- );
- expect(warningSpy).not.toHaveBeenCalled();
- warningSpy.mockRestore();
+ try {
+ const url = await getDownloadUrl({
+ version: "latest",
+ os: "windows",
+ arch: "arm64",
+ });
+
+ expect(url).toBe(
+ "https://github.com/oven-sh/bun/releases/download/bun-v1.3.10/bun-windows-aarch64.zip",
+ );
+ expect(warningSpy).not.toHaveBeenCalled();
+ } finally {
+ warningSpy.mockRestore();
+ }
});Also applies to: 207-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/download-url.spec.ts` around lines 165 - 181, The test creates a spy
via spyOn(core, "warning") (warningSpy) but calls warningSpy.mockRestore() only
at the end which can be skipped if an assertion throws; wrap the test assertions
and expectations in a try/finally (or otherwise ensure cleanup) so that
warningSpy.mockRestore() runs in the finally block to prevent the spy from
leaking into other tests; apply the same change to the other similar test block
(the one around lines 207-220) that also uses spyOn(core, "warning") and
warningSpy.mockRestore().
4 tasks
Collaborator
|
Thank you for opening this pull request so quickly. I really appreciate the effort. :) Anyway, thank you for the effort and for taking the time to work on this. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the version and architecture handling logic to support native Windows ARM64 builds starting from Bun version 1.3.10, while maintaining backward compatibility with older versions by falling back to x64-baseline builds.
Changes:
- Simplified
getArchitectureandgetAvx2functions by removing OS-specific parameters, moving platform-specific logic to the download URL builder - Added version-based Windows ARM64 support detection with fallback to x64-baseline for versions < 1.3.10
- Updated all tests to reflect the refactored API and added comprehensive Windows ARM64 test scenarios
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/utils.ts | Added MIN_WINDOWS_ARM64_VERSION constant, refactored getArchitecture/getAvx2 to remove OS parameter, added isWindowsArm64Supported and warnWindowsArm64 functions |
| src/download-url.ts | Integrated Windows ARM64 version checking logic, conditionally falls back to x64-baseline for unsupported versions |
| tests/utils.spec.ts | Added tests for isWindowsArm64Supported, updated getArchitecture/getAvx2 tests to match new simplified signatures |
| tests/download-url.spec.ts | Added comprehensive Windows ARM64 fallback test scenarios, updated MOCK_TAGS to include version 1.3.10 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| export function isWindowsArm64Supported(tag: string): boolean { | ||
| if (tag === "canary" || tag === "latest") return true; |
There was a problem hiding this comment.
The check for "latest" in this function appears to be unreachable in the current code flow. When version is "latest", it gets resolved to a concrete version tag (e.g., "bun-v1.3.10") in download-url.ts before being passed to this function. The function still works correctly via version comparison, but the "latest" special case check never executes. Consider documenting this or restructuring the code to make the intent clearer.
Suggested change
| if (tag === "canary" || tag === "latest") return true; | |
| // Treat "canary" as always supporting Windows arm64; all other tags are | |
| // evaluated via semantic version comparison against MIN_WINDOWS_ARM64_VERSION. | |
| if (tag === "canary") return true; |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.