zebra icon indicating copy to clipboard operation
zebra copied to clipboard

Tracking: Restore Windows support

Open conradoplg opened this issue 2 years ago • 12 comments

Motivation

We disabled Windows build in #3799 due to zcash_script requiring MinGW, and rocksdb requiring MSVC.

But we can restore Windows support if we wish. See discussion.

Specifications

Designs

Restore Windows support by making Zebra fully compile with MSVC or MinGW, by either:

  • Changing zcash_script so it compiles with MSVC
  • Changing rocksdb so it compiles with MinGW

Scope

  • [ ] Restore Windows support in CI using MSVC
    • [ ] Re-enable passing tests only
    • [ ] list failing tests here for future fixes
    • [ ] ...
  • [ ] Make zcash_script work with MSVC

Related Work

https://github.com/ZcashFoundation/zebra/issues/3800 is a compromise to allow some testing on Windows, that we can do beforehand.

conradoplg avatar Mar 08 '22 18:03 conradoplg

The linked discussion says "Delay this decision until we have users who want to use Windows?". I know I'm just one small voice, but I would probably run this on Windows if it was available. I've been running zcashd via ZecWallet Fullnode, but don't really trust how it gets released. A new binary blob just gets pushed once in a while and I hope that it doesn't do anything bad. My end goal is to have a reliable, trusted Zcash full node running on Windows that can store my ZCash in a Shielded Wallet on a Trezor. I'm hoping this will just be one link in that chain!

PRabahy avatar May 16 '23 03:05 PRabahy

Thank you for the feedback! Comments like this are great to help us understand how people want to use our software and prioritise work.

We'll definitely take this into consideration as we make a decision on which platforms we want to support.

mpguerra avatar May 16 '23 08:05 mpguerra

@PRabahy note that while zebrad should be a drop-in replacement for zcashd as a full node, it is not a drop-in replacement for zcashd as a wallet. The Zcash Foundation has not implemented any wallet logic in zebrad, and certainly not any wallet logic that is compatible with ZecWallet Full Node.

The solution to your problem is getting reliable Windows binaries built for zcashd. I've been working towards this for some time, such as introducing our Platform Tier Policy under which Windows binaries could be published as a Tier 2 platform (because the long-term blocker here has been our inability to run tests on Windows for various arcane CI reasons, meaning we could not release binaries under our Tier 1 policy, which historically was the only policy we had).

str4d avatar May 20 '23 11:05 str4d

@mpguerra this is potentially an extremely large project with an ongoing maintenance burden.

It involves modifying either:

  • all the parts of zcashd compiled by zcash_script which are incompatible with MSVC
  • all the parts of rocksdb which are incompatible with MinGW

And then:

  • temporarily depending on a fork of either zcashd or rocksdb (not sure if this will be required)
  • getting those patches upstreamed into either zcashd or rocksdb
  • depending on their subsequent release
  • every time they break compatibility, fixing the breakage and re-submitting our patches
  • adding the appropriate build tests to their CI, which should prevent compatibility breaks

Before I can fully estimate this project, we need to make a decision about which project to fix:

* Changing `zcash_script` so it compiles with MSVC
* Changing `rocksdb` so it compiles with MinGW

Then I think it would help to split this ticket based on the stages I listed above: patches, dependency update, and CI checks.

Maybe we could start by splitting out a ticket to investigate which option would be easier?

teor2345 avatar Aug 17 '23 22:08 teor2345

It might also help to split the ticket into stages:

  • get Windows builds working
  • add Windows builds to CI
  • fix tests that fail on Windows
  • add Windows tests to CI

teor2345 avatar Aug 17 '23 22:08 teor2345

For reference the first step is probably reverting https://github.com/ZcashFoundation/zebra/pull/3819/, letting CI run in a WIP PR and checking what breaks. My guess is that's probably easier to make zcash_script work on MSVC rather than making rocksdb work on MinGW.

conradoplg avatar Aug 18 '23 13:08 conradoplg

If we're just aiming for tier 2 support, then running compilation tests and some basic launch tests should be enough. I don't think we need to run the full test suite.

teor2345 avatar Aug 21 '23 03:08 teor2345

Let's split this issue up and turn it into a tracking issue. I think starting with #3800 is a good first step.

I'm going to un-schedule this for now though

mpguerra avatar Aug 28 '23 20:08 mpguerra

I think starting with #3800 is a good first step.

I'm not sure if we can link MSVC and MinGW code together, so if the MinGW side of this is a lot of effort, we could skip that part. Maybe we could split #3800 into MSVC and MinGW parts, and do MSVC first?

teor2345 avatar Aug 29 '23 02:08 teor2345

I think the idea behind #3800 was to do at least some testing on Windows, even if Zebra would not support it. But I think the idea here is to simply restore full support.

My suggestion for the path forward is:

  • Restore Windows support in CI in a WIP PR using MSVC (as we did before), and check what breaks (I was going to give a shot on this but I was afraid of breaking CI somehow since it has changed a lot since I disabled Windows support :sweat_smile: )
  • Make zcash_script work with MSVC, with the help of the output of the previous task. If for some reason this is too difficult, or not enough to restore Windows support, reevaluate next steps

conradoplg avatar Aug 29 '23 13:08 conradoplg

Sounds good!

I'd suggest splitting the first step into:

  • restore support for Windows in CI and turn on the tests that work immediately, then get that merged
  • open tickets for each test or group of tests that don't work, and fix them

That way there's not one big ticket or PR, and we're not breaking working tests while it's open for a long time.

teor2345 avatar Aug 29 '23 23:08 teor2345

I changed this into a tracking issue and started attempting to break it down a bit. If anyone has some time it would be helpful to break it down further

mpguerra avatar Aug 30 '23 09:08 mpguerra

We've successfully restored Windows support

mpguerra avatar Mar 18 '24 15:03 mpguerra