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 failing test for SSH clone #478

Closed
wants to merge 5 commits into from
Closed

Conversation

pietbrauer
Copy link
Member

Cloning with an ssh URL results in error Unsupported URL protocol which is the result of a libgit2 check if SSH is enabled. It seems that SSH is not enabled for objective-git, although the GIT_SSH flag is set.

Is this the expected behaviour?

Cloning with an ssh URL results in error
"Unsupported URL protocol" which is the result of a libgit2 check if SSH
is not enabled.
@phatblat
Copy link
Member

phatblat commented Jul 6, 2015

SSH used to work, so this is a regression 😦

While I like the idea of having a test like this, there are several reasons it could fail which have nothing to do with the framework. Also, the build server may not even be set up with an SSH key. Perhaps this test could be disabled by default and then manually enabled - or perhaps the logic depends on an environment variable to be present.

I think a better permanent test would be to assert that git_libgit2_features returns with the GIT_FEATURE_SSH bit set.

@pietbrauer
Copy link
Member Author

👍 Will rewrite it then and find a test file where it suites in.

@phatblat
Copy link
Member

phatblat commented Jul 6, 2015

You can disable the test with xdescribe or xit. That test will be a great help in verifying a fix for the SSH functionality (which I suspect is build related).

@pietbrauer
Copy link
Member Author

Yeah, it seems that GIT_SSH is not set when script/update_libgit2 is being built. Too I am lacking a ton of cmake knowledge.

@phatblat
Copy link
Member

phatblat commented Jul 6, 2015

I've worked with the build scripts and cmake a bit so I can look into this issue.

@pietbrauer
Copy link
Member Author

I would also love to work on this but have little hope. I tried adding -DCMAKE_C_FLAGS="${CMAKE_C_FLAGS} -DGIT_SSH" but that did not help.

@phatblat phatblat self-assigned this Jul 6, 2015
@phatblat
Copy link
Member

phatblat commented Jul 6, 2015

The libgit2 cmake build will disable SSH support if it doesn't find the libraries, which may be what's going on here. Are you seeing this when building for Mac, iOS or both?

@pietbrauer
Copy link
Member Author

Both as Mac is the test target and my iOS app also fails. Afaik the Mac build uses system OpenSSH and the iOS app depends on the OpenSSH iOS build.

@pietbrauer pietbrauer assigned pietbrauer and unassigned phatblat Jul 7, 2015
@pietbrauer
Copy link
Member Author

@phatblat I fixed the build and enabled SSH support. I also refactored the test to test for the correct error (SSH credentials not there).

@phatblat
Copy link
Member

phatblat commented Jul 7, 2015

Fantastic! 🎈 I'll test this out later this evening. I have an idea for getting those SSH credentials working.

@pietbrauer
Copy link
Member Author

Superb!! 🎉

On Tuesday, July 7, 2015, Ben Chatelain [email protected] wrote:

Fantastic! [image: 🎈] I'll test this out later this evening. I
have an idea for getting those SSH credentials working.


Reply to this email directly or view it on GitHub
#478 (comment)
.

@pietbrauer
Copy link
Member Author

@phatblat Did you have any chance to get to the tests or maybe share your approach?

@phatblat
Copy link
Member

Sorry, got tied up the last couple evenings.

I started adding some logic to pick up the SSH keys from ~/.ssh, but then I noticed that there is already an SSH clone test. This test doesn't run unless you have the GTUserName, GTPublicKey, GTPrivateKey and GTPrivateKeyPassword defined in the current environment.

screen shot 2015-07-09 at 6 56 33 pm

I presume these values are defined on the GitHub build server. I defined them as above and ran this test on my Mac. So, SSH is working fine on the Mac side - without these changes.

@phatblat
Copy link
Member

It's been challenging validating this change on iOS as my client app builds with Xcode 7 but I'm not able to build ObjectiveGit from source with 7. However, I built this branch with 6.4 and copied over the .framework and the GIT_FEATURE_SSH flag is coming back set from git_libgit2_features.

This looks like progress, but I'd like to do some deeper validation because this change forces GIT_SSH to be set, but that flag is supposed to automatically get set by cmake if it finds the SSH lib. I'm concerned about the lib not being found since that can cause issues when an app tries to actually use SSH.

@phatblat
Copy link
Member

I think this may be the real fix: phatblat/objective-git@aed2cc3. I misspelled LIBSSH2_INCLUDE_DIRS when I added that to the libgit2 build scripts.

@pietbrauer
Copy link
Member Author

I can verify it, my app still runs on Xcode 6.4. Will try it out today. If it fixes the issue, I will cherry pick and rebase this branch.

@phatblat
Copy link
Member

Also check out #483 to see if that works for you.

@phatblat phatblat mentioned this pull request Jul 10, 2015
@pietbrauer
Copy link
Member Author

Closing in favour of #483

@pietbrauer pietbrauer closed this Jul 10, 2015
@pietbrauer pietbrauer deleted the failing_clone_via_ssh branch July 10, 2015 14:07
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