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.

Sign up for GitHub

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

libgit2: Add support for OpenSSH instead of libssh2 #1031

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+26 −13
Open

libgit2: Add support for OpenSSH instead of libssh2 #1031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ tempfile = "3.1.0"
[features]
unstable = []
default = ["ssh", "https", "ssh_key_from_memory"]
ssh = ["libgit2-sys/ssh"]
ssh = ["ssh-libssh2"]
ssh-libssh2 = ["libgit2-sys/ssh-libssh2"]
ssh-openssh = ["libgit2-sys/ssh-openssh"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this (and libgit2-sys/Cargo.toml) comment that the openssh support is experimental with a comment here? Perhaps something like:

Suggested change
ssh-openssh = ["libgit2-sys/ssh-openssh"]
# Support for ssh-openssh is experimental.
ssh-openssh = ["libgit2-sys/ssh-openssh"]

Also, I'm a little uncertain about the interaction if both ssh-libssh2 and ssh-openssh is set. From what I can gather, ssh-libssh2 takes precedence, is that correct? That seems like it could potentially be awkward or confusing or difficult to get it configured properly, particularly with ssh-libssh2 in the default set, and with cargo's feature unification. I suspect it won't be obvious to most people who just see this list of features in Cargo.toml.

Some options for how this could be handled:

  • Document the interaction.
  • Remove ssh from the default set, and make it more explicit.
  • Detect if both are set and raise a compile-time error.

Unfortunately none of these seem like great options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If both features are set, I think libssh2 will take precedence, because we are just adding #define instructions, and the #ifdef in the libgit2 codebase appears to always check for libssh2 first. (example).

If we do (3), my understanding is that we would need to add more environment variables similar to LIBGIT2_NO_VENDOR to allow explicit disabling of one of the features? (Correct me if I'm wrong.) This makes it seems like doing (1) + (2) would be better.

ssh_key_from_memory = ["libgit2-sys/ssh_key_from_memory"]
https = ["libgit2-sys/https", "openssl-sys", "openssl-probe"]
vendored-libgit2 = ["libgit2-sys/vendored"]
vendored-openssl = ["openssl-sys/vendored", "libgit2-sys/vendored-openssl"]
ssh_key_from_memory = ["libgit2-sys/ssh_key_from_memory"]
zlib-ng-compat = ["libgit2-sys/zlib-ng-compat"]

[workspace]
Expand Down
6 changes: 4 additions & 2 deletions libgit2-sys/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ cc = { version = "1.0.43", features = ['parallel'] }
openssl-sys = { version = "0.9.45", optional = true }

[features]
ssh = ["libssh2-sys"]
ssh = ["ssh-libssh2"]
ssh-libssh2 = ["libssh2-sys"]
ssh-openssh = []
ssh_key_from_memory = ["ssh-libssh2"]
https = ["openssl-sys"]
ssh_key_from_memory = []
vendored = []
vendored-openssl = ["openssl-sys/vendored"]
zlib-ng-compat = ["libz-sys/zlib-ng", "libssh2-sys?/zlib-ng-compat"]
17 changes: 13 additions & 4 deletions libgit2-sys/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ fn try_system_libgit2() -> Result<pkg_config::Library, pkg_config::Error> {

fn main() {
let https = env::var("CARGO_FEATURE_HTTPS").is_ok();
let ssh = env::var("CARGO_FEATURE_SSH").is_ok();
let ssh_libssh2 = env::var("CARGO_FEATURE_SSH_LIBSSH2").is_ok();
let ssh_libssh2_key_from_memory = env::var("CARGO_FEATURE_SSH_KEY_FROM_MEMORY").is_ok();
let ssh_openssh = env::var("CARGO_FEATURE_SSH_OPENSSH").is_ok();
let vendored = env::var("CARGO_FEATURE_VENDORED").is_ok();
let zlib_ng_compat = env::var("CARGO_FEATURE_ZLIB_NG_COMPAT").is_ok();

Expand Down Expand Up @@ -176,13 +178,20 @@ The build is now aborting. To disable, unset the variable or use `LIBGIT2_NO_VEN
features.push_str("#define GIT_ARCH_64 1\n");
}

if ssh {
if ssh_openssh || ssh_libssh2 {
if let Some(path) = env::var_os("DEP_SSH2_INCLUDE") {
cfg.include(path);
}
features.push_str("#define GIT_SSH 1\n");
features.push_str("#define GIT_SSH_LIBSSH2 1\n");
features.push_str("#define GIT_SSH_LIBSSH2_MEMORY_CREDENTIALS 1\n");
if ssh_openssh {
features.push_str("#define GIT_SSH_EXEC 1\n");
}
if ssh_libssh2 {
features.push_str("#define GIT_SSH_LIBSSH2 1\n");
if ssh_libssh2_key_from_memory {
features.push_str("#define GIT_SSH_LIBSSH2_MEMORY_CREDENTIALS 1\n");
}
}
}
if https {
features.push_str("#define GIT_HTTPS 1\n");
Expand Down
6 changes: 3 additions & 3 deletions libgit2-sys/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
extern crate libz_sys as libz;

use libc::{c_char, c_int, c_uchar, c_uint, c_void, size_t};
#[cfg(feature = "ssh")]
#[cfg(feature = "ssh-libssh2")]
use libssh2_sys as libssh2;
use std::ffi::CStr;

Expand Down Expand Up @@ -4334,12 +4334,12 @@ pub fn openssl_init() {
#[doc(hidden)]
pub fn openssl_init() {}

#[cfg(feature = "ssh")]
#[cfg(feature = "ssh-libssh2")]
fn ssh_init() {
libssh2::init();
}

#[cfg(not(feature = "ssh"))]
#[cfg(not(feature = "ssh-libssh2"))]
fn ssh_init() {}

#[doc(hidden)]
Expand Down
2 changes: 1 addition & 1 deletion src/cred.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ echo password=$2
}

#[test]
#[cfg(feature = "ssh")]
#[cfg(feature = "ssh-libssh2")]
fn ssh_key_from_memory() {
let cred = Cred::ssh_key_from_memory(
"test",
Expand Down
2 changes: 1 addition & 1 deletion systest/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ build = "build.rs"
edition = "2018"

[dependencies]
libgit2-sys = { path = "../libgit2-sys", features = ['https', 'ssh'] }
libgit2-sys = { path = "../libgit2-sys", features = ['https', 'ssh-libssh2'] }
libc = "0.2"

[build-dependencies]
Expand Down