zellij
zellij copied to clipboard
Windows support
I am starting to track things that need work at the top now for people who just want to see the status or that want to help and need a "thing" to work at:
- [x] IPC (basic with 2 pipes)
- [ ] IPC (with the "correct" duplex pipe approach)
- [x] IPC performance investigation
- [x] Reading from pty
- [x] Writing to pty
- [x] Recognizing control characters
- [ ] Listing sessions
- [ ] Reattaching to sessions
Original Message:
I have gotten it to compile under windows and (hopefully) IPC working. I hope this did not break anything for non-windows systems, but I did not test it at the moment.
From here I hope the compiler will guide me to all the places that have yet to be implemented.
Hi, great work I am looking very forward to this! I tried to compile under Linux and found and fixed one compile error due to update of the sysinfo crate:
diff --git a/zellij-server/src/os_input_output.rs b/zellij-server/src/os_input_output.rs
index d23789cf..cafca34d 100644
--- a/zellij-server/src/os_input_output.rs
+++ b/zellij-server/src/os_input_output.rs
@@ -826,7 +826,7 @@ impl ServerOsApi for ServerOsInputOutput {
// See https://docs.rs/sysinfo/0.22.5/sysinfo/struct.ProcessRefreshKind.html#
system_info.refresh_processes_specifics(ProcessRefreshKind::default());
- if let Some(process) = system_info.process(pid.into()) {
+ if let Some(process) = system_info.process((pid.as_raw() as usize).into()) {
let cwd = process.cwd();
let cwd_is_empty = cwd.iter().next().is_none();
if !cwd_is_empty {
@@ -845,7 +845,7 @@ impl ServerOsApi for ServerOsInputOutput {
let mut cwds = HashMap::new();
for pid in pids {
- if let Some(process) = system_info.process(pid.into()) {
+ if let Some(process) = system_info.process((pid.as_raw() as usize).into()) {
let cwd = process.cwd();
let cwd_is_empty = cwd.iter().next().is_none();
if !cwd_is_empty {
After this change it compiles and runs a first short test without problem!
To make all the tests work again another simple fix is necessary:
diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs
index c3309f55..c9c455c8 100644
--- a/zellij-server/src/tab/unit/tab_integration_tests.rs
+++ b/zellij-server/src/tab/unit/tab_integration_tests.rs
@@ -103,6 +103,7 @@ impl ServerOsApi for FakeInputOutput {
&mut self,
_client_id: ClientId,
_stream: LocalSocketStream,
+ _sender: LocalSocketStream,
) -> Result<IpcReceiverWithContext<ClientToServerMsg>> {
unimplemented!()
}
diff --git a/zellij-server/src/tab/unit/tab_tests.rs b/zellij-server/src/tab/unit/tab_tests.rs
index b1e2110a..b2ba2850 100644
--- a/zellij-server/src/tab/unit/tab_tests.rs
+++ b/zellij-server/src/tab/unit/tab_tests.rs
@@ -77,6 +77,7 @@ impl ServerOsApi for FakeInputOutput {
&mut self,
_client_id: ClientId,
_stream: LocalSocketStream,
+ _sender: LocalSocketStream,
) -> Result<IpcReceiverWithContext<ClientToServerMsg>> {
unimplemented!()
}
diff --git a/zellij-server/src/unit/screen_tests.rs b/zellij-server/src/unit/screen_tests.rs
index c9de7afd..258ee360 100644
--- a/zellij-server/src/unit/screen_tests.rs
+++ b/zellij-server/src/unit/screen_tests.rs
@@ -188,6 +188,7 @@ impl ServerOsApi for FakeInputOutput {
&mut self,
_client_id: ClientId,
_stream: LocalSocketStream,
+ _sender: LocalSocketStream,
) -> Result<IpcReceiverWithContext<ClientToServerMsg>> {
unimplemented!()
}
Pleas Keep working on this!!! I would love to have support on Windows!!
I was naively thinking that zellij should work under windows if built from source ... this is a lot of work.
Appreciate the work you are doing on this.
I want to learn Rust just so that I can contribute to this PR. @iXialumy Thank you for working on this ❤️
If you wish to express support for this PR, please use the :+1: reaction on the original post rather than posting a reply. Some people are subscribed to this PR and receive notifications for all activity, including new comments.
I felt like a single emoji is not enough, and that I should express my gratitude to the person in question in another way.
I am kind of stuck now. I got to a point where zellij starts, connects a server and a client and does not crash instantly when typing something, but this is the point, where neither the compiler nor the logs give me a clear direction whats the next step to tackle.
@imsnif If you or anyone else knows where to look next please hint me, otherwise I will continue to poke in the dark until I found the right place to look at. :) Thanks for all the kind words reaffirming me to continue this poking around.
Hey @iXialumy - happy to see you're still at it. Kudos on your perseverance!
On the surface, this looks like at least two different problems - even though it's possible it's one core problem that creates an invalid state and causes everything else.
Without looking at the code, and from what you describe, I would guess you need to somehow handle spawning terminals. Specifically looking into how to translate what we're doing with openpty
here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/os_input_output.rs#L232 (and some related places).
When I go into a code base and try to fix something, I usually really hate it when someone who knows the code base well tells me something along the lines of "You're going about this the long way, you should really be doing..." - so, I'm not going to tell you that. But I will say that I think the compiler doesn't know a lot about architecture and system compatibility. The big chunk of work here is moving from pty to ConPty and adjusting the code to match, possibly even finding libraries that do this (since we're using direct syscalls on linux/mac).
I'd recommend giving a read to a blog post I wrote on the subject a while ago: https://poor.dev/blog/terminal-anatomy/
That being said, since I promised I will not do what I just did, I wish you good luck and have confidence in you finishing this up in whatever way works for you.
EDIT: another interesting place to look is here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/terminal_bytes.rs#L66 - which is the async task that reads bytes from the pty handle. Not sure how you're handling it, but could be that the pty is opened (I briefly saw you're using something there already) but not read from properly.
Hey @imsnif
First of all, thanks for the great feedback. That block post of yours is an absolute diamond.
I used your example code to figure out quite a big difference in reading from ptys in windows and unix systems. This is probably why my current implementation has no output. As for spawning the terminal, I can see in my running processes, that spawning the pty with its command was indeed successful already, so I am thinking, that I just do not read from it correctly yet.
Anyways thank you for taking your time to provide some feedback. I will take anything I can get :)
Well working on it finally paid off!!!
I found a stupid mistake I made on how I create the async reader. But now it works.
Still entering commands does not work, but I feel like I just cleared a major Bar :)
Super cool @iXialumy !! Great work. Looking forward to seeing more progress.
For reference, writing to the pty happens here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/pty_writer.rs#L42
Hope this helps.
First of all, @iXialumy thank you so much for your hard work!
I would love to help contribute where I can! I recently cloned your branch and implemented the get_terminal_size
function for windows (so that zellij takes up the entire terminal width/height).
I'm just not sure how we want to go about letting others contribute to this? Should I be making PRs into your fork? Or am I better off creating my own PRs into zellij-org so that this PR doesn't end up being too large in the end?
@imsnif, what do you think?
Please don't make PRs into zellij-org :)
@maxle5 Feel free to open a PR on my fork then. I think when I approve it, it should show up here too :)
I've kind of got command output showing up. It doesn't actually read user input, but I rigged it up to do an ls command after pressing enter.
The main issue was that the AsyncReader
sends back Ok(0)
when there's nothing left to read (like when waiting for user input), but TerminalBytes::listen
interprets the Ok(0)
as an EoF and kills the listener. I think the correct implementation would be to make AsyncReader properly awaitable, but I'm not sure how to do that with winpty-rs.
I'll make a PR into @iXialumy 's repo when it's a bit more ready.
And here it is reading and writing! I've got a PR into your repo @iXialumy. Getting closer...
@ulyssesdotcodes It looks like there are trailing tabs when isssuing commands (i.e. ls). Any fix for it so command starts at the beginning of line?
Turned on per-character read mode and it's somewhat useable. Tabs work, but panes do not yet.
@iXialumy, @ulyssesdotcodes: First great work. But would it be possible for someone who has managed to test the windows build to add some build or installation instructions for windows?
Then I would be able to test it and maybe help with it.
You should be able to follow the same instructions as the Linux build - cargo xtask etc.
On Sat, Mar 2, 2024, 11:09 Jakob Ledermann @.***> wrote:
@iXialumy https://github.com/iXialumy, @ulyssesdotcodes https://github.com/ulyssesdotcodes: First great work. But would it be possible for someone who has managed to test the windows build to add some build or installation instructions for windows?
Then I would be able to test it and maybe help with it.
— Reply to this email directly, view it on GitHub https://github.com/zellij-org/zellij/pull/2926#issuecomment-1974881546, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADVTU42GS7XQYDUBN4YX2DYWIPXRAVCNFSM6AAAAAA7GR4AUWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZUHA4DCNJUGY . You are receiving this because you were mentioned.Message ID: @.***>
@ulyssesdotcodes for me panes seem to be working too. So it seems input of "special characters"/commands is still kind of flaky. Ctrl+C for example kills zellij completely still.
Also detaching from a session seems to kill the session on the server too, or at the very least reattaching to the session does not work yet.
Latency between server and client has also degraded since I changed communication from file descriptors to networked for cross platform reasons.
@iXialumy yup, all the basic features are working with the latest updates - panes, tabs, layouts, etc.
Special characters should be working as much as possible. I copied what wezterm does for the virtual terminal. Not sure how to get ctrl + c working, but maybe we could catch and handle it?
I have not looked at sessions or performance yet. I agree that it has degraded unfortunately.
@ulyssesdotcodes It seems that ctrl+c and other unix signals work entirely differently under windows. It works with callback functions as far as I can tell, but I didn't find a library which provides a nice wrapper around it yet.
Regarding performance maybe @imsnif has an idea for how to communicate between the server and client in a faster, but still cross platform way. Otherwise we have to search for another way there ourselves.
Regarding performance maybe @imsnif has an idea for how to communicate between the server and client in a faster, but still cross platform way. Otherwise we have to search for another way there ourselves.
For what it's worth - I would not want to change anything about how they communicate in existing platforms. I'd very much prefer a windows-specific solution on windows and for things to stay as they are on linux/mac/etc.
@ulyssesdotcodes It seems to me, that we need to use winapi directly for the console handling. I am not sure where to register the handlers yet though.
For what it's worth - I would not want to change anything about how they communicate in existing platforms. I'd very much prefer a windows-specific solution on windows and for things to stay as they are on linux/mac/etc.
We can totally do that. I think it would be interesting to use the same codepath for both platforms, so they don't have to be maintained separately as well as producing less redundant code. But the decision is entirely yours.
We can totally do that. I think it would be interesting to use the same codepath for both platforms, so they don't have to be maintained separately as well as producing less redundant code. But the decision is entirely yours.
I appreciate the thought, but the I think the path of least resistance is no changes to existing functionality/platforms and only experiment on new platforms to find out what's good for them.
@ulyssesdotcodes It seems to me, that we need to use winapi directly for the console handling. I am not sure where to register the handlers yet though.
I have managed to react to the STRG+C
but am failing to find, how the SIGINT signal arrives at the actual running process like ping
.
If I take a look at https://github.com/zellij-org/zellij/pull/648/files it looks a lot like the target of CTRL-C is determined by linux directly. No clue how to do that in windows...
I found a way to forward CTRL-C
to the child process running in the pane... By treating the signal as input.
While testing I also found a crash that only occurs in the windows variant:
- open new pane
- type
exit
and close the shell in the new pane (first indication of problem is zellij does not detect the termination of the shell) - close the pane with
CTRL-P + x
Backtrace of the crash in the server:
Error occurred in server:
× Thread 'pty' panicked.
├─▶ Originating Thread(s)
│ 1. stdin_handler_thread: AcceptInput
│ 2. screen_thread: CloseFocusedPane
│ 3. pty_thread: ClosePane
│
├─▶ At zellij-server\src\pty.rs:1750:30
╰─▶ Program terminates: a fatal error occured
Caused by:
0: failed to run async task for pane 1
1: Unable to get process
Stack backtrace:
0: std::backtrace_rs::backtrace::dbghelp::trace
at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library\std\src\..\..\backtrace\src\backtrace\dbghelp.rs:98
1: std::backtrace_rs::backtrace::trace_unsynchronized
at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library\std\src\..\..\backtrace\src\backtrace\mod.rs:66
2: std::backtrace::Backtrace::create
at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library\std\src\backtrace.rs:331
3: std::backtrace::Backtrace::capture
at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library\std\src\backtrace.rs:297
4: anyhow::error::impl$1::from<std::io::error::Error>
at E:\.cargo\registry\src\index.crates.io-6f17d22bba15001f\anyhow-1.0.75\src\error.rs:551
5: core::result::impl$27::from_residual<tuple$<>,std::io::error::Error,anyhow::Error>
at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962\library\core\src\result.rs:1962
6: zellij_server::os_input_output::impl$2::kill
at .\zellij-server\src\os_input_output.rs:1023
7: zellij_server::pty::impl$1::close_pane::async_block$1
at .\zellij-server\src\pty.rs:1743
8: async_std::task::builder::impl$1::poll::closure$0<enum2$<zellij_server::pty::impl$1::close_pane::async_block_env$1>>
at E:\.cargo\registry\src\index.crates.io-6f17d22bba15001f\async-std-1.12.0\src\task\builder.rs:199
9: async_std::task::task_locals_wrapper::impl$0::set_current::closure$0<async_std::task::builder::impl$1::poll::closure_env$0<enum2$<zellij_server::pty::impl$1::close_pane::async_block_env$1>>,enum2$<core::task::poll::Poll<tuple$<> > > >
at E:\.cargo\registry\src\index.crates.io-6f17d22bba15001f\async-std-1.12.0\src\task\task_locals_wrapper.rs:60
10: std::thread::local::LocalKey<core::cell::Cell<ptr_const$<async_std::task::task_locals_wrapper::TaskLocalsWrapper> >
>::try_with<core::cell::Cell<ptr_const$<async_std::task::task_locals_wrapper::TaskLocalsWrapper> >,async_std::task::task_l
ocals_wrapper::im
at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962\library\std\src\thread\local.rs:270
11: std::thread::local::LocalKey<core::cell::Cell<ptr_const$<async_std::task::task_locals_wrapper::TaskLocalsWrapper> >
>::with<core::cell::Cell<ptr_const$<async_std::task::task_locals_wrapper::TaskLocalsWrapper> >,async_std::task::task_local
s_wrapper::impl$0
at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962\library\std\src\thread\local.rs:246
12: async_std::task::task_locals_wrapper::TaskLocalsWrapper::set_current<async_std::task::builder::impl$1::poll::closure
_env$0<enum2$<zellij_server::pty::impl$1::close_pane::async_block_env$1>
>,enum2$<core::task::poll::Poll<tuple$<> > > >
at E:\.cargo\registry\src\index.crates.io-6f17d22bba15001f\async-std-1.12.0\src\task\task_locals_wrapper.rs:5
5
13: async_std::task::builder::impl$1::poll<enum2$<zellij_server::pty::impl$1::close_pane::async_block_env$1> >
at E:\.cargo\registry\src\index.crates.io-6f17d22bba15001f\async-std-1.12.0\src\task\builder.rs:197
14: futures_lite::future::impl$12::poll<tuple$<>,async_std::task::builder::SupportTaskLocals<enum2$<zellij_server::pty::
impl$1::close_pane::async_block_env$1>
>,enum2$<async_executor::impl$5::run::async_fn$0::async_block_env$0<tuple$<>,async_std::task::builde
at E:\.cargo\registry\src\index.crates.io-6f17d22bba15001f\futures-lite-1.13.0\src\future.rs:526
15: async_executor::impl$5::run::async_fn$0<tuple$<>,async_std::task::builder::SupportTaskLocals<enum2$<zellij_server::p
ty::impl$1::close_pane::async_block_env$1> > >
at E:\.cargo\registry\src\index.crates.io-6f17d22bba15001f\async-executor-1.6.0\src\lib.rs:250
error: process didn't exit successfully: `target\debug\zellij.exe` (exit code: 1)
Thanks to @jakob-ledermann Ctrl+C is now correctly handled and panes close correctly. The main "killer" of usefulness now is the inability to detach and reattach to sessions. At the moment it says sessions are detaching successfully, but when trying to reattach no active sessions are found.