fix: make `cargo test testnet` work on windows #5937

fdefelici · 2025-03-18T13:57:38Z

Description

this patch makes cargo test testnet command work properly on windows.
Addressed:

compilation issue due to usage of unix apis
test runtime failure due to path naming not supported by windows

Applicable issues

fixes [windows dev] test compilation fails due to unix apis #5930

Additional info (benefits, drawbacks, caveats)

Checklist

Test coverage for new or modified code paths
Changelog is updated
Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
New clarity functions have corresponding PR in clarity-benchmarking repo
New integration test(s) added to bitcoin-tests.yml

…t run on windows. Now 'cargo test testnet' ends succesfully on windows os, stacks-network#5930

moodmosaic · 2025-03-21T16:41:48Z

What do I have to setup/install in order to try?

fdefelici · 2025-03-21T16:57:45Z

What do I have to setup/install in order to try?

If you mean locally on your machine:

Prerequisites

A windows os (even virtualized)
Cloning the PR Repo making sure you set before the git property core.autoclrf = input (as indicated in our readme)

Validation:

Run cargo test testnet and the process should end without compilation error and the all tests run succesfully

moodmosaic · 2025-03-21T22:22:11Z

Thanks! I can't reproduce this on windows 11 arm64:

rust:
- installed via rustup
- target: aarch64-pc-windows-msvc

visual studio:
- desktop development with c++
- msvc v143 - arm64 build tools
- windows 11 sdk
- c++ arm64 build tools
- clang compiler for arm64
- llvm support

tools confirmed:
- cl.exe
- link.exe
- vcvarsall.bat

@fdefelici, if your setup differs, let me know how to try it on a clean box. I can reset easily.

wileyj · 2025-03-22T04:53:25Z

i was able to get a VM with windows running on x64 to test this - but ran into a strange error that i can't reproduce on other systems.
the sha256 of the stx-genesis files doesn't match (and compile panics due to this). i'll have to verify the files in your fork @fdefelici , or it may be something with the version of sha256 on the windows VM

edit: it definitely seems to be a problem with the tool on windows. linux reports the sha256 of your branch correctly. i'll dig into it a bit more, see if i did something wrong here.

fdefelici · 2025-03-22T07:09:56Z

i was able to get a VM with windows running on x64 to test this - but ran into a strange error that i can't reproduce on other systems. the sha256 of the stx-genesis files doesn't match (and compile panics due to this). i'll have to verify the files in your fork @fdefelici , or it may be something with the version of sha256 on the windows VM

edit: it definitely seems to be a problem with the tool on windows. linux reports the sha256 of your branch correctly. i'll dig into it a bit more, see if i did something wrong here.

