Closed Fusion closed 2 years ago
Thanks for the PR, ill add some comments to the change files - and extra change needed: add windows to the CI process to test this (.github/workflows)
Also, in addition to adding windows, can you please bump up the test matrix to go 1.15/16/17?
@jpillora I've made a few modifications as per your comments. I think all that's missing is for goreleaser to actually release, which I could add in another PR. Please take a look at my recent modification to server.go, where I handle Windows session^H^H^H^H^H^Hshell exit.
Thanks Chris! Will try review tonight
On Thu, 30 Sep 2021 at 11:56 am Chris F Ravenscroft < @.***> wrote:
@jpillora https://github.com/jpillora I've made a few modifications as per your comments. I think all that's missing is for goreleaser to actually release, which I could add in another PR. Please take a look at my recent modification to server.go, where I handle Windows session^H^H^H^H^H^Hshell exit.
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/jpillora/sshd-lite/pull/1#issuecomment-930685859, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE2X4ZYUPEZO3LUSC5MAQ3UEO7UHANCNFSM5E4WUIHA .
Sorry, I should have been clearer here https://github.com/jpillora/sshd-lite/pull/1#discussion_r717359252
Can we put the fork of pty
in a separate repo and then depend on this forked repo using go.mod/sum
? That way we don't need to pull in a local copy
You know, I was not all that comfortable with "swallowing" that repo whole so I like this idea much better! This being said, since this project is not under the umbrella of an organization, I would worry that a separate repo could disappear just as well as the current fork. Maybe you could create this repo yourself so that both repos belong to the same account, and I would then reference the second repo as a submodule?
Disappearing repos a danger for all go modules - such is life 😄
On Fri, 1 Oct 2021 at 7:00 am Chris F Ravenscroft @.***> wrote:
You know, I was not all that comfortable with "swallowing" that repo whole so I like this idea much better! This being said, since this project is not under the umbrella of an organization, I would worry that a separate repo could disappear just as well as the current fork. Maybe you could create this repo yourself so that both repos belong to the same account, and I would then reference the second repo as a submodule?
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/jpillora/sshd-lite/pull/1#issuecomment-931685782, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE2X47E2TIZQCAHS3J4WELUETFXLANCNFSM5E4WUIHA .
Most important thing is that it can’t be secretly made malicious - and go.sum protects us from this
On Fri, 1 Oct 2021 at 7:00 am Chris F Ravenscroft @.***> wrote:
You know, I was not all that comfortable with "swallowing" that repo whole so I like this idea much better! This being said, since this project is not under the umbrella of an organization, I would worry that a separate repo could disappear just as well as the current fork. Maybe you could create this repo yourself so that both repos belong to the same account, and I would then reference the second repo as a submodule?
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/jpillora/sshd-lite/pull/1#issuecomment-931685782, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE2X47E2TIZQCAHS3J4WELUETFXLANCNFSM5E4WUIHA .
Sorry forgot to answer your actual question - let’s use the repo from https://github.com/creack/pty/pull/109 can directly use it directly + we’ll need a mod replace ( https://stackoverflow.com/questions/56671133/how-to-use-a-forked-module-with-versioned-go-modules-v1-11-go111module-on ) to allow it to compile
On Fri, 1 Oct 2021 at 7:00 am Chris F Ravenscroft @.***> wrote:
You know, I was not all that comfortable with "swallowing" that repo whole so I like this idea much better! This being said, since this project is not under the umbrella of an organization, I would worry that a separate repo could disappear just as well as the current fork. Maybe you could create this repo yourself so that both repos belong to the same account, and I would then reference the second repo as a submodule?
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/jpillora/sshd-lite/pull/1#issuecomment-931685782, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE2X47E2TIZQCAHS3J4WELUETFXLANCNFSM5E4WUIHA .
Feeling a bit dumb now. From what I can tell, I cannot use a replace
statement without a tag (form: vx.y.z) unless I am replacing using a local directory. Therefore I'm afraid I am not sure how I could use that repo?
All good, I can give it a try and report back, I think you can do go get @.***
On Fri, 1 Oct 2021 at 9:57 am Chris F Ravenscroft @.***> wrote:
Feeling a bit dumb now. From what I can tell, I cannot use a replace statement without a tag (form: vx.y.z) unless I am replacing using a local directory. Therefore I'm afraid I am not sure how I could use that repo?
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/jpillora/sshd-lite/pull/1#issuecomment-931786684, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE2X46EFQXNQJL6XEXJFW3UET2O7ANCNFSM5E4WUIHA .
Owow GitHub censored the command... trying it now...
I thought we could do
replace github.com/creack/pty => github.com/jeffreystoke/pty v0.0.0-20210929185349-8fab97d83d6f
then
$ go get -v github.com/jeffreystoke/pty@8fab97d83d6f
# but no dice
go get: github.com/jeffreystoke/pty@v1.1.12-0.20210831163441-8fab97d83d6f: parsing go.mod:
module declares its path as: github.com/creack/pty
but was required as: github.com/jeffreystoke/pty
Sorry for the run around! looks like we need to fork. Can you fork github.com/jeffreystoke/pty@8fab97d83d6f
, change the go.mod to match your user (ensure it compiles stand-alone), tag it with v1.1.13
and then we can just use that as a normal dependency
If there are future windows issues, I'll tag you and we can bump your fork - and hopefully - it gets merged downstream, then we just switch back to creack
Ok, done.
I'm going to look a bit more in depth into what goreleaser can do...
OK. goreleaser is in:make publish
Hey Chris, looks good! There is already a goreleaser file in .github/
, can we merge any changes into that one? Then we're good to go I think
A bit of context, CI action (also in .github) runs tests on every commit, and runs goreleaser (with github token for auto-releases) on every tag
OK. I hope this is what you had in mind!
Tested https://github.com/jpillora/sshd-lite/releases/tag/v1.3.0 on a Windows machine, works well, thanks Chris!
As suggested in this thread: https://gist.github.com/jpillora/b480fde82bff51a06238 it is indeed possible to support Windows.
A very interesting pull request, waiting to be integrated in the pty project (find it here: https://github.com/creack/pty/pull/109) offers seamless Windows integration.
Due to this relying on an unmerged upstream pull request, I am not really expecting you to just merge in this pull request and call it a day :)
"And what about the remaining 1%?" you may ask. Well, that remaining 1% is that, while resize et al work well, in order to exit a powershell or cmd session, you will need to hit
ENTER~.
as typingexit
in powershell does not close the SSH session. In fact, no session request is generated at this time, nor does the pty get EOF. So far, googling the issue has not helped much. I found this related issue when using Cygwin (https://stackoverflow.com/questions/42774454/how-to-exit-powershell-exe-inside-of-cmd) but no solution (I suspect the fix lies somewhere in the ConPTY API, which I am not very familiar with)