Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking: Restore Windows support #3801

Closed
5 tasks
conradoplg opened this issue Mar 8, 2022 · 13 comments
Closed
5 tasks

Tracking: Restore Windows support #3801

conradoplg opened this issue Mar 8, 2022 · 13 comments
Assignees
Labels
A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement C-tracking-issue Category: This is a tracking issue for other tasks S-needs-investigation Status: Needs further investigation S-needs-triage Status: A bug report needs triage

Comments

@conradoplg
Copy link
Collaborator

conradoplg commented Mar 8, 2022

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

#3800 is a compromise to allow some testing on Windows, that we can do beforehand.

@conradoplg conradoplg added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage P-Low ❄️ labels Mar 8, 2022
@mpguerra mpguerra moved this to 🆕 New in Zebra Sep 22, 2022
@mpguerra mpguerra added this to Zebra Sep 22, 2022
@PRabahy
Copy link

PRabahy commented May 16, 2023

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!

@mpguerra
Copy link
Contributor

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.

@str4d
Copy link
Contributor

str4d commented May 20, 2023

@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).

@teor2345 teor2345 added the S-needs-investigation Status: Needs further investigation label Aug 17, 2023
@teor2345
Copy link
Contributor

@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
Copy link
Contributor

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

@conradoplg
Copy link
Collaborator Author

For reference the first step is probably reverting #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.

@teor2345
Copy link
Contributor

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.

@mpguerra
Copy link
Contributor

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

@teor2345
Copy link
Contributor

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?

@conradoplg
Copy link
Collaborator Author

conradoplg commented Aug 29, 2023

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 😅 )
  • 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

@teor2345
Copy link
Contributor

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.

@mpguerra mpguerra changed the title Restore Windows support Tracking: Restore Windows support Aug 30, 2023
@mpguerra
Copy link
Contributor

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 mpguerra added the C-tracking-issue Category: This is a tracking issue for other tasks label Oct 26, 2023
@mpguerra mpguerra moved this from New to Sprint Backlog in Zebra Feb 20, 2024
@mpguerra mpguerra moved this from Sprint Backlog to In progress in Zebra Mar 6, 2024
@mpguerra
Copy link
Contributor

We've successfully restored Windows support

@github-project-automation github-project-automation bot moved this from In progress to Done in Zebra Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement C-tracking-issue Category: This is a tracking issue for other tasks S-needs-investigation Status: Needs further investigation S-needs-triage Status: A bug report needs triage
Projects
Archived in project
Development

No branches or pull requests

5 participants