It is the "correct" behaviour by now.
Did you set git property core.autoclrf = input as I mentioned to nikos? Eventually be aware of doing it before you clone the repository.
(I'm also working to fix the need for this configuration and I could submit a PR soon, driven by this issue #4613. As you can see in there it is the same problem you are esperiencing and I provided the same guideline to the user who report it).
So at the moment following our "official" readme, does the trick.

Anyhow considering that the problem is due to file end-of-line, another possibility is just edit the eol from CLRF to LF for the following files:

stx-genesis/chainstate.txt
stx-genesis/chainstate-test.txt
stx-genesis/name_zonefiles-test.txt
stx-genesis/name_zonefiles.txt

fdefelici · 2025-03-22T07:16:23Z

Thanks! I can't reproduce this on windows 11 arm64:
rust:
- installed via rustup
- target: aarch64-pc-windows-msvc

visual studio:
- desktop development with c++
- msvc v143 - arm64 build tools
- windows 11 sdk
- c++ arm64 build tools
- clang compiler for arm64
- llvm support

tools confirmed:
- cl.exe
- link.exe
- vcvarsall.bat
@fdefelici, if your setup differs, let me know how to try it on a clean box. I can reset easily.

The only difference I have is the target architecture. I have x86_64-pc-windows-msvc
What error do you get?

moodmosaic · 2025-03-22T07:47:58Z

@fdefelici, for the build output see: https://gist.github.com/moodmosaic/4a9022de3bd5ef8aa1ab6b3e0ac15a3c

fdefelici · 2025-03-22T08:07:55Z

@fdefelici, for the build output see: https://gist.github.com/moodmosaic/4a9022de3bd5ef8aa1ab6b3e0ac15a3c

Ok thanks. the problem is clear. It Is due the winapi crate that has not support for arm64.

As far that crate is obsolete we could think to migrate to the windows crate that should have better support for arm64. Anyhow Better to split in a specific issue/pr.

So, by now another prerequisite is that the architetture that must be x86 family

Basically the configuration that @wileyj is testing.

wileyj · 2025-03-22T22:38:42Z

i was able to get a VM with windows running on x64 to test this - but ran into a strange error that i can't reproduce on other systems. the sha256 of the stx-genesis files doesn't match (and compile panics due to this). i'll have to verify the files in your fork @fdefelici , or it may be something with the version of sha256 on the windows VM
edit: it definitely seems to be a problem with the tool on windows. linux reports the sha256 of your branch correctly. i'll dig into it a bit more, see if i did something wrong here.

It is the "correct" behaviour by now. Did you set git property core.autoclrf = input as I mentioned to nikos? Eventually be aware of doing it before you clone the repository. (I'm also working to fix the need for this configuration and I could submit a PR soon, driven by this issue #4613. As you can see in there it is the same problem you are esperiencing and I provided the same guideline to the user who report it). So at the moment following our "official" readme, does the trick.

Anyhow considering that the problem is due to file end-of-line, another possibility is just edit the eol from CLRF to LF for the following files:

stx-genesis/chainstate.txt

stx-genesis/chainstate-test.txt

stx-genesis/name_zonefiles-test.txt

stx-genesis/name_zonefiles.txt

confirmed - it was user error. i set the config value as autoclrf. retrying

wileyj · 2025-03-23T00:44:42Z

after fixing the gitconfig, the tests on this branch all pass.
very basic setup on my end: windows 11 evaluation VM, installed git and rustup (which installed missing dependencies).
git clone the fork/branch, cargo test testnet returns 0

fdefelici · 2025-03-23T09:46:14Z

@fdefelici, for the build output see: https://gist.github.com/moodmosaic/4a9022de3bd5ef8aa1ab6b3e0ac15a3c

Ok thanks. the problem is clear. It Is due the winapi crate that has not support for arm64.

As far that crate is obsolete we could think to migrate to the windows crate that should have better support for arm64. Anyhow Better to split in a specific issue/pr.

So, by now another prerequisite is that the architetture that must be x86 family

Basically the configuration that @wileyj is testing.

Tracked this issue here #5947

rdeioris · 2025-03-24T05:41:02Z

Tested successfully on an x64 system. I will try with an arm64 asap.

moodmosaic · 2025-03-24T10:29:20Z

OK, I got this to work on arm64 — specifically on a clean Windows 11 arm64 VM. This time I used only PortableGit and Skeeto's w64devkit. No MSVC, no Visual Studio, no Windows SDKs, no system-wide installs.

Notes:

rustup-init with aarch64-pc-windows-gnu failed (unsupported?)
Used x86_64-pc-windows-gnu under emulation instead:
```
./rustup-init.exe --default-host x86_64-pc-windows-gnu
```
Added w64devkit to PATH (otherwise cargo can't find gcc).
Build failed with:
```
ld.exe: cannot find -lgcc_eh
```
Rust's GNU toolchain expects libgcc_eh.a, but w64devkit doesn’t include it (by design, it uses compiler-rt).
Fixed by creating a dummy static archive:
```
cd w64devkit/x86_64-w64-mingw32/lib
ar rcs libgcc_eh.a
```
This keeps the linker happy. Confirmed here:
Error while using mingw as a linker for rust skeeto/w64devkit#52
Re-ran:
```
cargo test testnet
```

moodmosaic

Tested on Windows ARM64 with w64devkit. Left a small nit.

moodmosaic · 2025-03-24T10:38:55Z

stackslib/src/config/chain_data.rs

+echo ]
+)
+"#;
+        let path = "/tmp/test-get-unconfirmed-commits.bat";


This works in Git Bash but might not work in PowerShell, perhaps a more portable way could be

Suggested change

let path = "/tmp/test-get-unconfirmed-commits.bat";

let path = std::env::temp_dir().join("test-get-unconfirmed-commits.bat");

Yes this should be the right way. I was aware of that but didn't fix it here, because in all our codebase all tests doing filesystem I/O use /tmp as root folder (and if the folder doesn't exists it is automatically created).

