ci: update pwsh to use custom shell that fails-fast

m3dwards commented at 12:03 pm on June 3, 2025: contributor

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 = $true which will also fail calling any exe that returns a non-zero exit code.

Technically the step Adjust paths in test/config.ini only 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

DrahtBot commented at 12:03 pm on June 3, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32672.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept NACK	fanquake
Concept ACK	hodlinator, hebasto, davidgumberg

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

DrahtBot added the label Tests on Jun 3, 2025

fanquake added the label Windows on Jun 3, 2025

fanquake commented at 1:34 pm on July 2, 2025: member

cc @hebasto @hodlinator @davidgumberg for Concept ACK / NACK.

hodlinator commented at 1:43 pm on July 2, 2025: contributor

Concept ACK (but my PowerShell skill is 0). @fanquake you are currently registered as a N-A-C-K.

hebasto commented at 11:19 am on July 3, 2025: member

Concept ACK.

Defining a new custom job.defaults.run.shell for all Windows jobs allows us to revert f8619196ceb5c6a58125506d276d9515837f043a and use the same shell across all steps. This is especially convenient for reproducing failures locally.

hebasto commented at 12:41 pm on July 3, 2025: member

Left a couple of notes regarding the current implementation:

The output in the “Get tool information” step has changed:

 0--- old	2025-07-03 13:37:57.062194420 +0100
 1+++ new	2025-07-03 13:35:57.800242845 +0100
 2@@ -1,3 +1,15 @@
 3+
 4+Name                           Value
 5+----                           -----
 6+PSVersion                      7.4.10
 7+PSEdition                      Core
 8+GitCommitId                    7.4.10
 9+OS                             Microsoft Windows 10.0.20348
10+Platform                       Win32NT
11+PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
12+PSRemotingProtocolVersion      2.3
13+SerializationVersion           1.1.0.1
14+WSManStackVersion              3.0
15 cmake version 3.31.6
16 
17 CMake suite maintained and supported by Kitware (kitware.com/cmake).

The exit code is not propagated to the action’s status.

m3dwards commented at 8:52 am on July 4, 2025: contributor

The output in the “Get tool information” step has changed:

I just added this as a nice to have but happy to remove.

2. The exit code is not propagated to the action’s status.

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.

maflcko commented at 9:18 am on July 4, 2025: member

This is especially convenient for reproducing failures locally.

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: ./ci.py install; ./ci.py build; ./ci.py test; (or similar)

Just an unrelated general note. Anything is fine here and it is probably best to leave the decisions to the Windows devs here.

m3dwards commented at 9:30 am on July 4, 2025: contributor

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.

m3dwards marked this as a draft on Jul 4, 2025

maflcko commented at 10:19 am on July 4, 2025: member

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.

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.

m3dwards commented at 12:40 pm on July 4, 2025: contributor

@hebasto is there any reason we need Powershell at all? Can we just use bash (or another language) everywhere?

DrahtBot added the label Needs rebase on Oct 8, 2025

achow101 requested review from davidgumberg on Oct 22, 2025

hebasto commented at 3:51 pm on January 19, 2026: member

@hebasto is there any reason we need Powershell at all?

In my opinion, it is convenient to be able to reproduce CI commands locally without needing to install additional tools.

Can we just use bash (or another language) everywhere?

There are no technical obstacles to doing so.

ci: update pwsh to use custom shell that fails-fast

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.

6f0d49fde9

m3dwards force-pushed on Jan 28, 2026

DrahtBot removed the label Needs rebase on Jan 28, 2026

davidgumberg commented at 7:17 pm on January 28, 2026: contributor

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.

Concept ACK, seems reasonable to write the script in powershell and invoke it in the vendor ci.yml. I think the script should be moved out of the vendor-specific ci.yml as suggested by @maflcko.

Given that the script is mostly sequential command invocation, I’m not sure if there’s much benefit from rewriting it in python or another language, and agree with @hebasto that for reproducibility it would be nice to have it in powershell. If there is a good reason to not have the CI script in powershell, maybe it would suffice to have a powershell script that does something like winget install Python.Python.3 and runs a python script.

maflcko commented at 8:12 pm on January 28, 2026: member

Well, python is required and already installed on the GHA hosts, so the local runner will have to have it (pre-)installed in some way or another. I think it is fine to ignore the “install” part and only extract the “run” part, but anything is fine here. I’ll likely not going to review this pull, so I don’t want to give too much feedback :sweat_smile:

maflcko commented at 10:24 am on February 4, 2026: member

I’ve implemented the concept of my idea in https://github.com/bitcoin/bitcoin/pull/34500

ci: update pwsh to use custom shell that fails-fast #32672

Code Coverage & Benchmarks

Reviews

Conflicts