Remove more logic out of trychange.py into scm.py.

The goal is to (almost) completely remove git-try.

TEST=none

Review URL: http://codereview.chromium.org/501166

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@35144 0039d316-1c4b-4281-b951-d872f2087c98

scm.py

@@ -14,6 +14,10 @@ import xml.dom.minidom
 
 import gclient_utils
 
+def ValidateEmail(email):
+ return (re.match(r"^[a-zA-Z0-9._%-+]+@[a-zA-Z0-9._%-]+.[a-zA-Z]{2,6}$", email)
+         is not None)
+
 
 class GIT(object):
   COMMAND = "git"
@@ -78,9 +82,14 @@ class GIT(object):
 
   @staticmethod
   def GetBranchRef(cwd):
-    """Returns the short branch name, e.g. 'master'."""
+    """Returns the full branch reference, e.g. 'refs/heads/master'."""
     return GIT.Capture(['symbolic-ref', 'HEAD'], cwd).strip()
 
+  @staticmethod
+  def GetBranch(cwd):
+    """Returns the short branch name, e.g. 'master'."""
+    return GIT.ShortBranchName(GIT.BranchRef(cwd))
+
   @staticmethod
   def IsGitSvn(cwd):
     """Returns true if this repo looks like it's using git-svn."""
@@ -140,7 +149,7 @@ class GIT(object):
        e.g. 'origin', 'refs/heads/master'
     """
     remote = '.'
-    branch = GIT.ShortBranchName(GIT.GetBranchRef(cwd))
+    branch = GIT.GetBranch(cwd)
     upstream_branch = None
     upstream_branch = GIT.Capture(
         ['config', 'branch.%s.merge' % branch], error_ok=True).strip()
@@ -181,6 +190,21 @@ class GIT(object):
         diff[i] = '--- %s' % diff[i+1][4:]
     return ''.join(diff)
 
+  @staticmethod
+  def GetPatchName(cwd):
+    """Constructs a name for this patch."""
+    short_sha = GIT.Capture(['rev-parse', '--short=4', 'HEAD'], cwd).strip()
+    return "%s-%s" % (GIT.GetBranch(cwd), short_sha)
+
+  @staticmethod
+  def GetCheckoutRoot(cwd):
+    """Returns the top level directory of the current repository.
+
+    The directory is returned as an absolute path.
+    """
+    return os.path.abspath(GIT.Capture(['rev-parse', '--show-cdup'],
+                                       cwd).strip())
+
 
 class SVN(object):
   COMMAND = "svn"

tests/gclient_scm_test.py

@@ -360,7 +360,8 @@ from :3
   def testDir(self):
     members = [
         'COMMAND', 'Capture', 'CaptureStatus', 'FetchUpstreamTuple',
-        'GenerateDiff', 'GetBranchRef', 'GetEmail', 'GetSVNBranch',
+        'GenerateDiff', 'GetBranch', 'GetBranchRef', 'GetCheckoutRoot',
+        'GetEmail', 'GetPatchName', 'GetSVNBranch',
         'GetUpstream', 'IsGitSvn', 'ShortBranchName',
         'RunCommand', 'cleanup', 'diff', 'export', 'relpath', 'revert',
         'revinfo', 'runhooks', 'scm_name', 'status', 'update', 'url',

tests/scm_unittest.py

@@ -24,7 +24,7 @@ class RootTestCase(BaseSCMTestCase):
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
-        'GIT', 'SVN',
+        'GIT', 'SVN', 'ValidateEmail',
         'gclient_utils', 'os', 're', 'shutil', 'subprocess', 'sys', 'tempfile',
         'xml',
     ]
@@ -117,8 +117,9 @@ from :3
     self.mox.ReplayAll()
     members = [
         'COMMAND', 'Capture', 'CaptureStatus', 'FetchUpstreamTuple',
-        'GenerateDiff', 'GetBranchRef', 'GetEmail', 'GetSVNBranch',
-        'GetUpstream', 'IsGitSvn', 'ShortBranchName'
+        'GenerateDiff', 'GetBranch', 'GetBranchRef', 'GetCheckoutRoot',
+        'GetEmail', 'GetPatchName', 'GetSVNBranch',
+        'GetUpstream', 'IsGitSvn', 'ShortBranchName',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(scm.GIT, members)

tests/trychange_unittest.py

@@ -19,6 +19,8 @@ class TryChangeTestsBase(SuperMoxTestBase):
     self.mox.StubOutWithMock(trychange.gclient_utils, 'CheckCall')
     self.mox.StubOutWithMock(trychange.scm.GIT, 'Capture')
     self.mox.StubOutWithMock(trychange.scm.GIT, 'GenerateDiff')
+    self.mox.StubOutWithMock(trychange.scm.GIT, 'GetCheckoutRoot')
+    self.mox.StubOutWithMock(trychange.scm.GIT, 'GetPatchName')
     self.mox.StubOutWithMock(trychange.scm.GIT, 'GetEmail')
     self.mox.StubOutWithMock(trychange.scm.SVN, 'DiffItem')
     self.mox.StubOutWithMock(trychange.scm.SVN, 'GenerateDiff')
@@ -79,13 +81,11 @@ class GITUnittest(TryChangeTestsBase):
     self.compareMembers(trychange.GIT, members)
 
   def testBasic(self):
-    trychange.gclient_utils.CheckCall(
-        ['git', 'rev-parse', '--show-cdup']).AndReturn(self.fake_root)
-    trychange.os.path.abspath(self.fake_root).AndReturn(self.fake_root)
+    trychange.os.getcwd().AndReturn(self.fake_root)
+    trychange.scm.GIT.GetCheckoutRoot(self.fake_root).AndReturn(self.fake_root)
     trychange.scm.GIT.GenerateDiff(self.fake_root).AndReturn('a diff')
-    trychange.gclient_utils.CheckCall(
-        ['git', 'symbolic-ref', 'HEAD']).AndReturn('refs/heads/random branch')
-    trychange.scm.GIT.GetEmail('.').AndReturn('georges@example.com')
+    trychange.scm.GIT.GetPatchName(self.fake_root).AndReturn('bleh-1233')
+    trychange.scm.GIT.GetEmail(self.fake_root).AndReturn('georges@example.com')
     self.mox.ReplayAll()
     git = trychange.GIT(self.options)
     self.assertEqual(git.GetFileNames(), self.expected_files)

trychange.py

@@ -85,7 +85,6 @@ class SVN(SCM):
   def __init__(self, *args, **kwargs):
     SCM.__init__(self, *args, **kwargs)
     self.checkout_root = scm.SVN.GetCheckoutRoot(os.getcwd())
-    self.options.files
     if not self.options.diff:
       # Generate the diff from the scm.
       self.options.diff = self._GenerateDiff()
@@ -130,29 +129,13 @@ class GIT(SCM):
   """Gathers the options and diff for a git checkout."""
   def __init__(self, *args, **kwargs):
     SCM.__init__(self, *args, **kwargs)
-    self.checkout_root = os.path.abspath(
-        gclient_utils.CheckCall(['git', 'rev-parse', '--show-cdup']).strip())
+    self.checkout_root = scm.GIT.GetCheckoutRoot(os.getcwd())
     if not self.options.diff:
-      self.options.diff = self._GenerateDiff()
+      self.options.diff = scm.GIT.GenerateDiff(self.checkout_root)
     if not self.options.name:
-      self.options.name = self._GetPatchName()
+      self.options.name = scm.GIT.GetPatchName(self.checkout_root)
     if not self.options.email:
-      self.options.email = scm.GIT.GetEmail('.')
-
-  def _GenerateDiff(self):
-    """Get the diff we'll send to the try server. We ignore the files list."""
-    return scm.GIT.GenerateDiff(self.checkout_root)
-
-  def _GetPatchName(self):
-    """Construct a name for this patch."""
-    # TODO: perhaps include the hash of the current commit, to distinguish
-    # patches?
-    branch = gclient_utils.CheckCall(['git', 'symbolic-ref', 'HEAD']).strip()
-    if not branch.startswith('refs/heads/'):
-      # TODO(maruel): Find a better type.
-      raise NoTryServerAccess("Couldn't figure out branch name")
-    branch = branch[len('refs/heads/'):]
-    return branch
+      self.options.email = scm.GIT.GetEmail(self.checkout_root)
 
   def GetLocalRoot(self):
     """Return the path of the repository root."""