Adding Windows Support #13
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe CI configuration disables macOS, iOS/watchOS/visionOS, and Ubuntu jobs, leaving a Windows single-target test workflow active with a Swift version matrix. The composite action updates expand OS detection, separate OS/Swift version retrieval, and enable build/test and caching on Windows alongside Ubuntu and macOS. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions (Workflow)
participant R as Runner (windows-2025)
participant TS as compnerd/gha-setup-swift
participant Swift as Swift Toolchain
Dev->>GH: Push/PR triggers workflow
GH->>R: Start job test-single-target-windows (matrix: Swift versions)
rect rgba(200,230,255,0.4)
GH->>TS: Setup Swift (version/build from matrix)
TS-->>R: Swift toolchain installed
end
R->>Swift: swift test -c debug -v (SingleTargetPackage)
Swift-->>R: Test results per matrix entry
R-->>GH: Report status (fail-fast: false)
sequenceDiagram
autonumber
participant CA as Composite Action (action.yml)
participant Runner as Runner (macos/ubuntu/windows)
participant OS as OS Detector
participant Sys as System Env
CA->>OS: Detect OS
alt macos
OS-->>CA: os=macos
else ubuntu
OS-->>CA: os=ubuntu
CA->>Sys: Read /etc/os-release (VERSION_CODENAME)
Sys-->>CA: OS_VERSION exported
else windows
OS-->>CA: os=windows, OS_VERSION=windows
end
rect rgba(220,255,220,0.4)
CA->>Runner: Get Swift (ubuntu/windows)
CA->>Runner: Restore/Save cache (ubuntu/windows)
end
CA->>Runner: Build and Test (macos if !type, or ubuntu/windows)
Runner-->>CA: Build/test outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing touches🧪 Generate unit tests
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: 0
🧹 Nitpick comments (7)
.github/workflows/swift-test.yml (3)
175-178: Remove stale/incorrect commented URLs.The second URL duplicates “swift” and is misleading. Clean both for clarity.
- # https://download.swift.org/swift-6.2-branch/windows10/swift-6.2-DEVELOPMENT-SNAPSHOT-2025-09-06-a/swift-6.2-DEVELOPMENT-SNAPSHOT-2025-09-06-a-windows10.exe - - # https://download.swift.org/swift-6.2-branch/windows10/swift-swift-6.2-DEVELOPMENT-SNAPSHOT-2025-09-06-a/swift-swift-6.2-DEVELOPMENT-SNAPSHOT-2025-09-06-a-windows10.exe + # Example: https://download.swift.org/swift-6.2-branch/windows10/swift-6.2-DEVELOPMENT-SNAPSHOT-2025-09-06-a/swift-6.2-DEVELOPMENT-SNAPSHOT-2025-09-06-a-windows10.exe
185-192: Leverage the composite action you just made Windows-aware.To validate action.yml’s Windows path and keep workflows consistent, install Swift with compnerd and then invoke your action (remember: provide scheme as required by the action).
- name: Install Windows Swift toolchain uses: compnerd/gha-setup-swift@main with: swift-version: ${{ matrix.swift-version }} swift-build: ${{ matrix.swift-build }} - - run: | - swift test --package-path=test/SingleTargetPackage --verbose + - name: Build and Test via composite action + uses: ./ + with: + working-directory: test/SingleTargetPackage + scheme: SingleTargetPackage
21-21: Fix YAML lint nits (trailing spaces, excess blank lines).Trailing spaces and >2 consecutive blank lines can break linters.
-# (lines contain trailing spaces) +# (remove trailing spaces) -# (more trailing spaces) +# (remove trailing spaces) - # -Xbuild-tools-swiftc ... ${env:UCRTVersion} - + # -Xbuild-tools-swiftc ... ${env:UCRTVersion} + - # scheme: MultiTargetPackage + # scheme: MultiTargetPackageAlso collapse >2 consecutive blank lines near Line 196.
Also applies to: 34-34, 51-51, 71-71, 78-78, 151-151, 168-168, 171-171, 193-193, 196-196, 207-207
action.yml (4)
94-102: Preflight check for Swift availability (Windows/Ubuntu).The composite assumes Swift is preinstalled; on Windows that’s rarely true. Add a clear failure early to avoid confusing errors.
- name: Get Swift if: steps.detect-os.outputs.os == 'ubuntu' || steps.detect-os.outputs.os == 'windows' shell: bash working-directory: ${{ inputs.working-directory }} run: | + if ! command -v swift >/dev/null 2>&1; then + echo "Swift toolchain not found on PATH. Please install Swift (e.g., compnerd/gha-setup-swift on Windows or use a Swift container on Linux)." >&2 + exit 1 + fi SWIFT_VERSION=$(swift --version | head -n 1 | cut -d ' ' -f 3) echo "SWIFT_VERSION=$SWIFT_VERSION" >> $GITHUB_ENV
121-130: Cache key should hash the correct Package.resolved path.hashFiles() resolves from repo root, not inputs.working-directory. Use the full path to avoid cache miss collisions when working-directory ≠ root.
- key: spm-${{ env.OS_VERSION }}-${{ env.SWIFT_VERSION }}-${{ hashFiles('Package.resolved') }} + key: spm-${{ env.OS_VERSION }}-${{ env.SWIFT_VERSION }}-${{ hashFiles(format('{0}/Package.resolved', inputs.working-directory)) }}(Consider the same adjustment for the macOS cache key above for consistency.)
133-139: Clarify boolean precedence in job condition.Make evaluation explicit to avoid surprises.
- if: steps.detect-os.outputs.os == 'macos' && !inputs.type || steps.detect-os.outputs.os == 'ubuntu' || steps.detect-os.outputs.os == 'windows' + if: (steps.detect-os.outputs.os == 'macos' && !inputs.type) || steps.detect-os.outputs.os == 'ubuntu' || steps.detect-os.outputs.os == 'windows'
110-110: Trim trailing space.Minor YAML style cleanup.
- echo "OS_VERSION=$OS_VERSION" >> $GITHUB_ENV - + echo "OS_VERSION=$OS_VERSION" >> $GITHUB_ENV
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/swift-test.yml(1 hunks)action.yml(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/swift-test.yml
📄 CodeRabbit inference engine (CLAUDE.md)
When invoking this GitHub Action in workflows, provide the required
schemeinput (and other inputs as needed)
Files:
.github/workflows/swift-test.yml
🪛 YAMLlint (1.37.1)
action.yml
[error] 110-110: trailing spaces
(trailing-spaces)
.github/workflows/swift-test.yml
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 51-51: trailing spaces
(trailing-spaces)
[error] 71-71: trailing spaces
(trailing-spaces)
[error] 78-78: trailing spaces
(trailing-spaces)
[error] 151-151: trailing spaces
(trailing-spaces)
[error] 168-168: trailing spaces
(trailing-spaces)
[error] 171-171: trailing spaces
(trailing-spaces)
[error] 193-193: trailing spaces
(trailing-spaces)
[warning] 196-196: too many blank lines (3 > 2)
(empty-lines)
[error] 207-207: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
.github/workflows/swift-test.yml (2)
159-174: Matrix looks good for broad Swift coverage.Nice spread (6.2 dev snapshot down to 5.9). No issues spotted here.
153-156: No change required — windows-2025 is a valid GitHub-hosted runnerVerified: GitHub-hosted runners include windows-2025 (GA Apr 8, 2025). Keep runs-on: windows-2025; optionally use runs-on: windows-latest to track updates automatically.
action.yml (1)
137-138: Compatibility with Swift 5.9: avoid --force-resolved-versions or gate by version.Older toolchains may not support --force-resolved-versions. Either drop it or guard by $SWIFT_VERSION.
- swift build --build-tests --cache-path .cache --force-resolved-versions - swift test --enable-code-coverage --cache-path .cache --force-resolved-versions + if swift --version | grep -qE 'Swift version (6|7)\.'; then + swift build --build-tests --cache-path .cache --force-resolved-versions + swift test --enable-code-coverage --cache-path .cache --force-resolved-versions + else + swift build --build-tests --cache-path .cache + swift test --enable-code-coverage --cache-path .cache + fiPlease confirm 5.9 behavior in your environment.
Fixing detect-os Fixing AGain fixing script fixing windows container fixing swift for windows [skip ci] Update swift-test.yml Update swift-test.yml Update swift-test.yml Update swift-test.yml --accept-source-agreements --accept-package-agreements Update swift-test.yml Trying to add Path and refreshenv Update swift-test.yml Using Compared Actions (#15) * Update swift-test.yml * Fixing OS Version * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml * Update swift-test.yml
leogdion
force-pushed
the
12-windows
branch
from
21b2163 to
15484e2
Compare
Pull Request Review: Adding Windows SupportI've completed a comprehensive review of the Windows support implementation. Here's my detailed feedback: ✅ Positive AspectsCode Structure & Design:
Windows Implementation:
🚨 Critical Issues1. Missing OS Version Environment Variable ( # Current (problematic):
SWIFT_VERSION=$(swift --version | head -n 1 | cut -d ' ' -f 3)
echo "SWIFT_VERSION=$SWIFT_VERSION" >> $GITHUB_ENV
# Missing: OS_VERSION environment variableIssue: The code removes Fix Required: - name: Get Swift and OS versions
if: steps.detect-os.outputs.os == 'ubuntu' || steps.detect-os.outputs.os == 'windows'
shell: bash
working-directory: ${{ inputs.working-directory }}
run: |
SWIFT_VERSION=$(swift --version | head -n 1 | cut -d ' ' -f 3)
echo "SWIFT_VERSION=$SWIFT_VERSION" >> $GITHUB_ENV
if [[ "$RUNNER_OS" == "Windows" ]]; then
echo "OS_VERSION=windows" >> $GITHUB_ENV
else
OS_VERSION=$(. /etc/os-release && echo $VERSION_CODENAME)
echo "OS_VERSION=$OS_VERSION" >> $GITHUB_ENV
fi2. Inconsistent Runner Configuration
3. Missing Multi-Target Windows Testing
|
- Add comprehensive Windows support to GitHub Action with new input parameters - Implement Windows Swift toolchain installation using compnerd/gha-setup-swift - Add Windows-specific environment setup and caching strategy - Update workflow to test Windows support with single and multi-target packages - Enable previously commented Windows multi-target workflow - Simplify Windows testing by using action instead of manual setup 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Pull Request Review: Adding Windows SupportOverall AssessmentThis PR successfully adds Windows support to the Swift Build action, extending platform compatibility from Ubuntu/macOS to include Windows. The implementation follows established patterns and maintains consistency with existing code structure. ✅ StrengthsCode Quality & Architecture:
Documentation:
Testing Strategy:
🔍 Areas for Improvement1. Missing Ubuntu OS Version Detection (action.yml:318-324) Fix needed: # In "Get Swift and OS versions" step, add back:
OS_VERSION=$(. /etc/os-release && echo $VERSION_CODENAME)
echo "OS_VERSION=$OS_VERSION" >> $GITHUB_ENV2. Workflow File Missing Newline 3. Consider Windows Swift Version Validation 4. Test Coverage Gap 🛡️ Security Considerations
🚀 Performance Considerations
🧪 Test CoverageGood:
Suggestions:
📋 Required Changes
🎯 Optional Enhancements
VerdictApprove with required changes. The Windows support implementation is well-architected and follows project conventions. The missing Ubuntu OS_VERSION fix is critical for proper caching behavior. |
Critical fixes: - Add missing OS_VERSION detection to Ubuntu step (fixes cache key bug) - Clean up commented code blocks in Windows workflow - Update documentation to reflect Windows 2022/2025 support Enhanced testing: - Implement cross-product matrix for Windows testing - Test both Swift 6.1/6.2 on both Windows 2022/2025 runners - Expand coverage from 2 to 8 Windows test combinations (4 single + 4 multi target) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
PR Review: Adding Windows SupportI've thoroughly reviewed the Windows support implementation. Overall, this is a well-structured addition that follows existing patterns. Here's my detailed feedback: ✅ Positive Aspects
|
* Adding Windows Support (#13) * Improve robustness and platform handling (#31) - Update all README examples to use @v1.3.0 (#24) - Skip platform download for macOS builds to avoid no-op (#23) - Make XCODE_NAME robust with fallback logic and handle missing input (#22) - Fix bracket notation for hyphenated matrix keys in workflows (#18) - Align documentation (type input already included macos) (#21) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Summary by CodeRabbit