Do not assume windows_build_command is a single line #154
Conversation
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
`windows_build_command` was prefixed with a `Invoke-Program` to ensure we catch errors, but this then misses multiline commands. Move `Invoke-Program` up into the default command and then assume that it is passed if a non-default command is used. Note that `$ErrorActionPreference = 'Stop'` is not an option here - it only stops for cmdlets, not native commands. `$PSNativeCommandUseErrorActionPreference = $true` allows stopping on native commands too, but that was only introduced in PowerShell 7.3. windowsservercore-ltsc2022 has 5.1 by default (though we could install a newer if we wanted to). Update PR testing in this repo to use `Invoke-Program` to then actually catch failures.
bnbarham
commented
Aug 20, 2025
| @@ -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 }} | |||
There was a problem hiding this comment.
We could also leave this in, I am slightly concerned that Invoke-Program isn't being used in downstream overrides today.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Yeah, this is why I removed it. I'll go through them (I learned you can add NOT is:fork by the way)
There was a problem hiding this comment.
ahoppen
approved these changes
Aug 20, 2025
| @@ -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 }} | |||
There was a problem hiding this comment.
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
bnbarham
added a commit
to bnbarham/swift-syntax
that referenced
this pull request
Aug 21, 2025
Windows does not stop scripts when native commands exit with non-zero. Instead, their exit status has to be checked manually. This can be done through an `Invoke-Program` function that is added to the script running `windows_build_command`, which also previously prefixed the given command. This is changing in swiftlang/github-workflows#154 since it doesn't help with multi-line commands - update our modified `windows_build_command` to use `Invoke-Program` instead.
This was referenced Aug 21, 2025
bnbarham
added a commit
to bnbarham/swift-build
that referenced
this pull request
Aug 21, 2025
Windows does not stop scripts when native commands exit with non-zero. Instead, their exit status has to be checked manually. This can be done through an `Invoke-Program` function that is added to the script running `windows_build_command`, which also previously prefixed the given command. This is changing in swiftlang/github-workflows#154 since it doesn't help with multi-line commands - update our modified `windows_build_command` to use `Invoke-Program` instead.
bnbarham
added a commit
to bnbarham/vscode-swift
that referenced
this pull request
Aug 21, 2025
Windows does not stop scripts when native commands exit with non-zero. Instead, their exit status has to be checked manually. This can be done through an `Invoke-Program` function that is added to the script running `windows_build_command`, which also previously prefixed the given command. This is changing in swiftlang/github-workflows#154 since it doesn't help with multi-line commands - update our modified `windows_build_command` to use `Invoke-Program` instead.
bnbarham
added a commit
to bnbarham/swift-foundation-icu
that referenced
this pull request
Aug 21, 2025
Windows does not stop scripts when native commands exit with non-zero. Instead, their exit status has to be checked manually. This can be done through an `Invoke-Program` function that is added to the script running `windows_build_command`, which also previously prefixed the given command. This is changing in swiftlang/github-workflows#154 since it doesn't help with multi-line commands - update our modified `windows_build_command` to use `Invoke-Program` instead.
bnbarham
added a commit
to bnbarham/swift-format
that referenced
this pull request
Aug 21, 2025
Windows does not stop scripts when native commands exit with non-zero. Instead, their exit status has to be checked manually. This can be done through an `Invoke-Program` function that is added to the script running `windows_build_command`, which also previously prefixed the given command. This is changing in swiftlang/github-workflows#154 since it doesn't help with multi-line commands - update our modified `windows_build_command` to use `Invoke-Program` instead.
bnbarham
added a commit
to bnbarham/swift-toolchain-sqlite
that referenced
this pull request
Aug 21, 2025
Windows does not stop scripts when native commands exit with non-zero. Instead, their exit status has to be checked manually. This can be done through an `Invoke-Program` function that is added to the script running `windows_build_command`, which also previously prefixed the given command. This is changing in swiftlang/github-workflows#154 since it doesn't help with multi-line commands - update our modified `windows_build_command` to use `Invoke-Program` instead.
jakepetroules
approved these changes
Aug 22, 2025
bnbarham
added a commit
to bnbarham/vscode-swift
that referenced
this pull request
Aug 22, 2025
Windows does not stop scripts when native commands exit with non-zero. Instead, their exit status has to be checked manually. This can be done through an `Invoke-Program` function that is added to the script running `windows_build_command`, which also previously prefixed the given command. This is changing in swiftlang/github-workflows#154 since it doesn't help with multi-line commands - update our modified `windows_build_command` to use `Invoke-Program` instead.
plemarquand
pushed a commit
to swiftlang/vscode-swift
that referenced
this pull request
Aug 22, 2025
Windows does not stop scripts when native commands exit with non-zero. Instead, their exit status has to be checked manually. This can be done through an `Invoke-Program` function that is added to the script running `windows_build_command`, which also previously prefixed the given command. This is changing in swiftlang/github-workflows#154 since it doesn't help with multi-line commands - update our modified `windows_build_command` to use `Invoke-Program` instead.
bnbarham
added a commit
to swiftlang/swift-build
that referenced
this pull request
Aug 22, 2025
Windows does not stop scripts when native commands exit with non-zero. Instead, their exit status has to be checked manually. This can be done through an `Invoke-Program` function that is added to the script running `windows_build_command`, which also previously prefixed the given command. This is changing in swiftlang/github-workflows#154 since it doesn't help with multi-line commands - update our modified `windows_build_command` to use `Invoke-Program` instead.
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.
windows_build_commandwas prefixed with aInvoke-Programto ensure we catch errors, but this then misses multiline commands. MoveInvoke-Programup into the default command and then assume that it is passed if a non-default command is used.Note that
$ErrorActionPreference = 'Stop'is not an option here - it only stops for cmdlets, not native commands.$PSNativeCommandUseErrorActionPreference = $trueallows stopping on native commands too, but that was only introduced in PowerShell 7.3. windowsservercore-ltsc2022 has 5.1 by default (though we could install a newer if we wanted to).Update PR testing in this repo to use
Invoke-Programto then actually catch failures.