Windows Support

[TyPR124]

This is not quite ready yet, but far enough along that I can take feedback. I can answer questions and get into more detail later, but I want to get this posted so will keep this initial post shorter.

For now, I've pulled out all PtyProcess into a separate library, https://github.com/TyPR124/ptyproc

This code could be moved back into rexpect, or added back into this repo as a separate crate. I don't feel the need to "own" the code (but am willing to) and the repo above is not a published crate as of now.

To summarize the changes to PtyProcess, besides adding Windows support, the previous unix-specifc methods have been replaced with platform-independent methods that have the same functionality as their std counterparts (see: Child). Also of note, "set_kill_timeout" has been changed to "set_drop_timeout" and exactly what this means is explained on that methods docs.

Many of the existing tests are unix-specific, but the ones that aren't are all passing as well as a couple I added specifically for Windows. I do not consider this comprehensively tested yet and would greatly appreciate any more "real world"-y tests you could put it through (specifically talking about the Linux/Mac targets as I doubt you have anything built for Windows).

[philippkeller]

thanks for posting this, I didn't have yet a look at the code, the plan is to merge it into the new exp-and-windows branch. Now I already merged @zhiburt s changes in and now it has a merge conflict. Could you resolve these conflicts, or would this be easier for @zhiburt ?

About your question about moving the pty part into a different crate. Good that you bring this up, I was thinking that maybe the reader part could live in a separate repo as well, especially since @zhiburt made it quite a bit more "general" so it also could be used for other projects.

But then again I think the repo is not big enough to justify to move it to separate repos. Also I fear that it would be "drifting apart" and making it less stable.

Also maybe you and @zhiburt would like to be contributors to this github project so you could "own" your code better, especially since I was not using Rust too much in the past 2 years.

These are loose thoughts and it's maybe a bit hard to discuss this here in a github thread, so maybe we could open a chat in Telegram or maybe you know a good way to have these discussions?

[TyPR124]

Merge conflicts are resolved.

As far as separating things into their own crates, in my mind it makes sense to separate out PtyProcess from everything else, which would make ptyproc the crate for spawning PTY-enabled processes, and rexpect the crate for matching output of PTY-enabled streams. That also allows the majority of OS-specific code to be outside of rexpect, and puts all unsafe code outside of rexpect. That said, I will accept whatever is decided.

I would be okay with helping to maintain this crate/repo either way.

There is still a good amount of cleanup I need to do, as well as some things that could be done after the merge like adding spawn_cmd and/or spawn_powershell functions (and possibly making spawn_bash work on Windows via WSL). These would all be additions rather than breaking existing stuff.

As a final note for now, the method I'm using for Windows is only supported on Windows 10 (according to Microsoft, I haven't tested it myself yet on anything other than Win10) so that may be something to consider whether it is worth adding this specifically for Windows 10 support. (And assuming that isn't a deal-breaker, I will make sure it is documented in various places).

[philippkeller]

Thanks for resolving, I've now merged it into the exp-and-windows branch. I like your reasoning about the separate crate. Especially since a standalone PTY crate is useful for other use cases as well. I'll give myself some days to think about this but I'm not opposed to it :-)

I started looking through your code but it's a bit hard since I don't have a diff at hand. I guess it's best when I diff the old process.rs with your unix.rs, right? I'm a bit confused since e.g. I didn't find kill with a signal. Also: I see that you removed some of the comment, e.g. in process.rs there was a lengthy example at the beginning on how to use the module. Are you planning to move this into the documentation of PtyProcess?

[TyPR124]

Unix.rs will be the closest file to what was the old process.rs, however not much of it has changed.

The function spawn_pty is about the only part that will contain code from the old process.rs.

The remaining methods on PtyProcess no longer take a signal, as that is a concept that doesn't exist in Windows. So instead I opted to use the methods found on the standard libraries Child type, but also added the "try_wait_timeout" method to help keep consistent behavior with the old process.rs.

I can add methods that take a signal parameter, but they would be behind a unix-specific trait (similar to any extension trait in std::os::unix).

And comments/documentation/examples basically all need done still, which I plan on getting done this weekend (as well as any other changes that stem from this discussion).

[HikingDev]

Are there any plans to integrate Windows support into master? I see that exp-and-windows branch falls behind.

[souze]

At least I have no intention to merge the two branches at the moment. It looks like they have diverged a fair bit.

But if anyone creates a PR rebasing the important bits onto master, I'll have a go at reviewing/merging it.

Unfortunate that it was handled with a separate branch, and that a new group of maintainers was added who don't have the history.