Infact if you run cargo test testnet from both cmd or powershell the tests should ends succesfully.

For sure this need to be improved, but I would prefer doing it in more general fashion in this issue #5932.
So, if you agree, we can leave it as it is (to keep the tests homogeneous for now), and face it in the dedicated issue.

Yeah... AFAICT, the test runs the script, parses JSON output, and checks values. But the batch file won't run directly in PowerShell (unless they are prefixed with e.g. .\script.bat?).

Perhaps the safest is to invoke it explicitly via cmd.exe, e.g.:

let output = Command::new("cmd") .args(["/C", path]) .output() .expect("Failed to execute batch script");

Just to be sure and clear my understanding: did you tried running from powershell and you got some error?

I'm just asking for confirmation, because I tried to run from powershell these commands with success:

cargo run testnet

C:\tmp\test-get-unconfirmed-commits.bat

/tmp/test-get-unconfirmed-commits.bat

The only way I'm able to get an error (the one you describe) is when running the bat file with relative path.
So doing this gives error:

cd tmp test-get-unconfirmed-commits.bat //I got some red error and then this hint Suggestion [3,General]: The command test-get-unconfirmed-commits.bat was not found, but it exists in the current path. By default, Windows PowerShell does not load commands from the current path. If you trust the command, type ".\test-get-unconfirmed-commits.bat" instead. For more information, see "get-help about_Command_Precedence".

So by the fact we run it in the test with absolute path, we shouldn't have any error.

Instead if you got error, please share it, so I can collect it for later analysis (hopefully even the powershell version), and eventually I can apply the patch your proposing.

running cargo test testnet worked both on cmd and powershell on my end

rdeioris

LGTM (tested on x64 windows 11)

stackslib/src/net/mod.rs

…k#5930

Jiloc

Tested on Win11 x64, works great. Just left a comment for a documentation nit

stackslib/src/net/tests/convergence.rs

obycode

This all looks fine to me 👍

…twork#5930

stackslib/src/net/tests/convergence.rs

codecov · 2025-03-28T03:26:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.51%. Comparing base (51169a6) to head (bbd5236).
Report is 98 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #5937      +/-   ##
===========================================
+ Coverage    82.88%   84.51%   +1.63%     
===========================================
  Files          522      522              
  Lines       382715   381322    -1393     
  Branches       323      323              
===========================================
+ Hits        317232   322293    +5061     
+ Misses       65475    59021    -6454     
  Partials         8        8

Files with missing lines	Coverage Δ
stackslib/src/config/chain_data.rs	`75.11% <ø> (ø)`
stackslib/src/net/mod.rs	`78.68% <100.00%> (-0.02%)`	⬇️
stackslib/src/net/tests/convergence.rs	`93.77% <ø> (+11.49%)`	⬆️

... and 104 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e21c0dd...bbd5236. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

moodmosaic

Definitely improves support for Windows, working even on the bare-bones setup, as pointed out in #5937 (comment). Looks good to me.

fdefelici added 2 commits March 18, 2025 14:22

fix: test compilation on windows ends successfully, stacks-network#5930

1e01c13

fix: coordinator lock_info_primary_testnet and lock_info_other_testne…

00aa571

…t run on windows. Now 'cargo test testnet' ends succesfully on windows os, stacks-network#5930

fdefelici requested a review from a team as a code owner March 18, 2025 13:57

fdefelici self-assigned this Mar 18, 2025

aldur requested review from moodmosaic and rdeioris March 19, 2025 15:45

fdefelici mentioned this pull request Mar 23, 2025

[windows dev] replace obsolete winapi (and support arm64?!) #5947

Open

fdefelici mentioned this pull request Mar 23, 2025

fix: improve eol file management #5948

Merged

5 tasks

moodmosaic reviewed Mar 24, 2025

View reviewed changes

aldur requested a review from wileyj March 24, 2025 15:39

rdeioris approved these changes Mar 25, 2025

