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

libguile-ssh/session-func.c: Add NODELAY option #21

Merged
merged 2 commits into from
Jul 4, 2020
Merged

libguile-ssh/session-func.c: Add NODELAY option #21

merged 2 commits into from
Jul 4, 2020

Conversation

PromyLOPh
Copy link
Contributor

@PromyLOPh PromyLOPh commented Jul 2, 2020

This pull request adds the nodelay option libssh provides. It is intended to be used by guix, which currently opens its own socket and manually sets NODELAY. See https://issues.guix.gnu.org/41702#11

The Guile 2.2 build pipeline fails, because it’s using a really old version of libssh (0.8.0~20170825.94fa1e38-1ubuntu0.6). NODELAY was introduced in 2018 by https://git.libssh.org/projects/libssh.git/commit/?id=be22c0d442a1c5c016e2ebb99075b61614b5b447

* libguile-ssh/session-func.c (session_options, set_option): Add libssh’s
  nodelay option.
* tests/session.scm ("session-set!, valid values", "session-set!, invalid
  values"): Add unit-tests.
* modules/ssh/session.scm (make-session): Add key nodelay
@artyom-poptsov artyom-poptsov self-assigned this Jul 2, 2020
@artyom-poptsov
Copy link
Owner

Thanks for the patch!

There is a problem in Ubuntu 18.04 LTS: it ships with libssh version 0.8.0~20170825.94fa1e38-1ubuntu0.6, which is actually libssh 0.7. I tested it in a Docker container with Ubuntu GNU/Linux 18.04:

$ apt show libssh-dev 2>/dev/null | grep Version
Version: 0.8.0~20170825.94fa1e38-1ubuntu0.6
$ guile -c "(use-modules (ssh version)) (display (get-libssh-version)) (newline)"
0.7.0

Don't know for sure why the Ubuntu people decided to trick pkg-config into thinking that it's 0.8. But Guile-SSH will fail with your patch on this distribution, because SSH_OPTIONS_NODELAY appears only in actual libssh 0.8 (not the fake one 0.8 that is in Ubuntu 18.04.) That's the reason the tests fail here.

To fix that we need to check for libssh 0.8.1+ in configure.ac to make sure we have valid 0.8 -- that's the simplest solution (0.8.1 is quite old already so the most users will be okay with that I think.) Let me know if you come up with a better solution.

@artyom-poptsov artyom-poptsov merged commit b1cfee2 into artyom-poptsov:master Jul 4, 2020
@PromyLOPh
Copy link
Contributor Author

Not ideal but the best solution I can think of right now as well, thanks for merging! Would it be possible to create a new release of guile-ssh that we can then package and use?

@artyom-poptsov
Copy link
Owner

Sure. I'll see if I can fix some random test failures with Guile 2.2 that sometimes occur, and then I'll prepare a new release.

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

2 participants