Make it possible to refer to github-style URLs.

Make it possible to refer to github-style URLs.

BUG=362248
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=263252

[bradn, 2014-04-10 22:19:27]

https://codereview.chromium.org/233333003/diff/1/gclient.py
File gclient.py (right):

https://codereview.chromium.org/233333003/diff/1/gclient.py#newcode403
gclient.py:403: if url.startswith('git@'):
Move this down to where url is known to be a string.
Since this could be any username, may check against:
re.match('^[a-z]+[@].*', url) instead?

https://codereview.chromium.org/233333003/diff/1/gclient.py#newcode439
gclient.py:439: parsed_url = urlparse.urlparse(url)
in here.

https://codereview.chromium.org/233333003/diff/1/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/233333003/diff/1/gclient_utils.py#newcode78
gclient_utils.py:78: components = url.split('@', 1)
Couldn't you just change this to use rsplit instead?

https://codereview.chromium.org/233333003/diff/1/gclient_utils.py#newcode79
gclient_utils.py:79: if url.startswith('git@'):
Technically can't these be any username?
Though with the rsplit change you might not need to check for this explicitly.

[Sheridan Rawlins, 2014-04-11 01:05:45]

https://codereview.chromium.org/233333003/diff/1/gclient.py
File gclient.py (right):

https://codereview.chromium.org/233333003/diff/1/gclient.py#newcode403
gclient.py:403: if url.startswith('git@'):
Ah, too true -
https://www.kernel.org/pub/software/scm/git/docs/git-clone.html#_git_urls_a_i...
describes it as a "scp-like syntax".

On 2014/04/10 22:19:27, bradn wrote:
> Move this down to where url is known to be a string.
> Since this could be any username, may check against:
> mailto:re.match(, url) instead?

https://codereview.chromium.org/233333003/diff/1/gclient.py#newcode439
gclient.py:439: parsed_url = urlparse.urlparse(url)
On 2014/04/10 22:19:27, bradn wrote:
> in here.

Done.

https://codereview.chromium.org/233333003/diff/1/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/233333003/diff/1/gclient_utils.py#newcode78
gclient_utils.py:78: components = url.split('@', 1)
On 2014/04/10 22:19:27, bradn wrote:
> Couldn't you just change this to use rsplit instead?

Well, the code handles urls that don't lock the DEPS to an exact version; how
'bout this instead.

https://codereview.chromium.org/233333003/diff/1/gclient_utils.py#newcode79
gclient_utils.py:79: if url.startswith('git@'):
On 2014/04/10 22:19:27, bradn wrote:
> Technically can't these be any username?
> Though with the rsplit change you might not need to check for this explicitly.

Yes; Done.

[bradn, 2014-04-11 01:49:41]

https://codereview.chromium.org/233333003/diff/20001/gclient.py
File gclient.py (right):

https://codereview.chromium.org/233333003/diff/20001/gclient.py#newcode438
gclient.py:438: if not parsed_url[0] and not
re.match(r'^\w+\@[\w\.-]+\:[\w\/]+', parsed_url[2]):
>80 split with:
if (...
   ...):

https://codereview.chromium.org/233333003/diff/20001/gclient.py#newcode439
gclient.py:439: print 'match = %s' % re.match(r'^\w+\@\w+\:[\w\/]+',
parsed_url[2])
drop leftover print

https://codereview.chromium.org/233333003/diff/20001/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/233333003/diff/20001/gclient_utils.py#newcode80
gclient_utils.py:80: components = ['@'.join(components[:2])] +
components[1].split('@', 1)
Not sure that works.

Isn't more like (ie if its a git@.. url and the split was on the only @ sign,
then the @ wasn't for a revision so put it back together?):

if re.match(r'^\w+\@', url):
  if '@' not in components[0]:
    components = ['@'.join(components)]

[Sheridan Rawlins, 2014-04-11 04:15:16]

https://codereview.chromium.org/233333003/diff/20001/gclient.py
File gclient.py (right):

https://codereview.chromium.org/233333003/diff/20001/gclient.py#newcode438
gclient.py:438: if not parsed_url[0] and not
re.match(r'^\w+\@[\w\.-]+\:[\w\/]+', parsed_url[2]):
On 2014/04/11 01:49:41, bradn wrote:
> >80 split with:
> if (...
>    ...):

Done.

https://codereview.chromium.org/233333003/diff/20001/gclient.py#newcode439
gclient.py:439: print 'match = %s' % re.match(r'^\w+\@\w+\:[\w\/]+',
parsed_url[2])
On 2014/04/11 01:49:41, bradn wrote:
> drop leftover print

Done.

https://codereview.chromium.org/233333003/diff/20001/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/233333003/diff/20001/gclient_utils.py#newcode80
gclient_utils.py:80: components = ['@'.join(components[:2])] +
components[1].split('@', 1)
I should have started by tweaking the unit tests.  Thanks, done & verified.

On 2014/04/11 01:49:41, bradn wrote:
> Not sure that works.
> 
> Isn't more like (ie if its a git@.. url and the split was on the only @ sign,
> then the @ wasn't for a revision so put it back together?):
> 
> if re.match(r'^\w+\@', url):
>   if '@' not in components[0]:
>     components = mailto:[

[bradn, 2014-04-11 04:48:48]

lgtm

https://codereview.chromium.org/233333003/diff/40001/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/233333003/diff/40001/gclient_utils.py#newcode80
gclient_utils.py:80: components = [url]
ah, even better!

https://codereview.chromium.org/233333003/diff/40001/tests/gclient_utils_test.py
File tests/gclient_utils_test.py (right):

https://codereview.chromium.org/233333003/diff/40001/tests/gclient_utils_test...
tests/gclient_utils_test.py:127: url = "git@github.com:dart-lang/spark.git"
Oh yeah, tests :-)

[Sheridan Rawlins, 2014-04-11 15:49:02]

The CQ bit was checked by scr@chromium.org

[commit-bot: I haz the power, 2014-04-11 15:49:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scr@chromium.org/233333003/60001

[Sheridan Rawlins, 2014-04-11 15:49:17]

https://codereview.chromium.org/233333003/diff/40001/tests/gclient_utils_test.py
File tests/gclient_utils_test.py (right):

https://codereview.chromium.org/233333003/diff/40001/tests/gclient_utils_test...
tests/gclient_utils_test.py:127: url = "git@github.com:dart-lang/spark.git"
On 2014/04/11 04:48:48, bradn wrote:
> Oh yeah, tests :-)

Added one more for the gclient.py change.

[commit-bot: I haz the power, 2014-04-11 15:51:35]

Change committed as 263252