try windows edition #1
Conversation
robobun
force-pushed
the
windows-edition
branch
from
9915b97 to
ce37175
Compare
- Add Windows Dockerfile with Bun development environment - Add PowerShell build script for Windows - Add GitHub Actions workflow for daily Windows builds - Update .gitignore for Windows-specific files
RiskyMH
force-pushed
the
windows-edition
branch
from
ca1bac8 to
134603a
Compare
WalkthroughAdds Windows Docker build support through a multi-stage Dockerfile, a PowerShell build script, and a GitHub Actions workflow that automates daily Windows Docker image builds with version tagging and validation tests. Changes
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
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: 13
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/daily-build-windows.yml(1 hunks).gitignore(1 hunks)Dockerfile.windows(1 hunks)build-windows.ps1(1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
Dockerfile.windows
[low] 1-108: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-108: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 Hadolint (2.14.0)
Dockerfile.windows
[info] 16-16: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
[info] 56-56: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
[warning] 71-71: env is referenced but not assigned (for output from commands, use "$(env ...)" ).
(SC2154)
[error] 88-88: Parsing stopped here. Invalid use of parentheses?
(SC1088)
[info] 93-93: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
[info] 93-93: This \b will be a regular 'b' in this context.
(SC1001)
[info] 104-104: This \c will be a regular 'c' in this context.
(SC1001)
🔇 Additional comments (13)
.gitignore (1)
3-3: LGTM!The addition of
temp-bun-check/to the ignore list is appropriate for the Windows build workflow and aligns with existing artifact patterns.Dockerfile.windows (5)
1-6: LGTM!The escape character and shell configuration are appropriate for Windows containers. Setting
$ErrorActionPreference = 'Stop'ensures failures are caught early.
15-19: LGTM!The installation of Git and PowerShell Core via Chocolatey is straightforward, and the PATH configuration correctly includes all necessary directories.
47-53: LGTM!Adding Windows SDK components is necessary for the build toolchain. The quiet, non-restart flags are appropriate for containerized builds.
97-100: LGTM!Using
nanoserverfor the artifacts stage is the correct approach for Windows, asFROM scratchis not supported. The minimal image only contains the build artifacts.
102-108: LGTM!The run stage is correctly configured with a volume mount point, working directory, and explicit entrypoint. The static analysis warnings about missing
HEALTHCHECKand user creation are expected for Windows development containers and don't require changes.build-windows.ps1 (1)
19-28: LGTM!The success/failure handling is appropriate, and the usage instructions helpfully demonstrate both basic and volume-mounted container execution.
.github/workflows/daily-build-windows.yml (6)
8-23: LGTM!The job configuration is appropriate with correct permissions for reading the repository and writing to the container registry. The date format is ISO 8601 compliant.
44-49: LGTM!The GitHub Container Registry login is correctly configured using the standard action and built-in credentials.
220-252: LGTM!The artifacts stage correctly uses cache layering and doesn't need resource limits since it's only copying files.
254-288: LGTM!The run stage is correctly configured with appropriate cache layering and includes a
latesttag for easy reference.
190-209: The review comment is based on an unverified assumption.The original comment claims the PR description mentions "single-threaded compilation for stability," but verification of the commit message, code, and documentation reveals no such requirement. The
--cpu-quota="300000"setting correctly allows 3 CPU cores (verified via web search: 300000 µs ÷ 100000 µs period = 3 CPUs), and there is no evidence in the codebase that single-threaded compilation is required or intended.The Dockerfile.windows build step (
bun run build;) contains no threading flags, and the build-windows.ps1 script is a generic Docker build wrapper. Without evidence supporting the claimed single-threaded requirement, the perceived inconsistency does not exist.Likely an incorrect or invalid review comment.
51-122: Node.js preservation is intentional and appropriate.Verification confirms the commented-out Node.js path is a deliberate choice. The workflow uses only
actions/checkout@v4anddocker/login-action@v3, neither of which requires Node.js. No other workflows reference Node.js either. Preserving it is a conservative approach to the base runner environment, even if not strictly required by this specific workflow.
| on: | ||
| schedule: | ||
| - cron: "0 16 * * *" # Run daily at 8:00am Pacific Time (UTC-8) | ||
| workflow_dispatch: # Allow manual triggering |
There was a problem hiding this comment.
Update timezone comment for accuracy.
The comment states "8:00am Pacific Time (UTC-8)" but doesn't account for Daylight Saving Time. Pacific Time alternates between PST (UTC-8) and PDT (UTC-7), so 16:00 UTC would be 8:00am PST in winter or 9:00am PDT in summer.
Apply this diff for accuracy:
schedule:
- - cron: "0 16 * * *" # Run daily at 8:00am Pacific Time (UTC-8)
+ - cron: "0 16 * * *" # Run daily at 8:00am PST / 9:00am PDT (16:00 UTC)
workflow_dispatch: # Allow manual triggering📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on: | |
| schedule: | |
| - cron: "0 16 * * *" # Run daily at 8:00am Pacific Time (UTC-8) | |
| workflow_dispatch: # Allow manual triggering | |
| on: | |
| schedule: | |
| - cron: "0 16 * * *" # Run daily at 8:00am PST / 9:00am PDT (16:00 UTC) | |
| workflow_dispatch: # Allow manual triggering |
🤖 Prompt for AI Agents
.github/workflows/daily-build-windows.yml around lines 3 to 6: the inline
comment says "8:00am Pacific Time (UTC-8)" which is wrong during Daylight Saving
Time; update the comment to accurately reflect that 16:00 UTC corresponds to
8:00am Pacific Standard Time (UTC-8) in winter and 9:00am Pacific Daylight Time
(UTC-7) in summer, e.g. "Run daily at 8:00am Pacific Time (PT) — 16:00 UTC,
which is UTC-8 in winter (PST) and UTC-7 in summer (PDT)" or similar phrasing
that notes DST variation.
| - name: Get bootstrap script hash | ||
| id: bootstrap-hash | ||
| run: | | ||
| $content = (Invoke-WebRequest -Uri 'https://raw.githubusercontent.com/oven-sh/bun/main/scripts/bootstrap.ps1').Content | ||
| $bytes = [System.Text.Encoding]::UTF8.GetBytes($content) | ||
| $hash = (Get-FileHash -InputStream ([System.IO.MemoryStream]::new($bytes)) -Algorithm SHA256).Hash | ||
| $shortHash = $hash.Substring(0, 7) | ||
| echo "HASH=$shortHash" >> $env:GITHUB_OUTPUT | ||
| shell: pwsh | ||
|
|
||
| - name: Get Bun repo commit SHA | ||
| id: bun-sha | ||
| run: | | ||
| $sha = gh api repos/oven-sh/bun/commits/main --jq .sha | ||
| $shortSha = $sha.Substring(0, 7) | ||
| echo "SHA=$shortSha" >> $env:GITHUB_OUTPUT | ||
| shell: pwsh | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using longer hash prefixes for better collision resistance.
The 7-character hash prefixes provide ~28 bits of entropy, which is generally sufficient for tagging but could lead to collisions in large repositories or with many daily builds over time. Git typically uses 7-12 characters.
Consider increasing to 12 characters for better collision resistance:
$shortHash = $hash.Substring(0, 7)
+ # Alternative: $shortHash = $hash.Substring(0, 12)The current approach is acceptable for this use case, but be aware of potential collisions if the repository has high commit volume.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/daily-build-windows.yml around lines 24 to 43: the workflow
currently truncates both the bootstrap script hash and the Bun repo commit SHA
to 7 characters, which is low collision resistance; update the substring calls
to use 12 characters instead of 7 for both $shortHash and $shortSha so the
output uses 12-character prefixes (adjust any related documentation/consumers if
they expect 7 characters).
| - name: Push bootstrap stage | ||
| run: | | ||
| docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64-${{ steps.bootstrap-hash.outputs.HASH }} | ||
| docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64 | ||
| docker push ghcr.io/${{ github.repository }}-windows:bootstrap | ||
| shell: pwsh |
There was a problem hiding this comment.
Potential for inconsistent state with sequential pushes.
Pushing each tag separately means a network or permission error could leave some tags pushed and others not, creating an inconsistent state in the registry.
Consider collecting errors and reporting at the end:
- name: Push bootstrap stage
run: |
- docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64-${{ steps.bootstrap-hash.outputs.HASH }}
- docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64
- docker push ghcr.io/${{ github.repository }}-windows:bootstrap
+ $errors = @()
+ try { docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64-${{ steps.bootstrap-hash.outputs.HASH }} } catch { $errors += $_ }
+ try { docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64 } catch { $errors += $_ }
+ try { docker push ghcr.io/${{ github.repository }}-windows:bootstrap } catch { $errors += $_ }
+ if ($errors.Count -gt 0) {
+ Write-Host "Push errors: $($errors -join ', ')" -ForegroundColor Red
+ exit 1
+ }
shell: pwshAlternatively, accept the current approach as acceptable since failed pushes will be retried on the next daily run.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Push bootstrap stage | |
| run: | | |
| docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64-${{ steps.bootstrap-hash.outputs.HASH }} | |
| docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64 | |
| docker push ghcr.io/${{ github.repository }}-windows:bootstrap | |
| shell: pwsh | |
| - name: Push bootstrap stage | |
| run: | | |
| $errors = @() | |
| docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64-${{ steps.bootstrap-hash.outputs.HASH }} | |
| if ($LASTEXITCODE -ne 0) { $errors += "Push 1 failed" } | |
| docker push ghcr.io/${{ github.repository }}-windows:bootstrap-amd64 | |
| if ($LASTEXITCODE -ne 0) { $errors += "Push 2 failed" } | |
| docker push ghcr.io/${{ github.repository }}-windows:bootstrap | |
| if ($LASTEXITCODE -ne 0) { $errors += "Push 3 failed" } | |
| if ($errors.Count -gt 0) { | |
| Write-Host "Push errors: $($errors -join ', ')" -ForegroundColor Red | |
| exit 1 | |
| } | |
| shell: pwsh |
🤖 Prompt for AI Agents
In .github/workflows/daily-build-windows.yml around lines 146-151 the workflow
pushes each docker tag sequentially which can leave the registry in a
partial/inconsistent state if one push fails; update the step so each docker
push is executed but failures are collected rather than immediately exiting: run
each push command while capturing its exit status and the tag that failed,
continue to attempt the remaining pushes, then after all pushes check the
collected failures and if any exist print an aggregated error listing the failed
tags and exit non-zero so the job fails and the error is visible in the run logs
(this preserves attempts for all tags while ensuring failures are reported
atomically).
| - name: Build base stage | ||
| run: | | ||
| docker build ` | ||
| --isolation=process ` | ||
| --build-arg BOOTSTRAP_HASH=${{ steps.bootstrap-hash.outputs.HASH }} ` | ||
| --build-arg GIT_SHA=${{ steps.bun-sha.outputs.SHA }} ` | ||
| -f Dockerfile.windows ` | ||
| --target base ` | ||
| --cache-from ghcr.io/${{ github.repository }}-windows:bootstrap-amd64-${{ steps.bootstrap-hash.outputs.HASH }} ` | ||
| --cache-from ghcr.io/${{ github.repository }}-windows:base-amd64-${{ steps.bun-sha.outputs.SHA }} ` | ||
| -t ghcr.io/${{ github.repository }}-windows:base-amd64-${{ steps.bun-sha.outputs.SHA }} ` | ||
| -t ghcr.io/${{ github.repository }}-windows:base-amd64-${{ env.DATE }} ` | ||
| -t ghcr.io/${{ github.repository }}-windows:base-${{ env.DATE }} ` | ||
| -t ghcr.io/${{ github.repository }}-windows:base-amd64 ` | ||
| -t ghcr.io/${{ github.repository }}-windows:base ` | ||
| . | ||
| shell: pwsh |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Memory limit inconsistency across stages.
The base stage build doesn't specify --memory while bootstrap uses --memory 14g and prebuilt also uses --memory 14g. This might be intentional if the base stage is less memory-intensive, but consider documenting why or applying consistent limits.
For consistency, consider adding a memory limit:
run: |
docker build `
--isolation=process `
+ --memory 14g `
--build-arg BOOTSTRAP_HASH=${{ steps.bootstrap-hash.outputs.HASH }} `🤖 Prompt for AI Agents
.github/workflows/daily-build-windows.yml around lines 158 to 174: the docker
build for the "Build base stage" omits a --memory limit while other stages use
--memory 14g; add --memory 14g to the docker build arguments (same place other
stages set it) to make limits consistent, or alternatively add a comment
explaining why this stage intentionally omits the limit if that is desired.
| # === TEST === | ||
| - name: Test prebuilt container | ||
| run: | | ||
| @" | ||
| FROM ghcr.io/${{ github.repository }}-windows:prebuilt-amd64-${{ steps.bun-sha.outputs.SHA }} | ||
| RUN C:\workspace\bun\build\debug\bun-debug.exe --version | ||
| CMD ["C:\\workspace\\bun\\build\\debug\\bun-debug.exe", "--version"] | ||
| "@ | Out-File -FilePath Dockerfile.test -Encoding utf8 | ||
|
|
||
| docker build --isolation=process -f Dockerfile.test -t test-bun . | ||
| docker run --isolation=process --rm test-bun | ||
| shell: pwsh |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider expanding test coverage.
The current test only verifies that Bun can print its version. Consider adding more comprehensive validation to catch runtime issues.
Enhance the test to validate output and test basic functionality:
- name: Test prebuilt container
run: |
@"
FROM ghcr.io/${{ github.repository }}-windows:prebuilt-amd64-${{ steps.bun-sha.outputs.SHA }}
- RUN C:\workspace\bun\build\debug\bun-debug.exe --version
- CMD ["C:\\workspace\\bun\\build\\debug\\bun-debug.exe", "--version"]
+ RUN C:\workspace\bun\build\debug\bun-debug.exe --version > version.txt && `
+ if ((Get-Content version.txt) -notmatch '^\d+\.\d+\.\d+') { exit 1 }
+ RUN C:\workspace\bun\build\debug\bun-debug.exe --eval 'console.log(2 + 2)' > test.txt && `
+ if ((Get-Content test.txt) -ne '4') { exit 1 }
+ CMD ["C:\\workspace\\bun\\build\\debug\\bun-debug.exe", "--version"]
"@ | Out-File -FilePath Dockerfile.test -Encoding utf8
docker build --isolation=process -f Dockerfile.test -t test-bun .
- docker run --isolation=process --rm test-bun
+ $output = docker run --isolation=process --rm test-bun
+ if ($output -notmatch '^\d+\.\d+\.\d+') {
+ Write-Host "Unexpected version output: $output" -ForegroundColor Red
+ exit 1
+ }
+ Write-Host "Test passed: Bun version $output" -ForegroundColor Green
shell: pwshCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/daily-build-windows.yml around lines 290–301, the test step
only checks bun --version; expand it to validate runtime behavior by: generate a
Dockerfile.test that after FROM runs bun to print --version and also runs a
short script (e.g., bun -e "console.log('hello from bun')" or create a small JS
file and run bun run it) and exits with non-zero on failures; update the
workflow commands to build the image, run the container capturing stdout/stderr,
and assert the expected outputs (version string and the script output) and
successful exit code in the PowerShell step so the job fails when bun cannot
execute basic JS functionality.
| # Clone Bun repository to get bootstrap script | ||
| WORKDIR C:\workspace | ||
| RUN git clone https://github.com/oven-sh/bun.git C:\workspace\bun | ||
| WORKDIR C:\workspace\bun |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using shallow clone for faster builds.
The full Bun repository clone includes complete history, which may be unnecessary for building. A shallow clone would reduce build time and image size.
Apply this diff to use a shallow clone:
WORKDIR C:\workspace
-RUN git clone https://github.com/oven-sh/bun.git C:\workspace\bun
+RUN git clone --depth 1 https://github.com/oven-sh/bun.git C:\workspace\bun
WORKDIR C:\workspace\bunNote: This is optional as the workflow later does git pull to update to a specific commit, which may require full history depending on how far back the commit is.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Clone Bun repository to get bootstrap script | |
| WORKDIR C:\workspace | |
| RUN git clone https://github.com/oven-sh/bun.git C:\workspace\bun | |
| WORKDIR C:\workspace\bun | |
| # Clone Bun repository to get bootstrap script | |
| WORKDIR C:\workspace | |
| RUN git clone --depth 1 https://github.com/oven-sh/bun.git C:\workspace\bun | |
| WORKDIR C:\workspace\bun |
🤖 Prompt for AI Agents
Dockerfile.windows around lines 28 to 31: the current git clone pulls full
history; replace it with a shallow clone to speed builds and shrink the image by
using git clone with --depth 1 and optionally --branch <branch-or-tag> to pin to
the desired ref; if later workflow requires older history or checking out a
distant commit, add a step to run git fetch --unshallow (or fall back to a full
clone) before performing the git pull/checkout.
| RUN git pull; ` | ||
| $bootstrapPath = '.\scripts\bootstrap.ps1'; ` | ||
| $content = Get-Content $bootstrapPath -Raw; ` | ||
| $content = $content -replace '"--passive"', '"--quiet"'; ` | ||
| Set-Content -Path $bootstrapPath -Value $content; ` | ||
| pwsh -ExecutionPolicy Bypass -File $bootstrapPath -Optimize:$false; ` | ||
| git checkout $bootstrapPath; ` | ||
| refreshenv; |
There was a problem hiding this comment.
Fragile bootstrap script patching - consider upstream fix.
The string replacement approach (--passive → --quiet) is brittle and may break if the bootstrap script changes. The pattern could match unintended locations or fail to match if the script is updated.
Consider these approaches:
- Preferred: Contribute a patch upstream to make the bootstrap script container-aware
- Alternative: Use a more robust patching mechanism with validation:
RUN git pull; `
$bootstrapPath = '.\scripts\bootstrap.ps1'; `
$content = Get-Content $bootstrapPath -Raw; `
+ if (-not ($content -match '"--passive"')) { `
+ Write-Error "Expected pattern not found in bootstrap script"; `
+ exit 1; `
+ }; `
$content = $content -replace '"--passive"', '"--quiet"'; `
Set-Content -Path $bootstrapPath -Value $content; `
pwsh -ExecutionPolicy Bypass -File $bootstrapPath -Optimize:$false; `
git checkout $bootstrapPath; `
refreshenv;🤖 Prompt for AI Agents
In Dockerfile.windows around lines 38 to 45 the current method uses a simple
string replace to change "--passive" to "--quiet" in scripts\bootstrap.ps1 which
is brittle; either contribute an upstream patch to make the bootstrap script
container-aware (preferred) or replace the naive replace with a robust,
idempotent patch: read the file, locate the exact installer invocation line via
a strict regex or anchor comment, verify the match count is exactly 1, perform
the targeted substitution, write the file, and fail the build if the expected
match/replacement did not occur so we catch breakages; ensure the operation is
reversible/idempotent and add a short validation step (e.g., grep or ps check)
after modification to confirm the script now contains the intended flag.
| # Reload environment and add SDK bin to PATH | ||
| RUN $sdkBin = Get-ChildItem 'C:\Program Files (x86)\Windows Kits' -Recurse -Filter 'rc.exe' -ErrorAction SilentlyContinue | ` | ||
| Where-Object { $_.Directory.Name -eq 'x64' } | ` | ||
| Select-Object -First 1 | ForEach-Object { $_.Directory.FullName }; ` | ||
| if ($sdkBin) { [System.Environment]::SetEnvironmentVariable('SDK_BIN', $sdkBin, 'Machine') }; ` | ||
| refreshenv; | ||
|
|
||
| # Add SDK bin to PATH via ENV so CMake can find rc.exe and mt.exe | ||
| ENV PATH="${SDK_BIN};${PATH}" | ||
| ENV BUN_NO_CORE_DUMP=1 |
There was a problem hiding this comment.
Add error handling for missing SDK tools.
If rc.exe is not found, $sdkBin will be empty, and the PATH will have an empty component. While this may not break the build immediately, it will cause failures later when CMake tries to find rc.exe and mt.exe.
Apply this diff to add validation:
RUN $sdkBin = Get-ChildItem 'C:\Program Files (x86)\Windows Kits' -Recurse -Filter 'rc.exe' -ErrorAction SilentlyContinue | `
Where-Object { $_.Directory.Name -eq 'x64' } | `
Select-Object -First 1 | ForEach-Object { $_.Directory.FullName }; `
- if ($sdkBin) { [System.Environment]::SetEnvironmentVariable('SDK_BIN', $sdkBin, 'Machine') }; `
+ if (-not $sdkBin) { `
+ Write-Error "Windows SDK tools (rc.exe) not found"; `
+ exit 1; `
+ }; `
+ [System.Environment]::SetEnvironmentVariable('SDK_BIN', $sdkBin, 'Machine'); `
refreshenv;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Reload environment and add SDK bin to PATH | |
| RUN $sdkBin = Get-ChildItem 'C:\Program Files (x86)\Windows Kits' -Recurse -Filter 'rc.exe' -ErrorAction SilentlyContinue | ` | |
| Where-Object { $_.Directory.Name -eq 'x64' } | ` | |
| Select-Object -First 1 | ForEach-Object { $_.Directory.FullName }; ` | |
| if ($sdkBin) { [System.Environment]::SetEnvironmentVariable('SDK_BIN', $sdkBin, 'Machine') }; ` | |
| refreshenv; | |
| # Add SDK bin to PATH via ENV so CMake can find rc.exe and mt.exe | |
| ENV PATH="${SDK_BIN};${PATH}" | |
| ENV BUN_NO_CORE_DUMP=1 | |
| # Reload environment and add SDK bin to PATH | |
| RUN $sdkBin = Get-ChildItem 'C:\Program Files (x86)\Windows Kits' -Recurse -Filter 'rc.exe' -ErrorAction SilentlyContinue | ` | |
| Where-Object { $_.Directory.Name -eq 'x64' } | ` | |
| Select-Object -First 1 | ForEach-Object { $_.Directory.FullName }; ` | |
| if (-not $sdkBin) { ` | |
| Write-Error "Windows SDK tools (rc.exe) not found"; ` | |
| exit 1; ` | |
| }; ` | |
| [System.Environment]::SetEnvironmentVariable('SDK_BIN', $sdkBin, 'Machine'); ` | |
| refreshenv; | |
| # Add SDK bin to PATH via ENV so CMake can find rc.exe and mt.exe | |
| ENV PATH="${SDK_BIN};${PATH}" | |
| ENV BUN_NO_CORE_DUMP=1 |
🧰 Tools
🪛 Hadolint (2.14.0)
[info] 56-56: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
🤖 Prompt for AI Agents
In Dockerfile.windows around lines 55–64, the current PowerShell snippet
computes $sdkBin but doesn’t fail if rc.exe isn’t found, leaving SDK_BIN empty
and creating an empty PATH component; update the script to validate $sdkBin
immediately after discovery and fail fast if it’s empty (emit a clear error
message and exit with non-zero status), and only set the SDK_BIN environment
variable / add it to PATH when $sdkBin is present so the Docker build stops with
a helpful message instead of proceeding to later CMake failures.
| # Update to specific commit | ||
| RUN git pull | ||
|
|
There was a problem hiding this comment.
Missing git checkout to specific commit.
The GIT_SHA build argument is passed and logged, but git pull alone doesn't check out the specific commit. This means the build will use whatever commit is at the tip of the default branch, not the intended SHA.
Apply this diff to check out the specific commit:
WORKDIR C:\workspace\bun
# Update to specific commit
-RUN git pull
+RUN git fetch; `
+ git checkout $env:GIT_SHA📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Update to specific commit | |
| RUN git pull | |
| WORKDIR C:\workspace\bun | |
| # Update to specific commit | |
| RUN git fetch; ` | |
| git checkout $env:GIT_SHA | |
🤖 Prompt for AI Agents
In Dockerfile.windows around lines 75 to 77, the Dockerfile runs a plain `git
pull` so the build won't necessarily use the intended commit passed via GIT_SHA;
replace the pull with commands that fetch and check out the provided GIT_SHA:
ensure the GIT_SHA build-arg is declared/available, run a fetch from origin to
update refs, then force-checkout the exact commit SHA (or tag) specified by
GIT_SHA and optionally reset --hard to that commit so the working tree matches
exactly; this guarantees the build uses the exact commit passed in.
| RUN $env:PATH = $env:PATH + ';' + [System.Environment]::GetEnvironmentVariable('PATH','Machine') + ';C:\Users\ContainerAdministrator\.bun\bin'; ` | ||
| bun run build; ` | ||
| Remove-Item -Recurse -Force -ErrorAction SilentlyContinue C:\Users\ContainerAdministrator\AppData\Local\Temp\*; | ||
|
|
||
| # Test that the binary works | ||
| RUN C:\workspace\bun\build\debug\bun-debug.exe --version | ||
|
|
||
| CMD ["powershell.exe"] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Consider preserving cleanup errors for debugging.
The build process is well-structured with post-build cleanup and binary verification. However, using -ErrorAction SilentlyContinue on the cleanup might hide permission or locking issues.
Consider this change to make cleanup failures visible without breaking the build:
- Remove-Item -Recurse -Force -ErrorAction SilentlyContinue C:\Users\ContainerAdministrator\AppData\Local\Temp\*;
+ try { `
+ Remove-Item -Recurse -Force C:\Users\ContainerAdministrator\AppData\Local\Temp\* -ErrorAction Stop; `
+ } catch { `
+ Write-Warning "Cleanup failed: $_"; `
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN $env:PATH = $env:PATH + ';' + [System.Environment]::GetEnvironmentVariable('PATH','Machine') + ';C:\Users\ContainerAdministrator\.bun\bin'; ` | |
| bun run build; ` | |
| Remove-Item -Recurse -Force -ErrorAction SilentlyContinue C:\Users\ContainerAdministrator\AppData\Local\Temp\*; | |
| # Test that the binary works | |
| RUN C:\workspace\bun\build\debug\bun-debug.exe --version | |
| CMD ["powershell.exe"] | |
| RUN $env:PATH = $env:PATH + ';' + [System.Environment]::GetEnvironmentVariable('PATH','Machine') + ';C:\Users\ContainerAdministrator\.bun\bin'; ` | |
| bun run build; ` | |
| try { ` | |
| Remove-Item -Recurse -Force C:\Users\ContainerAdministrator\AppData\Local\Temp\* -ErrorAction Stop; ` | |
| } catch { ` | |
| Write-Warning "Cleanup failed: $_"; ` | |
| }; | |
| # Test that the binary works | |
| RUN C:\workspace\bun\build\debug\bun-debug.exe --version | |
| CMD ["powershell.exe"] |
🧰 Tools
🪛 Hadolint (2.14.0)
[error] 88-88: Parsing stopped here. Invalid use of parentheses?
(SC1088)
[info] 93-93: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
[info] 93-93: This \b will be a regular 'b' in this context.
(SC1001)
🤖 Prompt for AI Agents
In Dockerfile.windows around lines 88 to 95, the cleanup command uses
Remove-Item with -ErrorAction SilentlyContinue which suppresses errors and hides
permission/locking issues; change this to surface failures for debugging by
either removing -ErrorAction SilentlyContinue so errors are reported, or wrap
the Remove-Item call in a try/catch that logs the caught exception (e.g.,
Write-Error or Write-Host with the exception message) but does not exit the
build, ensuring cleanup errors are visible without failing the build.
Adds Windows Docker container support for Bun development.
Key features:
Usage:
See
README.windows.mdfor full documentation.