Reland "Update fetch protocol using --protocol-override flag"

This is a reland of commit 6817010e

Additional Changes:
This reland fixes https://crbug.com/1330995 by stopping gclient from accidentally updating the scheme when one is not present.

Original change's description:
> Update fetch protocol using --protocol-override flag
>
> This CL updates gclient sync to use the protocol of the URL specified in the solutions for cloning all the child dependencies of it.
>
> Bug: chrome-operations:170
> Change-Id: I33588059788b677fbae8c3b434100af5c7979a67
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3631600
> Reviewed-by: Joanna Wang <jojwang@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
> Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>

Bug: 1330995
Change-Id: I1447a5e884e41d671d8556c35193f1635f2f6936
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3684112Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>

fetch.py

@@ -24,6 +24,7 @@ import json
 import argparse
 import os
 import pipes
+import re
 import subprocess
 import sys
 import textwrap
@@ -34,7 +35,6 @@ from distutils import spawn
 
 
 SCRIPT_PATH = os.path.dirname(os.path.abspath(__file__))
-DEFAULT_PROTOCOL = 'https'
 
 #################################################
 # Checkout class definitions.
@@ -204,7 +204,7 @@ def handle_args(argv):
     '-p',
     '--protocol-override',
     type=str,
-    default=DEFAULT_PROTOCOL,
+    default=None,
     help='Protocol to use to fetch dependencies, defaults to https.')
 
   parser.add_argument('config', type=str,
@@ -259,6 +259,13 @@ def run(options, spec, root):
   assert 'type' in spec
   checkout_type = spec['type']
   checkout_spec = spec['%s_spec' % checkout_type]
+
+  # Replace https using the protocol specified in --protocol-override
+  if options.protocol_override is not None:
+    for solution in checkout_spec['solutions']:
+      solution['url'] = re.sub(
+        '^([a-z]+):', options.protocol_override + ':', solution['url'])
+
   try:
     checkout = CheckoutFactory(checkout_type, options, checkout_spec, root)
   except KeyError:

gclient.py

@@ -396,7 +396,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
   def __init__(self, parent, name, url, managed, custom_deps,
                custom_vars, custom_hooks, deps_file, should_process,
-               should_recurse, relative, condition, print_outbuf=False):
+               should_recurse, relative, condition, protocol='https',
+               print_outbuf=False):
     gclient_utils.WorkItem.__init__(self, name)
     DependencySettings.__init__(
         self, parent, url, managed, custom_deps, custom_vars,
@@ -462,6 +463,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     # dependency
     self.print_outbuf = print_outbuf
 
+    self.protocol = protocol
+
     if not self.name and self.parent:
       raise gclient_utils.Error('Dependency without name')
 
@@ -690,7 +693,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
             GitDependency(
                 parent=self,
                 name=name,
-                url=url,
+                # Update URL with parent dep's protocol
+                url=GitDependency.updateProtocol(url, self.protocol),
                 managed=True,
                 custom_deps=None,
                 custom_vars=self.custom_vars,
@@ -699,7 +703,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
                 should_process=should_process,
                 should_recurse=name in self.recursedeps,
                 relative=use_relative_paths,
-                condition=condition))
+                condition=condition,
+                protocol=self.protocol))
 
     deps_to_add.sort(key=lambda x: x.name)
     return deps_to_add
@@ -1342,6 +1347,16 @@ def _detect_host_os():
 class GitDependency(Dependency):
   """A Dependency object that represents a single git checkout."""
 
+  @staticmethod
+  def updateProtocol(url, protocol):
+    """Updates given URL's protocol"""
+    # only works on urls, skips local paths
+    if not url or not protocol or not re.match('([a-z]+)://', url) or \
+      re.match('file://', url):
+      return url
+
+    return re.sub('^([a-z]+):', protocol + ':', url)
+
   #override
   def GetScmName(self):
     """Always 'git'."""
@@ -1431,6 +1446,14 @@ solutions = %(solution_list)s
     self._cipd_root = None
     self.config_content = None
 
+  @staticmethod
+  def _getScheme(url):
+    """Returns the scheme part of the given URL"""
+    if not url or not re.match('^([a-z]+)://', url):
+      return None
+
+    return url.split('://')[0]
+
   def _CheckConfig(self):
     """Verify that the config matches the state of the existing checked-out
     solutions."""
@@ -1523,7 +1546,9 @@ it or fix the checkout.
             should_recurse=True,
             relative=None,
             condition=None,
-            print_outbuf=True))
+            print_outbuf=True,
+            # Pass parent URL protocol down the tree for child deps to use.
+            protocol=GClient._getScheme(s['url'])))
       except KeyError:
         raise gclient_utils.Error('Invalid .gclient file. Solution is '
                                   'incomplete: %s' % s)
@@ -1752,7 +1777,8 @@ it or fix the checkout.
               GitDependency(
                   parent=self,
                   name=entry,
-                  url=prev_url,
+                  # Update URL with parent dep's protocol
+                  url=GitDependency.updateProtocol(prev_url, self.protocol),
                   managed=False,
                   custom_deps={},
                   custom_vars={},
@@ -1761,7 +1787,8 @@ it or fix the checkout.
                   should_process=True,
                   should_recurse=False,
                   relative=None,
-                  condition=None))
+                  condition=None,
+                  protocol=self.protocol))
           if modified_files and self._options.delete_unversioned_trees:
             print('\nWARNING: \'%s\' is no longer part of this client.\n'
                   'Despite running \'gclient sync -D\' no action was taken '

tests/fetch_test.py

@@ -54,7 +54,7 @@ class TestUtilityFunctions(unittest.TestCase):
       no_history=False,
       force=False,
       config='foo',
-      protocol_override='https',
+      protocol_override=None,
       props=[]), response)
 
     response = fetch.handle_args([

tests/gclient_test.py

@@ -236,6 +236,7 @@ class GclientTest(trial_dir.TestCase):
         should_recurse=False,
         relative=False,
         condition=None,
+        protocol='https',
         print_outbuf=True)
     self.assertEqual('proto://host/path@revision', d.url)
 
@@ -258,6 +259,7 @@ class GclientTest(trial_dir.TestCase):
             should_recurse=True,
             relative=False,
             condition=None,
+            protocol='https',
             print_outbuf=True),
         gclient.Dependency(
             parent=obj,
@@ -272,6 +274,7 @@ class GclientTest(trial_dir.TestCase):
             should_recurse=False,
             relative=False,
             condition=None,
+            protocol='https',
             print_outbuf=True),
       ],
       [])
@@ -290,6 +293,7 @@ class GclientTest(trial_dir.TestCase):
             should_recurse=False,
             relative=False,
             condition=None,
+            protocol='https',
             print_outbuf=True),
       ],
       [])
@@ -1308,6 +1312,7 @@ class GclientTest(trial_dir.TestCase):
             should_recurse=True,
             relative=False,
             condition=None,
+            protocol='https',
             print_outbuf=True),
       ],
       [])