Skip to content

Use libgit2 fork to fix ssh url detection - #841

Closed
johnhaley81 wants to merge 1 commit into
masterfrom
fix-ssh-urls
Closed

Use libgit2 fork to fix ssh url detection#841
johnhaley81 wants to merge 1 commit into
masterfrom
fix-ssh-urls

Conversation

@johnhaley81

Copy link
Copy Markdown
Collaborator

This should add support for the ssh+git:// and git+ssh:// protocol schemes.

This should add support for the ssh+git:// and git+ssh:// protocol schemes.
@johnhaley81

Copy link
Copy Markdown
Collaborator Author

I don't want this merged until we have libgit2/libgit2#3555 merged. I definitely don't want to deviate from libgit2 and only want to pull this in as an advanced copy of the fix.

@maxkorp

maxkorp commented Jan 19, 2016

Copy link
Copy Markdown
Collaborator

deviate from libgit2

:shudder:

@tbranyen

tbranyen commented Mar 4, 2016

Copy link
Copy Markdown
Member

Is this PR still necessary @johnhaley81 ?

@johnhaley81

Copy link
Copy Markdown
Collaborator Author

@tbranyen so there was a bit of debate on whether or not this should be included since it's actually wrong URL formating. The issue is that somehow it got into git-core and people migrating from svn must have thought that svn+ssh carried over to git and started formatting their URL's like that. However it happened it's now a supported protocol format according to git-core so we (both libgit2 and NodeGit) have to support it as well.

I was waiting for the parent PR to get merged into libgit2 before I merged this as well since I'm trying to not maintain a deviated fork of libgit2. That just happened this morning so libgit2 now supports it although not in the latest (0.24.0) release. As far as I can tell we're good to merge this now.

@johnhaley81

Copy link
Copy Markdown
Collaborator Author

So libgit2/libgit2#3555 is merged now but I think a better course of action is to just update libgit2. I'm going to close this.

@johnhaley81 johnhaley81 closed this Apr 9, 2016
@johnhaley81
johnhaley81 deleted the fix-ssh-urls branch April 9, 2016 00:41
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