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 Cred::ssh_key_from_memory #321

Merged
merged 2 commits into from
Jul 2, 2018
Merged

Conversation

fooker
Copy link
Contributor

@fooker fooker commented Jun 19, 2018

@alexcrichton
Copy link
Member

Thanks! Should the keys though be &[u8] instead of &str?

@fooker
Copy link
Contributor Author

fooker commented Jun 27, 2018

Not really. They are in the same text format as stored in the file. It's mostly human readable.

@alexcrichton
Copy link
Member

Ok great! Could a test be added to ensure that's the case as well?

@agrinman
Copy link

@fooker I tried using this but get Error { code: -1, klass: -1, message: "this version of libgit2 was not built with ssh memory credentials.; class=Invalid (3)" }. Looks like libgit2 compilation flags needs to change as well? Not sure how to do this but can look into it.

@fooker
Copy link
Contributor Author

fooker commented Jul 2, 2018

@alexcrichton can you give me some hints how to implement such a test? I think this would involve some integration testing requiring a existing server to test against.

@agrinman I've seen the error message in several issues before. I don't have that problem on my systems. Looks like this is something platform dependent.

@fooker
Copy link
Contributor Author

fooker commented Jul 2, 2018

@agrinman look like HAVE_LIBSSH2_MEMORY_CREDENTIALS must be set while building libssh, which is an external library AFAIK.

@alexcrichton
Copy link
Member

@fooker oh I'm just thinking of calling this method and asserting it returns a success, no need to conned to a server, basically just feed it a generated SSH certificate

@alexcrichton alexcrichton merged commit 3ac7970 into rust-lang:master Jul 2, 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.

3 participants