diff --git a/gclient.py b/gclient.py index e29eaa80c..c804186e8 100644 --- a/gclient.py +++ b/gclient.py @@ -923,8 +923,15 @@ solutions = [ handle = urllib.urlopen(s.safesync_url) rev = handle.read().strip() handle.close() - if len(rev): - self._options.revisions.append('%s@%s' % (s.name, rev)) + scm = gclient_scm.CreateSCM(s.url, s.root.root_dir, s.name) + safe_rev = scm.GetUsableRev(rev=rev, options=self._options) + if not safe_rev: + raise gclient_utils.Error( + 'Despite our best attempts, we couldn\'t find a useful\n' + 'safesync_url revision for you.') + if self._options.verbose: + print('Using safesync_url revision: %s.\n' % safe_rev) + self._options.revisions.append('%s@%s' % (s.name, safe_rev)) if not self._options.revisions: return revision_overrides solutions_names = [s.name for s in self.dependencies] diff --git a/gclient_scm.py b/gclient_scm.py index 55aa1cca1..0b21dce99 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -483,6 +483,34 @@ class GitWrapper(SCMWrapper): files = self._Capture(['diff', '--name-only', merge_base]).split() file_list.extend([os.path.join(self.checkout_path, f) for f in files]) + def GetUsableRev(self, rev, options): + """Finds a useful revision for this repository. + + If SCM is git-svn and the head revision is less than |rev|, git svn fetch + will be called on the source.""" + sha1 = None + # As an optimization, only verify an SVN revision as [0-9]{1,6} for now to + # avoid making a network request. + if (scm.GIT.IsGitSvn(cwd=self.checkout_path) and + rev.isdigit() and len(rev) < 7): + local_head = scm.GIT.GetGitSvnHeadRev(cwd=self.checkout_path) + if not local_head or local_head < int(rev): + if options.verbose: + print('Running git svn fetch. This might take a while.\n') + scm.GIT.Capture(['svn', 'fetch'], cwd=self.checkout_path) + sha1 = scm.GIT.GetSha1ForSvnRev(cwd=self.checkout_path, rev=rev) + elif scm.GIT.IsValidRevision(cwd=self.checkout_path, rev=rev): + sha1 = rev + if not sha1: + raise gclient_utils.Error( + ( '%s is not a value hash. Safesync URLs with a git checkout\n' + 'currently require a git-svn remote or a safesync_url that\n' + 'provides git sha1s. Please add a git-svn remote or change\n' + 'your safesync_url. For more info, see:\n' + 'http://code.google.com/p/chromium/wiki/UsingNewGit' + '#Initial_checkout') % rev) + return sha1 + def FullUrlForRelativeUrl(self, url): # Strip from last '/' # Equivalent to unix basename @@ -1002,6 +1030,14 @@ class SVNWrapper(SCMWrapper): else: self._RunAndGetFileList(command, options, file_list) + def GetUsableRev(self, rev, options): + """Verifies the validity of the revision for this repository.""" + if not scm.SVN.IsValidRevision(url='%s@%s' % (self.url, rev)): + raise gclient_utils.Error( + ( '%s isn\'t a valid revision. Please check that your safesync_url is\n' + 'correct.') % rev) + return rev + def FullUrlForRelativeUrl(self, url): # Find the forth '/' and strip from there. A bit hackish. return '/'.join(self.url.split('/')[:4]) + url diff --git a/scm.py b/scm.py index 2b63901a9..da094fb59 100644 --- a/scm.py +++ b/scm.py @@ -380,6 +380,37 @@ class GIT(object): root = GIT.Capture(['rev-parse', '--show-cdup'], cwd=cwd).strip() return os.path.abspath(os.path.join(cwd, root)) + @staticmethod + def GetGitSvnHeadRev(cwd): + """Gets the most recently pulled git-svn revision.""" + try: + output = GIT.Capture(['svn', 'info'], cwd=cwd) + match = re.search(r'^Revision: ([0-9]+)$', output, re.MULTILINE) + return int(match.group(1)) if match else None + except (subprocess2.CalledProcessError, ValueError): + return None + + @staticmethod + def GetSha1ForSvnRev(cwd, rev): + """Returns a corresponding git sha1 for a SVN revision.""" + if not GIT.IsGitSvn(cwd=cwd): + return None + try: + lines = GIT.Capture( + ['svn', 'find-rev', 'r' + str(rev)], cwd=cwd).splitlines() + return lines[-1].strip() if lines else None + except subprocess2.CalledProcessError: + return None + + @staticmethod + def IsValidRevision(cwd, rev): + """Verifies the revision is a proper git revision.""" + try: + GIT.Capture(['rev-parse', rev], cwd=cwd) + return True + except subprocess2.CalledProcessError: + return False + @classmethod def AssertVersion(cls, min_version): """Asserts git's version is at least min_version.""" @@ -946,6 +977,15 @@ class SVN(object): cwd = parent return GetCasedPath(cwd) + @staticmethod + def IsValidRevision(url): + """Verifies the revision looks like an SVN revision.""" + try: + SVN.Capture(['info', url], cwd=None) + return True + except subprocess2.CalledProcessError: + return False + @classmethod def AssertVersion(cls, min_version): """Asserts svn's version is at least min_version.""" diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index 87d8acf7e..ac13c108b 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -93,10 +93,21 @@ class SVNWrapperTestCase(BaseTestCase): def testDir(self): members = [ - 'FullUrlForRelativeUrl', 'GetRevisionDate', 'RunCommand', - 'cleanup', 'diff', 'pack', 'relpath', 'revert', - 'revinfo', 'runhooks', 'status', 'update', - 'updatesingle', 'url', + 'FullUrlForRelativeUrl', + 'GetRevisionDate', + 'GetUsableRev', + 'RunCommand', + 'cleanup', + 'diff', + 'pack', + 'relpath', + 'revert', + 'revinfo', + 'runhooks', + 'status', + 'update', + 'updatesingle', + 'url', ] # If you add a member, be sure to add the relevant test! @@ -544,6 +555,26 @@ class SVNWrapperTestCase(BaseTestCase): self.checkstdout( ('________ found .hg directory; skipping %s\n' % self.relpath)) + def testGetUsableRevSVN(self): + # pylint: disable=E1101 + options = self.Options(verbose=True) + + # Mock SVN revision validity checking. + self.mox.StubOutWithMock( + gclient_scm.scm.SVN, 'IsValidRevision', True) + gclient_scm.scm.SVN.IsValidRevision(url='%s@%s' % (self.url, 1) + ).AndReturn(True) + gclient_scm.scm.SVN.IsValidRevision(url='%s@%s' % (self.url, 'fake') + ).AndReturn(False) + + self.mox.ReplayAll() + + svn_scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir) + # With an SVN checkout, 1 an example of a valid usable rev. + self.assertEquals(svn_scm.GetUsableRev(1, options), 1) + # With an SVN checkout, a fake or unknown rev should raise an excpetion. + self.assertRaises(gclient_scm.gclient_utils.Error, + svn_scm.GetUsableRev, 'fake', options) class BaseGitWrapperTestCase(GCBaseTestCase, StdoutCheck, TestCaseUtils, unittest.TestCase): @@ -646,13 +677,23 @@ from :3 unittest.TestCase.tearDown(self) rmtree(self.root_dir) - class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): def testDir(self): members = [ - 'FullUrlForRelativeUrl', 'GetRevisionDate', 'RunCommand', - 'cleanup', 'diff', 'pack', 'relpath', 'revert', - 'revinfo', 'runhooks', 'status', 'update', 'url', + 'FullUrlForRelativeUrl', + 'GetRevisionDate', + 'GetUsableRev', + 'RunCommand', + 'cleanup', + 'diff', + 'pack', + 'relpath', + 'revert', + 'revinfo', + 'runhooks', + 'status', + 'update', + 'url', ] # If you add a member, be sure to add the relevant test! @@ -929,6 +970,111 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): self.assertEquals(rev_info, '069c602044c5388d2d15c3f875b057c852003458') +class ManagedGitWrapperTestCaseMox(BaseTestCase): + class OptionsObject(object): + def __init__(self, verbose=False, revision=None, force=False): + self.verbose = verbose + self.revision = revision + self.deps_os = None + self.force = force + self.reset = False + self.nohooks = False + # TODO(maruel): Test --jobs > 1. + self.jobs = 1 + + def Options(self, *args, **kwargs): + return self.OptionsObject(*args, **kwargs) + + def setUp(self): + BaseTestCase.setUp(self) + self.fake_hash_1 = 't0ta11yf4k3' + self.fake_hash_2 = '3v3nf4k3r' + self.url = 'git://foo' + self.root_dir = '/tmp' + self.relpath = 'fake' + self.base_path = os.path.join(self.root_dir, self.relpath) + + def tearDown(self): + BaseTestCase.tearDown(self) + + def testGetUsableRevGit(self): + # pylint: disable=E1101 + options = self.Options(verbose=True) + + self.mox.StubOutWithMock(gclient_scm.scm.GIT, 'IsValidRevision', True) + gclient_scm.scm.GIT.IsValidRevision(cwd=self.base_path, rev=self.fake_hash_1 + ).AndReturn(True) + gclient_scm.scm.GIT.IsValidRevision(cwd=self.base_path, rev='1' + ).AndReturn(False) + + self.mox.StubOutWithMock(gclient_scm.scm.GIT, 'IsGitSvn', True) + gclient_scm.scm.GIT.IsGitSvn(cwd=self.base_path).MultipleTimes( + ).AndReturn(False) + + self.mox.ReplayAll() + + git_scm = gclient_scm.CreateSCM(url=self.url, root_dir=self.root_dir, + relpath=self.relpath) + # A [fake] git sha1 with a git repo should work (this is in the case that + # the LKGR gets flipped to git sha1's some day). + self.assertEquals(git_scm.GetUsableRev(self.fake_hash_1, options), + self.fake_hash_1) + # An SVN rev with a purely git repo should raise an exception. + self.assertRaises(gclient_scm.gclient_utils.Error, + git_scm.GetUsableRev, '1', options) + + def testGetUsableRevGitSvn(self): + # pylint: disable=E1101 + options = self.Options() + too_big = str(1e7) + + # Pretend like the git-svn repo's HEAD is at r2. + self.mox.StubOutWithMock(gclient_scm.scm.GIT, 'GetGitSvnHeadRev', True) + gclient_scm.scm.GIT.GetGitSvnHeadRev(cwd=self.base_path).MultipleTimes( + ).AndReturn(2) + + self.mox.StubOutWithMock(gclient_scm.scm.GIT, 'GetSha1ForSvnRev', True) + # r1 -> first fake hash, r3 -> second fake hash. + gclient_scm.scm.GIT.GetSha1ForSvnRev(cwd=self.base_path, rev='1' + ).AndReturn(self.fake_hash_1) + gclient_scm.scm.GIT.GetSha1ForSvnRev(cwd=self.base_path, rev='3' + ).AndReturn(self.fake_hash_2) + + # Ensure that we call git svn fetch if our LKGR is > the git-svn HEAD rev. + self.mox.StubOutWithMock(gclient_scm.scm.GIT, 'Capture', True) + gclient_scm.scm.GIT.Capture(['svn', 'fetch'], cwd=self.base_path) + + self.mox.StubOutWithMock(gclient_scm.scm.GIT, 'IsGitSvn', True) + gclient_scm.scm.GIT.IsGitSvn(cwd=self.base_path).MultipleTimes( + ).AndReturn(True) + + self.mox.StubOutWithMock(gclient_scm.scm.GIT, 'IsValidRevision', True) + gclient_scm.scm.GIT.IsValidRevision(cwd=self.base_path, rev=self.fake_hash_1 + ).AndReturn(True) + gclient_scm.scm.GIT.IsValidRevision(cwd=self.base_path, rev=too_big + ).AndReturn(False) + + self.mox.ReplayAll() + + git_svn_scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, + relpath=self.relpath) + # Given an SVN revision with a git-svn checkout, it should be translated to + # a git sha1 and be usable. + self.assertEquals(git_svn_scm.GetUsableRev('1', options), + self.fake_hash_1) + # Our fake HEAD rev is r2, so this should call git svn fetch to get more + # revs (pymox will complain if this doesn't happen). + self.assertEquals(git_svn_scm.GetUsableRev('3', options), + self.fake_hash_2) + # Given a git sha1 with a git-svn checkout, it should be used as is. + self.assertEquals(git_svn_scm.GetUsableRev(self.fake_hash_1, options), + self.fake_hash_1) + # We currently check for seemingly valid SVN revisions by assuming 6 digit + # numbers, so assure that numeric revs >= 1000000 don't work. + self.assertRaises(gclient_scm.gclient_utils.Error, + git_svn_scm.GetUsableRev, too_big, options) + + class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): def testUpdateCheckout(self): if not self.enabled: diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 2fbfe3dcf..a69b7cd8d 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -48,10 +48,24 @@ class RootTestCase(BaseSCMTestCase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'ElementTree', 'GetCasedPath', 'GenFakeDiff', 'GIT', 'SVN', + 'cStringIO', + 'determine_scm', + 'ElementTree', + 'gclient_utils', + 'GenFakeDiff', + 'GetCasedPath', + 'GIT', + 'glob', + 'logging', + 'only_int', + 'os', + 're', + 'subprocess2', + 'SVN', + 'sys', + 'tempfile', + 'time', 'ValidateEmail', - 'cStringIO', 'determine_scm', 'gclient_utils', 'glob', 'logging', - 'only_int', 'os', 're', 'subprocess2', 'sys', 'tempfile', 'time', ] # If this test fails, you should add the relevant test. self.compareMembers(scm, members) @@ -60,12 +74,26 @@ class RootTestCase(BaseSCMTestCase): class GitWrapperTestCase(BaseSCMTestCase): def testMembersChanged(self): members = [ - 'AssertVersion', 'Capture', 'CaptureStatus', - 'FetchUpstreamTuple', - 'GenerateDiff', 'GetBranch', 'GetBranchRef', 'GetCheckoutRoot', - 'GetDifferentFiles', 'GetEmail', 'GetPatchName', 'GetSVNBranch', - 'GetUpstreamBranch', 'IsGitSvn', 'MatchSvnGlob', 'ShortBranchName', + 'AssertVersion', + 'Capture', + 'CaptureStatus', 'current_version', + 'FetchUpstreamTuple', + 'GenerateDiff', + 'GetBranch', + 'GetBranchRef', + 'GetCheckoutRoot', + 'GetDifferentFiles', + 'GetEmail', + 'GetGitSvnHeadRev', + 'GetPatchName', + 'GetSha1ForSvnRev', + 'GetSVNBranch', + 'GetUpstreamBranch', + 'IsGitSvn', + 'IsValidRevision', + 'MatchSvnGlob', + 'ShortBranchName', ] # If this test fails, you should add the relevant test. self.compareMembers(scm.GIT, members) @@ -89,6 +117,66 @@ class GitWrapperTestCase(BaseSCMTestCase): 'branches/*:refs/remotes/*', True), 'refs/remotes/bleeding_edge') + +class RealGitTest(fake_repos.FakeReposTestBase): + def setUp(self): + super(RealGitTest, self).setUp() + self.enabled = self.FAKE_REPOS.set_up_git() + if self.enabled: + self.clone_dir = scm.os.path.join(self.FAKE_REPOS.git_root, 'repo_1') + + def testIsValidRevision(self): + if not self.enabled: + return + # Sha1's are [0-9a-z]{32}, so starting with a 'z' or 'r' should always fail. + self.assertFalse(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev='zebra')) + self.assertFalse(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev='r123456')) + # Valid cases + first_rev = self.githash('repo_1', 1) + self.assertTrue(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev=first_rev)) + self.assertTrue(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev='HEAD')) + + +class RealGitSvnTest(fake_repos.FakeReposTestBase): + def setUp(self): + super(RealGitSvnTest, self).setUp() + self.enabled = self.FAKE_REPOS.set_up_git() and self.FAKE_REPOS.set_up_svn() + if self.enabled: + self.tree_name = 'git-svn' + self.svn_url = scm.os.path.join(self.FAKE_REPOS.svn_base, 'trunk') + self.clone_dir = scm.os.path.join(self.FAKE_REPOS.git_root, + self.tree_name) + scm.os.makedirs(self.clone_dir) + self._capture(['svn', 'clone', '-q', '-q', self.svn_url, self.clone_dir]) + # git rev-list gives revisions in reverse chronological order. + hashes = reversed(self._capture(['rev-list', 'HEAD']).splitlines()) + # We insert a null value at 0 to do 1-based indexing, not 0-based, as SVN + # revisions are 1-based (i.e. they start at r1, not r0). + self.git_hashes = ([None] + list(hashes)) + + def tearDown(self): + scm.gclient_utils.rmtree(self.clone_dir) + + def _capture(self, cmd, **kwargs): + kwargs.setdefault('cwd', self.clone_dir) + return scm.GIT.Capture(cmd, **kwargs) + + def testGetGitSvnHeadRev(self): + if not self.enabled: + return + self.assertEquals(scm.GIT.GetGitSvnHeadRev(cwd=self.clone_dir), 2) + self._capture(['reset', '--hard', 'HEAD^']) + self.assertEquals(scm.GIT.GetGitSvnHeadRev(cwd=self.clone_dir), 1) + + def testGetGetSha1ForSvnRev(self): + if not self.enabled: + return + self.assertEquals(scm.GIT.GetSha1ForSvnRev(cwd=self.clone_dir, rev=1), + self.git_hashes[1]) + self.assertEquals(scm.GIT.GetSha1ForSvnRev(cwd=self.clone_dir, rev=2), + self.git_hashes[2]) + + class SVNTestCase(BaseSCMTestCase): def setUp(self): BaseSCMTestCase.setUp(self) @@ -98,11 +186,24 @@ class SVNTestCase(BaseSCMTestCase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'AssertVersion', 'Capture', 'CaptureRevision', 'CaptureLocalInfo', + 'AssertVersion', + 'Capture', + 'CaptureLocalInfo', 'CaptureRemoteInfo', - 'CaptureStatus', 'current_version', 'DiffItem', 'GenerateDiff', - 'GetCheckoutRoot', 'GetEmail', 'GetFileProperty', 'IsMoved', - 'IsMovedInfo', 'ReadSimpleAuth', 'Revert', 'RunAndGetFileList', + 'CaptureRevision', + 'CaptureStatus', + 'current_version', + 'DiffItem', + 'GenerateDiff', + 'GetCheckoutRoot', + 'GetEmail', + 'GetFileProperty', + 'IsMoved', + 'IsMovedInfo', + 'IsValidRevision', + 'ReadSimpleAuth', + 'Revert', + 'RunAndGetFileList', ] # If this test fails, you should add the relevant test. self.compareMembers(scm.SVN, members) @@ -288,6 +389,19 @@ class RealSvnTest(fake_repos.FakeReposTestBase): # Checkout and verify the tree. self.assertTree(self.tree, self.svn_root) + def testIsValidRevision(self): + if not self.enabled: + return + url_at_rev = self.svn_base + 'trunk/third_party@%s' + # Invalid or non-existent. + self.assertFalse(scm.SVN.IsValidRevision('url://totally_invalid/trunk/foo')) + self.assertFalse(scm.SVN.IsValidRevision(url_at_rev % 0)) + self.assertFalse(scm.SVN.IsValidRevision(url_at_rev % 123)) + # Valid. + self.assertTrue(scm.SVN.IsValidRevision(url_at_rev % 1)) + self.assertTrue(scm.SVN.IsValidRevision(url_at_rev % 2)) + self.assertTrue(scm.SVN.IsValidRevision(url_at_rev % 'HEAD')) + def testRevert(self): if not self.enabled: return