ci: update pwsh to use custom shell that fails-fast #32672
Conversation
Github Actions does set ErrorActionPreference to Stop but this does not apply when calling native executables. Use this custom shell that will fail fast when any command fails.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32672. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
cc @hebasto @hodlinator @davidgumberg for Concept ACK / NACK. |
|
Concept ACK (but my PowerShell skill is 0). @fanquake you are currently registered as a N-A-C-K. |
|
Concept ACK. Defining a new custom |
There was a problem hiding this comment.
Left a couple of notes regarding the current implementation:
- The output in the "Get tool information" step has changed:
--- old 2025-07-03 13:37:57.062194420 +0100
+++ new 2025-07-03 13:35:57.800242845 +0100
@@ -1,3 +1,15 @@
+
+Name Value
+---- -----
+PSVersion 7.4.10
+PSEdition Core
+GitCommitId 7.4.10
+OS Microsoft Windows 10.0.20348
+Platform Win32NT
+PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
+PSRemotingProtocolVersion 2.3
+SerializationVersion 1.1.0.1
+WSManStackVersion 3.0
cmake version 3.31.6
CMake suite maintained and supported by Kitware (kitware.com/cmake).- The exit code is not propagated to the action's status.
I just added this as a nice to have but happy to remove.
Unfortunately I think this is just how it works. When there is a failure it will always use code 1. Without having to write code that checks error codes and returns those codes each time we call a native executable I don't think it's possible to have the same error code. |
Just a general note on CI reproducibility: My recommendation would be to put as little code into vendor-specific locked-in yaml files and instead place CI code into vendor-agnostic scripts, so that they can be re-used outside of that vendor. I understand that it is nice to have the CI log split into sections, but this should also be possible with a script that allows several entry points: Just an unrelated general note. Anything is fine here and it is probably best to leave the decisions to the Windows devs here. |
|
After reading @maflcko's comment (which I agree with) and speaking with @hebasto offline I think it would be best that we change this to use powershell for all run steps on Windows jobs and look at condensing steps down into one or more scripts to aid in local reproducibility. As it stands, for someone to reproduce these steps locally they would have to jump between two shells. As I understand it, the change to use bash on windows was for the nicer fail fast behaviour but if we can get that in powershell now we should probably use the more native windows shell. Marking as draft for now until I rework. |
bash is also easier for non-windows devs to read and modify, so an alternative would also be python or rust as the ci runner script, but no strong opinion, I'd say anything is fine here. |
|
@hebasto is there any reason we need Powershell at all? Can we just use bash (or another language) everywhere? |
Github by default sets fail fast behaviour on pswh shell which means that if any powershell cmdlet fails the script will stop and exit. The problem is that this behaviour doesn't apply when calling native executables, it only applies to powershell cmdlets.
I think the safest thing is to whenever we use pwsh to enable
$PSNativeCommandUseErrorActionPreference = $truewhich will also fail calling any exe that returns a non-zero exit code.Technically the step
Adjust paths in test/config.inionly uses cmdlets so this step will not benefit from this change but I feel like it's good practice to still enable this feature in case this script gets modified in the future to call an exe.Here is a CI run that has a script that fails silently (look at Windows Native, VS 2022 -> Get tool information): https://github.com/m3dwards/bitcoin/actions/runs/15415032095/job/43375709475
And with this change applied, the script correctly fails: https://github.com/m3dwards/bitcoin/actions/runs/15416585565/job/43380685364