Issue 4906 - change cloning in ensure_dependecies

ensure_dependencies.py

 #!/usr/bin/env python
 
 # 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
 import re
@@ skipping 30 matching lines @@
 
 SKIP_DEPENDENCY_UPDATES = os.environ.get(
     'SKIP_DEPENDENCY_UPDATES', ''
 ).lower() not in ('', '0', 'false')
 
 
 class Mercurial():
     def istype(self, repodir):
         return os.path.exists(os.path.join(repodir, '.hg'))
 
-    def clone(self, source, target):
+    def clone(self, source, target, revision):
         if not source.endswith('/'):
             source += '/'
-        subprocess.check_call(['hg', 'clone', '--quiet', '--noupdate', source, target])
+        if not os.path.isdir(target):
+            os.makedirs(target)
+        subprocess.check_call(['hg', 'init', target])
+        config_path = os.path.join(target, '.hg', 'hgrc')
+        config = RawConfigParser()
+        config.add_section('paths')
+        config.set('paths', 'default', source)
+        with open(config_path, 'w') as stream:
+            config.write(stream)
+        self.pull(target)
 
     def get_revision_id(self, repo, rev=None):
         command = ['hg', 'id', '--repository', repo, '--id']
         if rev:
             command.extend(['--rev', rev])
 
         # Ignore stderr output and return code here: if revision lookup failed we
         # should simply return an empty string.
         result = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE).communicate()[0]
         return result.strip()
@@ skipping 23 matching lines @@
         _ensure_line_exists(ignore_path, module)
 
     def postprocess_url(self, url):
         return url
 
 
 class Git():
     def istype(self, repodir):
         return os.path.exists(os.path.join(repodir, '.git'))
 
-    def clone(self, source, target):
+    def clone(self, source, target, revision):
         source = source.rstrip('/')
         if not source.endswith('.git'):
             source += '.git'
-        subprocess.check_call(['git', 'clone', '--quiet', source, target])
+        if not os.path.isdir(target):
+            os.makedirs(target)
+        subprocess.check_call(['git', 'init', '--quiet'], cwd=target)
+        subprocess.check_call(['git', 'remote', 'add', 'origin', source], cwd=target)
+        self.pull(target)
+        # Manual call of update here because if the revision is the HEAD of the
+        # repository then update_repo, which is called after this clone method,
+        # cannot understand that checking out is still required and skips it.
+        # As the result there are no files.
+        resolved_revision = self.get_revision_id(target, revision)
+        self.update(target, resolved_revision, revision)
 
     def get_revision_id(self, repo, rev='HEAD'):
         command = ['git', 'rev-parse', '--revs-only', rev + '^{commit}']
         return subprocess.check_output(command, cwd=repo).strip()
 
     def pull(self, repo):
         # Fetch tracked branches, new tags and the list of available remote branches
         subprocess.check_call(['git', 'fetch', '--quiet', '--all', '--tags'], cwd=repo)
         # Next we need to ensure all remote branches are tracked
         newly_tracked = False
@@ skipping 125 matching lines @@
     return os.path.join(path, *normpath.split(posixpath.sep))
 
 
 def get_repo_type(repo):
     for name, repotype in repo_types.iteritems():
         if repotype.istype(repo):
             return name
     return 'hg'
 
 
-def ensure_repo(parentrepo, parenttype, target, type, root, sourcename):
-    if os.path.exists(target):
+def ensure_repo(parentrepo, parenttype, target, type, root, sourcename, revision):
+    repo = repo_types[type]
+    if repo.istype(target):
         return
 
     if SKIP_DEPENDENCY_UPDATES:
         logging.warning('SKIP_DEPENDENCY_UPDATES environment variable set, '
                         '%s not cloned', target)
         return
 
-    postprocess_url = repo_types[type].postprocess_url
-    root = postprocess_url(root)
-    sourcename = postprocess_url(sourcename)
+    root = repo.postprocess_url(root)
+    sourcename = repo.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)
+    repo.clone(url, target, revision)
     repo_types[parenttype].ignore(target, parentrepo)
 
 
 def update_repo(target, type, revision):
     resolved_revision = repo_types[type].get_revision_id(target, revision)
     current_revision = repo_types[type].get_revision_id(target)
 
     if resolved_revision != current_revision:
         if SKIP_DEPENDENCY_UPDATES:
             logging.warning('SKIP_DEPENDENCY_UPDATES environment variable set, '
@@ skipping 17 matching lines @@
         if level == 0:
             logging.warning('No dependencies file in directory %s, nothing to do...\n%s' % (repodir, USAGE))
         return
     if level >= 10:
         logging.warning('Too much subrepository nesting, ignoring %s' % repo)
         return
 
     if overrideroots is not None:
         config['_root'] = overrideroots
 
     for dir, sources in config.iteritems():

[Wladimir Palant, 2017/05/31 11:04:45]

IMHO, sorting here should do:

  for dir, sources in sorted(config.iteritems()):

[sergei, 2017/05/31 13:38:00]

Tested, it does work in our case and it is sufficient. Should I switch to it?

Actually, I would rather prefer to have the proposed change and check items out
in the order they appear in the "dependencies" file and not in some random order
nor sorted by paths. That will really increase the flexibility because it will
allow to handle in addition some other potential cases, the sorting on the other
hand will make the tool too smart and less flexible.

[Sebastian Noack, 2017/05/31 14:46:09]

Now where I understand the problem, I don't see why anybody would want to
checkout a repo in a subdirectory before checking out the parent directory.
Neither can I think of a scenario in which this "flexibility" is necessary.

Not to mention that re-implementing git/hg clone just to make this scenario
work, isn't great, if we can either require the dependencies file to list the
repositories in the "right" order, or sort them to ensure a compatible order.

Wladimir, Vasily: What do you think?

[Wladimir Palant, 2017/05/31 15:59:37]

Absolutely. This is a classic example where an unnecessarily complicated
approach is justified with "flexibility" that nobody really needs. We can always
do complicated later if we really need it.

         if (dir.startswith('_') or
             skipdependencies.intersection([s[0] for s in sources if s[0]])):
             continue
 
         target = safe_join(repodir, dir)
         parenttype = get_repo_type(repodir)
         _root = config.get('_root', {})
 
         for key in sources.keys() + _root.keys():
             if key == parenttype or key is None and vcs != '*':
                 vcs = key
         source, rev = merge_seqs(sources.get('*'), sources.get(vcs))
 
         if not (vcs and source and rev):
             logging.warning('No valid source / revision found to create %s' % target)
             continue
 
-        ensure_repo(repodir, parenttype, target, vcs, _root.get(vcs, ''), source)
+        ensure_repo(repodir, parenttype, target, vcs, _root.get(vcs, ''), source, rev)
         update_repo(target, vcs, rev)
         resolve_deps(target, level + 1, self_update=False,
                      overrideroots=overrideroots, skipdependencies=skipdependencies)
 
     if self_update and '_self' in config and '*' in config['_self']:
         source = safe_join(repodir, config['_self']['*'])
         try:
             with io.open(source, 'rb') as handle:
                 sourcedata = handle.read()
         except IOError as e:
@@ skipping 37 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)