Make utility for generating git url string #23040

omus · 2017-07-30T05:55:36Z

Part of #20725.

Various parts of the callback code create URLs for displaying information to the user. The git_url function allows us to consistently create these URL strings and allows us to test behaviour.

omus · 2017-07-30T05:58:21Z

base/libgit2/callbacks.jl

@@ -224,7 +229,7 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,

    if isset(allowed_types, Cuint(Consts.CREDTYPE_USERPASS_PLAINTEXT))
        defaultcreds = reset!(UserPasswordCredentials(true), -1)
-        credid = "$schema$host"
+        credid = "$(isempty(schema) ? "ssh" : schema)://$host"


Slight change to behaviour here. It used to be that when the schema was not defined in the URL the credential ID would be "$host" now it will be "ssh://$host".

I'm not sure defaulting to ssh is any better than requiring setting a schema

Note that using the scp-like syntax [email protected]:JuliaLang/julia.git or ssh://[email protected]/JuliaLang/julia.git both refer to the same git repo using the SSH protocol.

The advantage of defaulting to using SSH here is that both of these syntaxes will refer to the same credid.

See https://git-scm.com/book/id/v2/Git-on-the-Server-The-Protocols (under "The SSH Protocol" section)

We could just assign "ssh" to be the schema when one is missing but this would then modify the git_url output from [email protected]:repo.git to ssh://[email protected]/repo.git which would no longer resemble the URL passed in by the user.

what would happen on [email protected]/JuliaLang/julia.git ?

Based upon the behavour of git clone that URL should be invalid.

right but if it gets passed here will you be prepending ssh to it?

Currently yes. I'll update the URL_REGEX to fix this.

URL_REGEX no longer allows [email protected]/JuliaLang/julia.git

yuyichao · 2017-07-30T22:14:57Z

Just want to make sure this happens after alias resolution in .gitconfig right?

omus · 2017-07-30T22:43:00Z

Just want to make sure this happens after alias resolution in .gitconfig right?

I believe so. What is your concern?

yuyichao · 2017-07-30T23:18:04Z

Just to make sure it's not broken.

tkelman · 2017-07-31T21:39:09Z

test/libgit2.jl

@@ -183,15 +183,21 @@ end

    @testset "HTTPS URL, no path" begin
        m = match(LibGit2.URL_REGEX, "https://user:[email protected]:80")
-        @test m[:path] == ""
+        @test m[:path] == nothing


tkelman · 2017-07-31T21:46:38Z

base/libgit2/utils.jl

@@ -3,7 +3,7 @@
 # Parse "GIT URLs" syntax (URLs and a scp-like syntax). For details see:
 # https://git-scm.com/docs/git-clone#_git_urls_a_id_urls_a
 const URL_REGEX = r"""
-^(?:(?<scheme>ssh|git|https?)://)?
+^(?:(?<scheme>ssh|git|https?)://)?+


what's the difference with this? regex101.com tells me it's greedy vs possessive, whether or not it "gives back" but I'm not familiar with the consequences of that

Basically when https:// is matched the regular expression will stop any backtracking where this string could be repurposed elsewhere in the regular expression:

julia> match(possessive_regex, "https://git@server:repo") julia> match(original_regex, "https://git@server:repo") RegexMatch("https://git@server:repo", scheme=nothing, user="https", password="//git", host="server", port=nothing, path="repo")

Details on possessive quantifiers: http://perldoc.perl.org/perlretut.html#Possessive-quantifiers

oh ick. that's subtle, good catch and thanks for the explanation

Git URLs like `[email protected]:repo` are valid while URLs like `[email protected]/repo` are now treated as wrong.

Previously URLs like `http://[email protected]:repo` were being parsed incorrectly.

omus · 2017-08-02T19:33:19Z

Added some documentation. Ready to merge.

tkelman · 2017-08-02T20:01:05Z

base/libgit2/utils.jl

+  * `path::AbstractString=""`: the path to specify to use in the output if provided.
+
+# Examples
+


no blank line here

Should I remove the blank line after # Keywords as well?

tkelman · 2017-08-02T20:02:22Z

base/libgit2/utils.jl

+    be specified.
+  * `port::Union{AbstractString,Integer}=""`: the port number to use in the output if
+    provided. Cannot be specified when using the scp-like syntax.
+  * `path::AbstractString=""`: the path to specify to use in the output if provided.


"to specify to use" sounds a bit redundant

Weird. I don't remember writing it like that. Will fix.

omus · 2017-08-05T00:10:37Z

I think this is ready to go. Will merge on Tuesday.

Make utility for generating git url string

8c1a730

omus added the libgit2 The libgit2 library or the LibGit2 stdlib module label Jul 30, 2017

omus commented Jul 30, 2017

View reviewed changes

tkelman reviewed Jul 31, 2017

View reviewed changes

omus added 2 commits July 31, 2017 22:37

Disallow scp-like URLs not using : as separator

7c8a3e7

Git URLs like `[email protected]:repo` are valid while URLs like `[email protected]/repo` are now treated as wrong.

Disallow HTTP URLs using : as path separator

6814e1b

Previously URLs like `http://[email protected]:repo` were being parsed incorrectly.

omus force-pushed the cv/git-url branch from 33e12d8 to 6814e1b Compare August 1, 2017 03:37

tkelman reviewed Aug 2, 2017

View reviewed changes

[ci skip] Add docstring for git_url

0b0fffc

omus force-pushed the cv/git-url branch from 4caf118 to 0b0fffc Compare August 3, 2017 10:24

omus merged commit edbcb91 into master Aug 8, 2017

omus deleted the cv/git-url branch August 8, 2017 14:40

omus mentioned this pull request Aug 8, 2017

Support using EOF char (^D) to abort credential prompt #23092

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make utility for generating git url string #23040

Make utility for generating git url string #23040

omus commented Jul 30, 2017

omus Jul 30, 2017

tkelman Jul 30, 2017

omus Jul 30, 2017

omus Jul 30, 2017 •

edited

Loading

tkelman Jul 30, 2017

omus Jul 30, 2017

tkelman Jul 30, 2017

omus Jul 30, 2017

omus Jul 31, 2017

yuyichao commented Jul 30, 2017

omus commented Jul 30, 2017

yuyichao commented Jul 30, 2017

tkelman Jul 31, 2017

tkelman Jul 31, 2017

omus Jul 31, 2017 •

edited

Loading

tkelman Jul 31, 2017

omus commented Aug 2, 2017

tkelman Aug 2, 2017

omus Aug 2, 2017

tkelman Aug 2, 2017

omus Aug 2, 2017

omus commented Aug 5, 2017

		* `path::AbstractString=""`: the path to specify to use in the output if provided.

		# Examples

Make utility for generating git url string #23040

Make utility for generating git url string #23040

Conversation

omus commented Jul 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omus Jul 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuyichao commented Jul 30, 2017

omus commented Jul 30, 2017

yuyichao commented Jul 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omus Jul 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omus commented Aug 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omus commented Aug 5, 2017

omus Jul 30, 2017 •

edited

Loading

omus Jul 31, 2017 •

edited

Loading