changeset 932:dfb3afa6c619

stupid: Fail over to full revision when a PatchError is thrown (issue294) Also give an enhanced exception message when an AssertionError is thrown out of subvertpy. Can't test this as throwing an exception from the appropriate place leaves file handles open in the SVN repository, resulting in a failure to clean up in tearDown().
author Tim Delaney <timothy.c.delaney@gmail.com>
date Tue, 18 Sep 2012 13:18:22 +1000
parents e1dbd9646d6a
children a9f315eae67c
files hgsubversion/editor.py hgsubversion/stupid.py tests/run.py tests/test_pull_fallback.py
diffstat 4 files changed, 141 insertions(+), 7 deletions(-) [+]
line wrap: on
line diff
--- a/hgsubversion/editor.py
+++ b/hgsubversion/editor.py
@@ -356,7 +356,19 @@ class HgEditor(svnwrap.Editor):
             try:
                 if not self.meta.is_path_valid(self.current.file):
                     return
-                handler(window)
+                try:
+                    handler(window)
+                except AssertionError, e: # pragma: no cover
+                    # Enhance the exception message
+                    msg, others = e.args[0], e.args[1:]
+
+                    if msg:
+                        msg += '\n'
+
+                    msg += _TXDELT_WINDOW_HANDLER_FAILURE_MSG
+                    e.args = (msg,) + others
+                    raise e
+
                 # window being None means commit this file
                 if not window:
                     self.current.files[self.current.file] = target.getvalue()
@@ -369,3 +381,13 @@ class HgEditor(svnwrap.Editor):
                 self._exception_info = sys.exc_info()
                 raise
         return txdelt_window
+
+_TXDELT_WINDOW_HANDLER_FAILURE_MSG = (
+    "Your SVN repository may not be supplying correct replay deltas."
+    " It is strongly"
+    "\nadvised that you repull the entire SVN repository using"
+    " hg pull --stupid."
+    "\nAlternatively, re-pull just this revision using --stupid and verify"
+    " that the"
+    "\nchangeset is correct."
+)
--- a/hgsubversion/stupid.py
+++ b/hgsubversion/stupid.py
@@ -22,7 +22,7 @@ import util
 #  a
 #  a
 # +a
-# 
+#
 # Property changes on: a
 # ___________________________________________________________________
 # Added: svn:executable
@@ -239,11 +239,16 @@ def patchrepo(ui, meta, parentctx, patch
     try:
         touched = set()
         backend = svnbackend(ui, meta.repo, parentctx, store)
-        ret = patch.patchbackend(ui, backend, patchfp, 0, touched)
-        if ret < 0:
-            raise BadPatchApply('patching failed')
-        if ret > 0:
-            raise BadPatchApply('patching succeeded with fuzz')
+
+        try:
+            ret = patch.patchbackend(ui, backend, patchfp, 0, touched)
+            if ret < 0:
+                raise BadPatchApply('patching failed')
+            if ret > 0:
+                raise BadPatchApply('patching succeeded with fuzz')
+        except patch.PatchError, e:
+            raise BadPatchApply(str(e))
+
         files = {}
         for f in touched:
             try:
--- a/tests/run.py
+++ b/tests/run.py
@@ -20,6 +20,7 @@ def tests():
     import test_fetch_truncated
     import test_hooks
     import test_pull
+    import test_pull_fallback
     import test_push_command
     import test_push_renames
     import test_push_dirs
new file mode 100644
--- /dev/null
+++ b/tests/test_pull_fallback.py
@@ -0,0 +1,106 @@
+import test_util
+
+import re
+import mercurial
+from mercurial import commands
+from hgsubversion import stupid
+from hgsubversion import svnwrap
+from hgsubversion import wrappers
+
+class TestPullFallback(test_util.TestBase):
+    def setUp(self):
+        super(TestPullFallback, self).setUp()
+
+    def _loadupdate(self, fixture_name, *args, **kwargs):
+        kwargs = kwargs.copy()
+        kwargs.update(noupdate=False)
+        repo, repo_path = self.load_and_fetch(fixture_name, *args, **kwargs)
+        return repo, repo_path
+
+    def test_stupid_fallback_to_stupid_fullrevs(self):
+        return
+        to_patch = {
+            'mercurial.patch.patchbackend': _patchbackend_raise,
+            'stupid.diff_branchrev': stupid.diff_branchrev,
+            'stupid.fetch_branchrev': stupid.fetch_branchrev,
+        }
+
+        expected_calls = {
+            'mercurial.patch.patchbackend': 1,
+            'stupid.diff_branchrev': 1,
+            'stupid.fetch_branchrev': 1,
+        }
+
+        repo, repo_path = self._loadupdate(
+            'single_rev.svndump', stupid=True)
+
+        # Passing stupid=True doesn't seem to be working - force it
+        repo.ui.setconfig('hgsubversion', 'stupid', "true")
+        state = repo.parents()
+
+        calls, replaced = _monkey_patch(to_patch)
+
+        try:
+            self.add_svn_rev(repo_path, {'trunk/alpha': 'Changed'})
+            commands.pull(self.repo.ui, repo, update=True)
+            self.failIfEqual(state, repo.parents())
+            self.assertTrue('tip' in repo[None].tags())
+            self.assertEqual(expected_calls, calls)
+
+        finally:
+            _monkey_unpatch(replaced)
+
+def _monkey_patch(to_patch, start=None):
+    if start is None:
+        import sys
+        start = sys.modules[__name__]
+
+    calls = {}
+    replaced = {}
+
+    for path, replacement in to_patch.iteritems():
+        obj = start
+        owner, attr = path.rsplit('.', 1)
+
+        for a in owner.split('.', -1):
+            obj = getattr(obj, a)
+
+        replaced[path] = getattr(obj, attr)
+        calls[path] = 0
+
+        def outer(path=path, calls=calls, replacement=replacement):
+            def wrapper(*p, **kw):
+                calls[path] += 1
+                return replacement(*p, **kw)
+
+            return wrapper
+
+        setattr(obj, attr, outer())
+
+    return calls, replaced
+
+def _monkey_unpatch(to_patch, start=None):
+    if start is None:
+        import sys
+        start = sys.modules[__name__]
+
+    replaced = {}
+
+    for path, replacement in to_patch.iteritems():
+        obj = start
+        owner, attr = path.rsplit('.', 1)
+
+        for a in owner.split('.', -1):
+            obj = getattr(obj, a)
+
+        replaced[path] = getattr(obj, attr)
+        setattr(obj, attr, replacement)
+
+    return replaced
+
+def _patchbackend_raise(*p, **kw):
+    raise mercurial.patch.PatchError("patch failed")
+
+def suite():
+    import unittest, sys
+    return unittest.findTestCases(sys.modules[__name__])