Issue 2310 - Handle alternative syntax of Git SSH URLs

ensure_dependencies.py

 #!/usr/bin/env python
 # coding: utf-8
 
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import sys
 import os
 import posixpath
@@ skipping 92 matching lines @@
     subprocess.check_call(["git", "checkout", "--quiet", rev], cwd=repo)
 
   def ignore(self, target, repo):
     module = os.path.relpath(target, repo)
     exclude_file = os.path.join(repo, ".git", "info", "exclude")
     _ensure_line_exists(exclude_file, module)
 
   def postprocess_url(self, url):
     # Handle alternative syntax of SSH URLS
     if "@" in url and ":" in url and not urlparse.urlsplit(url).scheme:
       return "ssh://" + url.replace(":", "/", 1)

[Sebastian Noack, 2015/04/30 13:34:36]

How does git URLs specify ports?

[kzar, 2015/04/30 14:45:29]

Apparently they can't, you have to specify non-standard ports in your
`~/.ssh/config`.
http://stackoverflow.com/questions/1558719/using-a-remote-repository-with-non...

     return url
 
 repo_types = OrderedDict((
   ("hg", Mercurial()),
   ("git", Git()),
 ))
 
 def parse_spec(path, line):
   if "=" not in line:
     logging.warning("Invalid line in file %s: %s" % (path, line))
@@ skipping 65 matching lines @@
     return
 
   parenttype = get_repo_type(parentrepo)
   type = None
   for key in roots:
     if key == parenttype or (key in repo_types and type is None):
       type = key
   if type is None:
     raise Exception("No valid source found to create %s" % target)
 
-  root = repo_types[type].postprocess_url(roots[type])

[Sebastian Noack, 2015/04/30 13:34:36]

Nit, don't repeat yourself:

  postprocess_url = repo_types[type].postprocess_url

[kzar, 2015/04/30 14:45:29]

Done.

[Wladimir Palant, 2015/04/30 15:48:30]

Rather questionable advise in this context IMHO.

[Sebastian Noack, 2015/04/30 17:58:28]

How is caching one variable lookup + one item lookup + another attribute lookup
into a variable, reducing duplication, questionable?

[Wladimir Palant, 2015/04/30 18:34:07]

If it provides a negligible performance gain while reducing readability and
consistency with the surrounding code - sure, that's questionable. It's called
"premature optimization."

[Sebastian Noack, 2015/04/30 18:43:31]

I didn't even mention performance. "Don't repeat yourself" is mostly about code
readability ans maintainability. And I don't see how duplicating code helps
there.

-  sourcename = repo_types[type].postprocess_url(sourcename)
+  postprocess_url = repo_types[type].postprocess_url
+  root = postprocess_url(roots[type])
+  sourcename = postprocess_url(sourcename)
 
   if os.path.exists(root):
     url = os.path.join(root, sourcename)
   else:
     url = urlparse.urljoin(root, sourcename)
 
   logging.info("Cloning repository %s into %s" % (url, target))
   repo_types[type].clone(url, target)
 
   for repo in repo_types.itervalues():
@@ skipping 91 matching lines @@
   args = parser.parse_args()
 
   if args.quiet:
     logging.disable(logging.INFO)
 
   repos = args.repos
   if not len(repos):
     repos = [os.path.dirname(__file__)]
   for repo in repos:
     resolve_deps(repo)