diff --git a/scm.py b/scm.py index d23ab62d6..2e1355588 100644 --- a/scm.py +++ b/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" diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index 4e8f907ca..2bcc3a583 100755 --- a/tests/gclient_scm_test.py +++ b/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', diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 27696f73b..235f76b2c 100755 --- a/tests/scm_unittest.py +++ b/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) diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py index e0832f4a2..298865cb8 100644 --- a/tests/trychange_unittest.py +++ b/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) diff --git a/trychange.py b/trychange.py index 35a57433f..e29f054fd 100755 --- a/trychange.py +++ b/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."""