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

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.

21bc6915d2

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 ACK	hodlinator, hebasto
User requested bot ignore	fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32513 (ci: remove 3rd party js from windows dll gha job by m3dwards)

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.

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?

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

Code Coverage & Benchmarks

Reviews

Conflicts