Bug 1401309 - [mozversioncontrol] Use Repository.create() for instantiating repository objects, r?gps draft
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Tue, 19 Sep 2017 15:27:37 -0400
changeset 667681 3a0c72c2eb207322b626f9aeac3d63fa9d5c856b
parent 667680 6fdee40a409184041dac031bacc294a36e4f9971
child 667682 65941db28f7d07fab7b8cec31ea13932e31fba24
push id80814
push userahalberstadt@mozilla.com
push dateWed, 20 Sep 2017 15:42:29 +0000
reviewersgps
bugs1401309
milestone57.0a1
Bug 1401309 - [mozversioncontrol] Use Repository.create() for instantiating repository objects, r?gps The get_repository_* functions are a bit clunky to use. The method for instantiating a repository object that python/mozlint/vcs.py uses feels more natural and intuitive. This removes get_repository_object in favour of: Repository.create(path) Additionaly, calling create without a path argument will automatically detect the vcs by running 'hg root' and 'git rev-parser --show-toplevel' in that order. For now, get_repository_from_env and get_repository_from_build_config were left alone, though I suppose they could also be rolled into 'create' if we wanted to. MozReview-Commit-ID: 6rBSGmt4pgV
build/mach_bootstrap.py
build/moz.configure/init.configure
python/mozbuild/mozbuild/base.py
python/mozversioncontrol/mozversioncontrol/__init__.py
--- a/build/mach_bootstrap.py
+++ b/build/mach_bootstrap.py
@@ -163,17 +163,17 @@ def bootstrap(topsrcdir, mozilla_dir=Non
 
     def resolve_repository():
         import mozversioncontrol
 
         try:
             # This API doesn't respect the vcs binary choices from configure.
             # If we ever need to use the VCS binary here, consider something
             # more robust.
-            return mozversioncontrol.get_repository_object(path=mozilla_dir)
+            return mozversioncontrol.Repository.create(mozilla_dir)
         except (mozversioncontrol.InvalidRepoPath,
                 mozversioncontrol.MissingVCSTool):
             return None
 
     def telemetry_handler(context, data):
         # We have not opted-in to telemetry
         if 'BUILD_SYSTEM_TELEMETRY' not in os.environ:
             return
--- a/build/moz.configure/init.configure
+++ b/build/moz.configure/init.configure
@@ -463,22 +463,22 @@ def exposed_vcs_checkout_type(vcs_checko
             raise FatalCheckError('could not resolve Git binary info')
     elif vcs_checkout_type:
         raise FatalCheckError('unhandled VCS type: %s' % vcs_checkout_type)
 
 set_config('VCS_CHECKOUT_TYPE', exposed_vcs_checkout_type)
 
 # Obtain a Repository interface for the current VCS repository.
 @depends(check_build_environment, exposed_vcs_checkout_type, hg, git)
-@imports(_from='mozversioncontrol', _import='get_repository_object')
+@imports(_from='mozversioncontrol', _import='Repository')
 def vcs_repository(build_env, vcs_checkout_type, hg, git):
     if vcs_checkout_type == 'hg':
-        return get_repository_object(build_env.topsrcdir, hg=hg)
+        return Repository.create(build_env.topsrcdir, tool=hg)
     elif vcs_checkout_type == 'git':
-        return get_repository_object(build_env.topsrcdir, git=git)
+        return Repository.create(build_env.topsrcdir, tool=git)
     elif vcs_checkout_type:
         raise FatalCheckError('unhandled VCS type: %s' % vcs_checkout_type)
 
 @depends_if(vcs_repository)
 @checking('for sparse checkout')
 def vcs_sparse_checkout(repo):
     return repo.sparse_checkout_present()
 
--- a/python/mozbuild/mozbuild/base.py
+++ b/python/mozbuild/mozbuild/base.py
@@ -11,18 +11,18 @@ import multiprocessing
 import os
 import subprocess
 import sys
 import which
 
 from mach.mixin.process import ProcessExecutionMixin
 from mozversioncontrol import (
     get_repository_from_build_config,
-    get_repository_object,
     InvalidRepoPath,
+    Repository,
 )
 
 from .backend.configenvironment import ConfigEnvironment
 from .controller.clobber import Clobberer
 from .mozconfig import (
     MozconfigFindException,
     MozconfigLoadException,
     MozconfigLoader,
@@ -307,17 +307,17 @@ class MozbuildObject(ProcessExecutionMix
         top source directory.'''
         # We try to obtain a repo using the configured VCS info first.
         # If we don't have a configure context, fall back to auto-detection.
         try:
             return get_repository_from_build_config(self)
         except BuildEnvironmentNotFoundException:
             pass
 
-        return get_repository_object(self.topsrcdir)
+        return Repository.create(self.topsrcdir)
 
     def mozbuild_reader(self, config_mode='build', vcs_revision=None,
                         vcs_check_clean=True):
         """Obtain a ``BuildReader`` for evaluating moz.build files.
 
         Given arguments, returns a ``mozbuild.frontend.reader.BuildReader``
         that can be used to evaluate moz.build files for this repo.
 
--- a/python/mozversioncontrol/mozversioncontrol/__init__.py
+++ b/python/mozversioncontrol/mozversioncontrol/__init__.py
@@ -13,16 +13,28 @@ import which
 
 from distutils.version import LooseVersion
 
 
 class MissingVCSTool(Exception):
     """Represents a failure to find a version control tool binary."""
 
 
+class MissingVCSInfo(Exception):
+    """Represents a general failure to resolve a VCS interface."""
+
+
+class MissingConfigureInfo(MissingVCSInfo):
+    """Represents error finding VCS info from configure data."""
+
+
+class InvalidRepoPath(Exception):
+    """Represents a failure to find a VCS repo at a specified path."""
+
+
 def get_tool_path(tool):
     """Obtain the path of `tool`."""
     if os.path.isabs(tool) and os.path.exists(tool):
         return tool
 
     # We use subprocess in places, which expects a Win32 executable or
     # batch script. On some versions of MozillaBuild, we have "hg.exe",
     # "hg.bat," and "hg" (a Python script). "which" will happily return the
@@ -48,33 +60,63 @@ class Repository(object):
     calling a ``get_repository_*()`` helper function.
 
     Clients are recommended to use the object as a context manager. But not
     all methods require this.
     """
 
     __metaclass__ = abc.ABCMeta
 
-    def __init__(self, path, tool):
-        self.path = os.path.abspath(path)
+    def __init__(self, root, tool):
+        self.root = os.path.abspath(root)
         self._tool = get_tool_path(tool)
         self._env = os.environ.copy()
         self._version = None
 
     def __enter__(self):
         return self
 
     def __exit__(self, exc_type, exc_value, exc_tb):
         pass
 
     def _run(self, *args):
         return subprocess.check_output((self._tool, ) + args,
-                                       cwd=self.path,
+                                       cwd=self.root,
                                        env=self._env)
 
+    @classmethod
+    def find_vcs(cls):
+        # First check if we're in an hg repo, if not try git
+        commands = (
+            ['hg', 'root'],
+            ['git', 'rev-parse', '--show-toplevel'],
+        )
+
+        for cmd in commands:
+            proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+            output = proc.communicate()[0].strip()
+
+            if proc.returncode == 0:
+                return cmd[0], output
+        return 'none', ''
+
+    @classmethod
+    def create(cls, root=None, **kwargs):
+        if not root:
+            vcs, root = cls.find_vcs()
+        elif os.path.isdir(os.path.join(root, '.hg')):
+            vcs = 'hg'
+        elif os.path.isdir(os.path.join(root, '.git')):
+            vcs = 'git'
+
+        if vcs not in vcs_class or not root:
+            raise InvalidRepoPath('Unknown VCS, or not a source checkout: %s' % root)
+
+        return vcs_class[vcs](root, **kwargs)
+
     @property
     def tool_version(self):
         '''Return the version of the VCS tool in use as a `LooseVersion`.'''
         if self._version:
             return self._version
         info = self._run('--version').strip()
         match = re.search('version ([^\+\)]+)', info)
         if not match:
@@ -131,44 +173,44 @@ class Repository(object):
         By default, untracked and ignored files are not considered. If
         ``untracked`` or ``ignored`` are set, they influence the clean check
         to factor these file classes into consideration.
         """
 
 
 class HgRepository(Repository):
     '''An implementation of `Repository` for Mercurial repositories.'''
-    def __init__(self, path, hg='hg'):
+    def __init__(self, path, tool='hg'):
         import hglib.client
 
-        super(HgRepository, self).__init__(path, tool=hg)
+        super(HgRepository, self).__init__(path, tool=tool)
         self._env[b'HGPLAIN'] = b'1'
 
         # Setting this modifies a global variable and makes all future hglib
         # instances use this binary. Since the tool path was validated, this
         # should be OK. But ideally hglib would offer an API that defines
         # per-instance binaries.
         hglib.HGPATH = self._tool
 
         # Without connect=False this spawns a persistent process. We want
         # the process lifetime tied to a context manager.
-        self._client = hglib.client.hgclient(self.path, encoding=b'UTF-8',
+        self._client = hglib.client.hgclient(self.root, encoding=b'UTF-8',
                                              configs=None, connect=False)
 
     @property
     def name(self):
         return 'hg'
 
     def __enter__(self):
         if self._client.server is None:
             # The cwd if the spawned process should be the repo root to ensure
             # relative paths are normalized to it.
             old_cwd = os.getcwd()
             try:
-                os.chdir(self.path)
+                os.chdir(self.root)
                 self._client.open()
             finally:
                 os.chdir(old_cwd)
 
         return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         self._client.close()
@@ -180,17 +222,17 @@ class HgRepository(Repository):
         return self._client.rawcommand(args)
 
     def sparse_checkout_present(self):
         # We assume a sparse checkout is enabled if the .hg/sparse file
         # has data. Strictly speaking, we should look for a requirement in
         # .hg/requires. But since the requirement is still experimental
         # as of Mercurial 4.3, it's probably more trouble than its worth
         # to verify it.
-        sparse = os.path.join(self.path, '.hg', 'sparse')
+        sparse = os.path.join(self.root, '.hg', 'sparse')
 
         try:
             st = os.stat(sparse)
             return st.st_size > 0
         except OSError as e:
             if e.errno != errno.ENOENT:
                 raise
 
@@ -229,18 +271,18 @@ class HgRepository(Repository):
 
         # If output is empty, there are no entries of requested status, which
         # means we are clean.
         return not len(self._run_in_client(args).strip())
 
 
 class GitRepository(Repository):
     '''An implementation of `Repository` for Git repositories.'''
-    def __init__(self, path, git='git'):
-        super(GitRepository, self).__init__(path, tool=git)
+    def __init__(self, root, tool='git'):
+        super(GitRepository, self).__init__(root, tool=tool)
 
     @property
     def name(self):
         return 'git'
 
     def sparse_checkout_present(self):
         # Not yet implemented.
         return False
@@ -265,39 +307,20 @@ class GitRepository(Repository):
         if untracked:
             args.append('--untracked-files')
         if ignored:
             args.append('--ignored')
 
         return not len(self._run(*args).strip())
 
 
-class InvalidRepoPath(Exception):
-    """Represents a failure to find a VCS repo at a specified path."""
-
-
-def get_repository_object(path, hg='hg', git='git'):
-    '''Get a repository object for the repository at `path`.
-    If `path` is not a known VCS repository, raise an exception.
-    '''
-    if os.path.isdir(os.path.join(path, '.hg')):
-        return HgRepository(path, hg=hg)
-    elif os.path.exists(os.path.join(path, '.git')):
-        return GitRepository(path, git=git)
-    else:
-        raise InvalidRepoPath('Unknown VCS, or not a source checkout: %s' %
-                              path)
-
-
-class MissingVCSInfo(Exception):
-    """Represents a general failure to resolve a VCS interface."""
-
-
-class MissingConfigureInfo(MissingVCSInfo):
-    """Represents error finding VCS info from configure data."""
+vcs_class = {
+    'git': GitRepository,
+    'hg': HgRepository,
+}
 
 
 def get_repository_from_build_config(config):
     """Obtain a repository from the build configuration.
 
     Accepts an object that has a ``topsrcdir`` and ``subst`` attribute.
     """
     flavor = config.substs.get('VCS_CHECKOUT_TYPE')
@@ -306,19 +329,19 @@ def get_repository_from_build_config(con
     # that everything in the build system can be controlled via configure.
     if not flavor:
         raise MissingConfigureInfo('could not find VCS_CHECKOUT_TYPE '
                                    'in build config; check configure '
                                    'output and verify it could find a '
                                    'VCS binary')
 
     if flavor == 'hg':
-        return HgRepository(config.topsrcdir, hg=config.substs['HG'])
+        return HgRepository(config.topsrcdir, tool=config.substs['HG'])
     elif flavor == 'git':
-        return GitRepository(config.topsrcdir, git=config.substs['GIT'])
+        return GitRepository(config.topsrcdir, tool=config.substs['GIT'])
     else:
         raise MissingVCSInfo('unknown VCS_CHECKOUT_TYPE value: %s' % flavor)
 
 
 def get_repository_from_env():
     """Obtain a repository object by looking at the environment.
 
     If inside a build environment (denoted by presence of a ``buildconfig``
@@ -337,14 +360,14 @@ def get_repository_from_env():
         while path:
             yield path
             path, child = os.path.split(path)
             if child == '':
                 break
 
     for path in ancestors(os.getcwd()):
         try:
-            return get_repository_object(path)
+            return Repository.create(path)
         except InvalidRepoPath:
             continue
 
     raise MissingVCSInfo('Could not find Mercurial or Git checkout for %s' %
                          os.getcwd())