diff --git a/reviewboard/diffviewer/diffutils.py b/reviewboard/diffviewer/diffutils.py index dd6554d4449d82ce8097a1549f5f89abfa268f4c..f8c0795f8794abb828227bb61c66afbea2b03d5c 100644 --- a/reviewboard/diffviewer/diffutils.py +++ b/reviewboard/diffviewer/diffutils.py @@ -119,9 +119,11 @@ def get_original_file(filediff, request=None): if filediff.source_revision != PRE_CREATION: repository = filediff.diffset.repository - data = repository.get_file(filediff.source_file, - filediff.source_revision, - request) + data = repository.get_file( + filediff.source_file, + filediff.source_revision, + base_commit_id=filediff.diffset.base_commit_id, + request=request) # Repository.get_file doesn't know or care about how we need line # endings to work. So, we'll just transform every time. diff --git a/reviewboard/diffviewer/managers.py b/reviewboard/diffviewer/managers.py index 84dc38ab32ff37bf301a42ea6a8cd507b5121386..7e18210e117f5839381c99007af78a0bcc4ad285 100644 --- a/reviewboard/diffviewer/managers.py +++ b/reviewboard/diffviewer/managers.py @@ -98,6 +98,7 @@ class DiffSetManager(models.Manager): tool.get_parser(diff_file_contents), basedir, repository, + base_commit_id, request, check_existence=(not parent_diff_file_contents))) @@ -124,7 +125,7 @@ class DiffSetManager(models.Manager): # If the user supplied a base diff, we need to parse it and # later apply each of the files that are in the main diff for f in self._process_files(parent_parser, basedir, - repository, request, + repository, base_commit_id, request, check_existence=True, limit_to=diff_filenames): parent_files[f.origFile] = f @@ -182,8 +183,8 @@ class DiffSetManager(models.Manager): return diffset - def _process_files(self, parser, basedir, repository, request, - check_existence=False, limit_to=None): + def _process_files(self, parser, basedir, repository, base_commit_id, + request, check_existence=False, limit_to=None): tool = repository.get_scmtool() for f in parser.parse(): @@ -207,8 +208,10 @@ class DiffSetManager(models.Manager): not f.deleted and not f.moved and (check_existence and - not repository.get_file_exists(filename, revision, request))): - raise FileNotFoundError(filename, revision) + not repository.get_file_exists(filename, revision, + base_commit_id=base_commit_id, + request=request))): + raise FileNotFoundError(filename, revision, base_commit_id) f.origFile = filename f.origInfo = revision diff --git a/reviewboard/diffviewer/tests.py b/reviewboard/diffviewer/tests.py index 3dff91c3d80fdbb82be98a3264d51c0411705ad1..6bd36aea98f6dc1c49d20ec5ea3efda33da7e0c5 100644 --- a/reviewboard/diffviewer/tests.py +++ b/reviewboard/diffviewer/tests.py @@ -456,7 +456,7 @@ class DiffSetManagerTests(SpyAgency, TestCase): tool=Tool.objects.get(name='Subversion')) self.spy_on(repository.get_file_exists, - call_fake=lambda self, filename, revision, request: True) + call_fake=lambda *args, **kwargs: True) diffset = DiffSet.objects.create_from_data( repository, 'diff', diff, None, None, None, '/', None) @@ -492,7 +492,7 @@ class UploadDiffFormTests(SpyAgency, TestCase): tool=Tool.objects.get(name='Subversion')) self.spy_on(repository.get_file_exists, - call_fake=lambda self, filename, revision, request: True) + call_fake=lambda *args, **kwargs: True) form = UploadDiffForm( repository=repository, @@ -514,7 +514,7 @@ class UploadDiffFormTests(SpyAgency, TestCase): """Testing UploadDiffForm and filtering parent diff files""" saw_file_exists = {} - def get_file_exists(repository, filename, revision, request): + def get_file_exists(repository, filename, revision, *args, **kwargs): saw_file_exists[(filename, revision)] = True return True diff --git a/reviewboard/scmtools/errors.py b/reviewboard/scmtools/errors.py index cb614f93a148a2ce9d69bca312f0f498687078ab..d6a9845fdea21a12c2382f937333a92845adc49b 100644 --- a/reviewboard/scmtools/errors.py +++ b/reviewboard/scmtools/errors.py @@ -45,19 +45,26 @@ class InvalidRevisionFormatError(SCMError): class FileNotFoundError(SCMError): - def __init__(self, path, revision=None, detail=None): + def __init__(self, path, revision=None, detail=None, base_commit_id=None): from reviewboard.scmtools.core import HEAD - if revision == None or revision == HEAD: + if revision is None or revision == HEAD and base_commit_id is None: msg = "The file '%s' could not be found in the repository" % path + elif base_commit_id is not None and base_commit_id != revision: + msg = ("The file '%s' (r%s, commit %s) could not be found in " + "the repository" + % (path, revision)) else: msg = "The file '%s' (r%s) could not be found in the repository" \ % (path, revision) + if detail: msg += ': ' + detail + Exception.__init__(self, msg) self.revision = revision + self.base_commit_id = base_commit_id self.path = path self.detail = detail @@ -87,5 +94,3 @@ class UnverifiedCertificateError(SCMError): SCMError.__init__(self, _('A verified SSL certificate is required ' 'to connect to this repository.')) self.certificate = certificate - - diff --git a/reviewboard/scmtools/hg.py b/reviewboard/scmtools/hg.py index 7437c77d45b09325fd096cae6d0cd51a95db5281..86aa1d16cfd5ccd03d5240e3fb7c80adf5fc12e6 100644 --- a/reviewboard/scmtools/hg.py +++ b/reviewboard/scmtools/hg.py @@ -287,4 +287,4 @@ class HgClient(object): except Exception, e: # LookupError moves from repo to revlog in hg v0.9.4, so we # catch the more general Exception to avoid the dependency. - raise FileNotFoundError(path, rev, str(e)) + raise FileNotFoundError(path, rev, detail=str(e)) diff --git a/reviewboard/scmtools/localfile.py b/reviewboard/scmtools/localfile.py index 00d3e38988d14cce1ca538fb33e3464bbd34e5b6..b627974508590d3511174e93d971d705ae4cc6fd 100644 --- a/reviewboard/scmtools/localfile.py +++ b/reviewboard/scmtools/localfile.py @@ -22,7 +22,7 @@ class LocalFileTool(SCMTool): fp.close() return data except IOError, e: - raise FileNotFoundError(path, revision, str(e)) + raise FileNotFoundError(path, revision, detail=str(e)) def parse_diff_revision(self, file_str, revision_str, *args, **kwargs): return file_str, HEAD diff --git a/reviewboard/scmtools/models.py b/reviewboard/scmtools/models.py index 23c6a4f415083f70154a3e9b501350eec3dd3057..9e9aac8b5a3bdb906f873293f1e453be73d1c444 100644 --- a/reviewboard/scmtools/models.py +++ b/reviewboard/scmtools/models.py @@ -149,7 +149,7 @@ class Repository(models.Model): return None - def get_file(self, path, revision, request=None): + def get_file(self, path, revision, base_commit_id=None, request=None): """Returns a file from the repository. This will attempt to retrieve the file from the repository. If the @@ -165,11 +165,13 @@ class Repository(models.Model): # Basically, this fixes the massive regressions introduced by the # Django unicode changes. return cache_memoize( - self._make_file_cache_key(path, revision), - lambda: [self._get_file_uncached(path, revision, request)], + self._make_file_cache_key(path, revision, base_commit_id), + lambda: [self._get_file_uncached(path, revision, base_commit_id, + request)], large_data=True)[0] - def get_file_exists(self, path, revision, request=None): + def get_file_exists(self, path, revision, base_commit_id=None, + request=None): """Returns whether or not a file exists in the repository. If the repository is backed by a hosting service, this will go @@ -179,12 +181,13 @@ class Repository(models.Model): The result of this call will be cached, making future lookups of this path and revision on this repository faster. """ - key = self._make_file_exists_cache_key(path, revision) + key = self._make_file_exists_cache_key(path, revision, base_commit_id) if cache.get(make_cache_key(key)) == '1': return True - exists = self._get_file_exists_uncached(path, revision, request) + exists = self._get_file_exists_uncached(path, revision, + base_commit_id, request) if exists: cache_memoize(key, lambda: '1') @@ -219,16 +222,19 @@ class Repository(models.Model): def __unicode__(self): return self.name - def _make_file_cache_key(self, path, revision): + def _make_file_cache_key(self, path, revision, base_commit_id): """Makes a cache key for fetched files.""" - return "file:%s:%s:%s" % (self.pk, urlquote(path), urlquote(revision)) + return "file:%s:%s:%s:%s" % (self.pk, urlquote(path), + urlquote(revision), + urlquote(base_commit_id or '')) - def _make_file_exists_cache_key(self, path, revision): + def _make_file_exists_cache_key(self, path, revision, base_commit_id): """Makes a cache key for file existence checks.""" - return "file-exists:%s:%s:%s" % (self.pk, urlquote(path), - urlquote(revision)) + return "file-exists:%s:%s:%s:%s" % (self.pk, urlquote(path), + urlquote(revision), + urlquote(base_commit_id or '')) - def _get_file_uncached(self, path, revision, request): + def _get_file_uncached(self, path, revision, base_commit_id, request): """Internal function for fetching an uncached file. This is called by get_file if the file isn't already in the cache. @@ -236,16 +242,26 @@ class Repository(models.Model): fetching_file.send(sender=self, path=path, revision=revision, + base_commit_id=base_commit_id, request=request) - log_timer = log_timed("Fetching file '%s' r%s from %s" % - (path, revision, self), - request=request) + if base_commit_id: + timer_msg = "Fetching file '%s' r%s (base commit ID %s) from %s" \ + % (path, revision, base_commit_id, self) + else: + timer_msg = "Fetching file '%s' r%s from %s" \ + % (path, revision, self) + + log_timer = log_timed(timer_msg, request=request) hosting_service = self.hosting_service if hosting_service: - data = hosting_service.get_file(self, path, revision) + data = hosting_service.get_file( + self, + path, + revision, + base_commit_id=base_commit_id) else: data = self.get_scmtool().get_file(path, revision) @@ -254,12 +270,14 @@ class Repository(models.Model): fetched_file.send(sender=self, path=path, revision=revision, + base_commit_id=base_commit_id, request=request, data=data) return data - def _get_file_exists_uncached(self, path, revision, request): + def _get_file_exists_uncached(self, path, revision, base_commit_id, + request): """Internal function for checking that a file exists. This is called by get_file_eixsts if the file isn't already in the @@ -271,7 +289,7 @@ class Repository(models.Model): # First we check to see if we've fetched the file before. If so, # it's in there and we can just return that we have it. file_cache_key = make_cache_key( - self._make_file_cache_key(path, revision)) + self._make_file_cache_key(path, revision, base_commit_id)) if cache.has_key(file_cache_key): exists = True @@ -280,18 +298,24 @@ class Repository(models.Model): checking_file_exists.send(sender=self, path=path, revision=revision, + base_commit_id=base_commit_id, request=request) hosting_service = self.hosting_service if hosting_service: - exists = hosting_service.get_file_exists(self, path, revision) + exists = hosting_service.get_file_exists( + self, + path, + revision, + base_commit_id=base_commit_id) else: exists = self.get_scmtool().file_exists(path, revision) checked_file_exists.send(sender=self, path=path, revision=revision, + base_commit_id=base_commit_id, request=request, exists=exists) diff --git a/reviewboard/scmtools/svn.py b/reviewboard/scmtools/svn.py index 2999eb670dfa9becfae19974e581d6630163b4eb..c18a807b400db2f956e8989be0bd31b446a3f879 100644 --- a/reviewboard/scmtools/svn.py +++ b/reviewboard/scmtools/svn.py @@ -159,7 +159,7 @@ class SVNTool(SCMTool): except ClientError, e: stre = str(e) if 'File not found' in stre or 'path not found' in stre: - raise FileNotFoundError(path, revision, str(e)) + raise FileNotFoundError(path, revision, detail=str(e)) elif 'callback_ssl_server_trust_prompt required' in stre: raise SCMError( 'HTTPS certificate not accepted. Please ensure that ' diff --git a/reviewboard/scmtools/tests.py b/reviewboard/scmtools/tests.py index b2594379e51bdf34d9871c840e83616564aef05c..2d0ee26659d1cc861f0e7278279fb3758ed8ffbc 100644 --- a/reviewboard/scmtools/tests.py +++ b/reviewboard/scmtools/tests.py @@ -196,8 +196,8 @@ class RepositoryTests(DjangoTestCase): self.scmtool_cls.get_file = get_file - data1 = self.repository.get_file(path, revision, request) - data2 = self.repository.get_file(path, revision, request) + data1 = self.repository.get_file(path, revision, request=request) + data2 = self.repository.get_file(path, revision, request=request) self.assertEqual(data1, 'file data') self.assertEqual(data1, data2) @@ -220,7 +220,7 @@ class RepositoryTests(DjangoTestCase): revision = 'e965047' request = {} - self.repository.get_file(path, revision, request) + self.repository.get_file(path, revision, request=request) self.assertEqual(len(found_signals), 2) self.assertEqual(found_signals[0], @@ -244,8 +244,10 @@ class RepositoryTests(DjangoTestCase): self.scmtool_cls.file_exists = file_exists - exists1 = self.repository.get_file_exists(path, revision, request) - exists2 = self.repository.get_file_exists(path, revision, request) + exists1 = self.repository.get_file_exists(path, revision, + request=request) + exists2 = self.repository.get_file_exists(path, revision, + request=request) self.assertTrue(exists1) self.assertTrue(exists2) @@ -267,8 +269,10 @@ class RepositoryTests(DjangoTestCase): self.scmtool_cls.file_exists = file_exists - exists1 = self.repository.get_file_exists(path, revision, request) - exists2 = self.repository.get_file_exists(path, revision, request) + exists1 = self.repository.get_file_exists(path, revision, + request=request) + exists2 = self.repository.get_file_exists(path, revision, + request=request) self.assertFalse(exists1) self.assertFalse(exists2) @@ -296,9 +300,11 @@ class RepositoryTests(DjangoTestCase): self.scmtool_cls.get_file = get_file self.scmtool_cls.file_exists = file_exists - data = self.repository.get_file(path, revision, request) - exists1 = self.repository.get_file_exists(path, revision, request) - exists2 = self.repository.get_file_exists(path, revision, request) + self.repository.get_file(path, revision, request=request) + exists1 = self.repository.get_file_exists(path, revision, + request=request) + exists2 = self.repository.get_file_exists(path, revision, + request=request) self.assertTrue(exists1) self.assertTrue(exists2) @@ -324,7 +330,7 @@ class RepositoryTests(DjangoTestCase): revision = 'e965047' request = {} - self.repository.get_file_exists(path, revision, request) + self.repository.get_file_exists(path, revision, request=request) self.assertEqual(len(found_signals), 2) self.assertEqual(found_signals[0],