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

Add the ssh_key_from_memory optional feature. #331

Merged
merged 2 commits into from
Jul 26, 2018

Conversation

JMLX42
Copy link
Contributor

@JMLX42 JMLX42 commented Jul 16, 2018

Fix for https://github.com/alexcrichton/git2-rs/issues/320

This is done through a feature that will enable the GIT_SSH_MEMORY_CREDENTIALS flag when building libgit2-sys.

This feature is optional so that the current behavior is unchanged.

@alexcrichton
Copy link
Member

Thanks! It looks like though this is defined by libgit2's build system if it detects a symbol in libssh2, and maybe the detection is broken? Are you sure the libssh2 build you're using has the libssh2_userauth_publickey_frommemory symbol?

@JMLX42
Copy link
Contributor Author

JMLX42 commented Jul 16, 2018

Are you sure the libssh2 build you're using has the libssh2_userauth_publickey_frommemory symbol?

When I enable the feature, everything works as expected.
So I'm guessing it does, yes.

@alexcrichton
Copy link
Member

In that case perhaps this should be enabled unconditionally? Assuming that we put it on ourselves to find an appropriate libssh2 build?

@JMLX42
Copy link
Contributor Author

JMLX42 commented Jul 17, 2018

In that case perhaps this should be enabled unconditionally? Assuming that we put it on ourselves to find an appropriate libssh2 build?

Maybe. I made it optional because I don't know what would happen if someone uses it with a system libssh2 that does not support it.

In my case I'm also using the ssh2 crate. So I'm guessing the system's libssh2 is not used when I use git2-rs?

@alexcrichton
Copy link
Member

Hm yeah the API there seems to have been added 4 years ago, so it seems fine to unconditionally enable this.

@JMLX42
Copy link
Contributor Author

JMLX42 commented Jul 17, 2018

Hm yeah the API there seems to have been added 4 years ago, so it seems fine to unconditionally enable this.

Does that mean I remove the cargo "feature" thing, or make it enabled by default?

@alexcrichton
Copy link
Member

Indeed!

Users with older versions of libssh can still disable it if needed.
@JMLX42
Copy link
Contributor Author

JMLX42 commented Jul 26, 2018

@alexcrichton Here you go!

@alexcrichton alexcrichton merged commit 8ca0e92 into rust-lang:master Jul 26, 2018
@alexcrichton
Copy link
Member

Thanks!

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.

2 participants