penumbra-zone / penumbra

Penumbra is a fully private proof-of-stake network and decentralized exchange for the Cosmos ecosystem.
https://penumbra.zone
Apache License 2.0
381 stars 296 forks source link

Windows builds are broken #3298

Closed conorsch closed 1 year ago

conorsch commented 1 year ago

Describe the bug It's not currently possible to build Penumbra on Windows platforms.

To Reproduce Steps to reproduce the behavior:

  1. Boot a Windows machine and install rust and git, and LLVM (for clang).
  2. Run cmd.exe
  3. Clone Penumbra repo, chdir into it, check out v0.63.1, and run cargo build --release.
  4. Observe error message:
error[E0793]: reference to packed field is unaligned
    --> C:\Users\conor\.cargo\registry\src\index.crates.io-6f17d22bba15001f\ntapi-0.3.7\src\ntexapi.rs:2783:52
     |
2783 |         *tick_count.QuadPart_mut() = read_volatile(&(*USER_SHARED_DATA).u.TickCountQuad);
     |                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
     = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
     = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
    --> C:\Users\conor\.cargo\registry\src\index.crates.io-6f17d22bba15001f\ntapi-0.3.7\src\ntexapi.rs:2807:25
     |
2807 |         ((read_volatile(&(*USER_SHARED_DATA).u.TickCountQuad)
     |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
     = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
     = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

   Compiling js-sys v0.3.65
   Compiling serde_qs v0.8.5
   Compiling infer v0.2.3
   Compiling encode_unicode v0.3.6
   Compiling async-process v1.8.1
   Compiling quote v1.0.33
   Compiling headers v0.3.9
   Compiling async-dup v1.2.2
For more information about this error, try `rustc --explain E0793`.
error: could not compile `ntapi` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
^C  Building [===================>    ] 904/1081: librocksdb-sys(build)

Expected behavior Builds succeed on Windows.

Additional context Initial search identifies this problem may have been fixed a while back https://github.com/MSxDOS/ntapi/commit/24fc1e47677fc9f6e38e5f154e6011dc9b270da6 but our outdated version of vergen depends on an outdated version of ntapi. I suspect recent versions of Rust have made this problem more obvious, because Windows builds used to work, as recently as v0.61.0.

How hard to we want to try to support Windows as a build target? AFAICT, none of the development team are using it. Perhaps upgrading cargo-dist and adding a job in CI to run the cross-platform builds occasionally would help us keep an eye on this.

Refs https://github.com/penumbra-zone/penumbra/issues/3279

conorsch commented 1 year ago

How hard to we want to try to support Windows as a build target?

For now, let's focus on providing working Linux & macOS binaries. Once that's stable again, we can re-evaluate whether it's worth supporting Windows as a first-class build target.

conorsch commented 1 year ago

We've dropped support for Windows as a build platform. For mac and Linux, we have working binaries again, see https://github.com/penumbra-zone/penumbra/releases/tag/v0.63.2. We may want to discuss supporting Windows as a first-class build target in the future, but that deserves its own issue. As a sidenote, I suspect that dropping vergen from our build deps may simplify the Windows dependency story—but without regular use of those bins, compat is likely to break again.