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

Feature/windows pty #93

Merged
merged 43 commits into from
Sep 6, 2021
Merged

Conversation

alexadhy
Copy link
Contributor

@alexadhy alexadhy commented Jun 16, 2021

Fixes skycoin/skywire#778

Prequisites:

Changes:

  • Support dmsgpty on windows

How to test this PR:

Run it on windows 10 / windows server 2019 with Conpty support (on by default since late 2018). No support on win server 2016 and below, neither does win 8.1 and below.

Have redis binary installed and available from your %PATH%

Currently supports running dmsgpty-cli from powershell only, Instruction:

  • build the binaries for windows via make build-windows or manually.
  • run integration test via
> Set-ExecutionPolicy RemoteSigned -Scope CurrentUser 
> make integration-windows-start

Once all those is setup, open yet another powershell tab / window and do:

> .\bin\dmsgpty-cli.exe --confpath .\integration\configs\dmsgptyhost1_windows.json

If all is well, you should be able to do local pty:

Screen.Recording.2021-06-21.at.09.52.51.mov

or dmsgpty-ui:

> .\bin\dmsgpty-ui.exe --haddr <LOCATION_OF_YOUR_SOCK_FILE> # it's in C:\Users\<YOUR_USER>\dmsgpty1.sock or C:\Users\<YOUR_USER>\dmsgpty2.sock

Screen Shot 2021-06-21 at 16 13 55

To stop the integration environment, do:

> make integration-windows-stop

TODO:

  • Create a .ps1 script for integration test on windows, the same way it does for *nix.
  • Test dmsgpty-ui
  • Test remote dmsgpty
  • Resize support, probably poll it as SIGWINCH is not available under windows.
  • Use unix socket for all platforms (sc query afunix to test your windows support)
  • Alternatively, use TCP conn
  • Make sure that config is as unambiguous as possible and provide sane default for testing

Edit:

  • Add ExecutionPolicy set command, thanks @ersonp

Copy link
Contributor

@ersonp ersonp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dmsgpty is working on win 10

@ersonp
Copy link
Contributor

ersonp commented Jun 21, 2021

Everything is working on cmd , but the make integration-windows-start on powershell gives the following error.

.\integration\integration.ps1 : File C:\Users\Erson\Desktop\work\Go\dmsg\integration\integration.ps1 cannot be loaded
because running scripts is disabled on this system. For more information, see about_Execution_Policies at
https:/go.microsoft.com/fwlink/?LinkID=135170.
At line:1 char:1
+ .\integration\integration.ps1 start
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : SecurityError: (:) [], PSSecurityException
    + FullyQualifiedErrorId : UnauthorizedAccess
make: *** [Makefile:126: integration-windows-start] Error 1

@alexadhy alexadhy marked this pull request as ready for review June 21, 2021 09:14
@jdknives jdknives changed the title [WIP] Feature/windows pty Feature/windows pty Jun 21, 2021
@alexadhy
Copy link
Contributor Author

Everything is working on cmd , but the make integration-windows-start on powershell gives the following error.

.\integration\integration.ps1 : File C:\Users\Erson\Desktop\work\Go\dmsg\integration\integration.ps1 cannot be loaded
because running scripts is disabled on this system. For more information, see about_Execution_Policies at
https:/go.microsoft.com/fwlink/?LinkID=135170.
At line:1 char:1
+ .\integration\integration.ps1 start
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : SecurityError: (:) [], PSSecurityException
    + FullyQualifiedErrorId : UnauthorizedAccess
make: *** [Makefile:126: integration-windows-start] Error 1

This one is solved via Set-ExecutionPolicy RemoteSigned -Scope CurrentUser (already added to the topmost comment)

@alexadhy alexadhy mentioned this pull request Jun 22, 2021
2 tasks
@ersonp
Copy link
Contributor

ersonp commented Jun 28, 2021

logs.txt is the log of dmsgpty-host-1 generated from make integration-windows-start.
logs2.txt is the log of .\bin\dmsgpty-cli.exe --confpath .\integration\configs\dmsgptyhost1.json. Also the config file specified does not exist so i used this instead .\bin\dmsgpty-cli.exe --confpath .\integration\configs\dmsgptyhost1_windows.json

@alexadhy alexadhy mentioned this pull request Aug 5, 2021
@ersonp
Copy link
Contributor

ersonp commented Sep 3, 2021

dmsg-server seems to be having some issue
dmsg-discovery.txt
dmsg-server1.txt
dmsg-server2.txt
dmsg-host1.txt
dmsg-host2.txt
dmsg-pty-ui.txt

The hosts are stuck in a loop without a timeout.

Also in this make integration-windows-stop are the logs being printed twice or are we killing the services twice?

powershell -Command .\integration\integration.ps1 stop
Killing @{Name=Redis; Pid=4416}.Name
Killing @{Name=Redis; Pid=4416}.Name
Killing @{Name=Dmsg-Discovery; Pid=1884}.Name
Killing @{Name=Dmsg-Discovery; Pid=1884}.Name
Killing @{Name=Dmsg-Server1; Pid=9884}.Name
Killing @{Name=Dmsg-Server1; Pid=9884}.Name
Killing @{Name=Dmsg-Server2; Pid=9992}.Name
Killing @{Name=Dmsg-Server2; Pid=9992}.Name
Killing @{Name=DmsgPty-Host1; Pid=132}.Name
Killing @{Name=DmsgPty-Host1; Pid=132}.Name
Killing @{Name=DmsgPty-Host2; Pid=8396}.Name
Killing @{Name=DmsgPty-Host2; Pid=8396}.Name

@alexadhy
Copy link
Contributor Author

alexadhy commented Sep 4, 2021

dmsg-server seems to be having some issue
dmsg-discovery.txt
dmsg-server1.txt
dmsg-server2.txt
dmsg-host1.txt
dmsg-host2.txt
dmsg-pty-ui.txt

The hosts are stuck in a loop without a timeout.

Also in this make integration-windows-stop are the logs being printed twice or are we killing the services twice?

powershell -Command .\integration\integration.ps1 stop
Killing @{Name=Redis; Pid=4416}.Name
Killing @{Name=Redis; Pid=4416}.Name
Killing @{Name=Dmsg-Discovery; Pid=1884}.Name
Killing @{Name=Dmsg-Discovery; Pid=1884}.Name
Killing @{Name=Dmsg-Server1; Pid=9884}.Name
Killing @{Name=Dmsg-Server1; Pid=9884}.Name
Killing @{Name=Dmsg-Server2; Pid=9992}.Name
Killing @{Name=Dmsg-Server2; Pid=9992}.Name
Killing @{Name=DmsgPty-Host1; Pid=132}.Name
Killing @{Name=DmsgPty-Host1; Pid=132}.Name
Killing @{Name=DmsgPty-Host2; Pid=8396}.Name
Killing @{Name=DmsgPty-Host2; Pid=8396}.Name

Addressed in the ceb1204

Copy link
Contributor

@ersonp ersonp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good and is working well on windows.

Copy link
Contributor

@mrpalide mrpalide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Everything work as well as possible.

@jdknives jdknives merged commit 6a38c7f into skycoin:develop Sep 6, 2021
@alexadhy alexadhy deleted the feature/windows-pty branch September 8, 2021 09:45
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

Successfully merging this pull request may close these issues.

None yet

5 participants