View reviewed changes

stackslib/src/net/mod.rs Outdated Show resolved Hide resolved

fix: use underscore as separator and renaming variable, stacks-networ…

6388c0b

…k#5930

Jiloc approved these changes Mar 25, 2025

View reviewed changes

stackslib/src/net/tests/convergence.rs Outdated Show resolved Hide resolved

obycode previously approved these changes Mar 26, 2025

View reviewed changes

chore: improve comment about rlimit empty stub for windows, stacks-ne…

7c5cdeb

…twork#5930

fdefelici dismissed obycode’s stale review via 7c5cdeb March 27, 2025 08:22

fdefelici requested a review from obycode March 27, 2025 08:23

fdefelici requested a review from moodmosaic March 27, 2025 08:23

obycode reviewed Mar 27, 2025

View reviewed changes

stackslib/src/net/tests/convergence.rs Outdated Show resolved Hide resolved

chore: remove pr reference from comment, stacks-network#5930

bbd5236

moodmosaic approved these changes Mar 31, 2025

View reviewed changes

wileyj approved these changes Mar 31, 2025

View reviewed changes

obycode approved these changes Mar 31, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: make `cargo test testnet` work on windows #5937

fix: make `cargo test testnet` work on windows #5937

fdefelici commented Mar 18, 2025

moodmosaic commented Mar 21, 2025

fdefelici commented Mar 21, 2025

moodmosaic commented Mar 21, 2025

wileyj commented Mar 22, 2025 •

edited

Loading

fdefelici commented Mar 22, 2025 •

edited

Loading

fdefelici commented Mar 22, 2025

moodmosaic commented Mar 22, 2025

fdefelici commented Mar 22, 2025

wileyj commented Mar 22, 2025

wileyj commented Mar 23, 2025

fdefelici commented Mar 23, 2025

rdeioris commented Mar 24, 2025

moodmosaic commented Mar 24, 2025

moodmosaic left a comment

moodmosaic Mar 24, 2025

fdefelici Mar 25, 2025

moodmosaic Mar 25, 2025

fdefelici Mar 25, 2025

Jiloc Mar 25, 2025

rdeioris left a comment

Jiloc left a comment

obycode left a comment

codecov bot commented Mar 28, 2025 •

edited

Loading

moodmosaic left a comment

	let path = "/tmp/test-get-unconfirmed-commits.bat";
	let path = std::env::temp_dir().join("test-get-unconfirmed-commits.bat");

fix: make cargo test testnet work on windows #5937

Are you sure you want to change the base?

fix: make cargo test testnet work on windows #5937

Conversation

fdefelici commented Mar 18, 2025

Description

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

moodmosaic commented Mar 21, 2025

fdefelici commented Mar 21, 2025

moodmosaic commented Mar 21, 2025

wileyj commented Mar 22, 2025 • edited Loading

fdefelici commented Mar 22, 2025 • edited Loading

fdefelici commented Mar 22, 2025

moodmosaic commented Mar 22, 2025

fdefelici commented Mar 22, 2025

wileyj commented Mar 22, 2025

wileyj commented Mar 23, 2025

fdefelici commented Mar 23, 2025

rdeioris commented Mar 24, 2025

moodmosaic commented Mar 24, 2025

moodmosaic left a comment

Choose a reason for hiding this comment

moodmosaic Mar 24, 2025

Choose a reason for hiding this comment

fdefelici Mar 25, 2025

Choose a reason for hiding this comment

moodmosaic Mar 25, 2025

Choose a reason for hiding this comment

fdefelici Mar 25, 2025

Choose a reason for hiding this comment

Jiloc Mar 25, 2025

Choose a reason for hiding this comment

rdeioris left a comment

Choose a reason for hiding this comment

Jiloc left a comment

Choose a reason for hiding this comment

obycode left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 28, 2025 • edited Loading

Codecov Report

moodmosaic left a comment

Choose a reason for hiding this comment

fix: make `cargo test testnet` work on windows #5937

fix: make `cargo test testnet` work on windows #5937

wileyj commented Mar 22, 2025 •

edited

Loading

fdefelici commented Mar 22, 2025 •

edited

Loading

codecov bot commented Mar 28, 2025 •

edited

Loading