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

Windows support #219

Closed
Shayan-To opened this issue Mar 13, 2021 · 19 comments
Closed

Windows support #219

Shayan-To opened this issue Mar 13, 2021 · 19 comments

Comments

@Shayan-To
Copy link

Is Windows is going to be supported?

@jtdaugherty
Copy link
Owner

Hi @Shayan-To, there is currently no effort underway to support Windows. There have been a few attempts over the years, but nothing has stuck. If you or someone you know would like to work on Windows support, please let me know!

@noughtmare
Copy link
Contributor

noughtmare commented May 27, 2022

I've done a bit of investigation into what would be necessary, here are my findings:

All the relevant documentation is here: https://docs.microsoft.com/en-us/windows/console/

I collected this data by commenting out the unix package and its modules and seeing which errors were reported by GHC.

Terminfo is available via mingw-w64-x86_64-ncurses on msys2. This only needs small changes to the way the terminfo package is built. Although I don't know how compatible this is.

  1. Preparation:

Switch to use Handle as much as possible. This should retain the same behavior, but it is more portable.

  • FD -> Use System.IO.Handle directly
  • stdInput -> System.IO.stdin
  • stdOut -> System.IO.stdout
  • fdReadBuf -> hGetBuf(NonBlocking)
  • fdWriteBuf -> hPutBuf(NonBlocking)
  • openFd -> openFile
  • closeFd -> hClose
  • EnableEcho (ECHO) -> hSetEcho
  1. Basic useable implementation:
  • handleToFd -> handleToHANDLE/withHandleToHANDLE (see GHC issue #21661)
  • setFdOption NonBlockingRead -> ? ReadConsoleInput & GetNumberOfConsoleInputEvents or PeekConsoleInput & FlushConsoleInputBuffer
    (hGetBufNonBlocking doesn't work correctly on windows: GHC issue #21665)
  • set_term_timing
  • ProcessInput (ICANON), KeyboardInterrupts (ISIG), ExtendedFunction (IEXTEN) -> ENABLE_PROCESSED_INPUT (see this page from the docs)
  • get_window_size
  • windowChange event
  1. Full feature parity:
  • set_window_size
  • continueProcess -> possibly unnecessary?
  • MapCRToLF (ICRNL) -> possibly unnecessary?
  • StartStopOutput (IXON) -> possibly unnecessary?
  • get_tty_erase -> possibly unnecessary?
  • openPseudoTerminal (only used for mocking)

I want to see if I can implement some of these steps over the next few weeks.

@jtdaugherty
Copy link
Owner

jtdaugherty commented May 27, 2022 via email

@jtdaugherty
Copy link
Owner

I took a look over this and I have a few questions/thoughts.

Terminfo is available via mingw-w64-x86_64-ncurses on msys2.

Why is terminfo needed? I ask because 1) I'd like to avoid a dependency on mingw (so if there's some other native way to get it, I'd prefer that), and 2) mainly because I don't know of a reason why a Vty backend for Windows needs to use terminfo the way the other backends do. It could be implemented completely differently and not touch terminfo at all. If anything outside of the output drivers depends on terminfo, I'd rather change that (and thus decouple the rest of the library from some drivers' use of terminfo) rather than have the terminfo dependency leak into the Windows driver.

Switch to use Handle as much as possible.

Reading over the list, it looks like this section is about moving away from the unix package (which means using Handle rather than file descriptors). Right? I just want to clarify the motivation.

From an implementation approach perspective, to me it would be extremely helpful if the move to Handle can happen first, on a branch, without any work on Windows support in any other way. If we can get that change worked out on the current implementation, that's not only incremental progress toward making Windows support work, but it's also releasable work on this library that we can keep even if for some reason Windows support is abandoned before it's completed. I don't want this work to get wrapped up in the Windows support work in a way that it can't be disentangled.

Another reason I'd like to see the Handle migration happen first is because if for some reason it can't be done (i.e. due to lower-level APIs being needed that can't be gotten with just Handle), then I think it would be helpful to find that out sooner rather than later as it would be a show-stopping discovery.

@jtdaugherty jtdaugherty reopened this May 29, 2022
@jtdaugherty
Copy link
Owner

As for the post-Handle work to support Windows, I'm also thinking there may be places where we need to do conditional compilation for systems that can build unix and use lower-level APIs. If that happens, I'd like the approach to abstract over those needs if it doesn't already.

@noughtmare
Copy link
Contributor

I haven't looked too long at terminfo because it was pretty easy to use the mingw package. I agree that it would be nicer to use the native API. I should look at that in more detail.

I'm a bit conflicted about the Handle situation. I know that simply using Haskell's handles will not be enough to cover all those configuration options, but you can use functions like handleToFd (unix) and withHandleToHANDLE (win32) to get access to the lower level types.

If the high level Handle is good enough for most of the uses and only the initialization and deinitialization needs access to the low-level interface, then I think it would be worth it to switch to Handles. Otherwise, the alternative is to make an abstract NativeHandle type that is Fd on unix and HANDLE on windows.

One thing that I am wondering about is if vty has a good enough test suite. If we change it to use Handle then we have to be very sure that everything still works correctly. I'm especially worried that something like timing or the handling of special control characters will change slightly. I expect those things are pretty hard to notice.

@jtdaugherty
Copy link
Owner

Otherwise, the alternative is to make an abstract NativeHandle type that is Fd on unix and HANDLE on windows.

This strikes me as worthwhile no matter what, because it then makes it much easier to tell what kind of abstract interface we need for NativeHandle, and it makes it easier to concentrate any platform-specific implementation in functions that operate on NativeHandle. This seems like something we will ultimately need anyway; otherwise we run the risk of having to sprinkle ifdefs throughout the code in various spots, and I'd really love to have that stuff relegated to a windows-specific module that implements the handle interface or something like that.

One thing that I am wondering about is if vty has a good enough test suite.

Vty has a pretty large set of test programs, but 1) I did not write any of them and 2) frankly I don't know what they test or how much test coverage they provide. I have largely ignored the test suite over the years that I've maintained vty except to make sure that it doesn't fail; I get the impression it mostly tests Image-related internals, and I have so rarely needed to change Image that I haven't had to modify the test suite.

