Skip to content

fix(grey): fix Windows build errors across all build.rs and node.rs#811

Open
liuxiayu12 wants to merge 2 commits into
jarchain:masterfrom
liuxiayu12:fix/windows-build-all
Open

fix(grey): fix Windows build errors across all build.rs and node.rs#811
liuxiayu12 wants to merge 2 commits into
jarchain:masterfrom
liuxiayu12:fix/windows-build-all

Conversation

@liuxiayu12
Copy link
Copy Markdown

Summary

Fix three categories of Windows build errors that prevented compilation on Windows.

1. build.rs path escaping (all 4 build.rs files)

PathBuf::display() emits Windows backslash separators which Rust's lexer interprets as escape sequences inside include_bytes! string literals (e.g. \b, \j, \s, \o).

Fix: Add include_path() helpers to build-javm and build-pvm that replace backslashes with forward slashes. All 4 build.rs files now use these helpers instead of .display():

2. node.rs Unix-only signal handling

tokio::signal::unix is not available on Windows. The original code hardcoded SIGTERM, SIGUSR1, and SIGHUP signal registrations.

Fix: Wrap Unix-specific signal registrations in #[cfg(unix)] and provide std::future::pending::<()>() placeholders on non-Unix platforms so the select! arms compile but are simply never selected.

3. Comparison with #783

PR #783 fixed grey/build.rs and node.rs but missed the other 3 build.rs files that have the same .display() bug. This PR:

  • Fixes all 4 build.rs files uniformly through the build-javm/build-pvm helper approach
  • Adds include_path() to both build-javm and build-pvm crates (the latter is used by grey-bench)
  • Uses &Path instead of &PathBuf to satisfy clippy

Testing

cargo build --workspace                                                          # passes on Windows
cargo clippy -p grey -p build-javm -p build-pvm --all-targets -- -D warnings    # 0 warnings
cargo test -p javm                                                               # 133 passed, 0 failed
cargo fmt --all -- --check                                                       # no diff

Note: This is a Windows compatibility fix with no functional changes to Unix builds.

1825833563 and others added 2 commits April 25, 2026 16:50
Three categories of Windows build errors were preventing compilation:

1. build.rs path escaping (all 4 build.rs files):
   PathBuf::display() emits Windows backslash separators which Rust's
   lexer interprets as escape sequences inside include_bytes! string
   literals (e.g. \b, \j, \s, \o). Fix by adding include_path() helpers
   to build-javm and build-pvm that replace backslashes with forward
   slashes. All 4 build.rs files (grey, spec-tests, javm-guest-tests,
   grey-bench) now use these helpers instead of .display().

2. node.rs Unix-only signal handling:
   tokio::signal::unix is not available on Windows. The original code
   hardcoded SIGTERM, SIGUSR1, and SIGHUP signal registrations.
   Fix by wrapping Unix-specific signal registrations in #[cfg(unix)]
   and providing std::future::pending::<()>() placeholders on non-Unix
   platforms so the select! arms compile but are simply never selected.

3. Previous PR jarchain#783 only fixed grey/build.rs but missed spec-tests,
   javm-guest-tests, and grey-bench which have the same .display()
   bug. This PR fixes all 4 build.rs files uniformly through the
   build-javm/build-pvm helper approach.

Testing:
  cargo build --workspace              # passes on Windows
  cargo clippy -p grey -p build-javm -p build-pvm --all-targets -- -D warnings  # 0 warnings
  cargo test -p javm                   # 133 passed, 0 failed
  cargo fmt --all -- --check           # no diff
@github-actions
Copy link
Copy Markdown
Contributor

Genesis Review

Comparison targets:

How to review

Post a comment with the following format (rank from best to worst):

/review
difficulty: <commit1>, <commit2>, ..., <commitN>, currentPR
novelty: <commit1>, <commit2>, ..., <commitN>, currentPR
design: <commit1>, <commit2>, ..., <commitN>, currentPR
verdict: merge

Use the short commit hashes above and currentPR for this PR.
Each line ranks all comparison targets + this PR from best to worst.

To meta-review another reviewer's comment, react with 👍 or 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants