-
Notifications
You must be signed in to change notification settings - Fork 743
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
Multiple network stream support #986
base: master
Are you sure you want to change the base?
Conversation
Can someome @mobile-shell review this please? @cgull ? |
This didn’t pass the automated CI test, because we still support platforms with poor or missing C++11 support (that may change after this release, but for now it’s a requirement). When building on a C++11 system, |
@andersk I realize the tests are failing, just wanted someone to give a thumbs up/thumbs down whether this approach would ever be accepted before fixing it up. Does it need to be backwards compatible between versions? I can probably hack some support in if necessary. |
We haven't broken compatibility on the wire protocol since early 2012 and I'm not too enthusiastic to do it now -- so yeah, any protocol design here would probably need to be backwards compatible (or bundled with some huge major rev). You've probably seen the amount of anger we induced by turning down a backwards-compatible protocol change that added support for ssh-agent forwarding, so I just want to be upfront that we're unlikely to take major protocol changes to the Mosh/SSP protocol that haven't been discussed and reached consensus on the mailing lists. |
Keith, thanks for your answer. I've quickly scanned the mosh-devel ML archives up to the beginning of 2017 but couldn't easily find that thread/discussion... can you brief us on what would be an acceptable patch would look like if even backwards-compatible changes are (routinely?) rejected? I know that the stakes are high for such a popular, stable and great project, but I just want to manage expectations on whether we'll likely see this feature landing the main codebase at all or we shouldn't even be trying at this point? Perhaps there's some major rearchitecting going on for future major releases and this feature makes more sense there instead of trying to merge it now? |
Hello -- I appreciate your candor here. I was thinking of the discussions around #120 (and on the mailing list, https://www.mail-archive.com/[email protected]/msg00980.html). I think something like this could be part of a "mosh v2" (and I also have my ideas about what might be nice to have in "mosh v2"), but we honestly haven't even started the process of talking about what might be in scope for that. Probably we might start by brainstorming what we really want to do here, and in what dimensions we want to improve on mosh v1, and then build the protocol around that. That would be my druthers for the way to go, but it's a big effort not yet begun... It's not like we've started to rearchitect. |
It's almost a year later now, is there a mosh v2 in sight where this feature could be included, @keithw? The bounty for this feature getting implemented keeps increasing: #337 (comment) :) |
This adds the basic support for multiple network streams via a
Network::MultiplexerStream
. It aggregates the diffs for multiple child streams into one.This is intermediate work towards implementing #337.