Do not assume windows_build_command is a single line#154
Do not assume windows_build_command is a single line#154ahoppen merged 1 commit intoswiftlang:mainfrom
Conversation
`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.
| 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
left a comment
There was a problem hiding this comment.
LGTM but we should fix all existing innovations of this workflow to use Invoke-Program.
| 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
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.