With that said, I share your concern about tiny details being easy to miss. We'll just have to put it through its paces, and perhaps figure out what additional tests need to be written.

@noughtmare
Copy link
Contributor

OK, then I think the best course of action is to first consolidate all the unix dependent code behind such an abstract NativeHandle interface. Then we don't really need to use the Handle type. The second step would be to implement that interface on windows.

While doing that first step we should take into account the similarities and differences between windows and unix. We shouldn't make an abstraction that turns out to be unimplementable on windows.

I think this approach should also have the advantage that the existing unix implementation will stay completely the same, just moved behind an interface. So there should be no regressions at least. Whether the applications work correctly on windows can be dealt with in future issue reports.

@jtdaugherty
Copy link
Owner

Thanks, @noughtmare - that sounds like a great approach to me. I'm happy to review incrementally or wait until the NativeHandle move is complete. My only requests are:

  1. Just in case this is your style: please avoid force-pushes to any PR branch that you create; I find that some people do that, and it makes PRs much more difficult for me to evaluate.
  2. Please make commits as small and as logically contained as you can, and include any relevant context in the commit messages about your thinking as you go along. That will really help me review the changes.

@jtdaugherty
Copy link
Owner

I also had another thought about this, which is that as we think about these changes, I'd love to keep an eye on whether we could split up the package into a core (e.g. vty-core) and platform-specific backends (e.g. vty-unix and vty-windows). Not only does that test whether the exported interface is sufficiently extensible and complete, but it avoids mixing platform-specific code in the same package, which is usually a nightmare to maintain. It also makes it much easier to separate maintainers for different backends; I don't imagine I'll be able to maintain the Windows backend, but if it were in its own package, it would be so much more feasible to have another maintainer help with that part of the implementation.

@chhackett
Copy link

Hello, I want to let you know I would be very interested in working on the Windows port of vty with you and/or @noughtmare. I would very much like to discuss how to proceed with you when you have time. Or if you like, I could submit a PR with the NativeHandle change as a first step. Then perhaps you could look at the work I've done and we can decide the next step.

@jtdaugherty
Copy link
Owner

@chhackett thanks for your offer! I don't know whether @noughtmare still has plans to pick this back up so I'd like to hear from them first.

Looking back at the comment history, I am increasingly in favor of abstraction changes to support a core package and platform-specific packages. I strongly suspect that that means we don't need NativeHandle at all because I would consider the handle type in use to be an implementation detail that should be hidden by the library abstraction.

@noughtmare
Copy link
Contributor

I'm not working on anything at the moment.

My thought was that the platform independent part would use NativeHandle to represent the native handle type. But maybe it's possible to factor that out completely.

@hasufell
Copy link

hasufell commented Feb 1, 2023

@jtdaugherty is this something possible for GSoC? Or is the effort too much?

https://summer.haskell.org/ideas.html

@jtdaugherty
Copy link
Owner

@hasufell Unfortunately I don't think I'll have the time to devote to it, and I also would prefer to use a different mechanism to find a contributor. In particular, it isn't enough even to write the code; I strongly believe that the most important thing is to find a maintainer for the Windows support in vty (as a package) who will have a vested interest in maintaining it longer-term.

@hasufell
Copy link

hasufell commented Feb 2, 2023

I'm not sure we'll find a contributor if there's no one to mentor them.

@jtdaugherty
Copy link
Owner

Yes, that's true in the student case and for GSOC. I think that Windows support in Vty will be a big undertaking and will best be undertaken by someone with a lot of industrial software engineering experience. I'm not saying I can't be available to help and collaborate; I would definitely want to do so, but I don't have the resources to do it in a mentoring capacity for this project.

@jtdaugherty
Copy link
Owner

For those interested in following along, #260 captures the state of preparing for a release of Vty with support for Windows.

@jtdaugherty
Copy link
Owner

Windows support went live today via the releases for Vty 6 and vty-windows. Yay!

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

No branches or pull requests

5 participants