Skip to content

Do not assume windows_build_command is a single line #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 25, 2025
Merged

Do not assume windows_build_command is a single line #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/pull_request.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ jobs:
windows_build_command: |
mkdir MyPackage
cd MyPackage
swift package init --type library
swift build
Invoke-Program swift package init --type library
Invoke-Program swift build
enable_windows_docker: true

tests_without_docker:
Expand All @@ -59,8 +59,8 @@ jobs:
windows_build_command: |
mkdir MyPackage
cd MyPackage
swift package init --type library
swift build
Invoke-Program swift package init --type library
Invoke-Program swift build
enable_windows_docker: false

tests_macos:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/swift_package_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ on:
Windows Command Prompt command to build and test the package.
Note that Powershell does not automatically exit if a subcommand fails. The Invoke-Program utility is available to propagate non-zero exit codes.
It is strongly encouraged to run all command using `Invoke-Program` unless you want to continue on error eg. `Invoke-Program git apply patch.diff` instead of `git apply patch.diff`.
default: "swift test"
default: "Invoke-Program swift test"
macos_env_vars:
description: "Newline separated list of environment variables"
type: string
Expand Down Expand Up @@ -450,7 +450,7 @@ jobs:
Invoke-Program swift test --version
Invoke-Program cd $Source
${{ inputs.windows_pre_build_command }}
Invoke-Program ${{ inputs.windows_build_command }} ${{ (contains(matrix.swift_version, 'nightly') && inputs.swift_nightly_flags) || inputs.swift_flags }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also leave this in, I am slightly concerned that Invoke-Program isn't being used in downstream overrides today.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think we should prefix it with Invoke-Program. Otherwise, you might learn that the command fails if you only have a single one in your windows_build_commands and are then surprised that this behavior doesn’t hold up when you add a second command to the script.

Instead, what I would suggest is that we go through the existing use cases of this workflow with windows_build_command and fix them. There’s only a handful and many of them are forks: https://github.com/search?q=swiftlang%2Fgithub-workflows%2F.github%2Fworkflows%2Fswift_package_test.yml%40main+windows_build_command&type=code

Copy link
Contributor Author

@bnbarham bnbarham Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is why I removed it. I'll go through them (I learned you can add NOT is:fork by the way)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swiftlang/swift-syntax#3140
swiftlang/swift-build#746
swiftlang/vscode-swift#1800
swiftlang/swift-foundation-icu#77
swiftlang/swift-format#1060
swiftlang/swift-toolchain-sqlite#20

${{ inputs.windows_build_command }} ${{ (contains(matrix.swift_version, 'nightly') && inputs.swift_nightly_flags) || inputs.swift_flags }}
'@ >> $env:TEMP\test-script\run.ps1
# Docker build
- name: Docker Build / Test
Expand Down